Bug 706659 part 2: Support empty keypaths on objectStores. r=bent

dom/indexedDB/IDBDatabase.cpp

@@ -464,11 +464,8 @@ IDBDatabase::CreateObjectStore(const nsAString& aName,
 
   DatabaseInfo* databaseInfo = Info();
 
-  if (databaseInfo->ContainsStoreName(aName)) {
-    return NS_ERROR_DOM_INDEXEDDB_CONSTRAINT_ERR;
-  }
-
   nsString keyPath;
+  keyPath.SetIsVoid(true);
   bool autoIncrement = false;
 
   if (!JSVAL_IS_VOID(aOptions) && !JSVAL_IS_NULL(aOptions)) {
@@ -514,7 +511,11 @@ IDBDatabase::CreateObjectStore(const nsAString& aName,
     autoIncrement = !!boolVal;
   }
 
-  if (!IDBObjectStore::IsValidKeyPath(aCx, keyPath)) {
+  if (databaseInfo->ContainsStoreName(aName)) {
+    return NS_ERROR_DOM_INDEXEDDB_CONSTRAINT_ERR;
+  }
+
+  if (!keyPath.IsVoid() && !IDBObjectStore::IsValidKeyPath(aCx, keyPath)) {
     return NS_ERROR_DOM_SYNTAX_ERR;
   }
 
@@ -800,8 +801,8 @@ CreateObjectStoreHelper::DoDatabaseWork(mozIStorageConnection* aConnection)
 {
   nsCOMPtr<mozIStorageStatement> stmt =
     mTransaction->GetCachedStatement(NS_LITERAL_CSTRING(
-    "INSERT INTO object_store (id, name, key_path, auto_increment) "
-    "VALUES (:id, :name, :key_path, :auto_increment)"
+    "INSERT INTO object_store (id, auto_increment, name, key_path) "
+    "VALUES (:id, :auto_increment, :name, :key_path)"
   ));
   NS_ENSURE_TRUE(stmt, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
 
@@ -811,15 +812,17 @@ CreateObjectStoreHelper::DoDatabaseWork(mozIStorageConnection* aConnection)
                                        mObjectStore->Id());
   NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
 
+  rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("auto_increment"),
+                             mObjectStore->IsAutoIncrement() ? 1 : 0);
+  NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
+
   rv = stmt->BindStringByName(NS_LITERAL_CSTRING("name"), mObjectStore->Name());
   NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
 
-  rv = stmt->BindStringByName(NS_LITERAL_CSTRING("key_path"),
-                              mObjectStore->KeyPath());
-  NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
-
-  rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("auto_increment"),
-                             mObjectStore->IsAutoIncrement() ? 1 : 0);
+  rv = mObjectStore->HasKeyPath() ?
+    stmt->BindStringByName(NS_LITERAL_CSTRING("key_path"),
+                           mObjectStore->KeyPath()) :
+    stmt->BindNullByName(NS_LITERAL_CSTRING("key_path"));
   NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
 
   rv = stmt->Execute();

dom/indexedDB/IDBFactory.cpp

@@ -247,8 +247,18 @@ IDBFactory::LoadDatabaseInformation(mozIStorageConnection* aConnection,
 
     info->id = stmt->AsInt64(1);
 
-    rv = stmt->GetString(2, info->keyPath);
+    PRInt32 columnType;
+    nsresult rv = stmt->GetTypeOfIndex(2, &columnType);
     NS_ENSURE_SUCCESS(rv, rv);
+    if (columnType == mozIStorageStatement::VALUE_TYPE_NULL) {
+      info->keyPath.SetIsVoid(true);
+    }
+    else {
+      NS_ASSERTION(columnType == mozIStorageStatement::VALUE_TYPE_TEXT,
+                   "Should be a string");
+      rv = stmt->GetString(2, info->keyPath);
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
 
     info->autoIncrement = !!stmt->AsInt32(3);
     info->databaseId = aDatabaseId;

dom/indexedDB/IDBObjectStore.cpp

@@ -899,13 +899,13 @@ IDBObjectStore::GetAddInfo(JSContext* aCx,
 
   // Return DATA_ERR if a key was passed in and this objectStore uses inline
   // keys.
-  if (!JSVAL_IS_VOID(aKeyVal) && !mKeyPath.IsEmpty()) {
+  if (!JSVAL_IS_VOID(aKeyVal) && HasKeyPath()) {
     return NS_ERROR_DOM_INDEXEDDB_DATA_ERR;
   }
 
   JSAutoRequest ar(aCx);
 
-  if (mKeyPath.IsEmpty()) {
+  if (!HasKeyPath()) {
     // Out-of-line keys must be passed in.
     rv = aKey.SetFromJSVal(aCx, aKeyVal);
     NS_ENSURE_SUCCESS(rv, rv);
@@ -913,10 +913,6 @@ IDBObjectStore::GetAddInfo(JSContext* aCx,
   else {
     // Inline keys live on the object. Make sure that the value passed in is an
     // object.
-    if (JSVAL_IS_PRIMITIVE(aValue)) {
-      return NS_ERROR_DOM_INDEXEDDB_DATA_ERR;
-    }
-
     rv = GetKeyFromValue(aCx, aValue, mKeyPath, aKey);
     NS_ENSURE_SUCCESS(rv, rv);
   }
@@ -941,8 +937,12 @@ IDBObjectStore::GetAddInfo(JSContext* aCx,
   const size_t keyPathLen = mKeyPath.Length();
   JSBool ok = JS_FALSE;
 
-  if (!mKeyPath.IsEmpty() && aKey.IsUnset()) {
+  if (HasKeyPath() && aKey.IsUnset()) {
     NS_ASSERTION(mAutoIncrement, "Should have bailed earlier!");
+    
+    if (JSVAL_IS_PRIMITIVE(aValue)) {
+      return NS_ERROR_DOM_INDEXEDDB_DATA_ERR;
+    }
 
     JSObject* obj = JS_NewObject(aCx, &gDummyPropClass, nsnull, nsnull);
     NS_ENSURE_TRUE(obj, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);

dom/indexedDB/IDBObjectStore.h

@@ -145,6 +145,11 @@ public:
     return mKeyPath;
   }
 
+  const bool HasKeyPath() const
+  {
+    return !mKeyPath.IsVoid();
+  }
+
   IDBTransaction* Transaction()
   {
     return mTransaction;

dom/indexedDB/IndexedDatabase.h

@@ -51,7 +51,7 @@
 #include "nsStringGlue.h"
 #include "nsTArray.h"
 
-#define DB_SCHEMA_VERSION 6
+#define DB_SCHEMA_VERSION 7
 
 #define BEGIN_INDEXEDDB_NAMESPACE \
   namespace mozilla { namespace dom { namespace indexedDB {

dom/indexedDB/OpenDatabaseHelper.cpp

@@ -122,9 +122,9 @@ CreateTables(mozIStorageConnection* aDBConn)
   rv = aDBConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
     "CREATE TABLE object_store ("
       "id INTEGER PRIMARY KEY, "
-      "name TEXT NOT NULL, "
-      "key_path TEXT NOT NULL, "
       "auto_increment INTEGER NOT NULL DEFAULT 0, "
+      "name TEXT NOT NULL, "
+      "key_path TEXT, "
       "UNIQUE (name)"
     ");"
   ));
@@ -753,6 +753,72 @@ UpgradeSchemaFrom5To6(mozIStorageConnection* aConnection)
   return NS_OK;
 }
 
+nsresult
+UpgradeSchemaFrom6To7(mozIStorageConnection* aConnection)
+{
+  mozStorageTransaction transaction(aConnection, false,
+                                 mozIStorageConnection::TRANSACTION_IMMEDIATE);
+
+  // Turn off foreign key constraints before we do anything here.
+  nsresult rv = aConnection->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "PRAGMA foreign_keys = OFF;"
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = aConnection->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "CREATE TEMPORARY TABLE temp_upgrade ("
+      "id, "
+      "name, "
+      "key_path, "
+      "auto_increment, "
+    ");"
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = aConnection->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "INSERT INTO temp_upgrade "
+      "SELECT id, name, key_path, auto_increment "
+      "FROM object_store;"
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = aConnection->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "DROP TABLE object_store;"
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = aConnection->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "CREATE TABLE object_store ("
+      "id INTEGER PRIMARY KEY, "
+      "auto_increment INTEGER NOT NULL DEFAULT 0, "
+      "name TEXT NOT NULL, "
+      "key_path TEXT, "
+      "UNIQUE (name)"
+    ");"
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = aConnection->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "INSERT INTO object_store "
+      "SELECT id, auto_increment, name, nullif(key_path, '') "
+      "FROM temp_upgrade;"
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = aConnection->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "DROP TABLE temp_upgrade;"
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = aConnection->SetSchemaVersion(7);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = transaction.Commit();
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return NS_OK;
+}
+
 nsresult
 CreateDatabaseConnection(const nsAString& aName,
                          nsIFile* aDBFile,
@@ -804,7 +870,7 @@ CreateDatabaseConnection(const nsAString& aName,
   }
   else if (schemaVersion != DB_SCHEMA_VERSION) {
     // This logic needs to change next time we change the schema!
-    PR_STATIC_ASSERT(DB_SCHEMA_VERSION == 6);
+    PR_STATIC_ASSERT(DB_SCHEMA_VERSION == 7);
 
 #define UPGRADE_SCHEMA_CASE(_from, _to)                                        \
   if (schemaVersion == _from) {                                                \
@@ -819,6 +885,7 @@ CreateDatabaseConnection(const nsAString& aName,
 
     UPGRADE_SCHEMA_CASE(4, 5)
     UPGRADE_SCHEMA_CASE(5, 6)
+    UPGRADE_SCHEMA_CASE(6, 7)
 
 #undef UPGRADE_SCHEMA_CASE
 

dom/indexedDB/OpenDatabaseHelper.h

@@ -60,8 +60,8 @@ public:
                      bool aForDeletion)
     : HelperBase(aRequest), mOpenDBRequest(aRequest), mName(aName),
       mASCIIOrigin(aASCIIOrigin), mRequestedVersion(aRequestedVersion),
-      mForDeletion(aForDeletion), mCurrentVersion(0),
-      mDataVersion(DB_SCHEMA_VERSION), mDatabaseId(0), mLastObjectStoreId(0),
+      mForDeletion(aForDeletion), mDatabaseId(nsnull), mCurrentVersion(0),
+      mDataVersion(DB_SCHEMA_VERSION), mLastObjectStoreId(0),
       mLastIndexId(0), mState(eCreated), mResultCode(NS_OK)
   {
     NS_ASSERTION(!aForDeletion || !aRequestedVersion,

dom/indexedDB/test/test_complex_keyPaths.html

@@ -32,6 +32,11 @@
         { keyPath: "foo.id",  value: { foo: { id: undefined } } },
         { keyPath: "foo.id",  value: { foo: 47 } },
         { keyPath: "foo.id",  value: {} },
+        { keyPath: "",        value: "foopy",                        key: "foopy" },
+        { keyPath: "",        value: 2,                              key: 2 },
+        { keyPath: "",        value: undefined },
+        { keyPath: "",        value: { id: 12 } },
+        { keyPath: "",        value: /x/ },
         { keyPath: "foo.bar", value: { baz: 1, foo: { baz2: 2, bar: "xo" } },     key: "xo" },
         { keyPath: "foo.bar.baz", value: { foo: { bar: { bazz: 16, baz: 17 } } }, key: 17 },
         { keyPath: "foo..id", exception: true },
@@ -61,6 +66,7 @@
           try {
             let objectStore = db.createObjectStore(info.keyPath, { keyPath: info.keyPath });
             ok(!("exception" in info), "expected exception behavior observed" + test);
+            is(objectStore.keyPath, info.keyPath, "objectStore has correct keyPath property" + test);
             stores[info.keyPath] = objectStore;
           } catch (e) {
             ok("exception" in info, "expected exception behavior observed" + test);
@@ -103,6 +109,7 @@
           try {
             let index = store.createIndex(info.keyPath, info.keyPath);
             ok(!("exception" in info), "expected exception behavior observed");
+            is(index.keyPath, info.keyPath, "index has correct keyPath property");
             indexes[info.keyPath] = index;
           } catch (e) {
             ok("exception" in info, "expected exception behavior observed");

dom/indexedDB/test/test_create_objectStore.html

@@ -86,7 +86,7 @@
 
         is(objectStore.name, name, "Bad name");
         is(objectStore.keyPath, info.options && info.options.keyPath ?
-                                info.options.keyPath : "",
+                                info.options.keyPath : null,
            "Bad keyPath");
         if(objectStore.indexNames.length, 0, "Bad indexNames");
 

dom/indexedDB/test/test_transaction_abort.html

@@ -57,7 +57,7 @@
       is(transaction.onabort, null, "No abort listener");
 
       is(objectStore.name, "foo", "Correct name");
-      is(objectStore.keyPath, "", "Correct keyPath");
+      is(objectStore.keyPath, null, "Correct keyPath");
 
       is(objectStore.indexNames.length, 1, "Correct indexNames length");
       is(objectStore.indexNames[0], "fooindex", "Correct indexNames name");
@@ -83,7 +83,7 @@
       }
 
       is(objectStore.name, "foo", "Correct name");
-      is(objectStore.keyPath, "", "Correct keyPath");
+      is(objectStore.keyPath, null, "Correct keyPath");
 
       is(objectStore.indexNames.length, 1, "Correct indexNames length");
       is(objectStore.indexNames[0], "fooindex", "Correct indexNames name");