diff --git a/README.md b/README.md index afcd739..d6d0aca 100644 --- a/README.md +++ b/README.md @@ -350,16 +350,22 @@ await db.batch([ ### `chainedBatch = db.batch()` -Create a [chained batch](#chainedbatch), when `batch()` is called with zero arguments. A chained batch can be used to build and eventually commit an atomic batch of operations. Depending on how it's used, it is possible to obtain greater performance with this form of `batch()`. On several implementations however, it is just sugar. +Create a [chained batch](#chainedbatch), when `batch()` is called with zero arguments. A chained batch can be used to build and eventually commit an atomic batch of operations: ```js -await db.batch() +const chainedBatch = db.batch() .del('bob') .put('alice', 361) .put('kim', 220) - .write() + +// Commit +await chainedBatch.write() ``` +Depending on how it's used, it is possible to obtain greater overall performance with this form of `batch()`, mainly because its methods like `put()` can immediately copy the data of that singular operation to the underlying storage, rather than having to block the event loop while copying the data of multiple operations. However, on several `abstract-level` implementations, chained batch is just sugar and has no performance benefits. + +Due to its synchronous nature, it is not possible to create a chained batch before the database has finished opening. Be sure to call `await db.open()` before `chainedBatch = db.batch()`. This does not apply to other database methods. + ### `iterator = db.iterator([options])` Create an [iterator](#iterator). The optional `options` object may contain the following _range options_ to control the range of entries to be iterated: diff --git a/UPGRADING.md b/UPGRADING.md index 148c013..4037927 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -168,6 +168,36 @@ indexes.batch([{ type: 'del', key: 'blue', sublevel: colorIndex }]) db.batch([{ type: 'del', key: 'blue', sublevel: colorIndex }]) ``` +#### 1.5. Open before creating a chained batch + +It is no longer possible to create a chained batch while the database is opening. If you previously did: + +```js +const db = new ExampleLevel() + +const batch = db.batch().del('example') +await batch.write() +``` + +You must now do: + +```js +const db = new ExampleLevel() +await db.open() + +const batch = db.batch().del('example') +await batch.write() +``` + +Alternatively: + +```js +const db = new ExampleLevel() +await db.batch([{ type: 'del', key: 'example' }]) +``` + +As for why that last example works yet the same is not supported on a chained batch: the `put()`, `del()` and `clear()` methods of a chained batch are synchronous. This meant `abstract-level` (and `levelup` before it) had to jump through several hoops to make it work while the database is opening. Having such logic internally is fine, but the problem extended to the new [hooks](./README.md#hooks) feature and more specifically, the `prewrite` hook that runs on `put()` and `del()`. + ### 2. Private API #### 2.1. Promises all the way diff --git a/abstract-chained-batch.js b/abstract-chained-batch.js index aa4754a..f542bb7 100644 --- a/abstract-chained-batch.js +++ b/abstract-chained-batch.js @@ -265,13 +265,10 @@ class AbstractChainedBatch { _clear () {} async write (options) { + assertStatus(this) options = getOptions(options) - if (this[kStatus] !== 'open') { - throw new ModuleError('Batch is not open: cannot call write() after write() or close()', { - code: 'LEVEL_BATCH_NOT_OPEN' - }) - } else if (this[kLength] === 0) { + if (this[kLength] === 0) { return this.close() } else { this[kStatus] = 'writing' @@ -391,9 +388,12 @@ function assertStatus (batch) { }) } - // TODO (next major): enforce this regardless of hooks - if (batch[kPrewriteBatch] !== null && batch.db.status !== 'open') { - throw new ModuleError('Chained batch is not available until database is open', { + // Can technically be removed, because it's no longer possible to call db.batch() when + // status is not 'open', and db.close() closes the batch. Keep for now, in case of + // unforseen userland behaviors. + if (batch.db.status !== 'open') { + /* istanbul ignore next */ + throw new ModuleError('Database is not open', { code: 'LEVEL_DATABASE_NOT_OPEN' }) } diff --git a/abstract-level.js b/abstract-level.js index bea3263..474e8db 100644 --- a/abstract-level.js +++ b/abstract-level.js @@ -526,7 +526,6 @@ class AbstractLevel extends EventEmitter { // of classic-level, that should not be copied to individual operations. batch (operations, options) { if (!arguments.length) { - if (this[kStatus] === 'opening') return new DefaultChainedBatch(this) assertOpen(this) return this._chainedBatch() } diff --git a/lib/default-chained-batch.js b/lib/default-chained-batch.js index b2df084..8fbe0c4 100644 --- a/lib/default-chained-batch.js +++ b/lib/default-chained-batch.js @@ -1,10 +1,9 @@ 'use strict' const { AbstractChainedBatch } = require('../abstract-chained-batch') -const ModuleError = require('module-error') const kEncoded = Symbol('encoded') -// Functional default for chained batch, with support of deferred open +// Functional default for chained batch class DefaultChainedBatch extends AbstractChainedBatch { constructor (db) { // Opt-in to _add() instead of _put() and _del() @@ -21,19 +20,9 @@ class DefaultChainedBatch extends AbstractChainedBatch { } async _write (options) { - if (this.db.status === 'opening') { - return this.db.deferAsync(() => this._write(options)) - } else if (this.db.status === 'open') { - if (this[kEncoded].length === 0) return - - // Need to call the private rather than public method, to prevent - // recursion, double prefixing, double encoding and double hooks. - return this.db._batch(this[kEncoded], options) - } else { - throw new ModuleError('Batch is not open: cannot call write() after write() or close()', { - code: 'LEVEL_BATCH_NOT_OPEN' - }) - } + // Need to call the private rather than public method, to prevent + // recursion, double prefixing, double encoding and double hooks. + return this.db._batch(this[kEncoded], options) } } diff --git a/test/chained-batch-test.js b/test/chained-batch-test.js index bf7d3d2..29bebff 100644 --- a/test/chained-batch-test.js +++ b/test/chained-batch-test.js @@ -186,6 +186,52 @@ exports.batch = function (test, testCommon) { ]) }) + test('chained batch requires database to be open', async function (t) { + t.plan(5) + + const db1 = testCommon.factory() + const db2 = testCommon.factory() + + try { + db1.batch() + } catch (err) { + t.is(err.code, 'LEVEL_DATABASE_NOT_OPEN') + } + + await db2.open() + const batch = db2.batch() + await db2.close() + + try { + batch.put('beep', 'boop') + } catch (err) { + t.is(err.code, 'LEVEL_BATCH_NOT_OPEN') + } + + try { + batch.del('456') + } catch (err) { + t.is(err.code, 'LEVEL_BATCH_NOT_OPEN') + } + + try { + batch.clear() + } catch (err) { + t.is(err.code, 'LEVEL_BATCH_NOT_OPEN') + } + + try { + await batch.write() + } catch (err) { + t.is(err.code, 'LEVEL_BATCH_NOT_OPEN') + } + + // Should be a noop (already closed) + await batch.close() + + return Promise.all([db1.close(), db2.close()]) + }) + // NOTE: adapted from levelup test('chained batch with per-operation encoding options', async function (t) { t.plan(2) diff --git a/test/deferred-open-test.js b/test/deferred-open-test.js index 7274535..4786bba 100644 --- a/test/deferred-open-test.js +++ b/test/deferred-open-test.js @@ -37,27 +37,6 @@ exports.all = function (test, testCommon) { return db.close() }) - // NOTE: copied from levelup - test('deferred open(): chained batch() on new database', async function (t) { - // Create database, opens in next tick - const db = testCommon.factory() - const entries = 3 - const batch = db.batch() - - // Add entries with batch(), these should be deferred until the database is actually open - for (let i = 1; i <= entries; i++) { - batch.put('k' + i, 'v' + i) - } - - t.is(db.status, 'opening') - t.is(batch.length, entries) - - await batch.write() - await verifyValues(t, db, entries) - - return db.close() - }) - // NOTE: copied from levelup test('deferred open(): value of deferred operation is not stringified', async function (t) { const db = testCommon.factory({ valueEncoding: 'json' }) diff --git a/test/events/write.js b/test/events/write.js index 2f2e17f..054b789 100644 --- a/test/events/write.js +++ b/test/events/write.js @@ -2,7 +2,12 @@ module.exports = function (test, testCommon) { for (const deferred of [false, true]) { - for (const method of ['batch', 'chained batch', 'singular']) { + // Chained batch does not support deferred open + const batchMethods = deferred ? ['batch'] : ['batch', 'chained batch'] + const allMethods = batchMethods.concat(['singular']) + + for (const method of allMethods) { + // db.put() and db.del() do not support the sublevel option for (const withSublevel of (method === 'singular' ? [false] : [false, true])) { test(`db emits write event for ${method} put operation (deferred: ${deferred}, sublevel: ${withSublevel})`, async function (t) { t.plan(1) @@ -100,7 +105,7 @@ module.exports = function (test, testCommon) { } } - for (const method of ['batch', 'chained batch']) { + for (const method of batchMethods) { test(`db emits write event for multiple ${method} operations (deferred: ${deferred})`, async function (t) { t.plan(1) @@ -124,7 +129,7 @@ module.exports = function (test, testCommon) { }) } - for (const method of ['batch', 'chained batch', 'singular']) { + for (const method of allMethods) { test(`db emits write event for ${method} operation in favor of deprecated events (deferred: ${deferred})`, async function (t) { t.plan(5) diff --git a/test/self.js b/test/self.js index 609e943..210ddf8 100644 --- a/test/self.js +++ b/test/self.js @@ -478,12 +478,12 @@ test('test chained batch() (custom _chainedBatch) extensibility', async function t.is(spy.getCall(1).thisValue, test, '`this` on _chainedBatch() was correct') }) -test('test AbstractChainedBatch extensibility', function (t) { +test('test AbstractChainedBatch extensibility', async function (t) { const Batch = implement(AbstractChainedBatch) const db = testCommon.factory() + await db.open() const test = new Batch(db) t.ok(test.db === db, 'instance has db reference') - t.end() }) test('test AbstractChainedBatch expects a db', function (t) { @@ -868,7 +868,6 @@ require('./self/abstract-iterator-test') require('./self/iterator-test') require('./self/deferred-iterator-test') require('./self/deferred-operations-test') -require('./self/deferred-chained-batch-test') require('./self/async-iterator-test') require('./self/encoding-test') require('./self/sublevel-test') diff --git a/test/self/deferred-chained-batch-test.js b/test/self/deferred-chained-batch-test.js deleted file mode 100644 index 91b96f2..0000000 --- a/test/self/deferred-chained-batch-test.js +++ /dev/null @@ -1,94 +0,0 @@ -'use strict' - -const test = require('tape') -const { mockLevel } = require('../util') -const { DefaultChainedBatch } = require('../../lib/default-chained-batch') -const identity = (v) => v - -// NOTE: adapted from deferred-leveldown -test('deferred chained batch encodes once', async function (t) { - t.plan(8) - - let called = false - - const keyEncoding = { - format: 'utf8', - encode (key) { - t.is(called, false, 'not yet called') - t.is(key, 'foo') - return key.toUpperCase() - }, - decode: identity - } - - const valueEncoding = { - format: 'utf8', - encode (value) { - t.is(called, false, 'not yet called') - t.is(value, 'bar') - return value.toUpperCase() - }, - decode: identity - } - - const db = mockLevel({ - async _batch (array, options) { - called = true - t.is(array[0] && array[0].key, 'FOO') - t.is(array[0] && array[0].value, 'BAR') - }, - async _open (options) { - t.is(called, false, 'not yet called') - } - }, { encodings: { utf8: true } }, { - keyEncoding, - valueEncoding - }) - - db.once('open', function () { - t.is(called, true, 'called') - }) - - return db.batch().put('foo', 'bar').write() -}) - -test('deferred chained batch is closed upon failed open', function (t) { - t.plan(6) - - const db = mockLevel({ - async _open (options) { - t.pass('opening') - throw new Error('_open error') - }, - async _batch () { - t.fail('should not be called') - } - }) - - const batch = db.batch() - t.ok(batch instanceof DefaultChainedBatch) - - batch.put('foo', 'bar') - batch.del('123') - - batch.write().then(t.fail.bind(t), function (err) { - t.is(err.code, 'LEVEL_BATCH_NOT_OPEN') - - // Should account for userland code that ignores errors - try { - batch.put('beep', 'boop') - } catch (err) { - t.is(err && err.code, 'LEVEL_BATCH_NOT_OPEN') - } - - try { - batch.del('456') - } catch (err) { - t.is(err && err.code, 'LEVEL_BATCH_NOT_OPEN') - } - - batch.write().catch(function (err) { - t.is(err && err.code, 'LEVEL_BATCH_NOT_OPEN') - }) - }) -}) diff --git a/test/self/deferred-operations-test.js b/test/self/deferred-operations-test.js index b72ee4b..78f260d 100644 --- a/test/self/deferred-operations-test.js +++ b/test/self/deferred-operations-test.js @@ -75,10 +75,10 @@ test('deferred operations are called in order', function (t) { { type: 'put', key: '041', value: 'b' } ]) const it = db.iterator() - db.batch() - .put('050', 'c') - .put('051', 'd') - .write() + db.batch([ + { type: 'put', key: '050', value: 'c' }, + { type: 'put', key: '051', value: 'd' } + ]) it.next() db.clear({ gt: '060' }) diff --git a/test/self/encoding-test.js b/test/self/encoding-test.js index b3bdecf..5a8c7ba 100644 --- a/test/self/encoding-test.js +++ b/test/self/encoding-test.js @@ -217,85 +217,57 @@ for (const deferred of [false, true]) { }) // NOTE: adapted from encoding-down - test(`chainedBatch.put() and del() encode utf8 key and value (deferred: ${deferred})`, async function (t) { - t.plan(deferred ? 2 : 5) - - let db - - if (deferred) { - db = mockLevel({ - async _batch (array, options) { - t.same(array, [ - { type: 'put', key: '1', value: '2', keyEncoding: 'utf8', valueEncoding: 'utf8' }, - { type: 'del', key: '3', keyEncoding: 'utf8' } - ]) - t.same(options, {}) - } - }, utf8Manifest) - } else { - db = mockLevel({ - _chainedBatch () { - return mockChainedBatch(this, { - _put: function (key, value, options) { - t.same({ key, value }, { key: '1', value: '2' }) - - // May contain additional options just because it's cheaper to not remove them - t.is(options.keyEncoding, 'utf8') - t.is(options.valueEncoding, 'utf8') - }, - _del: function (key, options) { - t.is(key, '3') - t.is(options.keyEncoding, 'utf8') - } - }) - } - }, utf8Manifest) - } + deferred || test('chainedBatch.put() and del() encode utf8 key and value', async function (t) { + t.plan(5) - if (!deferred) await db.open() + const db = mockLevel({ + _chainedBatch () { + return mockChainedBatch(this, { + _put: function (key, value, options) { + t.same({ key, value }, { key: '1', value: '2' }) + + // May contain additional options just because it's cheaper to not remove them + t.is(options.keyEncoding, 'utf8') + t.is(options.valueEncoding, 'utf8') + }, + _del: function (key, options) { + t.is(key, '3') + t.is(options.keyEncoding, 'utf8') + } + }) + } + }, utf8Manifest) + + await db.open() await db.batch().put(1, 2).del(3).write() }) // NOTE: adapted from encoding-down - test(`chainedBatch.put() and del() take encoding options (deferred: ${deferred})`, async function (t) { - t.plan(deferred ? 2 : 5) - - let db + deferred || test('chainedBatch.put() and del() take encoding options', async function (t) { + t.plan(5) const putOptions = { keyEncoding: 'json', valueEncoding: 'json' } const delOptions = { keyEncoding: 'json' } - if (deferred) { - db = mockLevel({ - async _batch (array, options) { - t.same(array, [ - { type: 'put', key: '"1"', value: '{"x":[2]}', keyEncoding: 'utf8', valueEncoding: 'utf8' }, - { type: 'del', key: '"3"', keyEncoding: 'utf8' } - ]) - t.same(options, {}) - } - }, utf8Manifest) - } else { - db = mockLevel({ - _chainedBatch () { - return mockChainedBatch(this, { - _put: function (key, value, options) { - t.same({ key, value }, { key: '"1"', value: '{"x":[2]}' }) - - // May contain additional options just because it's cheaper to not remove them - t.is(options.keyEncoding, 'utf8') - t.is(options.valueEncoding, 'utf8') - }, - _del: function (key, options) { - t.is(key, '"3"') - t.is(options.keyEncoding, 'utf8') - } - }) - } - }, utf8Manifest) - } + const db = mockLevel({ + _chainedBatch () { + return mockChainedBatch(this, { + _put: function (key, value, options) { + t.same({ key, value }, { key: '"1"', value: '{"x":[2]}' }) + + // May contain additional options just because it's cheaper to not remove them + t.is(options.keyEncoding, 'utf8') + t.is(options.valueEncoding, 'utf8') + }, + _del: function (key, options) { + t.is(key, '"3"') + t.is(options.keyEncoding, 'utf8') + } + }) + } + }, utf8Manifest) - if (!deferred) await db.open() + await db.open() await db.batch().put('1', { x: [2] }, putOptions).del('3', delOptions).write() }) diff --git a/test/self/sublevel-test.js b/test/self/sublevel-test.js index 3a2f5c8..087b8e2 100644 --- a/test/self/sublevel-test.js +++ b/test/self/sublevel-test.js @@ -829,7 +829,8 @@ test('sublevel encodings', function (t) { }) for (const chained of [false, true]) { - for (const deferred of [false, true]) { + // Chained batch does not support deferred open + for (const deferred of (chained ? [false] : [false, true])) { test(`batch() with sublevel per operation (chained: ${chained}, deferred: ${deferred})`, async function (t) { t.plan(6)