Iterator refactor

iterator.js

@@ -9,7 +9,13 @@ function Iterator (db, options) {
   this.options = options
   AbstractIterator.call(this, db)
   this._order = options.reverse ? 'DESC': 'ASC'
-  this._limit = options.limit
+  this._limit = options.limit;
+  if (this._limit == null || this._limit === -1) {
+    this._limit = Infinity;
+  }
+  if (typeof this._limit !== 'number') throw new TypeError('options.limit must be a number')
+  if (this._limit === 0) return // skip further processing and wait for first call to _next
+
   this._count = 0
   this._done  = false
   var lower = ltgt.lowerBound(options)
@@ -21,17 +27,14 @@ function Iterator (db, options) {
       excludeLower: ltgt.lowerBoundExclusive(options),
       excludeUpper: ltgt.upperBoundExclusive(options)
     }) : null
-  } catch (e) {
+  } catch (err) {
     // The lower key is greater than the upper key.
-    // IndexedDB throws an error, but we'll just return 0 results.
+    // IndexedDB throws a DataError, but we'll just skip the iterator and return 0 results.
     this._keyRangeError = true
+    return;
   }
   this.callback = null
-}
-
-util.inherits(Iterator, AbstractIterator)
 
-Iterator.prototype.createIterator = function() {
   var self = this
 
   self.iterator = self.db.iterate(function () {
@@ -40,33 +43,57 @@ Iterator.prototype.createIterator = function() {
     keyRange: self._keyRange,
     autoContinue: false,
     order: self._order,
-    onError: function(err) { console.log('horrible error', err) },
-  })
+    onError: function(err) { console.error('horrible error', err) }
+  });
+
+  self.iterator.oncomplete = function () {
+    self._cursorEnded = true;
+    if (self.callback) self.callback()
+  }
 }
 
-// TODO the limit implementation here just ignores all reads after limit has been reached
-// it should cancel the iterator instead but I don't know how
+util.inherits(Iterator, AbstractIterator)
+
 Iterator.prototype.onItem = function (value, cursor, cursorTransaction) {
-  if (!cursor && this.callback) {
-    this.callback()
-    this.callback = false
-    return
-  }
-  var shouldCall = true
+  var self = this;
+  function emitAndContinue(cb) {
+    if (!cursor) {
+      cb()
+      return
+    }
+
+    self._count++;
 
-  if (!!this._limit && this._limit > 0 && this._count++ >= this._limit)
-    shouldCall = false
+    var key = self.options.keyAsBuffer !== false
+      ? Buffer(cursor.key)
+      : cursor.key
+    var value = self.options.valueAsBuffer !== false
+      ? Buffer(cursor.value)
+      : cursor.value
+    cb(null, key, value)
+    if (!self._cursorEnded) {
+      // IDBWrapper.limit only works if autoContinue is true
+      if (self._count < self._limit) cursor['continue']() // else timeout
+    }
+  }
 
-  if (shouldCall) this.callback(false, cursor.key, cursor.value)
-  if (cursor) cursor['continue']()
+  if (this.callback) {
+    emitAndContinue(this.callback)
+    this.callback = false
+  } else {
+    // wait for next handler
+    this._emitAndContinue = emitAndContinue
+  }
 }
 
 Iterator.prototype._next = function (callback) {
-  if (!callback) return new Error('next() requires a callback argument')
-  if (this._keyRangeError) return callback()
-  if (!this._started) {
-    this.createIterator()
-    this._started = true
+  if (this._keyRangeError || this._limit === 0) return callback()
+
+  if (this._emitAndContinue) {
+    this._emitAndContinue(callback)
+    this._emitAndContinue = false
+  } else {
+    // wait for cursor
+    this.callback = callback
   }
-  this.callback = callback
 }

test/custom-tests.js

@@ -12,6 +12,55 @@ module.exports.all = function(leveljs, tape, testCommon) {
 
   module.exports.setUp(leveljs, tape, testCommon)
 
+  tape('should use a callback only once per next-handler', function(t) {
+    var level = leveljs(testCommon.location())
+    level.open(function(err) {
+      t.notOk(err, 'no error')
+      level.put('akey', 'aval',  function (err) {
+        t.notOk(err, 'no error')
+        level.put('bkey', 'bval',  function (err) {
+          t.notOk(err, 'no error')
+          level.put('ckey', 'cval',  function (err) {
+            t.notOk(err, 'no error')
+
+            var iterator = level.iterator({ keyAsBuffer: false, valueAsBuffer: false })
+
+            iterator.next(function(err, key, value) {
+              t.notOk(err, 'no error')
+              t.equal(key, 'akey', 'should have akey')
+              t.equal(value, 'aval', 'should have avalue')
+
+              setTimeout(function() {
+                iterator.next(function(err, key, value) {
+                  t.notOk(err, 'no error')
+                  t.equal(key, 'bkey', 'should have bkey')
+                  t.equal(value, 'bval', 'should have bvalue')
+
+                  setTimeout(function() {
+                    iterator.next(function(err, key, value) {
+                      t.notOk(err, 'no error')
+                      t.equal(key, 'ckey', 'should have ckey')
+                      t.equal(value, 'cval', 'should have cvalue')
+
+                      setTimeout(function() {
+                        iterator.next(function(err, key, value) {
+                          t.notOk(err, 'no error')
+                          t.notOk(key, 'end, no key')
+                          t.notOk(value, 'end, no value')
+                          t.end()
+                        })
+                      }, 1)
+                    })
+                  }, 1)
+                })
+              }, 1)
+            })
+          })
+        })
+      })
+    })
+  })
+
   tape('store native JS types with raw = true', function(t) {
     var level = leveljs(testCommon.location())
     level.open(function(err) {
@@ -27,7 +76,7 @@ module.exports.all = function(leveljs, tape, testCommon) {
       })
     })
   })
-  
+
   // NOTE: in chrome (at least) indexeddb gets buggy if you try and destroy a db,
   // then create it again, then try and destroy it again. these avoid doing that
 
@@ -75,4 +124,19 @@ module.exports.all = function(leveljs, tape, testCommon) {
     })
   })
 
+  tape('zero results if gt key > lt key', function(t) {
+    var level = levelup('key-range-test', {db: leveljs})
+    level.open(function(err) {
+      t.notOk(err, 'no error')
+      var s = level.createReadStream({ gte: 'x', lt: 'b' });
+      var item;
+      s.on('readable', function() {
+        item = s.read()
+      })
+      s.on('end', function() {
+        t.end()
+      });
+    })
+  })
+
 }