IndexedDB: Key generators not rolled back if insertion fails or is aborted
authorjsbell@chromium.org <jsbell@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 4 Feb 2012 00:16:45 +0000 (00:16 +0000)
committerjsbell@chromium.org <jsbell@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 4 Feb 2012 00:16:45 +0000 (00:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=77060

Reviewed by Tony Chang.

Source/WebCore:

Test: storage/indexeddb/key-generator.html

* storage/IDBObjectStoreBackendImpl.cpp:
(WebCore::IDBObjectStoreBackendImpl::put): Add abort task to reset cache.
(WebCore::IDBObjectStoreBackendImpl::revertAutoIncrementKeyCache):
(WebCore):
(WebCore::IDBObjectStoreBackendImpl::putInternal): Reset cache on error.
* storage/IDBObjectStoreBackendImpl.h:
(IDBObjectStoreBackendImpl):

LayoutTests:

* storage/indexeddb/key-generator-expected.txt: Added.
* storage/indexeddb/key-generator.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@106705 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/storage/indexeddb/key-generator-expected.txt [new file with mode: 0644]
LayoutTests/storage/indexeddb/key-generator.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/storage/IDBObjectStoreBackendImpl.cpp
Source/WebCore/storage/IDBObjectStoreBackendImpl.h

index 351ec23..efdafd7 100644 (file)
@@ -1,3 +1,13 @@
+2012-02-03  Joshua Bell  <jsbell@chromium.org>
+
+        IndexedDB: Key generators not rolled back if insertion fails or is aborted
+        https://bugs.webkit.org/show_bug.cgi?id=77060
+
+        Reviewed by Tony Chang.
+
+        * storage/indexeddb/key-generator-expected.txt: Added.
+        * storage/indexeddb/key-generator.html: Added.
+
 2012-02-03  Tony Chang  <tony@chromium.org>
 
         positive and negative flex values are not being cleared on style changes
diff --git a/LayoutTests/storage/indexeddb/key-generator-expected.txt b/LayoutTests/storage/indexeddb/key-generator-expected.txt
new file mode 100644 (file)
index 0000000..509d248
--- /dev/null
@@ -0,0 +1,152 @@
+Test IndexedDB's key generator behavior.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+indexedDB = window.indexedDB || window.webkitIndexedDB
+PASS indexedDB != null is true
+IDBTransaction = window.IDBTransaction || window.webkitIDBTransaction
+PASS IDBTransaction != null is true
+IDBKeyRange = window.IDBKeyRange || window.webkitIDBKeyRange
+PASS IDBKeyRange != null is true
+
+Verify that each object store has an independent key generator.
+request = indexedDB.deleteDatabase('key-generator')
+request = indexedDB.open('key-generator')
+db = request.result
+request = db.setVersion('1')
+trans = request.result
+store1 = db.createObjectStore('store1', { autoIncrement: true })
+store1.put('a')
+request = store.get(1)
+store2 = db.createObjectStore('store2', { autoIncrement: true })
+store2.put('a')
+request = store.get(1)
+store1.put('b')
+request = store.get(2)
+store2.put('b')
+request = store.get(2)
+PASS Got "a" for key: 1
+PASS Got "a" for key: 1
+PASS Got "b" for key: 2
+PASS Got "b" for key: 2
+db.close()
+
+Verify that the key generator is not updated if insertion fails
+request = indexedDB.deleteDatabase('key-generator')
+request = indexedDB.open('key-generator')
+db = request.result
+request = db.setVersion('1')
+trans = request.result
+store = db.createObjectStore('store1', { autoIncrement: true })
+index = store.createIndex('index1', 'ix', { unique: true })
+store.put({ ix: 'a'})
+request = store.get(1)
+store.put({ ix: 'a'})
+store.put({ ix: 'b'})
+request = store.get(2)
+PASS Got {"ix":"a"} for key: 1
+PASS Got {"ix":"b"} for key: 2
+db.close()
+
+Verify that the key generator is not affected by item removal (delete or clear).
+request = indexedDB.deleteDatabase('key-generator')
+request = indexedDB.open('key-generator')
+db = request.result
+request = db.setVersion('1')
+trans = request.result
+store = db.createObjectStore('store1', { autoIncrement: true })
+store.put('a')
+request = store.get(1)
+store.delete(1)
+store.put('b')
+request = store.get(2)
+store.clear()
+store.put('c')
+request = store.get(3)
+store.put('d')
+request = store.get(4)
+PASS Got "a" for key: 1
+PASS Got "b" for key: 2
+PASS Got "c" for key: 3
+PASS Got "d" for key: 4
+db.close()
+
+Verify that the key generator is only set if and only if a numeric key greater than the last generated key is used.
+request = indexedDB.deleteDatabase('key-generator')
+request = indexedDB.open('key-generator')
+db = request.result
+request = db.setVersion('1')
+trans = request.result
+store = db.createObjectStore('store1', { autoIncrement: true })
+store.put('a')
+request = store.get(1)
+store.put('b', 3)
+request = store.get(3)
+store.put('c')
+request = store.get(4)
+store.put('d', -10)
+request = store.get(-10)
+store.put('e')
+request = store.get(5)
+store.put('f', 6.00001)
+request = store.get(6.00001)
+store.put('g')
+request = store.get(7)
+store.put('f', 8.9999)
+request = store.get(8.9999)
+store.put('g')
+request = store.get(9)
+store.put('h', 'foo')
+request = store.get("foo")
+store.put('i')
+request = store.get(10)
+store.put('j', [1000])
+request = store.get([1000])
+store.put('k')
+request = store.get(11)
+PASS Got "a" for key: 1
+PASS Got "b" for key: 3
+PASS Got "c" for key: 4
+PASS Got "d" for key: -10
+PASS Got "e" for key: 5
+PASS Got "f" for key: 6.00001
+PASS Got "g" for key: 7
+PASS Got "f" for key: 8.9999
+PASS Got "g" for key: 9
+PASS Got "h" for key: "foo"
+PASS Got "i" for key: 10
+PASS Got "j" for key: [1000]
+PASS Got "k" for key: 11
+db.close()
+
+Verify that aborting a transaction resets the key generator state.
+request = indexedDB.deleteDatabase('key-generator')
+request = indexedDB.open('key-generator')
+db = request.result
+request = db.setVersion('1')
+trans = request.result
+trans1 = db.transaction(['store'], IDBTransaction.READ_WRITE)
+store_t1 = trans1.objectStore('store')
+store_t1.put('a')
+request = store.get(1)
+store_t1.put('b')
+request = store.get(2)
+PASS Got "a" for key: 1
+PASS Got "b" for key: 2
+aborting...
+trans1.abort()
+aborted!
+trans2 = db.transaction(['store'], IDBTransaction.READ_WRITE)
+store_t2 = trans2.objectStore('store')
+store_t2.put('c')
+request = store.get(1)
+store_t2.put('d')
+request = store.get(2)
+PASS Got "c" for key: 1
+PASS Got "d" for key: 2
+db.close()
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/storage/indexeddb/key-generator.html b/LayoutTests/storage/indexeddb/key-generator.html
new file mode 100644 (file)
index 0000000..2124045
--- /dev/null
@@ -0,0 +1,223 @@
+<html>
+<head>
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+<script src="resources/shared.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script>
+
+description("Test IndexedDB's key generator behavior.");
+if (window.layoutTestController) 
+    layoutTestController.waitUntilDone();
+
+function test()
+{
+    evalAndLog("indexedDB = window.indexedDB || window.webkitIndexedDB");
+    shouldBeTrue("indexedDB != null");
+    evalAndLog("IDBTransaction = window.IDBTransaction || window.webkitIDBTransaction");
+    shouldBeTrue("IDBTransaction != null");
+    evalAndLog("IDBKeyRange = window.IDBKeyRange || window.webkitIDBKeyRange");
+    shouldBeTrue("IDBKeyRange != null");
+
+    runTests();
+}
+
+var tests = [];
+function defineTest(description, verchange, optional) {
+  tests.push(
+    {
+      description: description,
+      verchange: verchange,
+      optional: optional
+    }
+  );
+}
+
+
+function runTests() {
+
+    function nextTest() {
+        if (!tests.length) {
+            done();
+            return;
+        }
+
+        var test = tests.shift();
+        debug("");
+        debug(test.description);
+
+        evalAndLog("request = indexedDB.deleteDatabase('key-generator')");
+        request.onerror = unexpectedErrorCallback;
+        request.onsuccess = function () {
+            evalAndLog("request = indexedDB.open('key-generator')");
+            request.onerror = unexpectedErrorCallback;
+            request.onsuccess = function () {
+                evalAndLog("db = request.result");
+                evalAndLog("request = db.setVersion('1')");
+                request.onerror = unexpectedErrorCallback;
+                request.onsuccess = function () {
+                    evalAndLog("trans = request.result");
+                    trans.onabort = unexpectedAbortCallback;
+                    test.verchange(db, trans);
+                    trans.oncomplete = function () {
+
+                        function finishTest() {
+                            evalAndLog("db.close()");
+                            nextTest();
+                        }
+
+                        if (test.optional) {
+                            test.optional(db, finishTest);
+                        } else {
+                            finishTest();
+                        }
+                    };
+                };
+            };
+        };
+    }
+
+    nextTest();
+}
+
+function check(store, key, expected) {
+    window.store = store;
+    request = evalAndLog("request = store.get(" + JSON.stringify(key) + ")");
+    request.onerror = unexpectedErrorCallback;
+    request.onsuccess = function (e) {
+        window.expected = expected;
+        if (JSON.stringify(event.target.result) === JSON.stringify(expected)) {
+            testPassed("Got " + JSON.stringify(event.target.result) + " for key: " + JSON.stringify(key));
+        } else {
+            testFailed("Got " + JSON.stringify(event.target.result) + " for key: " + JSON.stringify(key) +
+                " expected: " + JSON.stringify(expected));
+        }
+    };
+}
+
+defineTest(
+    'Verify that each object store has an independent key generator.',
+    function (db, trans) {
+        evalAndLog("store1 = db.createObjectStore('store1', { autoIncrement: true })");
+        evalAndLog("store1.put('a')");
+        check(store1, 1, 'a');
+        evalAndLog("store2 = db.createObjectStore('store2', { autoIncrement: true })");
+        evalAndLog("store2.put('a')");
+        check(store2, 1, 'a');
+        evalAndLog("store1.put('b')");
+        check(store1, 2, 'b');
+        evalAndLog("store2.put('b')");
+        check(store2, 2, 'b');
+    }
+);
+
+defineTest(
+    'Verify that the key generator is not updated if insertion fails',
+    function (db, trans) {
+        trans.onerror = function(e) { e.preventDefault() };
+        evalAndLog("store = db.createObjectStore('store1', { autoIncrement: true })");
+        evalAndLog("index = store.createIndex('index1', 'ix', { unique: true })");
+        evalAndLog("store.put({ ix: 'a'})");
+        check(store, 1, {ix: 'a'});
+        evalAndLog("store.put({ ix: 'a'})");
+        evalAndLog("store.put({ ix: 'b'})");
+        check(store, 2, {ix: 'b'});
+    }
+);
+
+defineTest(
+    'Verify that the key generator is not affected by item removal (delete or clear).',
+    function (db, trans) {
+        evalAndLog("store = db.createObjectStore('store1', { autoIncrement: true })");
+        evalAndLog("store.put('a')");
+        check(store, 1, 'a');
+        evalAndLog("store.delete(1)");
+        evalAndLog("store.put('b')");
+        check(store, 2, 'b');
+        evalAndLog("store.clear()");
+        evalAndLog("store.put('c')");
+        check(store, 3, 'c');
+        // FIXME: IDBObjectStore.delete(IDBKeyRange) is not yet implemented in Chromium.
+        // http://crbug.com/101384
+        //evalAndLog("store.delete(IDBKeyRange.lowerBound(0))");
+        evalAndLog("store.put('d')");
+        check(store, 4, 'd');
+    }
+);
+
+defineTest(
+    'Verify that the key generator is only set if and only if a numeric key greater than the last generated key is used.',
+    function (db, trans) {
+        evalAndLog("store = db.createObjectStore('store1', { autoIncrement: true })");
+        evalAndLog("store.put('a')");
+        check(store, 1, 'a');
+        evalAndLog("store.put('b', 3)");
+        check(store, 3, 'b');
+        evalAndLog("store.put('c')");
+        check(store, 4, 'c');
+        evalAndLog("store.put('d', -10)");
+        check(store, -10, 'd');
+        evalAndLog("store.put('e')");
+        check(store, 5, 'e');
+        evalAndLog("store.put('f', 6.00001)");
+        check(store, 6.00001, 'f');
+        evalAndLog("store.put('g')");
+        check(store, 7, 'g');
+        evalAndLog("store.put('f', 8.9999)");
+        check(store, 8.9999, 'f');
+        evalAndLog("store.put('g')");
+        check(store, 9, 'g');
+        evalAndLog("store.put('h', 'foo')");
+        check(store, 'foo', 'h');
+        evalAndLog("store.put('i')");
+        check(store, 10, 'i');
+        evalAndLog("store.put('j', [1000])");
+        check(store, [1000], 'j');
+        evalAndLog("store.put('k')");
+        check(store, 11, 'k');
+
+        // FIXME: Repeat this test, but with a keyPath and inline key.
+    }
+);
+
+defineTest(
+    'Verify that aborting a transaction resets the key generator state.',
+    function (db, trans) {
+        db.createObjectStore('store', { autoIncrement: true });
+    },
+
+    function (db, callback) {
+        evalAndLog("trans1 = db.transaction(['store'], IDBTransaction.READ_WRITE)");
+        evalAndLog("store_t1 = trans1.objectStore('store')");
+        evalAndLog("store_t1.put('a')");
+        check(store_t1, 1, 'a');
+        evalAndLog("store_t1.put('b')");
+        check(store_t1, 2, 'b');
+
+        // Schedule the abort as a task (not run it synchronously)
+        store_t1.get(0).onsuccess = function () {
+            debug('aborting...');
+            evalAndLog("trans1.abort()");
+            trans1.onabort = function () {
+                debug('aborted!');
+
+                evalAndLog("trans2 = db.transaction(['store'], IDBTransaction.READ_WRITE)");
+                evalAndLog("store_t2 = trans2.objectStore('store')");
+                evalAndLog("store_t2.put('c')");
+                check(store_t2, 1, 'c');
+                evalAndLog("store_t2.put('d')");
+                check(store_t2, 2, 'd');
+
+                trans2.oncomplete = callback;
+            };
+        };
+    }
+);
+
+test();
+
+</script>
+</body>
+</html>
index aba7be9..9b3101d 100644 (file)
@@ -1,3 +1,20 @@
+2012-02-03  Joshua Bell  <jsbell@chromium.org>
+
+        IndexedDB: Key generators not rolled back if insertion fails or is aborted
+        https://bugs.webkit.org/show_bug.cgi?id=77060
+
+        Reviewed by Tony Chang.
+
+        Test: storage/indexeddb/key-generator.html
+
+        * storage/IDBObjectStoreBackendImpl.cpp:
+        (WebCore::IDBObjectStoreBackendImpl::put): Add abort task to reset cache.
+        (WebCore::IDBObjectStoreBackendImpl::revertAutoIncrementKeyCache):
+        (WebCore):
+        (WebCore::IDBObjectStoreBackendImpl::putInternal): Reset cache on error.
+        * storage/IDBObjectStoreBackendImpl.h:
+        (IDBObjectStoreBackendImpl):
+
 2012-02-03  Tony Chang  <tony@chromium.org>
 
         positive and negative flex values are not being cleared on style changes
index ce042be..f9bd780 100644 (file)
@@ -178,10 +178,18 @@ void IDBObjectStoreBackendImpl::put(PassRefPtr<SerializedScriptValue> prpValue,
         }
     }
 
