From a8dc520c281d9e158a995f2bc5ed08fcc65cd09c Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sat, 25 Sep 2021 13:59:18 +0200 Subject: [PATCH] Add `db.getMany(keys)` --- README.md | 21 +++- abstract-leveldown.js | 80 +++++++++++++ lib/common.js | 9 ++ package.json | 4 +- test/common.js | 1 + test/get-many-test.js | 240 +++++++++++++++++++++++++++++++++++++++ test/get-test.js | 51 ++++----- test/index.js | 4 + test/put-get-del-test.js | 33 ++++-- test/self.js | 3 + test/util.js | 14 +++ 11 files changed, 422 insertions(+), 38 deletions(-) create mode 100644 lib/common.js create mode 100644 test/get-many-test.js diff --git a/README.md b/README.md index 147bc60..4f9b90c 100644 --- a/README.md +++ b/README.md @@ -218,7 +218,17 @@ Get a value from the store by `key`. The optional `options` object may contain: - `asBuffer` _(boolean, default: `true`)_: Whether to return the `value` as a Buffer. If `false`, the returned type depends on the implementation. -The `callback` function will be called with an `Error` if the operation failed for any reason. If successful the first argument will be `null` and the second argument will be the value. +The `callback` function will be called with an `Error` if the operation failed for any reason, including if the key was not found. If successful the first argument will be `null` and the second argument will be the value. + +### `db.getMany(keys[, options][, callback])` + +Get multiple values from the store by an array of `keys`. The optional `options` object may contain: + +- `asBuffer` _(boolean, default: `true`)_: Whether to return the `value` as a Buffer. If `false`, the returned type depends on the implementation. + +The `callback` function will be called with an `Error` if the operation failed for any reason. If successful the first argument will be `null` and the second argument will be an array of values with the same order as `keys`. If a key was not found, the relevant value will be `undefined`. + +If no callback is provided, a promise is returned. ### `db.put(key, value[, options], callback)` @@ -435,6 +445,14 @@ Get a value by `key`. The `options` object will always have the following proper The default `_get()` invokes `callback` on a next tick with a `NotFound` error. It must be overridden. +### `db._getMany(keys, options, callback)` + +**This new method is optional for the time being. To enable its tests, set the [`getMany` option of the test suite](#excluding-tests) to `true`.** + +Get multiple values by an array of `keys`. The `options` object will always have the following properties: `asBuffer`. If an error occurs, call the `callback` function with an `Error`. Otherwise call `callback` with `null` as the first argument and an array of values as the second. If a key does not exist, set the relevant value to `undefined`. + +The default `_getMany()` invokes `callback` on a next tick with an array of values that is equal in length to `keys` and is filled with `undefined`. It must be overridden to support `getMany()` but this is currently an opt-in feature. If the implementation does support `getMany()` then `db.supports.getMany` must be set to true via the [constructor](#db--abstractleveldownmanifest). + ### `db._put(key, value, options, callback)` Store a new entry or overwrite an existing entry. There are no default options but `options` will always be an object. If putting failed, call the `callback` function with an `Error`. Otherwise call `callback` without any arguments. @@ -581,6 +599,7 @@ This also serves as a signal to users of your implementation. The following opti - `bufferKeys`: set to `false` if binary keys are not supported by the underlying storage - `seek`: set to `false` if your `iterator` does not implement `_seek` - `clear`: defaults to `false` until a next major release. Set to `true` if your implementation either implements `_clear()` itself or is suitable to use the default implementation of `_clear()` (which requires binary key support). +- `getMany`: defaults to `false` until a next major release. Set to `true` if your implementation implements `_getMany()`. - `snapshots`: set to `false` if any of the following is true: - Reads don't operate on a [snapshot](#iterator) - Snapshots are created asynchronously diff --git a/abstract-leveldown.js b/abstract-leveldown.js index eaa5129..ffe5a22 100644 --- a/abstract-leveldown.js +++ b/abstract-leveldown.js @@ -2,8 +2,12 @@ const supports = require('level-supports') const isBuffer = require('is-buffer') +const catering = require('catering') const AbstractIterator = require('./abstract-iterator') const AbstractChainedBatch = require('./abstract-chained-batch') +const getCallback = require('./lib/common').getCallback +const getOptions = require('./lib/common').getOptions + const hasOwnProperty = Object.prototype.hasOwnProperty const rangeOptions = ['lt', 'lte', 'gt', 'gte'] @@ -90,6 +94,51 @@ AbstractLevelDOWN.prototype._get = function (key, options, callback) { this._nextTick(function () { callback(new Error('NotFound')) }) } +AbstractLevelDOWN.prototype.getMany = function (keys, options, callback) { + callback = getCallback(options, callback) + callback = catering.fromCallback(callback) + options = getOptions(options) + + if (maybeError(this, callback)) { + return callback.promise + } + + if (!Array.isArray(keys)) { + this._nextTick(callback, new Error('getMany() requires an array argument')) + return callback.promise + } + + if (keys.length === 0) { + this._nextTick(callback, null, []) + return callback.promise + } + + if (typeof options.asBuffer !== 'boolean') { + options = { ...options, asBuffer: true } + } + + const serialized = new Array(keys.length) + + for (let i = 0; i < keys.length; i++) { + const key = keys[i] + const err = this._checkKey(key) + + if (err) { + this._nextTick(callback, err) + return callback.promise + } + + serialized[i] = this._serializeKey(key) + } + + this._getMany(serialized, options, callback) + return callback.promise +} + +AbstractLevelDOWN.prototype._getMany = function (keys, options, callback) { + this._nextTick(callback, null, new Array(keys.length).fill(undefined)) +} + AbstractLevelDOWN.prototype.put = function (key, value, options, callback) { if (typeof options === 'function') callback = options @@ -315,9 +364,40 @@ AbstractLevelDOWN.prototype._checkValue = function (value) { } } +// Undocumented, only here for levelup API parity +AbstractLevelDOWN.prototype.isOpen = function () { + return this.status === 'open' +} + +AbstractLevelDOWN.prototype.isClosed = function () { + return this.status === 'new' || this.status === 'closing' || this.status === 'closed' +} + // Expose browser-compatible nextTick for dependents // TODO: rename _nextTick to _queueMicrotask // TODO: after we drop node 10, also use queueMicrotask in node AbstractLevelDOWN.prototype._nextTick = require('./next-tick') module.exports = AbstractLevelDOWN + +function maybeError (db, callback) { + if (db.status !== 'open') { + // Exemption to support deferredOpen + if (db.type === 'deferred-leveldown') { + return false + } + + // TODO: deferred-leveldown and levelup are inconsistent: the former allows + // operations on any db status, the latter only if 'opening' or 'open'. We + // should define the scope of the deferredOpen feature. For now, pick the + // levelup behavior. Ref https://github.com/Level/community/issues/58 + if (db.supports.deferredOpen && db.status === 'opening') { + return false + } + + db._nextTick(callback, new Error('Database is not open')) + return true + } + + return false +} diff --git a/lib/common.js b/lib/common.js new file mode 100644 index 0000000..c354fa8 --- /dev/null +++ b/lib/common.js @@ -0,0 +1,9 @@ +'use strict' + +exports.getCallback = function (options, callback) { + return typeof options === 'function' ? options : callback +} + +exports.getOptions = function (options) { + return typeof options === 'object' && options !== null ? options : {} +} diff --git a/package.json b/package.json index edf63a9..122b226 100644 --- a/package.json +++ b/package.json @@ -21,6 +21,7 @@ "abstract-iterator.js", "abstract-leveldown.js", "index.js", + "lib", "next-tick-browser.js", "next-tick.js", "test", @@ -30,9 +31,10 @@ ], "dependencies": { "buffer": "^6.0.3", + "catering": "^2.0.0", "is-buffer": "^2.0.5", "level-concat-iterator": "^3.0.0", - "level-supports": "^2.0.0", + "level-supports": "^2.0.1", "queue-microtask": "^1.2.3" }, "devDependencies": { diff --git a/test/common.js b/test/common.js index dfb3a6e..5c672fa 100644 --- a/test/common.js +++ b/test/common.js @@ -31,6 +31,7 @@ function testCommon (options) { snapshots: options.snapshots !== false, seek: options.seek !== false, clear: !!options.clear, + getMany: !!options.getMany, // Support running test suite on a levelup db. All options below this line // are undocumented and should not be used by abstract-leveldown db's (yet). diff --git a/test/get-many-test.js b/test/get-many-test.js new file mode 100644 index 0000000..2b58787 --- /dev/null +++ b/test/get-many-test.js @@ -0,0 +1,240 @@ +'use strict' + +const isBuffer = require('is-buffer') +const isTypedArray = require('./util').isTypedArray +const assertAsync = require('./util').assertAsync + +let db + +exports.setUp = function (test, testCommon) { + test('setUp db', function (t) { + db = testCommon.factory() + db.open(t.end.bind(t)) + }) +} + +exports.args = function (test, testCommon) { + test('test getMany() requires an array argument (callback)', function (t) { + t.plan(4) + + db.getMany('foo', assertAsync(t, function (err) { + t.is(err && err.message, 'getMany() requires an array argument') + })) + db.getMany('foo', {}, assertAsync(t, function (err) { + t.is(err && err.message, 'getMany() requires an array argument') + })) + }) + + test('test getMany() requires an array argument (promise)', function (t) { + t.plan(3) + + db.getMany().catch(function (err) { + t.is(err && err.message, 'getMany() requires an array argument') + }) + db.getMany('foo').catch(function (err) { + t.is(err && err.message, 'getMany() requires an array argument') + }) + db.getMany('foo', {}).catch(function (err) { + t.is(err && err.message, 'getMany() requires an array argument') + }) + }) + + testCommon.serialize && test('test custom _serialize*', function (t) { + t.plan(3) + const db = testCommon.factory() + db._serializeKey = function (data) { return data } + db._getMany = function (keys, options, callback) { + t.same(keys, [{ foo: 'bar' }]) + this._nextTick(callback, null, ['foo']) + } + db.open(function () { + db.getMany([{ foo: 'bar' }], function (err) { + t.error(err) + db.close(t.error.bind(t)) + }) + }) + }) +} + +exports.getMany = function (test, testCommon) { + test('test getMany() support is reflected in manifest', function (t) { + t.is(db.supports && db.supports.getMany, true) + t.end() + }) + + test('test simple getMany()', function (t) { + db.put('foo', 'bar', function (err) { + t.error(err) + + function verify (err, values) { + t.error(err) + t.ok(Array.isArray(values), 'got an array') + t.is(values.length, 1, 'array has 1 element') + + const value = values[0] + let result + + if (!testCommon.encodings) { + t.isNot(typeof value, 'string', 'should not be string by default') + + if (isTypedArray(value)) { + result = String.fromCharCode.apply(null, new Uint16Array(value)) + } else { + t.ok(isBuffer(value)) + try { + result = value.toString() + } catch (e) { + t.error(e, 'should not throw when converting value to a string') + } + } + } else { + result = value + } + + t.is(result, 'bar') + } + + db.getMany(['foo'], function (err, values) { + verify(err, values) + + db.getMany(['foo'], {}, function (err, values) { + verify(err, values) + + db.getMany(['foo'], { asBuffer: false }, function (err, values) { + t.error(err) + t.is(values && typeof values[0], 'string', 'should be string if not buffer') + t.same(values, ['bar']) + t.end() + }) + }) + }) + }) + }) + + test('test getMany() with multiple keys', function (t) { + t.plan(5) + + db.put('beep', 'boop', function (err) { + t.ifError(err) + + db.getMany(['foo', 'beep'], { asBuffer: false }, function (err, values) { + t.ifError(err) + t.same(values, ['bar', 'boop']) + }) + + db.getMany(['beep', 'foo'], { asBuffer: false }, function (err, values) { + t.ifError(err) + t.same(values, ['boop', 'bar'], 'maintains order of input keys') + }) + }) + }) + + test('test empty getMany()', function (t) { + t.plan(2 * 3) + + for (const asBuffer in [true, false]) { + db.getMany([], { asBuffer }, assertAsync(t, function (err, values) { + t.ifError(err) + t.same(values, []) + })) + } + }) + + test('test not-found getMany()', function (t) { + t.plan(2 * 3) + + for (const asBuffer in [true, false]) { + db.getMany(['nope', 'another'], { asBuffer }, assertAsync(t, function (err, values) { + t.ifError(err) + t.same(values, [undefined, undefined]) + })) + } + }) + + test('test getMany() with promise', async function (t) { + t.same(await db.getMany(['foo'], { asBuffer: false }), ['bar']) + t.same(await db.getMany(['beep'], { asBuffer: false }), ['boop']) + t.same(await db.getMany(['foo', 'beep'], { asBuffer: false }), ['bar', 'boop']) + t.same(await db.getMany(['beep', 'foo'], { asBuffer: false }), ['boop', 'bar']) + t.same(await db.getMany(['beep', 'foo', 'nope'], { asBuffer: false }), ['boop', 'bar', undefined]) + t.same(await db.getMany([], { asBuffer: false }), []) + }) + + test('test simultaneous getMany()', function (t) { + db.put('hello', 'world', function (err) { + t.error(err) + + let completed = 0 + const done = function () { + if (++completed === 20) t.end() + } + + for (let i = 0; i < 10; ++i) { + db.getMany(['hello'], function (err, values) { + t.error(err) + t.is(values.length, 1) + t.is(values[0] && values[0].toString(), 'world') + done() + }) + } + + for (let i = 0; i < 10; ++i) { + db.getMany(['not found'], function (err, values) { + t.error(err) + t.same(values, [undefined]) + done() + }) + } + }) + }) + + testCommon.deferredOpen || test('test getMany() on new db', function (t) { + const db = testCommon.factory() + + if (db.type === 'deferred-leveldown') { + t.pass('exempted') + return t.end() + } + + db.getMany([], assertAsync(t, function (err) { + t.is(err && err.message, 'Database is not open') + db.close(t.end.bind(t)) + })) + }) + + test('test getMany() on closing db', function (t) { + const db = testCommon.factory() + + if (db.type === 'deferred-leveldown') { + t.pass('exempted') + return t.end() + } + + t.plan(4) + + db.open(function (err) { + t.ifError(err) + + db.close(function (err) { + t.ifError(err) + }) + + db.getMany([], assertAsync(t, function (err) { + t.is(err && err.message, 'Database is not open') + })) + }) + }) +} + +exports.tearDown = function (test, testCommon) { + test('tearDown', function (t) { + db.close(t.end.bind(t)) + }) +} + +exports.all = function (test, testCommon) { + exports.setUp(test, testCommon) + exports.args(test, testCommon) + exports.getMany(test, testCommon) + exports.tearDown(test, testCommon) +} diff --git a/test/get-test.js b/test/get-test.js index ac24aea..d3b625c 100644 --- a/test/get-test.js +++ b/test/get-test.js @@ -1,5 +1,6 @@ 'use strict' +const isBuffer = require('is-buffer') const verifyNotFoundError = require('./util').verifyNotFoundError const isTypedArray = require('./util').isTypedArray @@ -46,7 +47,7 @@ exports.args = function (test, testCommon) { const db = testCommon.factory() db._serializeKey = function (data) { return data } db._get = function (key, options, callback) { - t.deepEqual(key, { foo: 'bar' }) + t.same(key, { foo: 'bar' }) this._nextTick(callback) } db.open(function () { @@ -68,12 +69,12 @@ exports.get = function (test, testCommon) { let result if (!testCommon.encodings) { - t.ok(typeof value !== 'string', 'should not be string by default') + t.isNot(typeof value, 'string', 'should not be string by default') if (isTypedArray(value)) { result = String.fromCharCode.apply(null, new Uint16Array(value)) } else { - t.ok(typeof Buffer !== 'undefined' && value instanceof Buffer) + t.ok(isBuffer(value)) try { result = value.toString() } catch (e) { @@ -84,7 +85,7 @@ exports.get = function (test, testCommon) { result = value } - t.equal(result, 'bar') + t.is(result, 'bar') db.get('foo', {}, function (err, value) { // same but with {} t.error(err) @@ -97,7 +98,7 @@ exports.get = function (test, testCommon) { if (isTypedArray(value)) { result = String.fromCharCode.apply(null, new Uint16Array(value)) } else { - t.ok(typeof Buffer !== 'undefined' && value instanceof Buffer) + t.ok(isBuffer(value)) try { result = value.toString() } catch (e) { @@ -108,12 +109,12 @@ exports.get = function (test, testCommon) { result = value } - t.equal(result, 'bar') + t.is(result, 'bar') db.get('foo', { asBuffer: false }, function (err, value) { t.error(err) t.ok(typeof value === 'string', 'should be string if not buffer') - t.equal(value, 'bar') + t.is(value, 'bar') t.end() }) }) @@ -121,25 +122,23 @@ exports.get = function (test, testCommon) { }) }) - test('test simultaniously get()', function (t) { + test('test simultaneous get()', function (t) { db.put('hello', 'world', function (err) { t.error(err) - let r = 0 + let completed = 0 const done = function () { - if (++r === 20) { t.end() } + if (++completed === 20) t.end() } - let i = 0 - let j = 0 - for (; i < 10; ++i) { + for (let i = 0; i < 10; ++i) { db.get('hello', function (err, value) { t.error(err) - t.equal(value.toString(), 'world') + t.is(value.toString(), 'world') done() }) } - for (; j < 10; ++j) { + for (let i = 0; i < 10; ++i) { db.get('not found', function (err, value) { t.ok(err, 'should error') t.ok(verifyNotFoundError(err), 'should have correct error message') @@ -151,22 +150,18 @@ exports.get = function (test, testCommon) { }) test('test get() not found error is asynchronous', function (t) { - t.plan(5) + t.plan(4) - db.put('hello', 'world', function (err) { - t.error(err) - - let async = false + let async = false - db.get('not found', function (err, value) { - t.ok(err, 'should error') - t.ok(verifyNotFoundError(err), 'should have correct error message') - t.ok(typeof value === 'undefined', 'value is undefined') - t.ok(async, 'callback is asynchronous') - }) - - async = true + db.get('not found', function (err, value) { + t.ok(err, 'should error') + t.ok(verifyNotFoundError(err), 'should have correct error message') + t.ok(typeof value === 'undefined', 'value is undefined') + t.ok(async, 'callback is asynchronous') }) + + async = true }) } diff --git a/test/index.js b/test/index.js index fbd65b2..d761d38 100644 --- a/test/index.js +++ b/test/index.js @@ -26,6 +26,10 @@ function suite (options) { require('./del-test').all(test, testCommon) require('./put-get-del-test').all(test, testCommon) + if (testCommon.getMany) { + require('./get-many-test').all(test, testCommon) + } + require('./batch-test').all(test, testCommon) require('./chained-batch-test').all(test, testCommon) diff --git a/test/put-get-del-test.js b/test/put-get-del-test.js index cb7783f..ab85cfa 100644 --- a/test/put-get-del-test.js +++ b/test/put-get-del-test.js @@ -1,11 +1,12 @@ 'use strict' const verifyNotFoundError = require('./util').verifyNotFoundError +const assertAsync = require('./util').assertAsync const testBuffer = Buffer.from('testbuffer') let db -function makeGetDelErrorTests (test, type, key, expectedError) { +function makeGetDelErrorTests (test, testCommon, type, key, expectedError) { test('test get() with ' + type + ' causes error', function (t) { let async = false @@ -33,6 +34,22 @@ function makeGetDelErrorTests (test, type, key, expectedError) { async = true }) + + testCommon.getMany && test('test getMany() with ' + type + ' causes error', function (t) { + t.plan(2 * 4) + + db.getMany([key], assertAsync(t, function (err) { + t.ok(err, 'has error') + t.ok(err instanceof Error) + t.ok(err.message.match(expectedError), 'correct error message') + })) + + db.getMany(['valid', key], assertAsync(t, function (err) { + t.ok(err, 'has error') + t.ok(err instanceof Error) + t.ok(err.message.match(expectedError), 'correct error message') + })) + }) } function makePutErrorTest (test, type, key, value, expectedError) { @@ -96,8 +113,8 @@ function makePutGetDelSuccessfulTest (test, testCommon, type, key, value, expect }) } -function makeErrorKeyTest (test, type, key, expectedError) { - makeGetDelErrorTests(test, type, key, expectedError) +function makeErrorKeyTest (test, testCommon, type, key, expectedError) { + makeGetDelErrorTests(test, testCommon, type, key, expectedError) makePutErrorTest(test, type, key, 'foo', expectedError) } @@ -110,11 +127,11 @@ exports.setUp = function (test, testCommon) { } exports.errorKeys = function (test, testCommon) { - makeErrorKeyTest(test, 'null key', null, /key cannot be `null` or `undefined`/) - makeErrorKeyTest(test, 'undefined key', undefined, /key cannot be `null` or `undefined`/) - makeErrorKeyTest(test, 'empty String key', '', /key cannot be an empty String/) - makeErrorKeyTest(test, 'empty Buffer key', Buffer.alloc(0), /key cannot be an empty \w*Buffer/) - makeErrorKeyTest(test, 'empty Array key', [], /key cannot be an empty Array/) + makeErrorKeyTest(test, testCommon, 'null key', null, /key cannot be `null` or `undefined`/) + makeErrorKeyTest(test, testCommon, 'undefined key', undefined, /key cannot be `null` or `undefined`/) + makeErrorKeyTest(test, testCommon, 'empty String key', '', /key cannot be an empty String/) + makeErrorKeyTest(test, testCommon, 'empty Buffer key', Buffer.alloc(0), /key cannot be an empty \w*Buffer/) + makeErrorKeyTest(test, testCommon, 'empty Array key', [], /key cannot be an empty Array/) } exports.errorValues = function (test, testCommon) { diff --git a/test/self.js b/test/self.js index 6bf632f..0e6a38d 100644 --- a/test/self.js +++ b/test/self.js @@ -36,6 +36,9 @@ require('./del-test').args(test, testCommon) require('./get-test').setUp(test, testCommon) require('./get-test').args(test, testCommon) +require('./get-many-test').setUp(test, testCommon) +require('./get-many-test').args(test, testCommon) + require('./put-test').setUp(test, testCommon) require('./put-test').args(test, testCommon) diff --git a/test/util.js b/test/util.js index 5d83347..c97a838 100644 --- a/test/util.js +++ b/test/util.js @@ -1,5 +1,6 @@ 'use strict' +const nextTick = require('../next-tick') const nfre = /NotFound/i exports.verifyNotFoundError = function verifyNotFoundError (err) { @@ -10,3 +11,16 @@ exports.isTypedArray = function isTypedArray (value) { return (typeof ArrayBuffer !== 'undefined' && value instanceof ArrayBuffer) || (typeof Uint8Array !== 'undefined' && value instanceof Uint8Array) } + +exports.assertAsync = function (t, fn) { + let called = false + + nextTick(function () { + t.is(called, false, 'callback is asynchronous') + }) + + return function (...args) { + called = true + return fn(...args) + } +}