Bug 692652 - IndexedDB: Index updating is broken. r=sicking.

--HG-- extra : transplant_source : %92%5BBT%93%BA%3DO%D7%03v%B7%88%01%3AjZ%D0%81%AA
2024-09-13 09:24:08 -07:00 · 2011-11-21 20:18:19 -08:00 · 2011-11-21 20:18:19 -08:00 · e02b223f81
commit e02b223f81
parent 79a79a7fad
7 changed files with 340 additions and 66 deletions
--- a/dom/indexedDB/IDBObjectStore.cpp
+++ b/dom/indexedDB/IDBObjectStore.cpp
@ -664,7 +664,6 @@ IDBObjectStore::UpdateIndexes(IDBTransaction* aTransaction,
 #endif

  PRUint32 indexCount = aUpdateInfoArray.Length();
-  NS_ASSERTION(indexCount, "Don't call me!");

  nsCOMPtr<mozIStorageStatement> stmt;
  nsresult rv;
@ -702,34 +701,59 @@ IDBObjectStore::UpdateIndexes(IDBTransaction* aTransaction,

  NS_ASSERTION(aObjectDataId != LL_MININT, "Bad objectData id!");

-  for (PRUint32 indexIndex = 0; indexIndex < indexCount; indexIndex++) {
-    const IndexUpdateInfo& updateInfo = aUpdateInfoArray[indexIndex];
+  NS_NAMED_LITERAL_CSTRING(indexId, "index_id");
+  NS_NAMED_LITERAL_CSTRING(objectDataId, "object_data_id");
+  NS_NAMED_LITERAL_CSTRING(objectDataKey, "object_data_key");
+  NS_NAMED_LITERAL_CSTRING(value, "value");

-    stmt = aTransaction->IndexUpdateStatement(updateInfo.info.autoIncrement,
-                                              updateInfo.info.unique,
-                                              aOverwrite);
+  if (aOverwrite) {
+    stmt = aTransaction->IndexDataDeleteStatement(aAutoIncrement, false);
    NS_ENSURE_TRUE(stmt, NS_ERROR_FAILURE);

    mozStorageStatementScoper scoper2(stmt);

-    rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("index_id"),
-                               updateInfo.info.id);
+    rv = stmt->BindInt64ByName(objectDataId, aObjectDataId);
    NS_ENSURE_SUCCESS(rv, rv);

-    rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("object_data_id"),
-                               aObjectDataId);
+    rv = stmt->Execute();
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    stmt = aTransaction->IndexDataDeleteStatement(aAutoIncrement, true);
+    NS_ENSURE_TRUE(stmt, NS_ERROR_FAILURE);
+
+    mozStorageStatementScoper scoper3(stmt);
+
+    rv = stmt->BindInt64ByName(objectDataId, aObjectDataId);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    rv = stmt->Execute();
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  for (PRUint32 indexIndex = 0; indexIndex < indexCount; indexIndex++) {
+    const IndexUpdateInfo& updateInfo = aUpdateInfoArray[indexIndex];
+
+    NS_ASSERTION(updateInfo.info.autoIncrement == aAutoIncrement, "Huh?!");
+
+    // Insert new values.
+    stmt = aTransaction->IndexDataInsertStatement(aAutoIncrement,
+                                                  updateInfo.info.unique);
+    NS_ENSURE_TRUE(stmt, NS_ERROR_FAILURE);
+
+    mozStorageStatementScoper scoper4(stmt);
+
+    rv = stmt->BindInt64ByName(indexId, updateInfo.info.id);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    rv = stmt->BindInt64ByName(objectDataId, aObjectDataId);
    NS_ENSURE_SUCCESS(rv, rv);

    if (!updateInfo.info.autoIncrement) {
-      rv =
-        aObjectStoreKey.BindToStatement(stmt,
-                                        NS_LITERAL_CSTRING("object_data_key"));
+      rv = aObjectStoreKey.BindToStatement(stmt, objectDataKey);
      NS_ENSURE_SUCCESS(rv, rv);
    }

-    rv =
-      updateInfo.value.BindToStatementAllowUnset(stmt,
-                                                 NS_LITERAL_CSTRING("value"));
+    rv = updateInfo.value.BindToStatement(stmt, value);
    NS_ENSURE_SUCCESS(rv, rv);

    rv = stmt->Execute();
@ -1718,7 +1742,7 @@ AddHelper::DoDatabaseWork(mozIStorageConnection* aConnection)
  }

  // Update our indexes if needed.
-  if (!mIndexUpdateInfo.IsEmpty()) {
+  if (mOverwrite || !mIndexUpdateInfo.IsEmpty()) {
    PRInt64 objectDataId = autoIncrement ? mKey.ToInteger() : LL_MININT;
    rv = IDBObjectStore::UpdateIndexes(mTransaction, osid, mKey,
                                       autoIncrement, mOverwrite,
@ -2215,8 +2239,8 @@ CreateIndexHelper::InsertDataFromObjectStore(mozIStorageConnection* aConnection)
  bool hasResult;
  while (NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) {
    nsCOMPtr<mozIStorageStatement> insertStmt =
-      mTransaction->IndexUpdateStatement(mIndex->IsAutoIncrement(),
-                                         mIndex->IsUnique(), false);
+      mTransaction->IndexDataInsertStatement(mIndex->IsAutoIncrement(),
+                                             mIndex->IsUnique());
    NS_ENSURE_TRUE(insertStmt, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);

    mozStorageStatementScoper scoper2(insertStmt);
--- a/dom/indexedDB/IDBTransaction.cpp
+++ b/dom/indexedDB/IDBTransaction.cpp
@ -394,28 +394,13 @@ IDBTransaction::AddStatement(bool aCreate,
 }

 already_AddRefed<mozIStorageStatement>
-IDBTransaction::IndexUpdateStatement(bool aAutoIncrement,
-                                     bool aUnique,
-                                     bool aOverwrite)
+IDBTransaction::IndexDataInsertStatement(bool aAutoIncrement,
+                                         bool aUnique)
 {
  if (aAutoIncrement) {
    if (aUnique) {
-      if (aOverwrite) {
-        return GetCachedStatement(
-          "INSERT OR REPLACE INTO ai_unique_index_data "
-            "(index_id, ai_object_data_id, value) "
-          "VALUES (:index_id, :object_data_id, :value)"
-        );
-      }
      return GetCachedStatement(
        "INSERT INTO ai_unique_index_data "
-          "(index_id, aI_object_data_id, value) "
-        "VALUES (:index_id, :object_data_id, :value)"
-      );
-    }
-    if (aOverwrite) {
-      return GetCachedStatement(
-        "INSERT OR REPLACE INTO ai_index_data "
          "(index_id, ai_object_data_id, value) "
        "VALUES (:index_id, :object_data_id, :value)"
      );
@ -427,26 +412,12 @@ IDBTransaction::IndexUpdateStatement(bool aAutoIncrement,
    );
  }
  if (aUnique) {
-    if (aOverwrite) {
-      return GetCachedStatement(
-        "INSERT OR REPLACE INTO unique_index_data "
-          "(index_id, object_data_id, object_data_key, value) "
-        "VALUES (:index_id, :object_data_id, :object_data_key, :value)"
-      );
-    }
    return GetCachedStatement(
      "INSERT INTO unique_index_data "
        "(index_id, object_data_id, object_data_key, value) "
      "VALUES (:index_id, :object_data_id, :object_data_key, :value)"
    );
  }
-  if (aOverwrite) {
-    return GetCachedStatement(
-      "INSERT INTO index_data ("
-        "index_id, object_data_id, object_data_key, value) "
-      "VALUES (:index_id, :object_data_id, :object_data_key, :value)"
-    );
-  }
  return GetCachedStatement(
    "INSERT INTO index_data ("
      "index_id, object_data_id, object_data_key, value) "
@ -454,6 +425,34 @@ IDBTransaction::IndexUpdateStatement(bool aAutoIncrement,
  );
 }

+already_AddRefed<mozIStorageStatement>
+IDBTransaction::IndexDataDeleteStatement(bool aAutoIncrement,
+                                         bool aUnique)
+{
+  if (aAutoIncrement) {
+    if (aUnique) {
+      return GetCachedStatement(
+        "DELETE FROM ai_unique_index_data "
+        "WHERE ai_object_data_id = :object_data_id"
+      );
+    }
+    return GetCachedStatement(
+      "DELETE FROM ai_index_data "
+      "WHERE ai_object_data_id = :object_data_id"
+    );
+  }
+  if (aUnique) {
+    return GetCachedStatement(
+      "DELETE FROM unique_index_data "
+      "WHERE object_data_id = :object_data_id"
+    );
+  }
+  return GetCachedStatement(
+    "DELETE FROM index_data "
+    "WHERE object_data_id = :object_data_id"
+  );
+}
+
 already_AddRefed<mozIStorageStatement>
 IDBTransaction::GetCachedStatement(const nsACString& aQuery)
 {
--- a/dom/indexedDB/IDBTransaction.h
+++ b/dom/indexedDB/IDBTransaction.h
@ -118,9 +118,12 @@ public:
               bool aAutoIncrement);

  already_AddRefed<mozIStorageStatement>
-  IndexUpdateStatement(bool aAutoIncrement,
-                       bool aUnique,
-                       bool aOverwrite);
+  IndexDataInsertStatement(bool aAutoIncrement,
+                           bool aUnique);
+
+  already_AddRefed<mozIStorageStatement>
+  IndexDataDeleteStatement(bool aAutoIncrement,
+                           bool aUnique);

  already_AddRefed<mozIStorageStatement>
  GetCachedStatement(const nsACString& aQuery);
--- a/dom/indexedDB/Key.h
+++ b/dom/indexedDB/Key.h
@ -248,19 +248,6 @@ public:
    return NS_SUCCEEDED(rv) ? NS_OK : NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
  }

-  nsresult BindToStatementAllowUnset(mozIStorageStatement* aStatement,
-                                     const nsACString& aParamName) const
-  {
-    nsresult rv;
-
-    if (IsUnset()) {
-      rv = aStatement->BindStringByName(aParamName, EmptyString());
-      return NS_SUCCEEDED(rv) ? NS_OK : NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
-    }
-
-    return BindToStatement(aStatement, aParamName);
-  }
-
  nsresult SetFromStatement(mozIStorageStatement* aStatement,
                            PRUint32 aIndex)
  {
--- a/dom/indexedDB/test/Makefile.in
+++ b/dom/indexedDB/test/Makefile.in
@ -78,6 +78,7 @@ TEST_FILES = \
  test_index_getAll.html \
  test_index_getAllObjects.html \
  test_index_object_cursors.html \
+  test_index_update_delete.html \
  test_indexes.html \
  test_indexes_bad_values.html \
  test_key_requirements.html \
@ -108,6 +109,7 @@ TEST_FILES = \
  test_setVersion_abort.html \
  test_setVersion_events.html \
  test_setVersion_exclusion.html \
+  test_unique_index_update.html \
  test_writer_starvation.html \
  third_party_iframe1.html \
  third_party_iframe2.html \
--- a/dom/indexedDB/test/test_index_update_delete.html
+++ b/dom/indexedDB/test/test_index_update_delete.html
@ -0,0 +1,180 @@
+<!--
+  Any copyright is dedicated to the Public Domain.
+  http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html>
+<head>
+  <title>Indexed Database Property Test</title>
+
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+
+  <script type="text/javascript;version=1.7">
+    function testSteps()
+    {
+      let request = mozIndexedDB.open(window.location.pathname, 1);
+      request.onerror = errorHandler;
+      request.onupgradeneeded = grabEventAndContinueHandler;
+      request.onsuccess = grabEventAndContinueHandler;
+
+      let event = yield;
+
+      let db = event.target.result;
+      db.onerror = errorHandler;
+
+      for each (let autoIncrement in [false, true]) {
+        let objectStore =
+          db.createObjectStore(autoIncrement, { keyPath: "id",
+                                                autoIncrement: autoIncrement });
+
+        for (let i = 0; i < 10; i++) {
+          objectStore.add({ id: i, index: i });
+        }
+
+        for each (let unique in [false, true]) {
+          objectStore.createIndex(unique, "index", { unique: unique });
+        }
+
+        for (let i = 10; i < 20; i++) {
+          objectStore.add({ id: i, index: i });
+        }
+      }
+
+      event = yield;
+      is(event.type, "success", "expect a success event");
+
+      for each (let autoIncrement in [false, true]) {
+        let objectStore = db.transaction(autoIncrement)
+                            .objectStore(autoIncrement);
+
+        objectStore.count().onsuccess = grabEventAndContinueHandler;
+        let event = yield;
+
+        is(event.target.result, 20, "Correct number of entries in objectStore");
+
+        let objectStoreCount = event.target.result;
+        let indexCount = event.target.result;
+
+        for each (let unique in [false, true]) {
+          let index = db.transaction(autoIncrement, IDBTransaction.READ_WRITE)
+                        .objectStore(autoIncrement)
+                        .index(unique);
+
+          index.count().onsuccess = grabEventAndContinueHandler;
+          let event = yield;
+
+          is(event.target.result, indexCount,
+             "Correct number of entries in index");
+
+          let modifiedEntry = unique ? 5 : 10;
+          let keyRange = IDBKeyRange.only(modifiedEntry);
+
+          let sawEntry = false;
+          index.openCursor(keyRange).onsuccess = function(event) {
+            let cursor = event.target.result;
+            if (cursor) {
+              sawEntry = true;
+              is(cursor.key, modifiedEntry, "Correct key");
+
+              cursor.value.index = unique ? 30 : 35;
+              cursor.update(cursor.value).onsuccess = function(event) {
+                cursor.continue();
+              }
+            }
+            else {
+              continueToNextStep();
+            }
+          }
+          yield;
+
+          is(sawEntry, true, "Saw entry for key value " + modifiedEntry);
+
+          // Recount index. Shouldn't change.
+          index = db.transaction(autoIncrement, IDBTransaction.READ_WRITE)
+                    .objectStore(autoIncrement)
+                    .index(unique);
+
+          index.count().onsuccess = grabEventAndContinueHandler;
+          event = yield;
+
+          is(event.target.result, indexCount,
+             "Correct number of entries in index");
+
+          modifiedEntry = unique ? 30 : 35;
+          keyRange = IDBKeyRange.only(modifiedEntry);
+
+          sawEntry = false;
+          index.openCursor(keyRange).onsuccess = function(event) {
+            let cursor = event.target.result;
+            if (cursor) {
+              sawEntry = true;
+              is(cursor.key, modifiedEntry, "Correct key");
+
+              delete cursor.value.index;
+              cursor.update(cursor.value).onsuccess = function(event) {
+                indexCount--;
+                cursor.continue();
+              }
+            }
+            else {
+              continueToNextStep();
+            }
+          }
+          yield;
+
+          is(sawEntry, true, "Saw entry for key value " + modifiedEntry);
+
+          // Recount objectStore. Should be unchanged.
+          objectStore = db.transaction(autoIncrement, IDBTransaction.READ_WRITE)
+                          .objectStore(autoIncrement);
+
+          objectStore.count().onsuccess = grabEventAndContinueHandler;
+          event = yield;
+
+          is(event.target.result, objectStoreCount,
+             "Correct number of entries in objectStore");
+
+          // Recount index. Should be one item less.
+          index = objectStore.index(unique);
+
+          index.count().onsuccess = grabEventAndContinueHandler;
+          event = yield;
+
+          is(event.target.result, indexCount,
+             "Correct number of entries in index");
+
+          modifiedEntry = objectStoreCount - 1;
+
+          objectStore.delete(modifiedEntry).onsuccess =
+            grabEventAndContinueHandler;
+          event = yield;
+
+          objectStoreCount--;
+          indexCount--;
+
+          objectStore.count().onsuccess = grabEventAndContinueHandler;
+          event = yield;
+
+          is(event.target.result, objectStoreCount,
+             "Correct number of entries in objectStore");
+
+          index.count().onsuccess = grabEventAndContinueHandler;
+          event = yield;
+
+          is(event.target.result, indexCount,
+             "Correct number of entries in index");
+        }
+      }
+
+      finishTest();
+      yield;
+    }
+
+  </script>
+  <script type="text/javascript;version=1.7" src="helpers.js"></script>
+
+</head>
+
+<body onload="runTest();"></body>
+
+</html>
--- a/dom/indexedDB/test/test_unique_index_update.html
+++ b/dom/indexedDB/test/test_unique_index_update.html
@ -0,0 +1,79 @@
+<!--
+  Any copyright is dedicated to the Public Domain.
+  http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html>
+<head>
+  <title>Indexed Database Property Test</title>
+
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+
+  <script type="text/javascript;version=1.7">
+    function testSteps()
+    {
+      let request = mozIndexedDB.open(window.location.pathname, 1);
+      request.onerror = errorHandler;
+      request.onupgradeneeded = grabEventAndContinueHandler;
+      request.onsuccess = grabEventAndContinueHandler;
+
+      let event = yield;
+
+      let db = event.target.result;
+
+      for each (let autoIncrement in [false, true]) {
+        let objectStore =
+          db.createObjectStore(autoIncrement, { keyPath: "id",
+                                                autoIncrement: autoIncrement });
+        objectStore.createIndex("", "index", { unique: true });
+
+        for (let i = 0; i < 10; i++) {
+          objectStore.add({ id: i, index: i });
+        }
+      }
+
+      event = yield;
+      is(event.type, "success", "expect a success event");
+
+      for each (let autoIncrement in [false, true]) {
+        objectStore = db.transaction(autoIncrement, IDBTransaction.READ_WRITE)
+                        .objectStore(autoIncrement);
+
+        request = objectStore.put({ id: 5, index: 6 });
+        request.onsuccess = unexpectedSuccessHandler;
+        request.onerror = new ExpectError(IDBDatabaseException.CONSTRAINT_ERR);
+        event = yield;
+
+        event.preventDefault();
+
+        let keyRange = IDBKeyRange.only(5);
+
+        objectStore.index("").openCursor(keyRange).onsuccess = function(event) {
+          let cursor = event.target.result;
+          ok(cursor, "Must have a cursor here");
+
+          is(cursor.value.index, 5, "Still have the right index value");
+
+          cursor.value.index = 6;
+
+          request = cursor.update(cursor.value);
+          request.onsuccess = unexpectedSuccessHandler;
+          request.onerror =
+            new ExpectError(IDBDatabaseException.CONSTRAINT_ERR);
+        };
+
+        yield;
+      }
+
+      finishTest();
+      yield;
+    }
+
+  </script>
+  <script type="text/javascript;version=1.7" src="helpers.js"></script>
+
+</head>
+
+<body onload="runTest();"></body>
+
+</html>