Merge pull request #1 from peakji/seek-reset-state

Seek reset state

README.md

@@ -224,7 +224,8 @@ the `callback` function will be called with no arguments in any of the following
 
 * the iterator comes to the end of the store
 * the `end` key has been reached; or
-* the `limit` has been reached
+* the `limit` has been reached; or
+* the last `seek()` was out of range
 
 Otherwise, the `callback` function will be called with the following 3 arguments:
 

iterator.js

@@ -14,9 +14,15 @@ function Iterator (db, options) {
 
 util.inherits(Iterator, AbstractIterator)
 
-Iterator.prototype.seek = function (key) {
+Iterator.prototype.seek = function (target) {
+  if (this._ended)
+    throw new Error('cannot call seek() after end()')
+  if (this._nexting)
+    throw new Error('cannot call seek() before next() has completed')
+
   this.cache = null
-  this.binding.seek(key)
+  this.binding.seek(target)
+  this.finished = false
 }
 
 Iterator.prototype._next = function (callback) {

package.json

@@ -39,6 +39,8 @@
     "delayed": "~1.0.1",
     "du": "~0.1.0",
     "faucet": "0.0.1",
+    "iota-array": "~1.0.0",
+    "lexicographic-integer": "~1.1.0",
     "mkfiletree": "~1.0.1",
     "monotonic-timestamp": "~0.0.8",
     "node-uuid": "~1.4.3",

src/iterator.cc

@@ -173,6 +173,38 @@ bool Iterator::Read (std::string& key, std::string& value) {
   return false;
 }
 
+bool Iterator::OutOfRange (leveldb::Slice* target) {
+  if (lt != NULL) {
+    if (target->compare(*lt) >= 0)
+      return true;
+  } else if (lte != NULL) {
+    if (target->compare(*lte) > 0)
+      return true;
+  } else if (start != NULL && reverse) {
+    if (target->compare(*start) > 0)
+      return true;
+  }
+
+  if (end != NULL) {
+    int d = target->compare(*end);
+    if (reverse ? d < 0 : d > 0)
+      return true;
+  }
+
+  if (gt != NULL) {
+    if (target->compare(*gt) <= 0)
+      return true;
+  } else if (gte != NULL) {
+    if (target->compare(*gte) < 0)
+      return true;
+  } else if (start != NULL && !reverse) {
+    if (target->compare(*start) < 0)
+      return true;
+  }
+
+  return false;
+}
+
 bool Iterator::IteratorNext (std::vector<std::pair<std::string, std::string> >& result) {
   size_t size = 0;
   while(true) {
@@ -248,28 +280,39 @@ NAN_METHOD(Iterator::Seek) {
   dbIterator->Seek(*iterator->target);
   iterator->seeking = true;
 
-  if (dbIterator->Valid()) {
-    int cmp = dbIterator->key().compare(*iterator->target);
-    if (cmp > 0 && iterator->reverse) {
-      dbIterator->Prev();
-    } else if (cmp < 0 && !iterator->reverse) {
-      dbIterator->Next();
-    }
-  } else {
+  if (iterator->OutOfRange(iterator->target)) {
     if (iterator->reverse) {
-      dbIterator->SeekToLast();
-    } else {
       dbIterator->SeekToFirst();
+      dbIterator->Prev();
+    } else {
+      dbIterator->SeekToLast();
+      dbIterator->Next();
     }
+  }
+  else {
     if (dbIterator->Valid()) {
       int cmp = dbIterator->key().compare(*iterator->target);
       if (cmp > 0 && iterator->reverse) {
-        dbIterator->SeekToFirst();
         dbIterator->Prev();
       } else if (cmp < 0 && !iterator->reverse) {
-        dbIterator->SeekToLast();
         dbIterator->Next();
       }
+    } else {
+      if (iterator->reverse) {
+        dbIterator->SeekToLast();
+      } else {
+        dbIterator->SeekToFirst();
+      }
+      if (dbIterator->Valid()) {
+        int cmp = dbIterator->key().compare(*iterator->target);
+        if (cmp > 0 && iterator->reverse) {
+          dbIterator->SeekToFirst();
+          dbIterator->Prev();
+        } else if (cmp < 0 && !iterator->reverse) {
+          dbIterator->SeekToLast();
+          dbIterator->Next();
+        }
+      }
     }
   }
 

src/iterator.h

@@ -85,6 +85,7 @@ class Iterator : public Nan::ObjectWrap {
 private:
   bool Read (std::string& key, std::string& value);
   bool GetIterator ();
+  bool OutOfRange (leveldb::Slice* target);
 
   static NAN_METHOD(New);
   static NAN_METHOD(Seek);

test/iterator-test.js

@@ -3,6 +3,9 @@ const test       = require('tape')
     , leveldown  = require('../')
     , abstract   = require('abstract-leveldown/abstract/iterator-test')
     , make       = require('./make')
+    , iota       = require('iota-array')
+    , lexi       = require('lexicographic-integer')
+    , util       = require('util')
 
 abstract.all(leveldown, test, testCommon)
 
@@ -94,3 +97,118 @@ make('reverse seek from invalid range', function (db, t, done) {
     ite.end(done)
   })
 })
+
+make('iterator seek resets state', function (db, t, done) {
+  var ite = db.iterator()
+  ite.next(function (err, key, value) {
+    t.error(err, 'no error from next()')
+    t.equal(key.toString(), 'one', 'key matches')
+    t.ok(ite.cache, 'has cached items')
+    t.equal(ite.finished, true, 'finished')
+    ite.seek('two')
+    t.notOk(ite.cache, 'cache is removed')
+    t.equal(ite.finished, false, 'resets finished state')
+    ite.end(done)
+  })
+})
+
+make('iterator seek respects range', function (db, t, done) {
+  db.batch(pairs(10), function (err) {
+    t.error(err, 'no error from batch()')
+
+    var pending = 0
+
+    expect({ gt: '5' }, '4', undefined)
+    expect({ gt: '5' }, '5', undefined)
+    expect({ gt: '5' }, '6', '6')
+
+    expect({ gte: '5' }, '4', undefined)
+    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)
+
+    expect({ lte: '5' }, '4', '4')
+    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)
+
+    expect({ lte: '5', reverse: true }, '4', '4')
+    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')
+
+    expect({ gte: '5', reverse: true }, '4', undefined)
+    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)
+    expect({ gt: '7', lte:'8' }, '8', '8')
+
+    function expect (range, target, expected) {
+      pending++
+      var ite = db.iterator(range)
+
+      ite.seek(target)
+      ite.next(function (err, key, value) {
+        t.error(err, 'no error from next()')
+
+        var tpl = 'seek(%s) on %s yields %s'
+        var msg = util.format(tpl, target, util.inspect(range), expected)
+
+        if (expected === undefined)
+          t.equal(value, undefined, msg)
+        else
+          t.equal(value.toString(), expected, msg)
+
+        ite.end(function (err) {
+          t.error(err, 'no error from end()')
+          if (!--pending) done()
+        })
+      })
+    }
+  })
+})
+
+function pairs (length, opts) {
+  opts = opts || {}
+  return iota(length).filter(not(opts.not)).map(function (k) {
+    var key = opts.lex ? lexi.pack(k, 'hex') : '' + k
+    return { type: 'put', key: key, value: '' + k }
+  })
+}
+
+function not (n) {
+  if (typeof n === 'function') return function (k) { return !n(k) }
+  return function (k) { return k !== n }
+}
+
+function even (n) {
+  return n % 2 === 0
+}