diff --git a/README.md b/README.md index a0640f73..70ebc759 100644 --- a/README.md +++ b/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: diff --git a/iterator.js b/iterator.js index d5181a6c..9849d49e 100644 --- a/iterator.js +++ b/iterator.js @@ -14,11 +14,20 @@ function Iterator (db, options) { util.inherits(Iterator, AbstractIterator) -Iterator.prototype.seek = function (key) { - if (typeof key !== 'string') - throw new Error('seek requires a string 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') + + if (typeof target !== 'string' && !Buffer.isBuffer(target)) + throw new Error('seek() requires a string or buffer key') + if (target.length == 0) + throw new Error('cannot seek() to an empty key') + this.cache = null - this.binding.seek(key) + this.binding.seek(target) + this.finished = false } Iterator.prototype._next = function (callback) { diff --git a/package.json b/package.json index 51777673..5ff77f37 100644 --- a/package.json +++ b/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", diff --git a/src/iterator.cc b/src/iterator.cc index 5ac0bf67..234a0a46 100644 --- a/src/iterator.cc +++ b/src/iterator.cc @@ -56,6 +56,7 @@ Iterator::Iterator ( options->snapshot = database->NewSnapshot(); dbIterator = NULL; count = 0; + target = NULL; seeking = false; nexting = false; ended = false; @@ -64,6 +65,7 @@ Iterator::Iterator ( Iterator::~Iterator () { delete options; + ReleaseTarget(); if (start != NULL) { // Special case for `start` option: it won't be // freed up by any of the delete calls below. @@ -171,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 >& result) { size_t size = 0; while(true) { @@ -205,7 +239,19 @@ void Iterator::Release () { database->ReleaseIterator(id); } +void Iterator::ReleaseTarget () { + if (target != NULL) { + + if (!target->empty()) + delete[] target->data(); + + delete target; + target = NULL; + } +} + void checkEndCallback (Iterator* iterator) { + iterator->ReleaseTarget(); iterator->nexting = false; if (iterator->endWorker != NULL) { Nan::AsyncQueueWorker(iterator->endWorker); @@ -215,35 +261,52 @@ void checkEndCallback (Iterator* iterator) { NAN_METHOD(Iterator::Seek) { Iterator* iterator = Nan::ObjectWrap::Unwrap(info.This()); + + iterator->ReleaseTarget(); + + v8::Local targetBuffer = info[0].As(); + LD_STRING_OR_BUFFER_TO_COPY(_target, targetBuffer, target); + iterator->target = new leveldb::Slice(_targetCh_, _targetSz_); + iterator->GetIterator(); leveldb::Iterator* dbIterator = iterator->dbIterator; - Nan::Utf8String key(info[0]); - dbIterator->Seek(*key); + dbIterator->Seek(*iterator->target); iterator->seeking = true; - if (dbIterator->Valid()) { - int cmp = dbIterator->key().compare(*key); - 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(*key); + 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(); + } + } } } diff --git a/src/iterator.h b/src/iterator.h index bed53f8d..801d9e88 100644 --- a/src/iterator.h +++ b/src/iterator.h @@ -53,6 +53,7 @@ class Iterator : public Nan::ObjectWrap { leveldb::Status IteratorStatus (); void IteratorEnd (); void Release (); + void ReleaseTarget (); private: Database* database; @@ -60,6 +61,7 @@ class Iterator : public Nan::ObjectWrap { leveldb::Iterator* dbIterator; leveldb::ReadOptions* options; leveldb::Slice* start; + leveldb::Slice* target; std::string* end; bool seeking; bool reverse; @@ -83,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); diff --git a/test/iterator-test.js b/test/iterator-test.js index 87db93b7..82ec4d2c 100644 --- a/test/iterator-test.js +++ b/test/iterator-test.js @@ -3,20 +3,34 @@ 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) -make('iterator throws if key is not a string', function (db, t, done) { - var ite = db.iterator() - var error - try { - ite.seek() - } catch (e) { - error = e - } +make('iterator throws if key is not a string or buffer', function (db, t, done) { + var keys = [null, undefined, 1, true, false] + var pending = keys.length + + keys.forEach(function (key) { + var error + var ite = db.iterator() + + try { + ite.seek(key) + } catch (e) { + error = e + } + + t.ok(error, 'had error from seek()') + ite.end(end) + }) - t.ok(error, 'had error') - t.end() + function end(err) { + t.error(err, 'no error from end()') + if (!--pending) done() + } }) make('iterator is seekable', function (db, t, done) { @@ -35,6 +49,22 @@ make('iterator is seekable', function (db, t, done) { }) }) +make('iterator is seekable with buffer', function (db, t, done) { + var ite = db.iterator() + ite.seek(Buffer('two')) + ite.next(function (err, key, value) { + t.error(err, 'no error from next()') + t.equal(key.toString(), 'two', 'key matches') + t.equal(value.toString(), '2', 'value matches') + ite.next(function (err, key, value) { + t.error(err, 'no error from next()') + t.equal(key, undefined, 'end of iterator') + t.equal(value, undefined, 'end of iterator') + ite.end(done) + }) + }) +}) + make('reverse seek in the middle', function (db, t, done) { var ite = db.iterator({reverse: true, limit: 1}) ite.seek('three!') @@ -67,3 +97,153 @@ 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 before next has completed', function (db, t, done) { + var ite = db.iterator() + ite.next(function (err, key, value) { + t.error(err, 'no error from end()') + done() + }) + var error + try { + ite.seek("two") + } + catch (e) { + error = e; + } + t.ok(error, 'had error from seek() before next() has completed') +}) + +make('iterator seek after end', function (db, t, done) { + var ite = db.iterator() + ite.next(function (err, key, value) { + t.error(err, 'no error from next()') + ite.end(function (err){ + t.error(err, 'no error from end()') + var error + try { + ite.seek('two') + } + catch (e) { + error = e + } + t.ok(error, 'had error from seek() after 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 +} \ No newline at end of file