idb: fix renaming, make renaming tests pass

4 files changed, 257 insertions, 174 deletions

diff --git a/packages/idb-bridge/src/bridge-idb.ts b/packages/idb-bridge/src/bridge-idb.ts
index 2bfb01793..e23c78d4a 100644
--- a/packages/idb-bridge/src/bridge-idb.ts
+++ b/packages/idb-bridge/src/bridge-idb.ts
@@ -55,7 +55,7 @@ import {
   TransactionInactiveError,
   VersionError,
 } from "./util/errors";
-import { fakeDOMStringList } from "./util/fakeDOMStringList";
+import { FakeDOMStringList, fakeDOMStringList } from "./util/fakeDOMStringList";
 import FakeEvent from "./util/FakeEvent";
 import FakeEventTarget from "./util/FakeEventTarget";
 import { makeStoreKeyValue } from "./util/makeStoreKeyValue";
@@ -74,12 +74,6 @@ import { valueToKey } from "./util/valueToKey";
 export type CursorSource = BridgeIDBIndex | BridgeIDBObjectStore;
 
 /** @public */
-export interface FakeDOMStringList extends Array<string> {
-  contains: (value: string) => boolean;
-  item: (i: number) => string | undefined;
-}
-
-/** @public */
 export interface RequestObj {
   operation: () => Promise<any>;
   request?: BridgeIDBRequest | undefined;
@@ -828,7 +822,9 @@ export class BridgeIDBFactory {
         );
 
         // We need to expose the new version number to the upgrade transaction.
-        db._schema = this.backend.getCurrentTransactionSchema(backendTransaction);
+        db._schema = this.backend.getCurrentTransactionSchema(
+          backendTransaction,
+        );
 
         const transaction = db._internalTransaction(
           [],
@@ -1405,6 +1401,14 @@ export class BridgeIDBObjectStore implements IDBObjectStore {
     this._transaction._db._schema = this._backend.getCurrentTransactionSchema(
       btx,
     );
+
+    // We don't modify scope, as the scope of the transaction
+    // doesn't matter if we're in an upgrade transaction.
+    this._transaction._objectStoresCache.delete(oldName);
+    this._transaction._objectStoresCache.set(newName, this);
+    this._transaction._cachedObjectStoreNames = undefined;
+
+    this._name = newName;
   }
 
   public _store(value: any, key: IDBValidKey | undefined, overwrite: boolean) {
@@ -1910,9 +1914,19 @@ export class BridgeIDBTransaction
 
   _backendTransaction?: DatabaseTransaction;
 
-  _objectStoreNames: FakeDOMStringList;
+  _cachedObjectStoreNames: DOMStringList | undefined;
+
   get objectStoreNames(): DOMStringList {
-    return this._objectStoreNames as DOMStringList;
+    if (!this._cachedObjectStoreNames) {
+      if (this._openRequest) {
+        this._cachedObjectStoreNames = this._db.objectStoreNames;
+      } else {
+        this._cachedObjectStoreNames = fakeDOMStringList(
+          Array.from(this._scope).sort(),
+        );
+      }
+    }
+    return this._cachedObjectStoreNames;
   }
   mode: IDBTransactionMode;
   _db: BridgeIDBDatabase;
@@ -1961,7 +1975,6 @@ export class BridgeIDBTransaction
     this._backendTransaction = backendTransaction;
     this.mode = mode;
     this._db = db;
-    this._objectStoreNames = fakeDOMStringList(Array.from(this._scope).sort());
 
     this._db._transactions.push(this);
 
@@ -2049,12 +2062,24 @@ export class BridgeIDBTransaction
       throw new InvalidStateError();
     }
 
+    if (!this._db._schema.objectStores[name]) {
+      throw new NotFoundError();
+    }
+
+    if (!this._db._upgradeTransaction) {
+      if (!this._scope.has(name)) {
+        throw new NotFoundError();
+      }
+    }
+
     const objectStore = this._objectStoresCache.get(name);
     if (objectStore !== undefined) {
       return objectStore;
     }
 
-    return new BridgeIDBObjectStore(this, name);
+    const newObjectStore = new BridgeIDBObjectStore(this, name);
+    this._objectStoresCache.set(name, newObjectStore);
+    return newObjectStore;
   }
 
   // http://www.w3.org/TR/2015/REC-IndexedDB-20150108/#dfn-steps-for-asynchronously-executing-a-request
diff --git a/packages/idb-bridge/src/idb-wpt-ported/idbfactory-open.test.ts b/packages/idb-bridge/src/idb-wpt-ported/idbfactory-open.test.ts
index 102b54719..4ba7caa6f 100644
--- a/packages/idb-bridge/src/idb-wpt-ported/idbfactory-open.test.ts
+++ b/packages/idb-bridge/src/idb-wpt-ported/idbfactory-open.test.ts
@@ -1,4 +1,6 @@
 import test from "ava";
+import { BridgeIDBVersionChangeEvent } from "../bridge-idb";
+import FakeEvent from "../util/FakeEvent";
 import { createdb, format_value, idbFactory } from "./wptsupport";
 
 // IDBFactory.open() - request has no source
@@ -466,3 +468,62 @@ test("WPT idbfactory-open11.htm", async (t) => {
   });
   t.pass();
 });
