Skip to content

Commit

Permalink
- Fix: Distinguish deleted and pending deleted stores and indexes;
Browse files Browse the repository at this point in the history
    for insertion operations, do index checks independent of
    `indexNames` in case subsequently deleted by `deleteIndex` (indexeddbshim#276)
- Fix: Ensure same index instance object returned for same store object and
    index name
- Fix: Better error reporting for `IDBRequest.error` or `IDBRequest.result`
    errors.
- Testing (Grunt): Make QUnit tasks more granular
- Testing (QUnit): Avoid origin-checking
- Testing (W3C): Report on bad files not expected to be such (not in the list)
- Testing (Mocha): Add test for index instance clones

Current IndexedDB (and domstringlist) test statuses
  'Pass': 1102, (including 4 domstringlist tests but avoiding exclusions)
  'Fail': 69,
  'Timeout': 4,
  'Not Run': 2,
  'Total tests': 1177 (including 4 domstringlist tests but avoiding exclusions)

322 normal files (including 1 domstringlist file):
        288 are all good, 30 have at least some bad, 4 time out, 2 do not run
    7 excluded files (uncaught exceptions during testing)
  • Loading branch information
brettz9 committed Mar 31, 2017
1 parent bf75000 commit 9a81106
Show file tree
Hide file tree
Showing 23 changed files with 517 additions and 312 deletions.
8 changes: 7 additions & 1 deletion CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,13 @@ they were actually changes since a more recent version on `master`.
(`put`, `add`) and `IDBCursor.update`)
- Fix: Avoid `eval()` in cloning (issue #211, #236)
- Fix: Support cyclic values via typeson/typeson-registry (#267)
- Fix: Distinguish deleted and pending deleted stores and indexes;
for insertion operations, do index checks independent of
`indexNames` in case subsequently deleted by `deleteIndex` (#276)
- Fix: Ensure same index instance object returned for same store object and
index name
- Fix: Better error reporting for `IDBRequest.error` or `IDBRequest.result`
errors.
- Repo files: Rename test folders for ease in distinguishing
- Optimize: Only retrieve required SQLite columns
- Optimize: Have `IDBObjectStore` and `IDBIndex`'s `get` and
Expand Down Expand Up @@ -568,7 +575,6 @@ they were actually changes since a more recent version on `master`.

From old W3C (Node and browser), only the following are not passing:
IDBDatabase.close.js,
IDBObjectStore.createIndex.js,
TransactionBehavior.js

From new W3C (Node but potentially also browser):
Expand Down
4 changes: 2 additions & 2 deletions Gruntfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -304,13 +304,13 @@ module.exports = function (grunt) {
grunt.registerTask('build', ['eslint', 'browserify', 'uglify']);

const testJobs = ['build', 'connect'];
grunt.registerTask('nodequnit', testJobs.concat('node-qunit'));
grunt.registerTask('nodequnit', 'node-qunit');
grunt.registerTask('mocha', ['mochaTest:test']); // clean:mochaTests isn't working here as locked (even with force:true on it or grunt-wait) so we do in package.json
grunt.registerTask('fake', ['mochaTest:fake']);
grunt.registerTask('mock', ['mochaTest:mock']);
grunt.registerTask('w3c-old', ['mochaTest:w3cOld']);

grunt.registerTask('phantom-qunit', testJobs.concat('qunit'));
grunt.registerTask('phantom-qunit', ['connect', 'qunit']);

if (saucekey !== null) {
testJobs.push('saucelabs-qunit');
Expand Down
122 changes: 74 additions & 48 deletions dist/indexeddbshim-UnicodeIdentifiers-node.js

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions dist/indexeddbshim-UnicodeIdentifiers-node.min.js

Large diffs are not rendered by default.

122 changes: 74 additions & 48 deletions dist/indexeddbshim-UnicodeIdentifiers.js

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions dist/indexeddbshim-UnicodeIdentifiers.min.js

Large diffs are not rendered by default.

122 changes: 74 additions & 48 deletions dist/indexeddbshim-node.js

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions dist/indexeddbshim-node.min.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/indexeddbshim-node.min.js.map

Large diffs are not rendered by default.

122 changes: 74 additions & 48 deletions dist/indexeddbshim-noninvasive.js

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions dist/indexeddbshim-noninvasive.min.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/indexeddbshim-noninvasive.min.js.map

Large diffs are not rendered by default.

122 changes: 74 additions & 48 deletions dist/indexeddbshim.js

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions dist/indexeddbshim.min.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/indexeddbshim.min.js.map

Large diffs are not rendered by default.

32 changes: 22 additions & 10 deletions src/IDBIndex.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ IDBIndex.__createInstance = function (store, indexProperties) {
const oldName = me.name;
IDBTransaction.__assertVersionChange(me.objectStore.transaction);
IDBTransaction.__assertActive(me.objectStore.transaction);
if (me.__deleted) {
if (me.__deleted || me.__pendingDelete) {
throw createDOMException('InvalidStateError', 'This index has been deleted');
}
if (me.objectStore.__deleted) {
if (me.objectStore.__deleted || me.objectStore.__pendingDelete) {
throw createDOMException('InvalidStateError', "This index's object store has been deleted");
}
if (newName === oldName) {
Expand Down Expand Up @@ -104,20 +104,21 @@ IDBIndex.__clone = function (index, store) {
*/
IDBIndex.__createIndex = function (store, index) {
const idx = store.__indexes[index.name];
const columnExists = idx && idx.__deleted;

// Add the index to the IDBObjectStore
index.__pending = true;
store.__indexes[index.name] = index;
index.__pendingCreate = true;

store.indexNames.push(index.name);
store.__indexes[index.name] = index; // We add to indexes as needs to be available, e.g., if there is a subsequent deleteIndex call

// Create the index in WebSQL
const transaction = store.transaction;
transaction.__addNonRequestToTransactionQueue(function createIndex (tx, args, success, failure) {
const columnExists = idx && (idx.__deleted || idx.__recreated); // This check must occur here rather than earlier as properties may not have been set yet otherwise
let indexValues = {};

function error (tx, err) {
failure(createDOMException('UnknownError', 'Could not create index "' + index.name + '"', err));
failure(createDOMException('UnknownError', 'Could not create index "' + index.name + '"' + err.code + '::' + err.message, err));
}

function applyIndex (tx) {
Expand Down Expand Up @@ -162,7 +163,11 @@ IDBIndex.__createIndex = function (store, index) {
addIndexEntry(i + 1);
}
} else {
delete index.__pending;
delete index.__pendingCreate;
if (index.__deleted) {
delete index.__deleted;
index.__recreated = true;
}
indexValues = {};
success(store);
}
Expand Down Expand Up @@ -191,7 +196,9 @@ IDBIndex.__createIndex = function (store, index) {
*/
IDBIndex.__deleteIndex = function (store, index) {
// Remove the index from the IDBObjectStore
store.__indexes[index.name].__deleted = true;
index.__pendingDelete = true;
delete store.__indexClones[index.name];

store.indexNames.splice(store.indexNames.indexOf(index.name), 1);

// Remove the index in WebSQL
Expand All @@ -202,7 +209,12 @@ IDBIndex.__deleteIndex = function (store, index) {
}

// Update the object store's index list
IDBIndex.__updateIndexList(store, tx, success, error);
IDBIndex.__updateIndexList(store, tx, function (store) {
delete index.__pendingDelete;
delete index.__recreated;
index.__deleted = true;
success(store);
}, error);
}, undefined, store);
};

Expand Down Expand Up @@ -250,7 +262,7 @@ IDBIndex.prototype.__fetchIndexData = function (range, opType, nullDisallowed, c
count = util.enforceRange(count, 'unsigned long');
}

if (me.__deleted) {
if (me.__deleted || me.__pendingDelete) {
throw createDOMException('InvalidStateError', 'This index has been deleted');
}
if (me.objectStore.__deleted) {
Expand Down
86 changes: 52 additions & 34 deletions src/IDBObjectStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ IDBObjectStore.__createInstance = function (storeProperties, transaction) {
me.__autoIncrement = !!storeProperties.autoInc;

me.__indexes = {};
me.__indexClones = {};
me.__indexNames = DOMStringList.__createInstance();
const indexList = storeProperties.indexList;
for (const indexName in indexList) {
Expand All @@ -58,7 +59,7 @@ IDBObjectStore.__createInstance = function (storeProperties, transaction) {
},
set: function (name) {
const me = this;
if (me.__deleted) {
if (me.__deleted || me.__pendingDelete) {
throw createDOMException('InvalidStateError', 'This store has been deleted');
}
IDBTransaction.__assertVersionChange(me.transaction);
Expand Down Expand Up @@ -122,6 +123,7 @@ IDBObjectStore.__clone = function (store, transaction) {
*/
IDBObjectStore.__createObjectStore = function (db, store) {
// Add the object store to the IDBDatabase
store.__pendingCreate = true;
db.__objectStores[store.name] = store;
db.objectStoreNames.push(store.name);

Expand All @@ -141,6 +143,8 @@ IDBObjectStore.__createObjectStore = function (db, store) {
tx.executeSql(sql, [], function (tx, data) {
Sca.encode(store.keyPath, function (encodedKeyPath) {
tx.executeSql('INSERT INTO __sys__ VALUES (?,?,?,?,?)', [util.escapeSQLiteStatement(store.name), encodedKeyPath, store.autoIncrement, '{}', 1], function () {
delete store.__pendingCreate;
delete store.__deleted;
success(store);
}, error);
});
Expand All @@ -156,15 +160,16 @@ IDBObjectStore.__createObjectStore = function (db, store) {
*/
IDBObjectStore.__deleteObjectStore = function (db, store) {
// Remove the object store from the IDBDatabase
store.__deleted = true;
store.__pendingDelete = true;
db.__objectStores[store.name] = undefined;
db.objectStoreNames.splice(db.objectStoreNames.indexOf(store.name), 1);

const storeClone = db.__versionTransaction.__storeClones[store.name];
if (storeClone) {
storeClone.__indexNames = DOMStringList.__createInstance();
storeClone.__indexes = {};
storeClone.__deleted = true;
storeClone.__indexClones = {};
storeClone.__pendingDelete = true;
}

// Remove the object store from WebSQL
Expand All @@ -181,6 +186,12 @@ IDBObjectStore.__deleteObjectStore = function (db, store) {
if (data.rows.length > 0) {
tx.executeSql('DROP TABLE ' + util.escapeStoreNameForSQL(store.name), [], function () {
tx.executeSql('DELETE FROM __sys__ WHERE "name" = ?', [util.escapeSQLiteStatement(store.name)], function () {
delete store.__pendingDelete;
store.__deleted = true;
if (storeClone) {
delete storeClone.__pendingDelete;
storeClone.__deleted = true;
}
success();
}, error);
}, error);
Expand Down Expand Up @@ -295,18 +306,33 @@ IDBObjectStore.prototype.__insertData = function (tx, encoded, value, clonedKeyO
// The `ConstraintError` to occur for `add` upon a duplicate will occur naturally in attempting an insert
// We process the index information first as it will stored in the same table as the store
const paramMap = {};
const indexPromises = me.indexNames.map((indexName) => {
const indexPromises = Object.keys(
// We do not iterate `indexNames` as those can be modified synchronously (e.g.,
// `deleteIndex` could, by its synchronous removal from `indexNames`, prevent
// iteration here of an index though per IndexedDB test
// `idbobjectstore_createIndex4-deleteIndex-event_order.js`, `createIndex`
// should be allowed to first fail even in such a case).
me.__indexes
).map((indexName) => {
// While this may sometimes resolve sync and sometimes async, the
// idea is to avoid, where possible, unnecessary delays (and
// consuming code ought to only see a difference in the browser
// where we can't control the transaction timeout anyways).
return new SyncPromise((resolve, reject) => {
const index = me.__indexes[indexName];
if (index.__pending) {
if (
// `createIndex` was called synchronously after the current insertion was added to
// the transaction queue so not yet ready to be checked (e.g., if two items with
/// the same key were added and then a unique index was created, it should not abort
// yet, as we're still handling the insertions)
// the transaction queue so although it was added to `__indexes`, it is not yet
// ready to be checked here for the insertion as it will be when running the
// `createIndex` operation (e.g., if two items with the same key were added and
// *then* a unique index was created, it should not continue to err and abort
// yet, as we're still handling the insertions which must be processed (e.g., to
// add duplicates which then cause a unique index to fail))
index.__pendingCreate ||
// If already deleted (and not just slated for deletion (by `__pendingDelete`
// after this add), we avoid checks
index.__deleted
) {
resolve();
return;
}
Expand Down Expand Up @@ -392,7 +418,7 @@ IDBObjectStore.prototype.add = function (value /* , key */) {
if (arguments.length === 0) {
throw new TypeError('No value was specified');
}
if (me.__deleted) {
if (me.__deleted || me.__pendingDelete) {
throw createDOMException('InvalidStateError', 'This store has been deleted');
}
IDBTransaction.__assertActive(me.transaction);
Expand All @@ -413,7 +439,7 @@ IDBObjectStore.prototype.put = function (value /*, key */) {
if (arguments.length === 0) {
throw new TypeError('No value was specified');
}
if (me.__deleted) {
if (me.__deleted || me.__pendingDelete) {
throw createDOMException('InvalidStateError', 'This store has been deleted');
}
IDBTransaction.__assertActive(me.transaction);
Expand Down Expand Up @@ -467,7 +493,7 @@ IDBObjectStore.prototype.__get = function (query, getKey, getAll, count) {
if (count !== undefined) {
count = util.enforceRange(count, 'unsigned long');
}
if (me.__deleted) {
if (me.__deleted || me.__pendingDelete) {
throw createDOMException('InvalidStateError', 'This store has been deleted');
}
IDBTransaction.__assertActive(me.transaction);
Expand Down Expand Up @@ -569,7 +595,7 @@ IDBObjectStore.prototype['delete'] = function (query) {
throw new TypeError('A parameter was missing for `IDBObjectStore.delete`.');
}

if (me.__deleted) {
if (me.__deleted || me.__pendingDelete) {
throw createDOMException('InvalidStateError', 'This store has been deleted');
}
IDBTransaction.__assertActive(me.transaction);
Expand Down Expand Up @@ -601,7 +627,7 @@ IDBObjectStore.prototype.clear = function () {
if (!(this instanceof IDBObjectStore)) {
throw new TypeError('Illegal invocation');
}
if (me.__deleted) {
if (me.__deleted || me.__pendingDelete) {
throw createDOMException('InvalidStateError', 'This store has been deleted');
}
IDBTransaction.__assertActive(me.transaction);
Expand All @@ -626,7 +652,7 @@ IDBObjectStore.prototype.count = function (/* query */) {
if (!(me instanceof IDBObjectStore)) {
throw new TypeError('Illegal invocation');
}
if (me.__deleted) {
if (me.__deleted || me.__pendingDelete) {
throw createDOMException('InvalidStateError', 'This store has been deleted');
}
IDBTransaction.__assertActive(me.transaction);
Expand All @@ -641,7 +667,7 @@ IDBObjectStore.prototype.openCursor = function (/* query, direction */) {
if (!(me instanceof IDBObjectStore)) {
throw new TypeError('Illegal invocation');
}
if (me.__deleted) {
if (me.__deleted || me.__pendingDelete) {
throw createDOMException('InvalidStateError', 'This store has been deleted');
}
const cursor = IDBCursorWithValue.__createInstance(query, direction, me, me, 'key', 'value');
Expand All @@ -654,7 +680,7 @@ IDBObjectStore.prototype.openKeyCursor = function (/* query, direction */) {
if (!(me instanceof IDBObjectStore)) {
throw new TypeError('Illegal invocation');
}
if (me.__deleted) {
if (me.__deleted || me.__pendingDelete) {
throw createDOMException('InvalidStateError', 'This store has been deleted');
}
const [query, direction] = arguments;
Expand All @@ -671,30 +697,22 @@ IDBObjectStore.prototype.index = function (indexName) {
if (arguments.length === 0) {
throw new TypeError('No index name was specified');
}
if (me.__deleted) {
if (me.__deleted || me.__pendingDelete) {
throw createDOMException('InvalidStateError', 'This store has been deleted');
}
IDBTransaction.__assertNotFinished(me.transaction);
const index = me.__indexes[indexName];
if (!index || index.__deleted) {
throw createDOMException('NotFoundError', 'Index "' + indexName + '" does not exist on ' + me.name);
}
/*
// const storeClone = me.transaction.objectStore(me.name); // Ensure clone is made if not present
// const indexes = storeClone.__indexes;
const storeClones = me.transaction.__storeClones;
if (!storeClones[me.name] || storeClones[me.name].__deleted) { // The latter condition is to allow store
// recreation to create new clone object
storeClones[me.name] = IDBObjectStore.__clone(me, me.transaction);
}

const indexes = storeClones[me.name].__indexes;
if (!indexes[indexName]) {
indexes[indexName] = IDBIndex.__clone(index, me);
if (!me.__indexClones[indexName] ||
me.__indexes[indexName].__pendingDelete ||
me.__indexes[indexName].__deleted
) {
me.__indexClones[indexName] = IDBIndex.__clone(index, me);
}
return indexes[indexName];
*/
return IDBIndex.__clone(index, me);
return me.__indexClones[indexName];
};

/**
Expand All @@ -718,11 +736,11 @@ IDBObjectStore.prototype.createIndex = function (indexName, keyPath /* , optiona
throw new TypeError('No key path was specified');
}
IDBTransaction.__assertVersionChange(me.transaction);
if (me.__deleted) {
if (me.__deleted || me.__pendingDelete) {
throw createDOMException('InvalidStateError', 'This store has been deleted');
}
IDBTransaction.__assertActive(me.transaction);
if (me.__indexes[indexName] && !me.__indexes[indexName].__deleted) {
if (me.__indexes[indexName] && !me.__indexes[indexName].__deleted && !me.__indexes[indexName].__pendingDelete) {
throw createDOMException('ConstraintError', 'Index "' + indexName + '" already exists on ' + me.name);
}

Expand Down Expand Up @@ -758,7 +776,7 @@ IDBObjectStore.prototype.deleteIndex = function (name) {
throw new TypeError('No index name was specified');
}
IDBTransaction.__assertVersionChange(me.transaction);
if (me.__deleted) {
if (me.__deleted || me.__pendingDelete) {
throw createDOMException('InvalidStateError', 'This store has been deleted');
}
IDBTransaction.__assertActive(me.transaction);
Expand Down
2 changes: 1 addition & 1 deletion src/IDBRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ IDBRequest.__super = function IDBRequest () {
configurable: true,
get: function () {
if (this.__readyState !== 'done') {
throw createDOMException('InvalidStateError', 'The request is still pending.');
throw createDOMException('InvalidStateError', "Can't get " + prop + '; the request is still pending.');
}
return this['__' + prop];
}
Expand Down
3 changes: 2 additions & 1 deletion src/IDBTransaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,8 @@ IDBTransaction.prototype.objectStore = function (objectStoreName) {
}

if (!me.__storeClones[objectStoreName] ||
me.__storeClones[objectStoreName].__deleted) { // The latter condition is to allow store
me.__storeClones[objectStoreName].__pendingDelete ||
me.__storeClones[objectStoreName].__deleted) { // The latter conditions are to allow store
// recreation to create new clone object
me.__storeClones[objectStoreName] = IDBObjectStore.__clone(store, me);
}
Expand Down
Loading

0 comments on commit 9a81106

Please sign in to comment.