Firefox compatibility, IndexedDB fixes; batch remove fixes; issue #108. #130
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Doesn't actually make this adapter work on Firefox, but at least keeps it from crashing the test suite.
…t suite. This makes the new 'click to test an adapter' functionality work on Firefox.
Firefox's IndexedDB implementation is significantly slower than webkit's, which causes sporadic failures of the test suite.
…eak. This prevents a leak in Chrome. Granted, this is actually a bug in chrome: the request objects should get garbage collected which should also free up the event listeners. But this doesn't appear to be happening (as shown in Chrome's memory tracing tools).
Numeric constants for transaction mode are deprecated.
Spec says that cursor is null if the range is empty, but firefox makes the cursor undefined instead.
…refox). Transaction creation inside a versionchange transaction is disallowed.
…Increment. Both Firefox and Webkit have issues with the autoIncrement feature of IndexedDB. Let's assume that those bugs will be fixed by the time the spec becomes final and indexedDB is unprefixed -- but until that time, don't use the feature and use the same this.uuid() key-generator that the other implementations use.
The previous implementations suffered from race conditions: the order of the results was not guaranteed to match the order given in the parameters.
The webkit-sqlite, indexed-db, and dom adapters all fail this new test in various ways.
Implement batch remove. While we're at it, make the "is this a key or an object containing a key" test consistent with that used in the other adapters.
Passing an array to get() could previously return a result array shorter than the parameter array, since keys not present in the lawnchair were missing from the result. Add 'null' in the place of the missing result, to be consistent with other adapters. Implement batch remove (using code borrowed from the indexedDB adapter).
Update the batch() implementation to match that used in the other adapters. Removes the need for setInterval() to poll for completion. Fix sql quoting issues in get() implementation, and ensure that the order of returned results matches the order of the keys provided as arguments in the batch get case. Implement batch remove. Update the "is this a key or an object containing a key" test to match that used in the other adapters.
…ers. Also fixes issue brianleroux#108 with the nuke() method, while we're at it. In addition, a few minor changes to try to make memory and window-name more nearly identical, although a number of unnecessary whitespace diffs remain.
brianleroux
added a commit
that referenced
this pull request
Oct 17, 2012
Firefox compatibility, IndexedDB fixes; batch remove fixes; issue #108.
|
this is great---don't worry about the formatting. have some conference-driven-development coming up for this so I'll do the tidy =) |
|
Incidentally, one thing I noticed was that a lot of adapters share basically the same implementation of batch set/get/remove. It might be worth providing a default implementation in Lawnchair.prototype and then doing something like: get: function(keyOrArray) {
if (this.isArray(keyOrArray)) {
return Lawnchair.prototype.get.call(this, keyOrArray);
}
/* ... rest of single-key implementation here ... */
}Similarly for remove. And any adapter which doesn't have an efficient batch put could just not override batch and use a default implementation of batch() from Lawnchair.prototype. Might be a worthwhile refactoring for your conference-driven-development. ;-) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I can split this patch set into four pieces if you like. But you've been pulling my patches promptly so I thought I'd combine the pull requests. There are four things here:
First three patches in the set fix firefox compatibility issues with the test suite.
The remainder of the patches in the first batch fix the IndexedDB adapter and make it work on latest Chrome and latest Firefox. These patches should resolve Updated IndexedDB Adapter #120 (I reviewed Paul Lewis' patch and incorporated his non-whitespace fixes), and hopefully indexed-db adaptor 0.6.30 not working on Firefox 12 #103, Firefox and indexed-db adapter #126 and Uncaught Error: InvalidStateError: DOM IDBDatabase Exception 11 indexed-db.js:105 #127 as well (at least the test suite works!).
The third set of patches (from 023826f on) adds a test case for batch add/get/remove to the test suite. Then the patches go and fix all the bugs in the dom, webkit-sqlite, and indexeddb adapters which were uncovered by the new test case. This should resolve websql.lite adapter - SQL escaping error on get #118, webkit-sqlite changes to use parameters for the IDs on get #119, and "remove" method parameters change according to active adapter #94.
The final patch resolves window-name adapter nuke function #108 and fixes the "independent data store" failure in the test suite for the memory and window-name adapters.