Iterator refactor

[timkuijsten]

Did some refactor of the iterator to fix the tests and fix some minor issues.

commit d9605d4
Author: Tim Kuijsten
Date: Fri Feb 5 01:17:42 2016 +0100

fix iterator when limit is 0

uncovered when testing with abstract-leveldown2 tests

commit 6ac2eb8
Author: Tim Kuijsten
Date: Fri Feb 5 00:53:49 2016 +0100

add test to check for key range error handling

commit c1b6428
Author: Tim Kuijsten
Date: Fri Feb 5 00:25:50 2016 +0100

refactor iterator, multiple fixes, make tests pass

Fixes #43 #40 and #3. Unfortunately not easy to split out in separate commits.

1. maxogden/level.js#43 "snapshots seem not to work"

The reason snapshots didn't work in the test, is because the iterator was only
opened on the first call to _next. The new version opens the iterator right
away in the constructor (if there is no KeyRange error).

2. maxogden/level.js#40 "abstract-leveldown tests aren't all passing anymore"

Partially fixed by merging PR #42 and further fixed by this commit.

3. maxogden/level.js#3 "stop batch reads after limit is reached"

The solution is stop calling cursor.continue() once the limit is reached. The
transaction will automatically timeout as prescribed by the spec. idb-wrapper
can't be used for this because limit is only respected if autoContinue is true.

All 563 tests pass on:
* Safari 9.0.3
* Firefox 45.0b2
* Firefox 46.0a2
* Iridium 44.1 (Chrome/44.0.2403.157)

commit bfe717b
Merge: 2c72508 7939006
Author: Tim Kuijsten
Date: Fri Feb 5 00:20:23 2016 +0100

Merge remote-tracking branch 'upstream/fix-for-new-abstract-leveldown'

relates to maxogden/level.js#40

fixes the problem with keys. doesn't fix the problem with snapshots

commit 7939006
Author: Julian Gruber
Date: Tue Jul 7 09:24:34 2015 +0200

iterator: data as buffers by default

[juliangruber]

+1

[JamesKyburz]

lgtm

[mcollina]

LGTM

[timkuijsten]

The only note I'd like to add, is that I did not use this code in a real application yet. This is because my own application depends on binary key support. So I've only run this rewrite using the test cases.

[timkuijsten]

I just noticed a difference with 2.2.3. There the callback given to next() might be called multiple times. The indexedDB cursor just starts iterating after the first call to next() and (re)uses the callback that was passed to the last call to next. I've attached a test that shows callbacks given to next are used multiple times. Furthermore, in 2.2.3 the cursor always continues right away until the end. This makes that the indexedDB cursor never times out.

The code in this PR calls the callback given to next() only once, and the cursor only continues after the callback is handled. This fixes #3 but introduces the problem that if the callback isn't called fast enough (within 1 or 2ms on my machine), the cursor stops iterating and the transaction completes even if not all items have been iterated. This also becomes clear by the attached test (maybe the timeout has to be increased a bit).

This makes the current PR not good enough and leaves me with two questions:

1. Is it ok to call a callback that is passed to _next() multiple times?
2. What should we do with the default indexedDB behavior that if a cursor is not continued fast enough, it stops iterating?

A possible solution for the second question is to keep track of the last emitted item and reopen a new cursor if it was aborted prematurely (if a callback to _next() took too long).

[timkuijsten]

I've reworked the native IndexedDB iterator to reopen the cursor if it timed out prematurely (proposed in #46) and think we should drop this PR. I think there are too many moving parts to fix this for 2.x and should concentrate on stabilizing the work for 3.x.

Is it ok if I close this one?

[vweevers]

Superseded by #68.