+
+// IDBFactory.open() - upgradeneeded gets VersionChangeEvent
+test("WPT idbfactory-open12.htm", async (t) => {
+  const indexedDB = idbFactory;
+
+  var db: any;
+  var open_rq = createdb(t, undefined, 9);
+
+  await new Promise<void>((resolve, reject) => {
+    open_rq.onupgradeneeded = function (e: any) {
+      db = e.target.result;
+
+      t.true(
+        e instanceof BridgeIDBVersionChangeEvent,
+        "e instanceof IDBVersionChangeEvent",
+      );
+      t.deepEqual(e.oldVersion, 0, "oldVersion");
+      t.deepEqual(e.newVersion, 9, "newVersion");
+      t.deepEqual(e.type, "upgradeneeded", "event type");
+
+      t.deepEqual(db.version, 9, "db.version");
+    };
+    open_rq.onsuccess = function (e) {
+      t.true(e instanceof FakeEvent, "e instanceof Event");
+      t.false(
+        e instanceof BridgeIDBVersionChangeEvent,
+        "e not instanceof IDBVersionChangeEvent",
+      );
+      t.deepEqual(e.type, "success", "event type");
+      resolve();
+    };
+  });
+
+  await new Promise<void>((resolve, reject) => {
+    /**
+     * Second test
+     */
+    db.onversionchange = function () {
+      db.close();
+    };
+
+    var open_rq2 = createdb(t, db.name, 10);
+    open_rq2.onupgradeneeded = function (e: any) {
+      var db2 = e.target.result;
+      t.true(
+        e instanceof BridgeIDBVersionChangeEvent,
+        "e instanceof IDBVersionChangeEvent",
+      );
+      t.deepEqual(e.oldVersion, 9, "oldVersion");
+      t.deepEqual(e.newVersion, 10, "newVersion");
+      t.deepEqual(e.type, "upgradeneeded", "event type");
+
+      t.deepEqual(db2.version, 10, "new db.version");
+
+      resolve();
+    };
+  });
+  t.pass();
+});
diff --git a/packages/idb-bridge/src/idb-wpt-ported/idbobjectstore-rename-store.test.ts b/packages/idb-bridge/src/idb-wpt-ported/idbobjectstore-rename-store.test.ts
index 0f872fa51..40f4dda98 100644
--- a/packages/idb-bridge/src/idb-wpt-ported/idbobjectstore-rename-store.test.ts
+++ b/packages/idb-bridge/src/idb-wpt-ported/idbobjectstore-rename-store.test.ts
@@ -1,13 +1,10 @@
 import test, { ExecutionContext } from "ava";
-import { BridgeIDBRequest } from "..";
-import { EventTarget, IDBDatabase } from "../idbtypes";
 import {
   checkStoreContents,
   checkStoreGenerator,
   checkStoreIndexes,
   createBooksStore,
   createDatabase,
-  createdb,
   createNotBooksStore,
   migrateDatabase,
 } from "./wptsupport";
