Bug 706068: Make complex keys work on auto-increment object stores. Patch by khuey and sicking. r=sicking on parts by khuey and r=bent on parts by sicking.
authorJonas Sicking <jonas@sicking.cc>
Fri, 02 Dec 2011 18:32:46 -0800
changeset 82849 56aba64236d8f4cf9dc70848485348888937aab7
parent 82848 c1b5a0721e6c6619980d287fed8f01ed1e832d67
child 82850 127374ca4f92a21a88159e43717ad9e7937947c1
push id519
push userakeybl@mozilla.com
push dateWed, 01 Feb 2012 00:38:35 +0000
treeherdermozilla-beta@788ea1ef610b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssicking, bent
bugs706068
milestone11.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 706068: Make complex keys work on auto-increment object stores. Patch by khuey and sicking. r=sicking on parts by khuey and r=bent on parts by sicking.
dom/indexedDB/IDBDatabase.cpp
dom/indexedDB/IDBObjectStore.cpp
dom/indexedDB/test/test_complex_keyPaths.html
--- a/dom/indexedDB/IDBDatabase.cpp
+++ b/dom/indexedDB/IDBDatabase.cpp
@@ -510,18 +510,23 @@ IDBDatabase::CreateObjectStore(const nsA
     }
     autoIncrement = !!boolVal;
   }
 
   if (databaseInfo->ContainsStoreName(aName)) {
     return NS_ERROR_DOM_INDEXEDDB_CONSTRAINT_ERR;
   }
 
