Refactor chained batch

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
 

abstract-chained-batch.js

@@ -1,17 +1,13 @@
 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
 }
 
-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')
@@ -24,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)
 
@@ -42,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
@@ -54,13 +50,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()
@@ -69,18 +66,16 @@ 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)
+}
 
-  // @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

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()
@@ -194,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()
   })
 }
 

test/self.js

@@ -335,30 +335,67 @@ 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)
 
   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()
 })
 
+test('test write() extensibility with null options', function (t) {
+  var spy = sinon.spy()
+  var Test = implement(AbstractChainedBatch, { _write: spy })
+  var test = new Test({ test: true })
+
+  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 write() extensibility with options', function (t) {
+  var spy = sinon.spy()
+  var Test = implement(AbstractChainedBatch, { _write: spy })
+  var test = new Test({ test: true })
+
+  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'