@@ -15,183 +12,175 @@ import {
 // IndexedDB: object store renaming support
 // IndexedDB object store rename in new transaction
 test("WPT idbobjectstore-rename-store.html (subtest 1)", async (t) => {
-  await new Promise<void>((resolve, reject) => {
-    let bookStore: any = null;
-    let bookStore2: any = null;
-    let renamedBookStore: any = null;
-    let renamedBookStore2: any = null;
-
-    return createDatabase(t, (database, transaction) => {
-      bookStore = createBooksStore(t, database);
+  let bookStore: any = null;
+  let bookStore2: any = null;
+  let renamedBookStore: any = null;
+  let renamedBookStore2: any = null;
+  await createDatabase(t, (database, transaction) => {
+    bookStore = createBooksStore(t, database);
+  })
+    .then((database) => {
+      t.deepEqual(
+        database.objectStoreNames as any,
+        ["books"],
+        'Test setup should have created a "books" object store',
+      );
+      const transaction = database.transaction("books", "readonly");
+      bookStore2 = transaction.objectStore("books");
+      return checkStoreContents(
+        t,
+        bookStore2,
+        "The store should have the expected contents before any renaming",
+      ).then(() => database.close());
     })
-      .then((database) => {
+    .then(() =>
+      migrateDatabase(t, 2, (database, transaction) => {
+        renamedBookStore = transaction.objectStore("books");
+        renamedBookStore.name = "renamed_books";
+
         t.deepEqual(
-          database.objectStoreNames as any,
-          ["books"],
-          'Test setup should have created a "books" object store',
+          renamedBookStore.name,
+          "renamed_books",
+          "IDBObjectStore name should change immediately after a rename",
         );
-        const transaction = database.transaction("books", "readonly");
-        bookStore2 = transaction.objectStore("books");
-        return checkStoreContents(
-          t,
-          bookStore2,
-          "The store should have the expected contents before any renaming",
-        ).then(() => database.close());
-      })
-      .then(() =>
-        migrateDatabase(t, 2, (database, transaction) => {
-          renamedBookStore = transaction.objectStore("books");
-          renamedBookStore.name = "renamed_books";
-
-          t.deepEqual(
-            renamedBookStore.name,
-            "renamed_books",
-            "IDBObjectStore name should change immediately after a rename",
-          );
-          t.deepEqual(
-            database.objectStoreNames as any,
-            ["renamed_books"],
-            "IDBDatabase.objectStoreNames should immediately reflect the " +
-              "rename",
-          );
-          t.deepEqual(
-            transaction.objectStoreNames as any,
-            ["renamed_books"],
-            "IDBTransaction.objectStoreNames should immediately reflect the " +
-              "rename",
-          );
-          t.deepEqual(
-            transaction.objectStore("renamed_books"),
-            renamedBookStore,
-            "IDBTransaction.objectStore should return the renamed object " +
-              "store when queried using the new name immediately after the " +
-              "rename",
-          );
-          t.throws(
-            () => transaction.objectStore("books"),
-            { name: "NotFoundError" },
-            "IDBTransaction.objectStore should throw when queried using the " +
-              "renamed object store's old name immediately after the rename",
-          );
-        }),
-      )
-      .then((database) => {
         t.deepEqual(
           database.objectStoreNames as any,
           ["renamed_books"],
-          "IDBDatabase.objectStoreNames should still reflect the rename " +
-            "after the versionchange transaction commits",
-        );
-        const transaction = database.transaction("renamed_books", "readonly");
-        renamedBookStore2 = transaction.objectStore("renamed_books");
-        return checkStoreContents(
-          t,
-          renamedBookStore2,
-          "Renaming an object store should not change its records",
-        ).then(() => database.close());
-      })
-      .then(() => {
-        t.deepEqual(
-          bookStore.name,
-          "books",
-          "IDBObjectStore obtained before the rename transaction should " +
-            "not reflect the rename",
+          "IDBDatabase.objectStoreNames should immediately reflect the " +
+            "rename",
         );
         t.deepEqual(
-          bookStore2.name,
-          "books",
-          "IDBObjectStore obtained before the rename transaction should " +
-            "not reflect the rename",
+          transaction.objectStoreNames as any,
+          ["renamed_books"],
+          "IDBTransaction.objectStoreNames should immediately reflect the " +
+            "rename",
         );
         t.deepEqual(
-          renamedBookStore.name,
-          "renamed_books",
-          "IDBObjectStore used in the rename transaction should keep " +
-            "reflecting the new name after the transaction is committed",
+          transaction.objectStore("renamed_books"),
+          renamedBookStore,
+          "IDBTransaction.objectStore should return the renamed object " +
+            "store when queried using the new name immediately after the " +
+            "rename",
         );
-        t.deepEqual(
-          renamedBookStore2.name,
-          "renamed_books",
-          "IDBObjectStore obtained after the rename transaction should " +
-            "reflect the new name",
+        t.throws(
+          () => transaction.objectStore("books"),
+          { name: "NotFoundError" },
+          "IDBTransaction.objectStore should throw when queried using the " +
+            "renamed object store's old name immediately after the rename",
         );
-      });
-  });
-  t.pass();
+      }),
+    )
+    .then((database) => {
+      t.deepEqual(
+        database.objectStoreNames as any,
+        ["renamed_books"],
+        "IDBDatabase.objectStoreNames should still reflect the rename " +
+          "after the versionchange transaction commits",
+      );
+      const transaction = database.transaction("renamed_books", "readonly");
+      renamedBookStore2 = transaction.objectStore("renamed_books");
+      return checkStoreContents(
+        t,
+        renamedBookStore2,
+        "Renaming an object store should not change its records",
+      ).then(() => database.close());
+    })
+    .then(() => {
+      t.deepEqual(
+        bookStore.name,
+        "books",
+        "IDBObjectStore obtained before the rename transaction should " +
+          "not reflect the rename",
+      );
+      t.deepEqual(
+        bookStore2.name,
+        "books",
+        "IDBObjectStore obtained before the rename transaction should " +
+          "not reflect the rename",
+      );
+      t.deepEqual(
+        renamedBookStore.name,
+        "renamed_books",
+        "IDBObjectStore used in the rename transaction should keep " +
+          "reflecting the new name after the transaction is committed",
+      );
+      t.deepEqual(
+        renamedBookStore2.name,
+        "renamed_books",
+        "IDBObjectStore obtained after the rename transaction should " +
+          "reflect the new name",
+      );
+    });
 });
 
 // IndexedDB: object store renaming support
 // IndexedDB object store rename in the transaction where it is created
 test("WPT idbobjectstore-rename-store.html (subtest 2)", async (t) => {
-  await new Promise<void>((resolve, reject) => {
-    let renamedBookStore: any = null,
-      renamedBookStore2: any = null;
-    return createDatabase(t, (database, transaction) => {
-      renamedBookStore = createBooksStore(t, database);
-      renamedBookStore.name = "renamed_books";
+  let renamedBookStore: any = null,
+    renamedBookStore2: any = null;
+  await createDatabase(t, (database, transaction) => {
+    renamedBookStore = createBooksStore(t, database);
+    renamedBookStore.name = "renamed_books";
 
-      t.deepEqual(
-        renamedBookStore.name,
-        "renamed_books",
-        "IDBObjectStore name should change immediately after a rename",
-      );
+    t.deepEqual(
+      renamedBookStore.name,
+      "renamed_books",
+      "IDBObjectStore name should change immediately after a rename",
+    );
+    t.deepEqual(
+      database.objectStoreNames as any,
+      ["renamed_books"],
+      "IDBDatabase.objectStoreNames should immediately reflect the " + "rename",
+    );
+    t.deepEqual(
+      transaction.objectStoreNames as any,
+      ["renamed_books"],
+      "IDBTransaction.objectStoreNames should immediately reflect the " +
+        "rename",
+    );
+    t.deepEqual(
+      transaction.objectStore("renamed_books"),
+      renamedBookStore,
+      "IDBTransaction.objectStore should return the renamed object " +
+        "store when queried using the new name immediately after the " +
+        "rename",
+    );
+    t.throws(
+      () => transaction.objectStore("books"),
+      { name: "NotFoundError" },
+      "IDBTransaction.objectStore should throw when queried using the " +
+        "renamed object store's old name immediately after the rename",
+    );
+  })
+    .then((database) => {
       t.deepEqual(
         database.objectStoreNames as any,
         ["renamed_books"],
-        "IDBDatabase.objectStoreNames should immediately reflect the " +
-          "rename",
+        "IDBDatabase.objectStoreNames should still reflect the rename " +
+          "after the versionchange transaction commits",
       );
+      const transaction = database.transaction("renamed_books", "readonly");
+      renamedBookStore2 = transaction.objectStore("renamed_books");
+      return checkStoreContents(
+        t,
+        renamedBookStore2,
+        "Renaming an object store should not change its records",
+      ).then(() => database.close());
+    })
+    .then(() => {
       t.deepEqual(
-        transaction.objectStoreNames as any,
-        ["renamed_books"],
-        "IDBTransaction.objectStoreNames should immediately reflect the " +
-          "rename",
+        renamedBookStore.name,
+        "renamed_books",
+        "IDBObjectStore used in the rename transaction should keep " +
+          "reflecting the new name after the transaction is committed",
       );
       t.deepEqual(
-        transaction.objectStore("renamed_books"),
-        renamedBookStore,
-        "IDBTransaction.objectStore should return the renamed object " +
-          "store when queried using the new name immediately after the " +
-          "rename",
-      );
-      t.throws(
-        () => transaction.objectStore("books"),
-        { name: "NotFoundError" },
-        "IDBTransaction.objectStore should throw when queried using the " +
-          "renamed object store's old name immediately after the rename",
+        renamedBookStore2.name,
+        "renamed_books",
+        "IDBObjectStore obtained after the rename transaction should " +
+          "reflect the new name",
       );
-    })
-      .then((database) => {
-        t.deepEqual(
-          database.objectStoreNames as any,
-          ["renamed_books"],
-          "IDBDatabase.objectStoreNames should still reflect the rename " +
-            "after the versionchange transaction commits",
-        );
-        const transaction = database.transaction("renamed_books", "readonly");
-        renamedBookStore2 = transaction.objectStore("renamed_books");
-        return checkStoreContents(
-          t,
-          renamedBookStore2,
-          "Renaming an object store should not change its records",
-        ).then(() => database.close());
-      })
-      .then(() => {
-        t.deepEqual(
-          renamedBookStore.name,
-          "renamed_books",
-          "IDBObjectStore used in the rename transaction should keep " +
-            "reflecting the new name after the transaction is committed",
-        );
-        t.deepEqual(
-          renamedBookStore2.name,
-          "renamed_books",
-          "IDBObjectStore obtained after the rename transaction should " +
-            "reflect the new name",
-        );
-      });
-  });
-  t.pass();
+    });
 });
 
 // Renames the 'books' store to 'renamed_books'.
@@ -333,13 +322,13 @@ test("WPT idbobjectstore-rename-store.html (IndexedDB object store swapping via
           "IDBDatabase.objectStoreNames should immediately reflect the swap",
         );
 
-        t.deepEqual(
+        t.is(
           transaction.objectStore("books"),
           notBookStore,
           'IDBTransaction.objectStore should return the original "books" ' +
             'store when queried with "not_books" after the swap',
         );
-        t.deepEqual(
+        t.is(
           transaction.objectStore("not_books"),
           bookStore,
           "IDBTransaction.objectStore should return the original " +
@@ -452,9 +441,12 @@ test("WPT idbobjectstore-rename-store.html (IndexedDB object store rename string
   t.pass();
 });
 
-function rename_test_macro(t: ExecutionContext, escapedName: string) {
+function rename_test_macro(
+  t: ExecutionContext,
+  escapedName: string,
+): Promise<void> {
   const name = JSON.parse('"' + escapedName + '"');
-  createDatabase(t, (database, transaction) => {
+  return createDatabase(t, (database, transaction) => {
     createBooksStore(t, database);
   })
     .then((database) => {
diff --git a/packages/idb-bridge/src/util/fakeDOMStringList.ts b/packages/idb-bridge/src/util/fakeDOMStringList.ts
index 09ef77003..0549e1283 100644
--- a/packages/idb-bridge/src/util/fakeDOMStringList.ts
+++ b/packages/idb-bridge/src/util/fakeDOMStringList.ts
@@ -14,10 +14,12 @@
  * permissions and limitations under the License.
  */
 
+import { DOMStringList } from "../idbtypes";
+
 /** @public */
 export interface FakeDOMStringList extends Array<string> {
   contains: (value: string) => boolean;
-  item: (i: number) => string | undefined;
+  item: (i: number) => string | null;
 }
 
 // Would be nicer to sublcass Array, but I'd have to sacrifice Node 4 support to do that.
@@ -26,13 +28,16 @@ export const fakeDOMStringList = (arr: string[]): FakeDOMStringList => {
   const arr2 = arr.slice();
 
   Object.defineProperty(arr2, "contains", {
-    // tslint:disable-next-line object-literal-shorthand
     value: (value: string) => arr2.indexOf(value) >= 0,
   });
 
   Object.defineProperty(arr2, "item", {
-    // tslint:disable-next-line object-literal-shorthand
-    value: (i: number) => arr2[i],
+    value: (i: number) => {
+      if (i < 0 || i >= arr2.length) {
+        return null;
+      }
+      return arr2[i];
+    },
   });
 
   return arr2 as FakeDOMStringList;