From 979d34f42515f24c5854f7e7087c237b108f4263 Mon Sep 17 00:00:00 2001
From: Vincent Weevers <mail@vincentweevers.nl>
Date: Sat, 3 Apr 2021 16:50:13 +0200
Subject: [PATCH] Breaking: remove legacy range options (start & end)
An error is now thrown when an iterator is created with one of these
options. Ref https://github.com/Level/community/issues/86
---
README.md | 6 --
abstract-leveldown.js | 6 +-
test/clear-range-test.js | 4 +-
test/common.js | 8 +--
test/iterator-range-test.js | 128 +-----------------------------------
test/iterator-seek-test.js | 16 -----
test/self.js | 20 +++++-
7 files changed, 31 insertions(+), 157 deletions(-)
diff --git a/README.md b/README.md
index 6f66f56c..daf68305 100644
--- a/README.md
+++ b/README.md
@@ -251,11 +251,6 @@ Returns an [`iterator`](#iterator). Accepts the following range options:
- `reverse` _(boolean, default: `false`)_: iterate entries in reverse order. Beware that a reverse seek can be slower than a forward seek.
- `limit` _(number, default: `-1`)_: limit the number of entries collected by this iterator. This number represents a _maximum_ number of entries and may not be reached if you get to the end of the range first. A value of `-1` means there is no limit. When `reverse=true` the entries with the highest keys will be returned instead of the lowest keys.
-Legacy options:
-
-- `start`: instead use `gte`
-- `end`: instead use `lte`.
-
**Note** Zero-length strings, buffers and arrays as well as `null` and `undefined` are invalid as keys, yet valid as range options. These types are significant in encodings like [`bytewise`](https://github.com/deanlandolt/bytewise) and [`charwise`](https://github.com/dominictarr/charwise) as well as some underlying stores like IndexedDB. Consumers of an implementation should assume that `{ gt: undefined }` is _not_ the same as `{}`. An implementation can choose to:
- [_Serialize_](#db_serializekeykey) or [_encode_][encoding-down] these types to make them meaningful
@@ -570,7 +565,6 @@ This also serves as a signal to users of your implementation. The following opti
- Reads don't operate on a [snapshot](#iterator)
- Snapshots are created asynchronously
- `createIfMissing` and `errorIfExists`: set to `false` if `db._open()` does not support these options.
-- `legacyRange`: set to `false` if your iterator does not support the legacy `start` and `end` range options.
This metadata will be moved to manifests (`db.supports`) in the future.
diff --git a/abstract-leveldown.js b/abstract-leveldown.js
index 5039cf32..5c842ce1 100644
--- a/abstract-leveldown.js
+++ b/abstract-leveldown.js
@@ -5,7 +5,7 @@ var AbstractIterator = require('./abstract-iterator')
var AbstractChainedBatch = require('./abstract-chained-batch')
var nextTick = require('./next-tick')
var hasOwnProperty = Object.prototype.hasOwnProperty
-var rangeOptions = 'start end gt gte lt lte'.split(' ')
+var rangeOptions = ['lt', 'lte', 'gt', 'gte']
function AbstractLevelDOWN (manifest) {
this.status = 'new'
@@ -256,6 +256,10 @@ function cleanRangeOptions (db, options) {
for (var k in options) {
if (!hasOwnProperty.call(options, k)) continue
+ if (k === 'start' || k === 'end') {
+ throw new Error('Legacy range options ("start" and "end") have been removed')
+ }
+
var opt = options[k]
if (isRangeOption(k)) {
diff --git a/test/clear-range-test.js b/test/clear-range-test.js
index 21586318..4fe3a6d2 100644
--- a/test/clear-range-test.js
+++ b/test/clear-range-test.js
@@ -151,12 +151,12 @@ exports.range = function (test, testCommon) {
reverse: true
}, data.slice(0, 51))
- // Starting key is actually '00' so it should avoid it
+ // First key is actually '00' so it should avoid it
rangeTest('lte=0', {
lte: '0'
}, data)
- // Starting key is actually '00' so it should avoid it
+ // First key is actually '00' so it should avoid it
rangeTest('lt=0', {
lt: '0'
}, data)
diff --git a/test/common.js b/test/common.js
index de402e65..0b4d9381 100644
--- a/test/common.js
+++ b/test/common.js
@@ -10,6 +10,10 @@ function testCommon (options) {
throw new TypeError('test must be a function')
}
+ if (options.legacyRange != null) {
+ throw new Error('The legacyRange option has been removed')
+ }
+
return {
test: test,
factory: factory,
@@ -26,10 +30,6 @@ function testCommon (options) {
seek: options.seek !== false,
clear: !!options.clear,
- // Allow skipping 'start' and 'end' tests
- // TODO (next major): drop legacy range options
- legacyRange: options.legacyRange !== false,
-
// 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).
promises: !!options.promises,
diff --git a/test/iterator-range-test.js b/test/iterator-range-test.js
index a691503a..7c65b7d2 100644
--- a/test/iterator-range-test.js
+++ b/test/iterator-range-test.js
@@ -35,12 +35,9 @@ exports.setUp = function (test, testCommon) {
exports.range = function (test, testCommon) {
function rangeTest (name, opts, expected) {
- if (!testCommon.legacyRange && ('start' in opts || 'end' in opts)) {
- return
- }
-
opts.keyAsBuffer = false
opts.valueAsBuffer = false
+
test(name, function (t) {
collectEntries(db.iterator(opts), function (err, result) {
t.error(err)
@@ -55,18 +52,6 @@ exports.range = function (test, testCommon) {
if (!opts.reverse && !('limit' in opts)) {
var reverseOpts = xtend(opts, { reverse: true })
- // Swap start & end options
- if (('start' in opts) && ('end' in opts)) {
- reverseOpts.end = opts.start
- reverseOpts.start = opts.end
- } else if ('start' in opts) {
- reverseOpts.end = opts.start
- delete reverseOpts.start
- } else if ('end' in opts) {
- reverseOpts.start = opts.end
- delete reverseOpts.end
- }
-
rangeTest(
name + ' (flipped)',
reverseOpts,
@@ -85,44 +70,23 @@ exports.range = function (test, testCommon) {
gte: '00'
}, data)
- rangeTest('test iterator with start=00 - legacy', {
- start: '00'
- }, data)
-
rangeTest('test iterator with gte=50', {
gte: '50'
}, data.slice(50))
- rangeTest('test iterator with start=50 - legacy', {
- start: '50'
- }, data.slice(50))
-
rangeTest('test iterator with lte=50 and reverse=true', {
lte: '50',
reverse: true
}, data.slice().reverse().slice(49))
- rangeTest('test iterator with start=50 and reverse=true - legacy', {
- start: '50',
- reverse: true
- }, data.slice().reverse().slice(49))
-
rangeTest('test iterator with gte=49.5 (midway)', {
gte: '49.5'
}, data.slice(50))
- rangeTest('test iterator with start=49.5 (midway) - legacy', {
- start: '49.5'
- }, data.slice(50))
-
rangeTest('test iterator with gte=49999 (midway)', {
gte: '49999'
}, data.slice(50))
- rangeTest('test iterator with start=49999 (midway) - legacy', {
- start: '49999'
- }, data.slice(50))
-
rangeTest('test iterator with lte=49.5 (midway) and reverse=true', {
lte: '49.5',
reverse: true
@@ -138,27 +102,14 @@ exports.range = function (test, testCommon) {
reverse: true
}, data.slice().reverse().slice(50))
- rangeTest('test iterator with start=49.5 (midway) and reverse=true - legacy', {
- start: '49.5',
- reverse: true
- }, data.slice().reverse().slice(50))
-
rangeTest('test iterator with lte=50', {
lte: '50'
}, data.slice(0, 51))
- rangeTest('test iterator with end=50 - legacy', {
- end: '50'
- }, data.slice(0, 51))
-
rangeTest('test iterator with lte=50.5 (midway)', {
lte: '50.5'
}, data.slice(0, 51))
- rangeTest('test iterator with end=50.5 (midway) - legacy', {
- end: '50.5'
- }, data.slice(0, 51))
-
rangeTest('test iterator with lte=50555 (midway)', {
lte: '50555'
}, data.slice(0, 51))
@@ -167,10 +118,6 @@ exports.range = function (test, testCommon) {
lt: '50555'
}, data.slice(0, 51))
- rangeTest('test iterator with end=50555 (midway) - legacy', {
- end: '50555'
- }, data.slice(0, 51))
-
rangeTest('test iterator with gte=50.5 (midway) and reverse=true', {
gte: '50.5',
reverse: true
@@ -181,31 +128,21 @@ exports.range = function (test, testCommon) {
reverse: true
}, data.slice().reverse().slice(0, 49))
- rangeTest('test iterator with end=50.5 (midway) and reverse=true - legacy', {
- end: '50.5',
- reverse: true
- }, data.slice().reverse().slice(0, 49))
-
rangeTest('test iterator with gt=50 and reverse=true', {
gt: '50',
reverse: true
}, data.slice().reverse().slice(0, 49))
- // end='0', starting key is actually '00' so it should avoid it
+ // first key is actually '00' so it should avoid it
rangeTest('test iterator with lte=0', {
lte: '0'
}, [])
- // end='0', starting key is actually '00' so it should avoid it
+ // first key is actually '00' so it should avoid it
rangeTest('test iterator with lt=0', {
lt: '0'
}, [])
- // end='0', starting key is actually '00' so it should avoid it
- rangeTest('test iterator with end=0 - legacy', {
- end: '0'
- }, [])
-
rangeTest('test iterator with gte=30 and lte=70', {
gte: '30',
lte: '70'
@@ -216,11 +153,6 @@ exports.range = function (test, testCommon) {
lt: '71'
}, data.slice(30, 71))
- rangeTest('test iterator with start=30 and end=70 - legacy', {
- start: '30',
- end: '70'
- }, data.slice(30, 71))
-
rangeTest('test iterator with gte=30 and lte=70 and reverse=true', {
lte: '70',
gte: '30',
@@ -233,12 +165,6 @@ exports.range = function (test, testCommon) {
reverse: true
}, data.slice().reverse().slice(29, 70))
- rangeTest('test iterator with start=70 and end=30 and reverse=true - legacy', {
- start: '70',
- end: '30',
- reverse: true
- }, data.slice().reverse().slice(29, 70))
-
rangeTest('test iterator with limit=20', {
limit: 20
}, data.slice(0, 20))
@@ -248,11 +174,6 @@ exports.range = function (test, testCommon) {
gte: '20'
}, data.slice(20, 40))
- rangeTest('test iterator with limit=20 and start=20 - legacy', {
- limit: 20,
- start: '20'
- }, data.slice(20, 40))
-
rangeTest('test iterator with limit=20 and reverse=true', {
limit: 20,
reverse: true
@@ -264,12 +185,6 @@ exports.range = function (test, testCommon) {
reverse: true
}, data.slice().reverse().slice(20, 40))
- rangeTest('test iterator with limit=20 and start=79 and reverse=true - legacy', {
- limit: 20,
- start: '79',
- reverse: true
- }, data.slice().reverse().slice(20, 40))
-
// the default limit value from levelup is -1
rangeTest('test iterator with limit=-1 should iterate over whole database', {
limit: -1
@@ -284,21 +199,11 @@ exports.range = function (test, testCommon) {
lte: '50'
}, data.slice(0, 20))
- rangeTest('test iterator with end after limit - legacy', {
- limit: 20,
- end: '50'
- }, data.slice(0, 20))
-
rangeTest('test iterator with lte before limit', {
limit: 50,
lte: '19'
}, data.slice(0, 20))
- rangeTest('test iterator with end before limit - legacy', {
- limit: 50,
- end: '19'
- }, data.slice(0, 20))
-
rangeTest('test iterator with gte after database end', {
gte: '9a'
}, [])
@@ -307,28 +212,15 @@ exports.range = function (test, testCommon) {
gt: '9a'
}, [])
- rangeTest('test iterator with start after database end - legacy', {
- start: '9a'
- }, [])
-
rangeTest('test iterator with lte after database end and reverse=true', {
lte: '9a',
reverse: true
}, data.slice().reverse())
- rangeTest('test iterator with start after database end and reverse=true - legacy', {
- start: '9a',
- reverse: true
- }, data.slice().reverse())
-
rangeTest('test iterator with lt after database end', {
lt: 'a'
}, data.slice())
- rangeTest('test iterator with end after database end - legacy', {
- end: 'a'
- }, data.slice())
-
rangeTest('test iterator with lt at database end', {
lt: data[data.length - 1].key
}, data.slice(0, -1))
@@ -337,10 +229,6 @@ exports.range = function (test, testCommon) {
lte: data[data.length - 1].key
}, data.slice())
- rangeTest('test iterator with end at database end - legacy', {
- end: data[data.length - 1].key
- }, data.slice())
-
rangeTest('test iterator with lt before database end', {
lt: data[data.length - 2].key
}, data.slice(0, -2))
@@ -349,10 +237,6 @@ exports.range = function (test, testCommon) {
lte: data[data.length - 2].key
}, data.slice(0, -1))
- rangeTest('test iterator with end before database end - legacy', {
- end: data[data.length - 2].key
- }, data.slice(0, -1))
-
rangeTest('test iterator with lte and gte after database and reverse=true', {
lte: '9b',
gte: '9a',
@@ -364,12 +248,6 @@ exports.range = function (test, testCommon) {
gt: '9a',
reverse: true
}, [])
-
- rangeTest('test iterator with start and end after database and reverse=true - legacy', {
- start: '9b',
- end: '9a',
- reverse: true
- }, [])
}
exports.tearDown = function (test, testCommon) {
diff --git a/test/iterator-seek-test.js b/test/iterator-seek-test.js
index d153c9a0..f9878daf 100644
--- a/test/iterator-seek-test.js
+++ b/test/iterator-seek-test.js
@@ -196,10 +196,6 @@ exports.seek = function (test, testCommon) {
expect({ gte: '5' }, '5', '5')
expect({ gte: '5' }, '6', '6')
- expect({ start: '5' }, '4', undefined)
- expect({ start: '5' }, '5', '5')
- expect({ start: '5' }, '6', '6')
-
expect({ lt: '5' }, '4', '4')
expect({ lt: '5' }, '5', undefined)
expect({ lt: '5' }, '6', undefined)
@@ -208,10 +204,6 @@ exports.seek = function (test, testCommon) {
expect({ lte: '5' }, '5', '5')
expect({ lte: '5' }, '6', undefined)
- expect({ end: '5' }, '4', '4')
- expect({ end: '5' }, '5', '5')
- expect({ end: '5' }, '6', undefined)
-
expect({ lt: '5', reverse: true }, '4', '4')
expect({ lt: '5', reverse: true }, '5', undefined)
expect({ lt: '5', reverse: true }, '6', undefined)
@@ -220,10 +212,6 @@ exports.seek = function (test, testCommon) {
expect({ lte: '5', reverse: true }, '5', '5')
expect({ lte: '5', reverse: true }, '6', undefined)
- expect({ start: '5', reverse: true }, '4', '4')
- expect({ start: '5', reverse: true }, '5', '5')
- expect({ start: '5', reverse: true }, '6', undefined)
-
expect({ gt: '5', reverse: true }, '4', undefined)
expect({ gt: '5', reverse: true }, '5', undefined)
expect({ gt: '5', reverse: true }, '6', '6')
@@ -232,10 +220,6 @@ exports.seek = function (test, testCommon) {
expect({ gte: '5', reverse: true }, '5', '5')
expect({ gte: '5', reverse: true }, '6', '6')
- expect({ end: '5', reverse: true }, '4', undefined)
- expect({ end: '5', reverse: true }, '5', '5')
- expect({ end: '5', reverse: true }, '6', '6')
-
expect({ gt: '7', lt: '8' }, '7', undefined)
expect({ gte: '7', lt: '8' }, '7', '7')
expect({ gte: '7', lt: '8' }, '8', undefined)
diff --git a/test/self.js b/test/self.js
index 8dfc6255..4ed4ce72 100644
--- a/test/self.js
+++ b/test/self.js
@@ -16,7 +16,6 @@ var testCommon = require('./common')({
})
var rangeOptions = ['gt', 'gte', 'lt', 'lte']
-var legacyRangeOptions = ['start', 'end']
// Test the suite itself as well as the default implementation,
// excluding noop operations that can't pass the test suite.
@@ -930,7 +929,7 @@ test('.status', function (t) {
})
test('_setupIteratorOptions', function (t) {
- var keys = legacyRangeOptions.concat(rangeOptions)
+ var keys = rangeOptions.slice()
var db = new AbstractLevelDOWN()
function setupOptions (constrFn) {
@@ -948,7 +947,7 @@ test('_setupIteratorOptions', function (t) {
t.end()
}
- t.plan(6)
+ t.plan(7)
t.test('default options', function (t) {
t.same(db._setupIteratorOptions(), {
@@ -1014,4 +1013,19 @@ test('_setupIteratorOptions', function (t) {
})
verifyOptions(t, db._setupIteratorOptions(options))
})
+
+ t.test('rejects legacy range options', function (t) {
+ t.plan(2)
+
+ for (var key of ['start', 'end']) {
+ var options = {}
+ options[key] = 'x'
+
+ try {
+ db._setupIteratorOptions(options)
+ } catch (err) {
+ t.is(err.message, 'Legacy range options ("start" and "end") have been removed')
+ }
+ }
+ })
})