From 3630442d47be97bd583519467f373fdebcc5a040 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sun, 8 Jul 2018 18:47:35 +0200 Subject: [PATCH 1/8] make AbstractChainedBatch#_clear consistent with _put and _del --- abstract-chained-batch.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/abstract-chained-batch.js b/abstract-chained-batch.js index ffad2bde..a23c7b52 100644 --- a/abstract-chained-batch.js +++ b/abstract-chained-batch.js @@ -54,13 +54,14 @@ AbstractChainedBatch.prototype._del = function (key) { AbstractChainedBatch.prototype.clear = function () { this._checkWritten() - this._operations = [] this._clear() return this } -AbstractChainedBatch.prototype._clear = function noop () {} +AbstractChainedBatch.prototype._clear = function () { + this._operations = [] +} AbstractChainedBatch.prototype.write = function (options, callback) { this._checkWritten() From 337cd37a93ef7ebea3f239ffddb8ec1604ea6cae Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sun, 8 Jul 2018 18:56:01 +0200 Subject: [PATCH 2/8] add AbstractChainedBatch#_write with options (closes #200) --- abstract-chained-batch.js | 12 ++++-------- test/self.js | 7 ++++--- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/abstract-chained-batch.js b/abstract-chained-batch.js index a23c7b52..b3c2cb8b 100644 --- a/abstract-chained-batch.js +++ b/abstract-chained-batch.js @@ -73,15 +73,11 @@ AbstractChainedBatch.prototype.write = function (options, callback) { if (typeof options !== 'object') { options = {} } this._written = true + this._write(options, callback) +} - // @ts-ignore - if (typeof this._write === 'function') { return this._write(callback) } - - if (typeof this._db._batch === 'function') { - return this._db._batch(this._operations, options, callback) - } - - process.nextTick(callback) +AbstractChainedBatch.prototype._write = function (options, callback) { + this._db._batch(this._operations, options, callback) } module.exports = AbstractChainedBatch diff --git a/test/self.js b/test/self.js index b5e24a97..ce4bfc31 100644 --- a/test/self.js +++ b/test/self.js @@ -350,11 +350,12 @@ test('test write() extensibility', function (t) { t.equal(spy.callCount, 1, 'got _write() call') t.equal(spy.getCall(0).thisValue, test, '`this` on _write() was correct') - t.equal(spy.getCall(0).args.length, 1, 'got one argument') + t.equal(spy.getCall(0).args.length, 2, 'got two arguments') + t.same(spy.getCall(0).args[0], {}, 'got options') // awkward here cause of nextTick & an internal wrapped cb - t.equal(typeof spy.getCall(0).args[0], 'function', 'got a callback function') + t.equal(typeof spy.getCall(0).args[1], 'function', 'got a callback function') t.equal(spycb.callCount, 0, 'spycb not called') - spy.getCall(0).args[0]() + spy.getCall(0).args[1]() t.equal(spycb.callCount, 1, 'spycb called, i.e. was our cb argument') t.end() }) From fb0390a0d37bf5c8271f8941adfb8547685fd824 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sun, 8 Jul 2018 19:03:30 +0200 Subject: [PATCH 3/8] fix AbstractChainedBatch#write(null) --- abstract-chained-batch.js | 4 +++- test/self.js | 12 ++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/abstract-chained-batch.js b/abstract-chained-batch.js index b3c2cb8b..a717cded 100644 --- a/abstract-chained-batch.js +++ b/abstract-chained-batch.js @@ -70,7 +70,9 @@ AbstractChainedBatch.prototype.write = function (options, callback) { if (typeof callback !== 'function') { throw new Error('write() requires a callback argument') } - if (typeof options !== 'object') { options = {} } + if (typeof options !== 'object' || options === null) { + options = {} + } this._written = true this._write(options, callback) diff --git a/test/self.js b/test/self.js index ce4bfc31..857fd653 100644 --- a/test/self.js +++ b/test/self.js @@ -360,6 +360,18 @@ test('test write() extensibility', function (t) { t.end() }) +test('test write() extensibility with null options', function (t) { + var spy = sinon.spy() + var Test = implement(AbstractChainedBatch, { _write: spy }) + var test = new Test('foobar') + + test.write(null, function () {}) + + t.equal(spy.callCount, 1, 'got _write() call') + t.same(spy.getCall(0).args[0], {}, 'got options') + t.end() +}) + test('test put() extensibility', function (t) { var spy = sinon.spy() var expectedKey = 'key' From a7fc3ebcf2e10d73065ed04dc8fa9525514cc9df Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sun, 8 Jul 2018 19:04:57 +0200 Subject: [PATCH 4/8] test AbstractChainedBatch#write(options) --- test/self.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/self.js b/test/self.js index 857fd653..14cdc2d0 100644 --- a/test/self.js +++ b/test/self.js @@ -372,6 +372,18 @@ test('test write() extensibility with null options', function (t) { t.end() }) +test('test write() extensibility with options', function (t) { + var spy = sinon.spy() + var Test = implement(AbstractChainedBatch, { _write: spy }) + var test = new Test('foobar') + + test.write({ test: true }, function () {}) + + t.equal(spy.callCount, 1, 'got _write() call') + t.same(spy.getCall(0).args[0], { test: true }, 'got options') + t.end() +}) + test('test put() extensibility', function (t) { var spy = sinon.spy() var expectedKey = 'key' From e19da29162370fbc55d45f5b419f2f2925aa7e71 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sun, 8 Jul 2018 19:20:17 +0200 Subject: [PATCH 5/8] make db argument of AbstractChainedBatch required --- abstract-chained-batch.js | 4 ++++ test/self.js | 22 +++++++++++++++++----- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/abstract-chained-batch.js b/abstract-chained-batch.js index a717cded..f43d97fb 100644 --- a/abstract-chained-batch.js +++ b/abstract-chained-batch.js @@ -1,4 +1,8 @@ function AbstractChainedBatch (db) { + if (typeof db !== 'object' || db === null) { + throw new TypeError('First argument must be an abstract-leveldown compliant store') + } + this._db = db this._operations = [] this._written = false diff --git a/test/self.js b/test/self.js index 14cdc2d0..46479264 100644 --- a/test/self.js +++ b/test/self.js @@ -335,16 +335,28 @@ test('test chained batch() (custom _chainedBatch) extensibility', function (t) { test('test AbstractChainedBatch extensibility', function (t) { var Test = implement(AbstractChainedBatch) - var test = new Test('foobar') - t.equal(test._db, 'foobar', 'db set on instance') + var test = new Test({ test: true }) + t.same(test._db, { test: true }, 'db set on instance') t.end() }) +test('test AbstractChainedBatch expects a db', function (t) { + t.plan(1) + + var Test = implement(AbstractChainedBatch) + + try { + Test() + } catch (err) { + t.is(err.message, 'First argument must be an abstract-leveldown compliant store') + } +}) + test('test write() extensibility', function (t) { var spy = sinon.spy() var spycb = sinon.spy() var Test = implement(AbstractChainedBatch, { _write: spy }) - var test = new Test('foobar') + var test = new Test({ test: true }) test.write(spycb) @@ -363,7 +375,7 @@ test('test write() extensibility', function (t) { test('test write() extensibility with null options', function (t) { var spy = sinon.spy() var Test = implement(AbstractChainedBatch, { _write: spy }) - var test = new Test('foobar') + var test = new Test({ test: true }) test.write(null, function () {}) @@ -375,7 +387,7 @@ test('test write() extensibility with null options', function (t) { test('test write() extensibility with options', function (t) { var spy = sinon.spy() var Test = implement(AbstractChainedBatch, { _write: spy }) - var test = new Test('foobar') + var test = new Test({ test: true }) test.write({ test: true }, function () {}) From 6af06a4958bca033c7b138cc95da4f5c40ace50a Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sun, 8 Jul 2018 19:22:48 +0200 Subject: [PATCH 6/8] make AbstractChainedBatch#_db required --- abstract-chained-batch.js | 14 +++----------- test/chained-batch-test.js | 5 +++++ 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/abstract-chained-batch.js b/abstract-chained-batch.js index f43d97fb..2c46fe23 100644 --- a/abstract-chained-batch.js +++ b/abstract-chained-batch.js @@ -8,14 +8,6 @@ function AbstractChainedBatch (db) { this._written = false } -AbstractChainedBatch.prototype._serializeKey = function (key) { - return this._db._serializeKey(key) -} - -AbstractChainedBatch.prototype._serializeValue = function (value) { - return this._db._serializeValue(value) -} - AbstractChainedBatch.prototype._checkWritten = function () { if (this._written) { throw new Error('write() already called on this batch') @@ -28,8 +20,8 @@ AbstractChainedBatch.prototype.put = function (key, value) { var err = this._db._checkKey(key, 'key') if (err) { throw err } - key = this._serializeKey(key) - value = this._serializeValue(value) + key = this._db._serializeKey(key) + value = this._db._serializeValue(value) this._put(key, value) @@ -46,7 +38,7 @@ AbstractChainedBatch.prototype.del = function (key) { var err = this._db._checkKey(key, 'key') if (err) { throw err } - key = this._serializeKey(key) + key = this._db._serializeKey(key) this._del(key) return this diff --git a/test/chained-batch-test.js b/test/chained-batch-test.js index 157d4c79..2c6e65f0 100644 --- a/test/chained-batch-test.js +++ b/test/chained-batch-test.js @@ -33,6 +33,11 @@ exports.setUp = function (test, testCommon) { } exports.args = function (test, testCommon) { + test('test batch has _db', function (t) { + t.ok(db.batch()._db === db) + t.end() + }) + test('test batch#put() with missing `value`', function (t) { db.batch().put('foo1') t.end() From ea45c8f549569fe7a5af65eaeb8a47627215aab5 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sun, 8 Jul 2018 19:37:32 +0200 Subject: [PATCH 7/8] future-proof serialize test on chained batch --- test/chained-batch-test.js | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/test/chained-batch-test.js b/test/chained-batch-test.js index 2c6e65f0..b1be0c9e 100644 --- a/test/chained-batch-test.js +++ b/test/chained-batch-test.js @@ -199,22 +199,35 @@ exports.args = function (test, testCommon) { }) test('test custom _serialize*', function (t) { - var _db = Object.create(db) - _db._serializeKey = _db._serializeValue = function (data) { return data } + t.plan(4) + var _db = Object.create(db) var batch = _db.batch() var ops = collectBatchOps(batch) - batch - .put({ foo: 'bar' }, { beep: 'boop' }) - .del({ bar: 'baz' }) + _db._serializeKey = function (key) { + t.same(key, { foo: 'bar' }) + return 'key1' + } + + _db._serializeValue = function (value) { + t.same(value, { beep: 'boop' }) + return 'value1' + } + + batch.put({ foo: 'bar' }, { beep: 'boop' }) + + _db._serializeKey = function (key) { + t.same(key, { bar: 'baz' }) + return 'key2' + } + + batch.del({ bar: 'baz' }) t.deepEqual(ops, [ - { type: 'put', key: { foo: 'bar' }, value: { beep: 'boop' } }, - { type: 'del', key: { bar: 'baz' } } + { type: 'put', key: 'key1', value: 'value1' }, + { type: 'del', key: 'key2' } ]) - - t.end() }) } From 37491e10264708559908fcb59f99fedb9fe2bba6 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Fri, 13 Jul 2018 11:05:59 +0200 Subject: [PATCH 8/8] update chained batch docs --- README.md | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index cd0e0b0e..e315735d 100644 --- a/README.md +++ b/README.md @@ -362,7 +362,7 @@ Free up underlying resources. This method is guaranteed to only be called once. ### `chainedBatch = AbstractChainedBatch(db)` -Provided with the current instance of `abstract-leveldown` by default. +The first argument to this constructor must be an instance of your `AbstractLevelDOWN` implementation. The constructor will set `chainedBatch._db` which is used to access `db._serialize*` and ensures that `db` will not be garbage collected in case there are no other references to it. #### `chainedBatch._put(key, value)` @@ -374,21 +374,11 @@ Queue a `del` operation on this batch. #### `chainedBatch._clear()` -Perform additional cleanup when `clear()` is called. - -#### `chainedBatch._write(callback)` - -If the `_write` method is defined on `chainedBatch`, it must atomically commit the queued operations. If this fails, call the `callback` function with an `Error`. Otherwise call `callback` without any arguments. - -If the `_write` method is not defined, `db._batch` will be used instead. - -#### `chainedBatch._serializeKey(key)` - -A proxy to [`db._serializeKey(key)`](#private-serialize-key). +Clear all queued operations on this batch. -#### `chainedBatch._serializeValue(value)` +#### `chainedBatch._write(options, callback)` -A proxy to [`db._serializeValue(value)`](#private-serialize-value). +The default `_write` method uses `db._batch`. If the `_write` method is overridden it must atomically commit the queued operations. There are no default options but `options` will always be an object. If committing fails, call the `callback` function with an `Error`. Otherwise call `callback` without any arguments. ## Test Suite