Skip to content
This repository was archived by the owner on Mar 23, 2023. It is now read-only.

Windows #4

Merged
merged 8 commits into from
Nov 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .flowconfig
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
.*/radium/.*
.*/standard-changelog/.*
.*/conventional-changelog-core/.*
.*/node_modules/.*

[include]

Expand Down
27 changes: 27 additions & 0 deletions appveyor.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
version: "{build}"

environment:
matrix:
- nodejs_version: "6"

matrix:
fast_finish: true

install:
# Install Node.js
- ps: Install-Product node $env:nodejs_version

# Upgrade npm
- npm install -g npm

# Output our current versions for debugging
- node --version
- npm --version

# Install our package dependencies
- npm install

test_script:
- npm run test

build: off
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"main": "src/index.js",
"scripts": {
"lint": "aegir-lint",
"test": "aegir-test --env node",
"test": "aegir-test --env node --timeout 20000",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this now? Have timeouts increased with this changes? If so, we should have individual timeouts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes a lot of tests were skipped. In particular the many (3 * 400) test takes a long time on my top-end laptop when sharding.

"flow": "flow",
"release": "aegir-release --env node --docs",
"release-minor": "aegir-release --type minor --env node --docs",
Expand Down Expand Up @@ -33,13 +33,13 @@
"homepage": "https://github.com/ipfs/js-datastore-fs#readme",
"dependencies": {
"datastore-core": "^0.3.0",
"glob": "^7.1.2",
"graceful-fs": "^4.1.11",
"interface-datastore": "^0.3.0",
"level-js": "^2.2.4",
"leveldown": "^1.7.2",
"levelup": "^1.3.8",
"mkdirp": "^0.5.1",
"pull-glob": "^1.0.6",
"pull-stream": "^3.6.0",
"write-file-atomic": "^2.1.0"
},
Expand All @@ -57,4 +57,4 @@
"David Dias <daviddias.p@gmail.com>",
"Friedel Ziegelmayer <dignifiedquire@gmail.com>"
]
}
}
25 changes: 12 additions & 13 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

const fs = require('graceful-fs')
const pull = require('pull-stream')
const glob = require('pull-glob')
const glob = require('glob')
const setImmediate = require('async/setImmediate')
const waterfall = require('async/series')
const each = require('async/each')
Expand Down Expand Up @@ -136,7 +136,9 @@ class FsDatastore {
throw new Error(`Invalid extension: ${path.extname(file)}`)
}

return new Key(file.slice(this.path.length, -ext.length))
let keyname = file.slice(this.path.length, -ext.length)
keyname = keyname.split(path.sep).join('/')
return new Key(keyname)
}

/**
Expand Down Expand Up @@ -259,7 +261,13 @@ class FsDatastore {
* @returns {PullStream}
*/
query (q /* : Query<Buffer> */) /* : QueryResult<Buffer> */ {
let tasks = [glob(path.join(this.path, '**', '*' + this.opts.extension))]
// glob expects a POSIX path
let prefix = q.prefix || '**'
let pattern =
path.join(this.path, prefix, '*' + this.opts.extension)
.split(path.sep)
.join('/')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be the inverse? Split by the key separator / and the join with the os specific path sep?

Copy link
Contributor Author

@richardschneider richardschneider Nov 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beg to differ. The path.join creates an os specific path. The split(path.sep) then gives an array from the os specific ath and then join('/') creates a POSIX specific path, which is what glob wants,

Copy link
Member

@dryajov dryajov Nov 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah glob.sync does expect POSIX / separators - makes sense 👍

let tasks = [pull.values(glob.sync(pattern))]

if (!q.keysOnly) {
tasks.push(pull.asyncMap((f, cb) => {
Expand All @@ -277,19 +285,10 @@ class FsDatastore {
tasks.push(pull.map(f => ({ key: this._decode(f) })))
}

let filters = []

if (q.prefix != null) {
const prefix = q.prefix
filters.push((e, cb) => cb(null, e.key.toString().startsWith(prefix)))
}

if (q.filters != null) {
filters = filters.concat(q.filters)
tasks = tasks.concat(q.filters.map(f => asyncFilter(f)))
}

tasks = tasks.concat(filters.map(f => asyncFilter(f)))

if (q.orders != null) {
tasks = tasks.concat(q.orders.map(o => asyncSort(o)))
}
Expand Down
7 changes: 3 additions & 4 deletions test/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ describe('FsDatastore', () => {
})

it('query', (done) => {
const fs = new FsStore(path.join(__dirname, 'test-repo/blocks'))
const fs = new FsStore(path.join(__dirname, 'test-repo', 'blocks'))

pull(
fs.query({}),
Expand Down Expand Up @@ -139,9 +139,8 @@ describe('FsDatastore', () => {
}
})
})

// TODO: depends on sharding query fix
describe.skip('interface-datastore (sharding(fs))', () => {

describe('interface-datastore (sharding(fs))', () => {
const dir = utils.tmpdir()

require('interface-datastore/src/tests')({
Expand Down