-  if (!keyPath.IsVoid() && !IDBObjectStore::IsValidKeyPath(aCx, keyPath)) {
-    return NS_ERROR_DOM_SYNTAX_ERR;
+  if (!keyPath.IsVoid()) {
+    if (keyPath.IsEmpty() && autoIncrement) {
+      return NS_ERROR_DOM_INVALID_ACCESS_ERR;
+    }
+    if (!IDBObjectStore::IsValidKeyPath(aCx, keyPath)) {
+      return NS_ERROR_DOM_SYNTAX_ERR;
+    }
   }
 
   nsAutoPtr<ObjectStoreInfo> newInfo(new ObjectStoreInfo());
 
   newInfo->name = aName;
   newInfo->id = databaseInfo->nextObjectStoreId++;
   newInfo->keyPath = keyPath;
   newInfo->autoIncrement = autoIncrement;
--- a/dom/indexedDB/IDBObjectStore.cpp
+++ b/dom/indexedDB/IDBObjectStore.cpp
@@ -910,17 +910,17 @@ IDBObjectStore::GetAddInfo(JSContext* aC
 
   JSAutoRequest ar(aCx);
 
   if (!HasKeyPath()) {
     // Out-of-line keys must be passed in.
     rv = aKey.SetFromJSVal(aCx, aKeyVal);
     NS_ENSURE_SUCCESS(rv, rv);
   }
-  else {
+  else if (!mAutoIncrement) {
     // Inline keys live on the object. Make sure that the value passed in is an
     // object.
     rv = GetKeyFromValue(aCx, aValue, mKeyPath, aKey);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   // Return DATA_ERR if no key was specified this isn't an autoIncrement
   // objectStore.
@@ -938,42 +938,115 @@ IDBObjectStore::GetAddInfo(JSContext* aC
   aUpdateInfoArray.SetCapacity(count); // Pretty good estimate
   for (PRUint32 indexesIndex = 0; indexesIndex < count; indexesIndex++) {
     const IndexInfo& indexInfo = info->indexes[indexesIndex];
 
     rv = AppendIndexUpdateInfo(indexInfo.id, indexInfo.keyPath,
                                indexInfo.unique, indexInfo.multiEntry,
                                aCx, aValue, aUpdateInfoArray);
     NS_ENSURE_SUCCESS(rv, rv);
-
   }
 
-  const jschar* keyPathChars =
-    reinterpret_cast<const jschar*>(mKeyPath.get());
-  const size_t keyPathLen = mKeyPath.Length();
-  JSBool ok = JS_FALSE;
-
-  if (HasKeyPath() && aKey.IsUnset()) {
-    NS_ASSERTION(mAutoIncrement, "Should have bailed earlier!");
-    
+  nsString targetObjectPropName;
+  JSObject* targetObject = nsnull;
+
+  if (mAutoIncrement && HasKeyPath()) {
+    NS_ASSERTION(aKey.IsUnset(), "Shouldn't have gotten the key yet!");
+
     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);
-
-    jsval key = OBJECT_TO_JSVAL(obj);
- 
-    ok = JS_DefineUCProperty(aCx, JSVAL_TO_OBJECT(aValue), keyPathChars,
-                             keyPathLen, key, nsnull, nsnull,
-                             JSPROP_ENUMERATE);
-    NS_ENSURE_TRUE(ok, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
-
-    // From this point on we have to try to remove the property.
+    KeyPathTokenizer tokenizer(mKeyPath, '.');
+    NS_ASSERTION(tokenizer.hasMoreTokens(),
+                 "Shouldn't have empty keypath and autoincrement");
+
+    JSObject* obj = JSVAL_TO_OBJECT(aValue);
+    while (tokenizer.hasMoreTokens()) {
+      const nsDependentSubstring& token = tokenizer.nextToken();
+  
+      NS_ASSERTION(!token.IsEmpty(), "Should be a valid keypath");
+  
+      const jschar* keyPathChars = token.BeginReading();
+      const size_t keyPathLen = token.Length();
+  
+      JSBool hasProp;
+      if (!targetObject) {
+        // We're still walking the chain of existing objects
+
+        JSBool ok = JS_HasUCProperty(aCx, obj, keyPathChars, keyPathLen,
+                                     &hasProp);
+        NS_ENSURE_TRUE(ok, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
+
+        if (hasProp) {
+          // Get if the property exists...
+          jsval intermediate;
+          JSBool ok = JS_GetUCProperty(aCx, obj, keyPathChars, keyPathLen,
+                                       &intermediate);
+          NS_ENSURE_TRUE(ok, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
+
+          if (tokenizer.hasMoreTokens()) {
+            // ...and walk to it if there are more steps...
+            if (JSVAL_IS_PRIMITIVE(intermediate)) {
+              return NS_ERROR_DOM_INDEXEDDB_DATA_ERR;
+            }
+            obj = JSVAL_TO_OBJECT(intermediate);
+          }
+          else {
+            // ...otherwise use it as key
+            aKey.SetFromJSVal(aCx, intermediate);
+            if (aKey.IsUnset()) {
+              return NS_ERROR_DOM_INDEXEDDB_DATA_ERR;
+            }
+          }
+        }
+        else {
+          // If the property doesn't exist, fall into below path of starting
+          // to define properties
+          targetObject = obj;
+          targetObjectPropName = token;
+        }
+      }
+
+      if (targetObject) {
+        // We have started inserting new objects or are about to just insert
+        // the first one.
+        if (tokenizer.hasMoreTokens()) {
+          // If we're not at the end, we need to add a dummy object to the chain.
+          JSObject* dummy = JS_NewObject(aCx, nsnull, nsnull, nsnull);
+          if (!dummy) {
+            rv = NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
+            break;
+          }
+  
+          if (!JS_DefineUCProperty(aCx, obj, token.BeginReading(),
+                                   token.Length(),
+                                   OBJECT_TO_JSVAL(dummy), nsnull, nsnull,
+                                   JSPROP_ENUMERATE)) {
+            rv = NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
+            break;
+          }
+  
+          obj = dummy;
+        }
+        else {
+          JSObject* dummy = JS_NewObject(aCx, &gDummyPropClass, nsnull, nsnull);
+          if (!dummy) {
+            rv = NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
+            break;
+          }
+  
+          if (!JS_DefineUCProperty(aCx, obj, token.BeginReading(),
+                                   token.Length(), OBJECT_TO_JSVAL(dummy),
+                                   nsnull, nsnull, JSPROP_ENUMERATE)) {
+            rv = NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
+          }
+        }
+      }
+    }
   }
 
   JSStructuredCloneCallbacks callbacks = {
     nsnull,
     StructuredCloneWriteDummyProp,
     nsnull
   };
   *aOffsetToKeyProp = 0;
@@ -981,23 +1054,25 @@ IDBObjectStore::GetAddInfo(JSContext* aC
   // We guard on rv being a success because we need to run the property
   // deletion code below even if we should not be serializing the value
   if (NS_SUCCEEDED(rv) && 
       !IDBObjectStore::SerializeValue(aCx, aCloneBuffer, aValue, &callbacks,
                                       aOffsetToKeyProp)) {
     rv = NS_ERROR_DOM_DATA_CLONE_ERR;
   }
 
-  if (ok) {
+  if (targetObject) {
     // If this fails, we lose, and the web page sees a magical property
     // appear on the object :-(
     jsval succeeded;
-    ok = JS_DeleteUCProperty2(aCx, JSVAL_TO_OBJECT(aValue), keyPathChars,
-                              keyPathLen, &succeeded);
-    NS_ENSURE_TRUE(ok, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
+    if (!JS_DeleteUCProperty2(aCx, targetObject,
+                              targetObjectPropName.get(),
+                              targetObjectPropName.Length(), &succeeded)) {
+      return NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
+    }
     NS_ASSERTION(JSVAL_IS_BOOLEAN(succeeded), "Wtf?");
     NS_ENSURE_TRUE(JSVAL_TO_BOOLEAN(succeeded),
                    NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
   }
 
   return rv;
 }
 
--- a/dom/indexedDB/test/test_complex_keyPaths.html
+++ b/dom/indexedDB/test/test_complex_keyPaths.html
@@ -45,19 +45,20 @@
         { keyPath: "foo ",    exception: true },
         { keyPath: "foo[bar]",exception: true },
         { keyPath: "$('id').stuff", exception: true },
         { keyPath: "foo.2.bar", exception: true },
         { keyPath: "foo. .bar", exception: true },
         { keyPath: ".bar",    exception: true },
       ];
 
-      let request = mozIndexedDB.open(name, 1);
-      request.onerror = errorHandler;
-      request.onupgradeneeded = grabEventAndContinueHandler;
+      let openRequest = mozIndexedDB.open(name, 1);
+      openRequest.onerror = errorHandler;
+      openRequest.onupgradeneeded = grabEventAndContinueHandler;
+      openRequest.onsuccess = unexpectedSuccessHandler;
       let event = yield;
       let db = event.target.result;
 
       let stores = {};
 
       // Test creating object stores and inserting data
       for (let i = 0; i < keyPaths.length; i++) {
         let info = keyPaths[i];
@@ -132,16 +133,78 @@
           e = yield;
           is(e.target.result, 0, "index should be empty");
         }
 
         store.clear().onsuccess = grabEventAndContinueHandler;
         yield;
       }
 
+      // Autoincrement and complex key paths
+      let aitests = [{ v: {},                           k: 1, res: { foo: { id: 1 }} },
+                     { v: { value: "x" },               k: 2, res: { value: "x", foo: { id: 2 }} },
+                     { v: { value: "x", foo: {} },      k: 3, res: { value: "x", foo: { id: 3 }} },
+                     { v: { v: "x", foo: { x: "y" } },  k: 4, res: { v: "x", foo: { x: "y", id: 4 }} },
+                     { v: { value: 2, foo: { id: 10 }}, k: 10 },
+                     { v: { value: 2 },                 k: 11, res: { value: 2, foo: { id: 11 }} },
+                     { v: true,                         },
+                     { v: { value: 2, foo: 12 },        },
+                     { v: { foo: { id: true }},         },
+                     { v: { foo: { x: 5, id: {} }},     },
+                     { v: undefined,                    },
+                     { v: { foo: undefined },           },
+                     { v: { foo: { id: undefined }},    },
+                     { v: null,                         },
+                     { v: { foo: null },                },
+                     { v: { foo: { id: null }},         },
+                     ];
+
+      store = db.createObjectStore("gen", { keyPath: "foo.id", autoIncrement: true });
+      for (let i = 0; i < aitests.length; ++i) {
+        let test = aitests[i];
+
+        let preValue = JSON.stringify(test.v);
+        try {
+          store.add(test.v).onsuccess = grabEventAndContinueHandler;
+          ok("k" in test, "shouldn't have thrown");
+          is(JSON.stringify(test.v), preValue, "put didn't modify value");
+        }
+        catch(e) {
+          ok(!("k" in test), "expected exception behavior observed");
+          ok(e instanceof IDBDatabaseException, "Got a IDBDatabaseException");
+          is(e.code, IDBDatabaseException.DATA_ERR, "expect a DATA_ERR");
+
+          is(JSON.stringify(test.v), preValue, "put didn't modify value");
+
+          continue;
+        }
+
+        let e = yield;
+        is(e.target.result, test.k, "got correct return key");
+
+        store.get(test.k).onsuccess = grabEventAndContinueHandler;
+        e = yield;
+        is(JSON.stringify(e.target.result), JSON.stringify(test.res || test.v),
+           "expected value stored");
+      }
+
+      // Can't handle autoincrement and empty keypath
+      try {
+        store = db.createObjectStore("storefail", { keyPath: "", autoIncrement: true });
+        ok(false, "Should have thrown");
+      }
+      catch(e) {
+        ok(true, "Did throw");
+        ok(e instanceof DOMException, "Got a DOMException");
+        is(e.code, DOMException.INVALID_ACCESS_ERR, "expect a INVALID_ACCESS_ERR");
+      }
+
+      openRequest.onsuccess = grabEventAndContinueHandler;
+      yield;
+
       finishTest();
       yield;
     }
   </script>
   <script type="text/javascript;version=1.7" src="helpers.js"></script>
 </head>
 
 <body onload="runTest();"></body>