Skip to content

Commit

Permalink
Breaking: remove legacy range options (start & end)
Browse files Browse the repository at this point in the history
An error is now thrown when an iterator is created with one of these
options. Ref Level/community#86
  • Loading branch information
vweevers committed Apr 9, 2021
1 parent a1fd94e commit 979d34f
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 157 deletions.
6 changes: 0 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.

Expand Down
6 changes: 5 additions & 1 deletion abstract-leveldown.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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)) {
Expand Down
4 changes: 2 additions & 2 deletions test/clear-range-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions test/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
128 changes: 3 additions & 125 deletions test/iterator-range-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand All @@ -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
Expand All @@ -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))
Expand All @@ -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
Expand All @@ -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'
Expand All @@ -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',
Expand All @@ -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))
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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'
}, [])
Expand All @@ -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))
Expand All @@ -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))
Expand All @@ -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',
Expand All @@ -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) {
Expand Down
Loading

0 comments on commit 979d34f

Please sign in to comment.