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

fix: add buffer and cleanup #22

Merged
merged 3 commits into from
Apr 6, 2020
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
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,15 @@
},
"homepage": "https://github.com/ipfs/js-datastore-core#readme",
"devDependencies": {
"aegir": "^19.0.3",
"aegir": "^21.4.5",
"async-iterator-all": "^1.0.0",
"chai": "^4.2.0",
"dirty-chai": "^2.0.1"
},
"dependencies": {
"buffer": "^5.5.0",
"debug": "^4.1.1",
"interface-datastore": "~0.7.0"
"interface-datastore": "^0.8.2"
},
"engines": {
"node": ">=6.0.0",
Expand Down
14 changes: 4 additions & 10 deletions src/mount.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class MountDatastore {
* @returns {{Datastore, Key, Key}}
*/
_lookup (key) {
for (let mount of this.mounts) {
for (const mount of this.mounts) {
if (mount.prefix.toString() === key.toString() || mount.prefix.isAncestorOf(key)) {
const s = replaceStartWith(key.toString(), mount.prefix.toString())
return {
Expand Down Expand Up @@ -156,15 +156,9 @@ class MountDatastore {

function _many (iterable) {
return (async function * () {
let completed = iterable.map(() => false)
while (!completed.every(Boolean)) {
for (const [idx, itr] of iterable.entries()) {
const it = await itr.next()
if (it.done) {
completed[idx] = true
continue
}
yield it.value
for (let i = 0; i < iterable.length; i++) {
for await (const v of iterable[i]) {
yield v
}
}
})()
Expand Down
1 change: 1 addition & 0 deletions src/sharding.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict'

const { Buffer } = require('buffer')
const Key = require('interface-datastore').Key

const sh = require('./shard')
Expand Down
20 changes: 8 additions & 12 deletions src/tiered.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class TieredDatastore {

async open () {
try {
await (this.stores.map((store) => store.open()))
await Promise.all(this.stores.map((store) => store.open()))
} catch (err) {
throw Errors.dbOpenFailedError()
}
Expand All @@ -43,18 +43,14 @@ class TieredDatastore {
throw Errors.notFoundError()
}

has (key) {
return new Promise(async (resolve) => {
await Promise.all(this.stores.map(async (store) => {
const has = await store.has(key)

if (has) {
resolve(true)
}
}))
async has (key) {
for (const s of this.stores) {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe Promise.race here? The previous code looked through all nested stores in parallel, this makes it serial but it should never have waited for them all to complete before resolving to a value.

Copy link
Member Author

Choose a reason for hiding this comment

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

race is weird (http://bluebirdjs.com/docs/api/promise.race.html), with https://github.com/sindresorhus/p-any we would need to try catch twice to return a boolean.
do you think its worth it ? this looks nice and simple

Copy link
Member

Choose a reason for hiding this comment

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

The semantics are a little convoluted. Let's merge this then we can look at refactoring it if it proves to be a bottleneck.

if (await s.has(key)) {
return true
}
}

resolve(false)
})
return false
}

async delete (key) {
Expand Down
7 changes: 4 additions & 3 deletions test/keytransform.spec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-env mocha */
'use strict'

const { Buffer } = require('buffer')
const chai = require('chai')
chai.use(require('dirty-chai'))
const expect = chai.expect
Expand Down Expand Up @@ -37,9 +38,9 @@ describe('KeyTransformDatastore', () => {
'foo/bar/baz/barb'
].map((s) => new Key(s))
await Promise.all(keys.map((key) => kStore.put(key, Buffer.from(key.toString()))))
let kResults = Promise.all(keys.map((key) => kStore.get(key)))
let mResults = Promise.all(keys.map((key) => mStore.get(new Key('abc').child(key))))
let results = await Promise.all([kResults, mResults])
const kResults = Promise.all(keys.map((key) => kStore.get(key)))
const mResults = Promise.all(keys.map((key) => mStore.get(new Key('abc').child(key))))
const results = await Promise.all([kResults, mResults])
expect(results[0]).to.eql(results[1])

const mRes = await all(mStore.query({}))
Expand Down
1 change: 1 addition & 0 deletions test/mount.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/* eslint max-nested-callbacks: ["error", 8] */
'use strict'

const { Buffer } = require('buffer')
const chai = require('chai')
chai.use(require('dirty-chai'))
const assert = chai.assert
Expand Down
7 changes: 4 additions & 3 deletions test/namespace.spec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-env mocha */
'use strict'

const { Buffer } = require('buffer')
const chai = require('chai')
chai.use(require('dirty-chai'))
const expect = chai.expect
Expand Down Expand Up @@ -30,9 +31,9 @@ describe('KeyTransformDatastore', () => {
].map((s) => new Key(s))

await Promise.all(keys.map(key => store.put(key, Buffer.from(key.toString()))))
let nResults = Promise.all(keys.map((key) => store.get(key)))
let mResults = Promise.all(keys.map((key) => mStore.get(new Key(prefix).child(key))))
let results = await Promise.all([nResults, mResults])
const nResults = Promise.all(keys.map((key) => store.get(key)))
const mResults = Promise.all(keys.map((key) => mStore.get(new Key(prefix).child(key))))
const results = await Promise.all([nResults, mResults])
const mRes = await all(mStore.query({}))
const nRes = await all(store.query({}))

Expand Down
1 change: 1 addition & 0 deletions test/sharding.spec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-env mocha */
'use strict'

const { Buffer } = require('buffer')
const chai = require('chai')
chai.use(require('dirty-chai'))
const expect = chai.expect
Expand Down
7 changes: 6 additions & 1 deletion test/tiered.spec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-env mocha */
'use strict'

const { Buffer } = require('buffer')
const chai = require('chai')
chai.use(require('dirty-chai'))
const expect = chai.expect
Expand All @@ -12,7 +13,7 @@ const TieredStore = require('../src').TieredDatastore

describe('Tiered', () => {
describe('all stores', () => {
let ms = []
const ms = []
let store
beforeEach(() => {
ms.push(new MemoryStore())
Expand Down Expand Up @@ -40,6 +41,10 @@ describe('Tiered', () => {
expect(exists).to.be.eql(true)
})

it('has - key not found', async () => {
expect(await store.has(new Key('hello1'))).to.be.eql(false)
})

it('has and delete', async () => {
const k = new Key('hello')
const v = Buffer.from('world')
Expand Down