Breaking: remove legacy range options (start & end)

An error is now thrown when an iterator is created with one of these options. Ref Level/community#86
Level · Apr 9, 2021 · 979d34f · 979d34f
1 parent a1fd94e
commit 979d34f
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 157 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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) {