From 525a2c97fa6a8212d2841db7d1de0db40269a330 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Mon, 30 Sep 2019 14:10:57 +0200 Subject: [PATCH] Breaking: parent db must support deferredOpen DeferredOpen means that the db opens itself and defers operations until it's open. Currently that's only supported by level(up) and friends. Before, subleveldown would also accept abstract-leveldown db's that were not wrapped in levelup. Opening and closing a sublevel no longer opens or closes the parent db. The sublevel does wait for the parent to open (which in the case of levelup already happens automatically) but never initiates a state change. Drops support of old modules: - memdb (use level-mem instead) - deferred-leveldown < 2.0.0 (and thus levelup < 2.0.0) - abstract-leveldown < 2.4.0 Closes #84, #83 and #60. --- leveldown.js | 61 +++++++++----- matchdown.js | 1 + package.json | 5 +- test/index.js | 215 +++++++++++++++++++++++++++++++++++++------------- 4 files changed, 206 insertions(+), 76 deletions(-) diff --git a/leveldown.js b/leveldown.js index 5a26e9f..da32f82 100644 --- a/leveldown.js +++ b/leveldown.js @@ -64,12 +64,36 @@ function SubDown (db, prefix, opts) { }) this.db = db - this.leveldown = null - this.ownPrefix = separator + prefix + separator - this.prefix = this.ownPrefix + this.prefix = separator + prefix + separator this._beforeOpen = opts.open var self = this + var manifest = db.supports || {} + + // The parent db must open itself or be (re)opened by the user because a + // sublevel can't (shouldn't) initiate state changes on the rest of the db. + if (!manifest.deferredOpen && !reachdown.is(db, 'levelup')) { + throw new Error('The parent db must support deferredOpen') + } + + var subdb = reachdown(db, 'subleveldown') + + if (subdb) { + // Old subleveldown doesn't know its prefix and leveldown until opened + if (!subdb.prefix || !subdb.leveldown) { + throw new Error('Incompatible with subleveldown < 5.0.0') + } + + this.prefix = subdb.prefix + this.prefix + this.leveldown = subdb.leveldown + } else { + this.leveldown = reachdown(db, matchdown, false) + } + + // Old deferred-leveldown doesn't expose its underlying db until opened + if (reachdown.is(this.leveldown, 'deferred-leveldown')) { + throw new Error('Incompatible with deferred-leveldown < 2.0.0') + } this._wrap = { gt: function (x) { @@ -91,28 +115,29 @@ inherits(SubDown, abstract.AbstractLevelDOWN) SubDown.prototype.type = 'subleveldown' +// TODO: remove _open() once abstract-leveldown supports deferredOpen, +// because that means we can always do operations on this.leveldown. SubDown.prototype._open = function (opts, cb) { var self = this - this.db.open(function (err) { - if (err) return cb(err) + // TODO: make _isOpening public in levelup or add a method like + // ready(cb) which waits for - but does not initiate - a state change. + var m = typeof this.db.isOpening === 'function' ? 'isOpening' : '_isOpening' - var subdb = reachdown(self.db, 'subleveldown') + if (this.db[m]()) { + this.db.once('open', onopen) + } else { + this._nextTick(onopen) + } - if (subdb && subdb.prefix) { - self.prefix = subdb.prefix + self.ownPrefix - self.leveldown = subdb.leveldown - } else { - self.leveldown = reachdown(self.db, matchdown, false) - } + function onopen () { + if (!self.db.isOpen()) return cb(new Error('Parent db is not open')) - if (self._beforeOpen) self._beforeOpen(cb) - else cb() - }) -} + // TODO: add hooks to abstract-leveldown + if (self._beforeOpen) return self._beforeOpen(cb) -SubDown.prototype._close = function (cb) { - this.leveldown.close(cb) + cb() + } } SubDown.prototype._serializeKey = function (key) { diff --git a/matchdown.js b/matchdown.js index 089ce39..45a0e10 100644 --- a/matchdown.js +++ b/matchdown.js @@ -3,6 +3,7 @@ module.exports = function matchdown (db, type) { if (type === 'levelup') return false if (type === 'encoding-down') return false if (type === 'deferred-leveldown') return false + if (type === 'subleveldown') return false return true } diff --git a/package.json b/package.json index 3131320..941d43d 100644 --- a/package.json +++ b/package.json @@ -16,12 +16,12 @@ "test": "test" }, "dependencies": { - "abstract-leveldown": "^6.1.1", + "abstract-leveldown": "^6.2.3", "encoding-down": "^6.2.0", "inherits": "^2.0.3", "level-option-wrap": "^1.1.0", "levelup": "^4.3.1", - "reachdown": "^1.0.0" + "reachdown": "^1.1.0" }, "devDependencies": { "after": "^0.8.2", @@ -31,7 +31,6 @@ "hallmark": "^2.0.0", "level-community": "^3.0.0", "level-concat-iterator": "^2.0.1", - "memdb": "^1.3.1", "memdown": "^5.0.0", "nyc": "^14.0.0", "standard": "^14.0.0", diff --git a/test/index.js b/test/index.js index a0b6309..d06f5f8 100644 --- a/test/index.js +++ b/test/index.js @@ -8,9 +8,9 @@ var subdown = require('../leveldown') var subdb = require('..') var levelup = require('levelup') var reachdown = require('reachdown') -var memdb = require('memdb') var abstract = require('abstract-leveldown') var inherits = require('util').inherits +var EventEmitter = require('events') // Test abstract-leveldown compliance function runSuite (factory) { @@ -45,49 +45,66 @@ runSuite(function factory () { // Test without a user-provided levelup layer runSuite(function factory () { - return subdown(memdown(), 'test') + var down = memdown() + var emitter = new EventEmitter() + + if (!down.supports.deferredOpen) { + // Simulate a future abstract-leveldown that + // supports deferredOpen just like levelup + down.supports.deferredOpen = true + down.isOpen = function () { return this.status === 'open' } + down.isOpening = function () { return this.status === 'opening' } + down.once = emitter.once.bind(emitter) + down.open(function (err) { + if (err) throw err + emitter.emit('open') + }) + down.open = function () { throw new Error('Explicit open is not simulated') } + } + + return subdown(down, 'test') }) // Additional tests for this implementation test('SubDown constructor', function (t) { t.test('can be called without new', function (t) { - var sub = subdown() + var sub = subdown(levelup(memdown())) t.is(sub instanceof subdown, true, 'instanceof subdown') t.end() }) t.test('missing prefix and missing separator', function (t) { - var sub = subdown() + var sub = subdown(levelup(memdown())) t.is(sub.prefix, '!!') t.end() }) t.test('prefix and missing separator', function (t) { - var sub = subdown({}, 'prefix') + var sub = subdown(levelup(memdown()), 'prefix') t.is(sub.prefix, '!prefix!') t.end() }) t.test('prefix and separator (as string)', function (t) { - var sub = subdown({}, 'prefix', '%') + var sub = subdown(levelup(memdown()), 'prefix', '%') t.is(sub.prefix, '%prefix%') t.end() }) t.test('prefix and separator (as options)', function (t) { - var sub = subdown({}, 'prefix', { separator: '%' }) + var sub = subdown(levelup(memdown()), 'prefix', { separator: '%' }) t.is(sub.prefix, '%prefix%') t.end() }) t.test('prefix with same initial character as separator is sliced', function (t) { - var sub = subdown({}, '!prefix') + var sub = subdown(levelup(memdown()), '!prefix') t.is(sub.prefix, '!prefix!') t.end() }) t.test('prefix with same ending character as separator is sliced', function (t) { - var sub = subdown({}, 'prefix!') + var sub = subdown(levelup(memdown()), 'prefix!') t.is(sub.prefix, '!prefix!') t.end() }) // TODO we're currently not guarded by multiple separators in the prefix // t.test('repeated separator is slices off from prefix parameter', function (t) { - // var sub = subdown({}, '!!prefix!!') + // var sub = subdown(levelup(memdown()), '!!prefix!!') // t.is(sub.prefix, '!prefix!') // t.end() // }) @@ -103,18 +120,25 @@ test('SubDb main function', function (t) { }) }) - t.test('error from open() bubbles up', function (t) { + t.test('error from open() does not bubble up', function (t) { t.plan(1) - var mockdb = { - open: function (cb) { + var mockdb = mock(abstract.AbstractLevelDOWN, { + _open: function (opts, cb) { process.nextTick(cb, new Error('error from underlying store')) } - } + }) - subdb(mockdb, 'test').on('error', (err) => { + var db = levelup(mockdb) + var sub = subdb(db, 'test') + + db.on('error', (err) => { t.is(err.message, 'error from underlying store') }) + + sub.on('error', (err) => { + t.fail(err) + }) }) t.test('levelup *down is set to subdown which has correct storage', function (t) { @@ -155,18 +179,114 @@ test('SubDb main function', function (t) { }) }) - t.test('wrap a closed levelup and re-open levelup', function (t) { - t.plan(3) + t.test('cannot create a sublevel on a closed db', function (t) { + t.plan(4) var db = levelup(memdown()) db.once('open', function () { db.close(function (err) { t.error(err, 'no error') - var sub = subdb(db, 'test') - sub.once('open', function () { - t.pass('subdb openen') + + subdb(db, 'test').on('error', function (err) { + t.is(err.message, 'Parent db is not open', 'sublevel not opened') }) + db.open(function (err) { t.error(err, 'no error') + + subdb(db, 'test').on('open', function () { + t.pass('sublevel opened') + }) + }) + }) + }) + }) + + t.test('can close db and sublevel once opened', function (t) { + t.plan(3) + + levelup(memdown(), function (err, db) { + t.ifError(err, 'no open error') + var sub = subdb(db, 'test') + + sub.once('open', function () { + db.close(function (err) { + t.ifError(err, 'no close error') + }) + + // Noop, shouldn't error + sub.close(function (err) { + t.ifError(err, 'no close error') + }) + }) + }) + }) + + t.test('cannot close db while sublevel is opening', function (t) { + t.plan(5) + + levelup(memdown(), function (err, db) { + t.ifError(err, 'no open error') + var sub = subdb(db, 'test') + + sub.on('error', (err) => { + t.is(err.message, 'Parent db is not open') + }) + + db.close(function (err) { + t.ifError(err, 'no close error') + t.is(reachdown(sub, 'subleveldown').status, 'new') + t.is(reachdown(sub).status, 'closed') + }) + + sub.close(function () { + t.fail('should not be called, because opening never finished') + }) + }) + }) + + t.test('cannot create sublevel while db is closing', function (t) { + t.plan(6) + + levelup(memdown(), function (err, db) { + t.ifError(err, 'no open error') + + db.close(function (err) { + t.ifError(err, 'no close error') + t.is(reachdown(sub, 'subleveldown').status, 'opening') + t.is(reachdown(sub).status, 'closed') + + sub.on('error', (err) => { + t.is(err.message, 'Parent db is not open') + t.is(reachdown(sub, 'subleveldown').status, 'new') + }) + }) + + var sub = subdb(db, 'test') + + sub.on('open', function () { + t.fail('should not open') + }) + }) + }) + + t.test('can reopen a sublevel without affecting encoding-down state of db', function (t) { + t.plan(3) + var db = levelup(encoding(memdown())) + + db.once('open', function () { + var sub = subdb(db, 'test') + + sub.close(function (err) { + t.ifError(err, 'no close error') + + // Previously, subleveldown would open a sublevel via levelup yet close + // it via the innermost db (memdown). So at this point, the intermediate + // encoding-down layer would still be open, leading levelup to believe + // that encoding-down and its underlying memdown db need not be opened. + // See https://github.com/Level/subleveldown/issues/60. + sub.open(function (err) { + t.error(err, 'no open error') + t.is(reachdown(sub).status, 'open') }) }) }) @@ -277,11 +397,8 @@ test('SubDb main function', function (t) { t.test('errors from iterator bubble up', function (t) { t.plan(2) - var mockdb = { - open: function (cb) { - process.nextTick(cb) - }, - iterator: function () { + var mockdb = mock(abstract.AbstractLevelDOWN, { + _iterator: function () { return { next: function (cb) { process.nextTick(cb, new Error('next() error from underlying store')) @@ -291,9 +408,9 @@ test('SubDb main function', function (t) { } } } - } + }) - var sub = subdb(mockdb, 'test') + var sub = subdb(levelup(mockdb), 'test') var it = sub.iterator() it.next(function (err) { @@ -508,37 +625,25 @@ test('subleveldown on intermediate layer', function (t) { }) }) -test('legacy memdb (old levelup)', function (t) { - t.plan(7) - - // Should not result in double json encoding - var db = memdb({ valueEncoding: 'json' }) - var sub = subdb(db, 'test', { valueEncoding: 'json' }) - - // Integration with memdb still works because subleveldown waits to reachdown - // until the (old levelup) db is open. Reaching down then correctly lands on - // the memdown db. If subleveldown were to reachdown immediately it'd land on - // the old deferred-leveldown (which when unopened doesn't have a reference to - // the memdown db yet) so we'd be unable to persist anything. - t.is(Object.getPrototypeOf(reachdown(db)).constructor.name, 'DeferredLevelDOWN') +function getKey (entry) { + return entry.key +} - sub.put('key', { a: 1 }, function (err) { - t.ifError(err, 'no put error') +function implement (ctor, methods) { + function Test () { + ctor.apply(this, arguments) + } - sub.get('key', function (err, value) { - t.ifError(err, 'no get error') - t.same(value, { a: 1 }) - }) + inherits(Test, ctor) - t.is(Object.getPrototypeOf(reachdown(db)).constructor.name, 'MemDOWN') + for (var k in methods) { + Test.prototype[k] = methods[k] + } - reachdown(db).get('!test!key', { asBuffer: false }, function (err, value) { - t.ifError(err, 'no get error') - t.is(value, '{"a":1}') - }) - }) -}) + return Test +} -function getKey (entry) { - return entry.key +function mock (ctor, methods) { + var Test = implement(ctor, methods) + return new Test() }