-    if (!transaction->scheduleTask(createCallbackTask(&IDBObjectStoreBackendImpl::putInternal, objectStore, value, key, putMode, callbacks, transaction)))
+    if (!transaction->scheduleTask(
+            createCallbackTask(&IDBObjectStoreBackendImpl::putInternal, objectStore, value, key, putMode, callbacks, transaction),
+            // FIXME: One of these per put() is overkill, since it's simply a cache invalidation.
+            createCallbackTask(&IDBObjectStoreBackendImpl::revertAutoIncrementKeyCache, objectStore)))
         ec = IDBDatabaseException::TRANSACTION_INACTIVE_ERR;
 }
 
+void IDBObjectStoreBackendImpl::revertAutoIncrementKeyCache(ScriptExecutionContext*, PassRefPtr<IDBObjectStoreBackendImpl> objectStore)
+{
+    objectStore->resetAutoIncrementKeyCache();
+}
+
 void IDBObjectStoreBackendImpl::putInternal(ScriptExecutionContext*, PassRefPtr<IDBObjectStoreBackendImpl> objectStore, PassRefPtr<SerializedScriptValue> prpValue, PassRefPtr<IDBKey> prpKey, PutMode putMode, PassRefPtr<IDBCallbacks> callbacks, PassRefPtr<IDBTransactionBackendInterface> transaction)
 {
     RefPtr<SerializedScriptValue> value = prpValue;
@@ -204,6 +212,7 @@ void IDBObjectStoreBackendImpl::putInternal(ScriptExecutionContext*, PassRefPtr<
                     // https://bugs.webkit.org/show_bug.cgi?id=77374
                     RefPtr<SerializedScriptValue> valueAfterInjection = injectKeyIntoKeyPath(autoIncKey, value, objectStore->m_keyPath);
                     if (!valueAfterInjection) {
+                        objectStore->resetAutoIncrementKeyCache();
                         callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::DATA_ERR, "The generated key could not be inserted into the object using the keyPath."));
                         return;
                     }
