Bug 706659 part 1: Make complex keypaths ignore index rather than throwing in all situations if it can't get key for index. r=bent
authorJonas Sicking <jonas@sicking.cc>
Fri, 02 Dec 2011 18:32:46 -0800
changeset 81206 1b9ca56d4aab132ab290aeecafbcf3740894aaa5
parent 81205 99e27f500ad32a2e3fffc65372f87f5ea677dc8e
child 81207 1fddf8667d2180f6a347ccff45cc7504fa18d4b1
push id21564
push usermak77@bonardo.net
push dateSat, 03 Dec 2011 11:10:17 +0000
treeherdermozilla-central@a68c96c1d8e0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbent
bugs706659
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 706659 part 1: Make complex keypaths ignore index rather than throwing in all situations if it can't get key for index. r=bent
dom/indexedDB/IDBObjectStore.cpp
dom/indexedDB/test/test_complex_keyPaths.html
--- a/dom/indexedDB/IDBObjectStore.cpp
+++ b/dom/indexedDB/IDBObjectStore.cpp
@@ -424,36 +424,33 @@ inline
 nsresult
 GetKeyFromValue(JSContext* aCx,
                 jsval aVal,
                 const nsAString& aKeyPath,
                 Key& aKey)
 {
   NS_ASSERTION(aCx, "Null pointer!");
   // aVal can be primitive iff the key path is empty.
-  NS_ASSERTION(!JSVAL_IS_PRIMITIVE(aVal) || aKeyPath.IsEmpty(),
-               "Why are we here!?");
   NS_ASSERTION(IDBObjectStore::IsValidKeyPath(aCx, aKeyPath),
                "This will explode!");
 
   KeyPathTokenizer tokenizer(aKeyPath, '.');
 
   jsval intermediate = aVal;
   while (tokenizer.hasMoreTokens()) {
-    nsString token(tokenizer.nextToken());
-
-    if (!token.Length()) {
-      return NS_ERROR_DOM_SYNTAX_ERR;
-    }
-
-    const jschar* keyPathChars = token.get();
+    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();
 
     if (JSVAL_IS_PRIMITIVE(intermediate)) {
-      return NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
+      intermediate = JSVAL_VOID;
+      break;
     }
 
     JSBool ok = JS_GetUCProperty(aCx, JSVAL_TO_OBJECT(intermediate),
                                  keyPathChars, keyPathLen, &intermediate);
     NS_ENSURE_TRUE(ok, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
   }
 
   if (NS_FAILED(aKey.SetFromJSVal(aCx, intermediate))) {
@@ -619,20 +616,16 @@ IDBObjectStore::GetIndexUpdateInfo(Objec
     if (!aUpdateInfoArray.SetCapacity(count)) {
       NS_ERROR("Out of memory!");
       return NS_ERROR_OUT_OF_MEMORY;
     }
 
     for (PRUint32 indexesIndex = 0; indexesIndex < count; indexesIndex++) {
       const IndexInfo& indexInfo = aObjectStoreInfo->indexes[indexesIndex];
 
-      if (JSVAL_IS_PRIMITIVE(aObject) && !indexInfo.keyPath.IsEmpty()) {
-        continue;
-      }
-
       Key value;
       nsresult rv = GetKeyFromValue(aCx, aObject, indexInfo.keyPath, value);
       NS_ENSURE_SUCCESS(rv, rv);
 
       if (value.IsUnset()) {
         // Not a value we can do anything with, ignore it.
         continue;
       }
--- a/dom/indexedDB/test/test_complex_keyPaths.html
+++ b/dom/indexedDB/test/test_complex_keyPaths.html
@@ -13,160 +13,127 @@
     function testSteps()
     {
       const nsIIDBObjectStore = Components.interfaces.nsIIDBObjectStore;
       const nsIIDBTransaction = Components.interfaces.nsIIDBTransaction;
 
       // Test object stores
 
       const name = window.location.pathname;
-      const objectStoreInfo = [
-        { name: "a", options: { keyPath: "id"} },
-        { name: "b", options: { keyPath: "foo.id"} },
-        { name: "c", options: { keyPath: ""} },
-        { name: "d", options: { keyPath: "foo..id"}, exception: true },
-        { name: "e", options: { keyPath: "foo."}, exception: true },
-        { name: "f", options: { keyPath: "foo.bar" } },
-        { name: "g", options: { keyPath: "fo o" }, exception: true},
-        { name: "h", options: { keyPath: "foo " }, exception: true},
-        { name: "i", options: { keyPath: "foo[bar]" }, exception: true },
-        { name: "j", options: { keyPath: "$('id').stuff" }, exception: true },
-        { name: "k", options: { keyPath: "foo.2.bar" }, exception: true }
-      ];
-
-      const indexInfo = [
-        { name: "1", keyPath: "id" },
-        { name: "2", keyPath: "foo..id", exception: true },
-        { name: "3", keyPath: "foo.", exception: true },
-        { name: "4", keyPath: "foo.baz" },
-        { name: "5", keyPath: "fo o", exception: true },
-        { name: "6", keyPath: "foo ", exception: true },
-        { name: "7", keyPath: "foo[bar]", exception: true },
-        { name: "8", keyPath: "$('id').stuff", exception: true },
-        { name: "9", keyPath: "foo.2.bar", exception: true },
-        { name: "10", keyPath: "foo.bork" },
+      const keyPaths = [
+        { keyPath: "id",      value: { id: 5 },                      key: 5 },
+        { keyPath: "id",      value: { id: "14", iid: 12 },          key: "14" },
+        { keyPath: "id",      value: { iid: "14", id: 12 },          key: 12 },
+        { keyPath: "id",      value: {} },
+        { keyPath: "id",      value: { id: {} } },
+        { keyPath: "id",      value: { id: /x/ } },
+        { keyPath: "id",      value: 2 },
+        { keyPath: "id",      value: undefined },
+        { keyPath: "foo.id",  value: { foo: { id: 7 } },             key: 7 },
+        { keyPath: "foo.id",  value: { id: 7, foo: { id: "asdf" } }, key: "asdf" },
+        { keyPath: "foo.id",  value: { foo: { id: undefined } } },
+        { keyPath: "foo.id",  value: { foo: 47 } },
+        { keyPath: "foo.id",  value: {} },
+        { 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 },
+        { keyPath: "foo.",    exception: true },
+        { keyPath: "fo o",    exception: true },
+        { 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;
-      request.onsuccess = unexpectedSuccessHandler;
       let event = yield;
       let db = event.target.result;
 
-      for (let i = 0; i < objectStoreInfo.length; i++) {
-        let info = objectStoreInfo[i];
-        try {
-          let objectStore = info.hasOwnProperty("options") ?
-                            db.createObjectStore(info.name, info.options) :
-                            db.createObjectStore(info.name);
-          ok(!info.hasOwnProperty("exception"), "expected exception behavior observed");
-        } catch (e) {
-          ok(info.hasOwnProperty("exception"), "expected exception behavior observed");
-          ok(e instanceof DOMException, "Got a DOM Exception");
-          is(e.code, DOMException.SYNTAX_ERR, "expect a syntax error");
+      let stores = {};
+
+      // Test creating object stores and inserting data
+      for (let i = 0; i < keyPaths.length; i++) {
+        let info = keyPaths[i];
+        test = " for " + JSON.stringify(info);
+        if (!stores[info.keyPath]) {
+          try {
+            let objectStore = db.createObjectStore(info.keyPath, { keyPath: info.keyPath });
+            ok(!("exception" in info), "expected exception behavior observed" + test);
+            stores[info.keyPath] = objectStore;
+          } catch (e) {
+            ok("exception" in info, "expected exception behavior observed" + test);
+            ok(e instanceof DOMException, "Got a DOM Exception" + test);
+            is(e.code, DOMException.SYNTAX_ERR, "expect a syntax error" + test);
+            continue;
+          }
         }
-      }
+
+        let store = stores[info.keyPath];
 
-      request.onsuccess = grabEventAndContinueHandler;
-      yield;
-
-      let trans = db.transaction(["f"], IDBTransaction.READ_WRITE);
-      let objectStore = trans.objectStore("f");
+        try {
+          request = store.add(info.value);
+          ok("key" in info, "successfully created request to insert value" + test);
+        } catch (e) {
+          ok(!("key" in info), "threw when attempted to insert" + test);
+          ok(e instanceof IDBDatabaseException, "Got a IDBDatabaseException" + test);
+          is(e.code, IDBDatabaseException.DATA_ERR, "expect a DATA_ERR error" + test);
+          continue;
+        }
 
-      objectStore.put({foo: {baz: -1, bar: 72, bork: true}});
-      objectStore.put({foo: {baz: 2, bar: 1, bork: false}});
+        request.onerror = errorHandler;
+        request.onsuccess = grabEventAndContinueHandler;
 
-      try {
-        objectStore.put({foo: {}});
-        ok(false, "Should have thrown!");
-      } catch (e) {
-        ok(true, "Putting an object without the key should throw");
+        let e = yield;
+        is(e.type, "success", "inserted successfully" + test);
+        is(e.target, request, "expected target" + test);
+        is(request.result, info.key, "found correct key" + test);
+
+        store.clear().onsuccess = grabEventAndContinueHandler;
+        yield;
       }
 
-      trans.onerror = errorHandler;
-      trans.oncomplete = grabEventAndContinueHandler;
-
-      yield;
-
-      let trans = db.transaction(["f"], IDBTransaction.READ);
-      let objectStore = trans.objectStore("f");
-      let request = objectStore.openCursor();
-
-      request.onerror = errorHandler;
-      request.onsuccess = grabEventAndContinueHandler;
-
-      let event = yield;
-
-      let cursor = event.target.result;
-      is(cursor.value.foo.baz, 2, "got things in the right order");
-
-      cursor.continue();
-
-      let event = yield
-
-      let cursor = event.target.result;
-      is(cursor.value.foo.baz, -1, "got things in the right order");
-
-      db.close();
-
-      // Test indexes
-
-      let request = mozIndexedDB.open(name, 2);
-      request.onerror = errorHandler;
-      request.onupgradeneeded = grabEventAndContinueHandler;
-
-      let event = yield;
-      let db = event.target.result;
-
-      let trans = event.target.transaction;
-      let objectStore = trans.objectStore("f");
+      // Attempt to create indexes and insert data
+      let store = db.createObjectStore("indexStore");
+      let indexes = {};
+      for (let i = 0; i < keyPaths.length; i++) {
+        let info = keyPaths[i];
+        if (!indexes[info.keyPath]) {
+          try {
+            let index = store.createIndex(info.keyPath, info.keyPath);
+            ok(!("exception" in info), "expected exception behavior observed");
+            indexes[info.keyPath] = index;
+          } catch (e) {
+            ok("exception" in info, "expected exception behavior observed");
+            ok(e instanceof DOMException, "Got a DOM Exception");
+            is(e.code, DOMException.SYNTAX_ERR, "expect a syntax error");
+            continue;
+          }
+        }
+        
+        let index = indexes[info.keyPath];
 
-      let indexes = [];
-      for (let i = 0; i < indexInfo.length; i++) {
-        let info = indexInfo[i];
-        try {
-          indexes[i] = info.hasOwnProperty("options") ?
-                       objectStore.createIndex(info.name, info.keyPath, info.options) :
-                       objectStore.createIndex(info.name, info.keyPath);
-          ok(!info.hasOwnProperty("exception"), "expected exception behavior observed");
-        } catch (e) {
-          ok(info.hasOwnProperty("exception"), "expected exception behavior observed");
-          ok(e instanceof DOMException, "Got a DOM Exception");
-          is(e.code, DOMException.SYNTAX_ERR, "expect a syntax error");
+        request = store.add(info.value, 1);
+        if ("key" in info) {
+          index.getKey(info.key).onsuccess = grabEventAndContinueHandler;
+          e = yield;
+          is(e.target.result, 1, "found value when reading from index");
         }
-      }
-      request.onsuccess = grabEventAndContinueHandler;
-
-      yield;
-
-      let trans = db.transaction(["f"], IDBTransaction.READ);
-      let objectStore = trans.objectStore("f");
+        else {
+          index.count().onsuccess = grabEventAndContinueHandler;
+          e = yield;
+          is(e.target.result, 0, "index should be empty");
+        }
 
-      let request = objectStore.index("4").openCursor();
-      request.onsuccess = grabEventAndContinueHandler;
-
-      let event = yield;
-
-      let cursor = event.target.result;
-
-      is(cursor.value.foo.bar, 72, "got things in the right order");
-
-      cursor.continue();
-      yield;
-
-      is(cursor.value.foo.bar, 1, "got things in the right order");
-
-      let request = objectStore.index("10").openCursor();
-      request.onerror = errorHandler;
-      request.onsuccess = grabEventAndContinueHandler;
-
-      let event = yield;
-
-      is(event.target.result, null, "should have no results");
+        store.clear().onsuccess = grabEventAndContinueHandler;
+        yield;
+      }
 
       finishTest();
       yield;
     }
   </script>
   <script type="text/javascript;version=1.7" src="helpers.js"></script>
 </head>