@@ -221,6 +230,7 @@ void IDBObjectStoreBackendImpl::putInternal(ScriptExecutionContext*, PassRefPtr<
 
     RefPtr<IDBBackingStore::ObjectStoreRecordIdentifier> recordIdentifier = objectStore->m_backingStore->createInvalidRecordIdentifier();
     if (putMode == AddOnly && objectStore->m_backingStore->keyExistsInObjectStore(objectStore->m_databaseId, objectStore->id(), *key, recordIdentifier.get())) {
+        objectStore->resetAutoIncrementKeyCache();
         callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::CONSTRAINT_ERR, "Key already exists in the object store."));
         return;
     }
@@ -237,13 +247,15 @@ void IDBObjectStoreBackendImpl::putInternal(ScriptExecutionContext*, PassRefPtr<
         ASSERT(indexKey->valid());
 
         if ((!index->multiEntry() || indexKey->type() != IDBKey::ArrayType) && !index->addingKeyAllowed(indexKey.get(), key.get())) {
+            objectStore->resetAutoIncrementKeyCache();
             callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::CONSTRAINT_ERR, "One of the derived (from a keyPath) keys for an index does not satisfy its uniqueness requirements."));
             return;
         }
 
-       if (index->multiEntry() && indexKey->type() == IDBKey::ArrayType) {
+        if (index->multiEntry() && indexKey->type() == IDBKey::ArrayType) {
            for (size_t j = 0; j < indexKey->array().size(); ++j) {
                 if (!index->addingKeyAllowed(indexKey->array()[j].get(), key.get())) {
+                    objectStore->resetAutoIncrementKeyCache();
                     callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::CONSTRAINT_ERR, "One of the derived (from a keyPath) keys for an index does not satisfy its uniqueness requirements."));
                     return;
                 }
index 7c12d84..bdbf1f1 100644 (file)
@@ -100,6 +100,7 @@ private:
     // These are used as setVersion transaction abort tasks.
     static void removeIndexFromMap(ScriptExecutionContext*, PassRefPtr<IDBObjectStoreBackendImpl>, PassRefPtr<IDBIndexBackendImpl>);
     static void addIndexToMap(ScriptExecutionContext*, PassRefPtr<IDBObjectStoreBackendImpl>, PassRefPtr<IDBIndexBackendImpl>);
+    static void revertAutoIncrementKeyCache(ScriptExecutionContext*, PassRefPtr<IDBObjectStoreBackendImpl>);
 
     RefPtr<IDBBackingStore> m_backingStore;