Skip to content
This repository has been archived by the owner on Jul 23, 2022. It is now read-only.

Commit

Permalink
Migrate from idb-keyval to native IndexedDB (#81)
Browse files Browse the repository at this point in the history
Up until now, we basically had only one table and it had conceptually only one column, so we didn't have to worry about atomicity guarantees; everything was automatically atomic. As we prepare to store more kinds of data, we need transactional consistency, to avoid a situation where, e.g., one tab does a read-update-write and in doing so overwrites a write made concurrently from another tab with stale data. So idb-keyval is not for us anymore.

This change prepares for that migration.

Currently, there's no support for database versioning; if the code is deployed as-is and then later the version is upgraded, stuff will break. I'm hoping to fix this later; see #80.
  • Loading branch information
taymonbeal authored May 21, 2021
1 parent 0c7afa8 commit 2fca9a4
Show file tree
Hide file tree
Showing 15 changed files with 273 additions and 42 deletions.
2 changes: 1 addition & 1 deletion frame/auction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
/** @fileoverview Selection of ads, and creation of tokens to display them. */

import { isArray, isKeyValueObject } from "../lib/shared/types";
import { getAllAds } from "./database";
import { getAllAds } from "./db_schema";
import { FetchJsonStatus, tryFetchJson } from "./fetch";

/**
Expand Down
2 changes: 1 addition & 1 deletion frame/auction_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
queryParamsMatching,
} from "../testing/url";
import { runAdAuction } from "./auction";
import { setInterestGroupAds } from "./database";
import { setInterestGroupAds } from "./db_schema";

describe("runAdAuction", () => {
clearStorageBeforeAndAfter();
Expand Down
32 changes: 20 additions & 12 deletions frame/database.ts → frame/db_schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@

/**
* @fileoverview CRUD operations on our data model for persistent storage in
* idb-keyval, with runtime type checking.
* IndexedDB, with runtime type checking.
*/

import * as idbKeyval from "idb-keyval";
import { isArray } from "../lib/shared/types";
import { useStore } from "./indexeddb";

/** An `Ad` from the public API serialized into storage format. */
export type Ad = [renderingUrl: string, price: number];
Expand All @@ -24,30 +24,38 @@ function isInterestGroupAd(value: unknown): value is Ad {
}

/**
* Stores an interest group in idb-keyval. If there's already one with the same
* Stores an interest group in IndexedDB. If there's already one with the same
* name, it is overwritten.
*/
export function setInterestGroupAds(name: string, ads: Ad[]): Promise<void> {
return idbKeyval.set(name, ads);
return useStore("readwrite", (store) => {
store.put(ads, name);
});
}

/** Deletes an interest group from idb-keyval. */
/** Deletes an interest group from IndexedDB. */
export function deleteInterestGroup(name: string): Promise<void> {
return idbKeyval.del(name);
return useStore("readwrite", (store) => {
store.delete(name);
});
}

/** Returns all ads from all interest groups currently stored in idb-keyval. */
/** Returns all ads from all interest groups currently stored in IndexedDB. */
export function getAllAds(): Promise<
Generator<Ad, /* TReturn= */ void, /* TNext= */ void>
> {
return idbKeyval.entries().then(function* (entries) {
for (const [key, ads] of entries) {
let interestGroups: unknown[];
return useStore("readonly", (store) => {
const cursor = store.getAll();
cursor.onsuccess = () => {
interestGroups = cursor.result;
};
}).then(function* () {
for (const ads of interestGroups) {
function check(condition: boolean): asserts condition {
if (!condition) {
throw new Error(
`Malformed entry in idb-keyval for ${JSON.stringify(
key
)}: ${JSON.stringify(ads)}`
`Malformed entry in IndexedDB: ${JSON.stringify(ads)}`
);
}
}
Expand Down
18 changes: 10 additions & 8 deletions frame/database_test.ts → frame/db_schema_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,34 @@
*/

import "jasmine";
import * as idbKeyval from "idb-keyval";
import { clearStorageBeforeAndAfter } from "../testing/storage";
import {
Ad,
deleteInterestGroup,
getAllAds,
setInterestGroupAds,
} from "./database";
} from "./db_schema";
import { useStore } from "./indexeddb";

describe("database:", () => {
describe("db_schema:", () => {
clearStorageBeforeAndAfter();

describe("getAllAds", () => {
it("should read an ad from idb-keyval", async () => {
it("should read an ad from IndexedDB", async () => {
const ads: Ad[] = [["about:blank", 0.02]];
await setInterestGroupAds("interest group name", ads);
expect([...(await getAllAds())]).toEqual(ads);
});

it("should read ads from multiple entries in idb-keyval", async () => {
it("should read ads from multiple entries in IndexedDB", async () => {
const ad1: Ad = ["about:blank#1", 0.01];
const ad2: Ad = ["about:blank#2", 0.02];
const ad3: Ad = ["about:blank#3", 0.03];
await idbKeyval.set("interest group name 1", [ad1]);
await idbKeyval.set("interest group name 2", []);
await idbKeyval.set("interest group name 3", [ad2, ad3]);
await useStore("readwrite", (store) => {
store.add([ad1], "interest group name 1");
store.add([], "interest group name 2");
store.add([ad2, ad3], "interest group name 3");
});
expect([...(await getAllAds())]).toEqual([ad1, ad2, ad3]);
});
});
Expand Down
2 changes: 1 addition & 1 deletion frame/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
} from "../lib/shared/protocol";
import { isArray } from "../lib/shared/types";
import { runAdAuction } from "./auction";
import { setInterestGroupAds, deleteInterestGroup } from "./database";
import { setInterestGroupAds, deleteInterestGroup } from "./db_schema";

/**
* Handles a `MessageEvent` representing a request to the FLEDGE API, and sends
Expand Down
2 changes: 1 addition & 1 deletion frame/handler_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
import { assert, assertType, nonNullish } from "../lib/shared/types";
import { FakeServerHandler, setFakeServerHandler } from "../testing/http";
import { clearStorageBeforeAndAfter } from "../testing/storage";
import { Ad, getAllAds } from "./database";
import { Ad, getAllAds } from "./db_schema";
import { handleRequest } from "./handler";

describe("handleRequest", () => {
Expand Down
95 changes: 95 additions & 0 deletions frame/indexeddb.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/**
* @license
* Copyright 2021 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

/**
* @fileoverview Utilities for performing IndexedDB operations, in order to
* persistently store and retrieve data client-side.
*/

import { assert, assertionError } from "../lib/shared/types";

const DB_NAME = "fledge-shim";
const STORE_NAME = "interest-groups";

const dbPromise = new Promise<IDBDatabase>((resolve, reject) => {
const dbRequest = indexedDB.open(DB_NAME, /* version= */ 1);
dbRequest.onupgradeneeded = ({ oldVersion, newVersion }) => {
// This should be called iff the database is just now being created for the
// first time.
assert(oldVersion === 0);
assert(newVersion === 1);
dbRequest.result.createObjectStore(STORE_NAME);
};
dbRequest.onsuccess = () => {
resolve(dbRequest.result);
};
dbRequest.onerror = () => {
reject(dbRequest.error);
};
dbRequest.onblocked = () => {
// Since the version number is 1 (the lowest allowed), it shouldn't be
// possible for an earlier version of the same database to already be open.
reject(assertionError());
};
});

/**
* Runs an arbitrary operation on the IndexedDB object store. `callback` has to
* be synchronous, but it can create IndexedDB requests, and those requests'
* `onsuccess` handlers can create further requests, and so forth; the
* transaction will be committed and the promise resolved after such a task
* finishes with no further pending requests. Such requests need not register
* `onerror` handlers, unless they need to do fine-grained error handling; if an
* exception is thrown and not caught, the transaction will be aborted without
* committing any writes, and the promise rejected.
*/
export async function useStore(
txMode: IDBTransactionMode,
callback: (store: IDBObjectStore) => void
): Promise<void> {
const db = await dbPromise;
return new Promise((resolve, reject) => {
// The FLEDGE API does not offer callers any guarantees about when writes
// will be committed; for example, `joinAdInterestGroup` has a synchronous
// API that triggers a background task but does not allow the caller to
// await that task. Therefore, strict durability is not required for
// correctness. So we'll improve latency and user battery life by opting
// into relaxed durability, which allows the browser and OS to economize on
// potentially expensive writes to disk.
const tx = db.transaction(STORE_NAME, txMode, { durability: "relaxed" });
tx.oncomplete = () => {
resolve();
};
tx.onabort = () => {
reject(tx.error);
};
// No need to explicitly install an onerror handler since an error aborts
// the transaction.
const store = tx.objectStore(STORE_NAME);
try {
callback(store);
} catch (error: unknown) {
tx.abort();
throw error;
}
});
}

declare global {
interface IDBDatabase {
/**
* The `options` parameter is in the IndexedDB spec and is supported by
* Chrome, but is absent from the default TypeScript type definitions.
*
* @see https://www.w3.org/TR/IndexedDB/#database-interface
*/
transaction(
storeNames: string | Iterable<string>,
mode?: IDBTransactionMode,
options?: { durability?: "default" | "strict" | "relaxed" }
): IDBTransaction;
}
}
101 changes: 101 additions & 0 deletions frame/indexeddb_test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/**
* @license
* Copyright 2021 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

import { assertInstance } from "../lib/shared/types";
import { clearStorageBeforeAndAfter } from "../testing/storage";
import { useStore } from "./indexeddb";

describe("useStore", () => {
clearStorageBeforeAndAfter();

const value = "IndexedDB value";
const key = "IndexedDB key";

it("should read its own writes across multiple transactions", async () => {
await useStore("readwrite", (store) => {
store.put(value, key);
});
await useStore("readonly", (store) => {
const retrievalRequest = store.get(key);
retrievalRequest.onsuccess = () => {
expect(retrievalRequest.result).toBe(value);
};
});
});

it("should reject if the transaction is aborted", () =>
expectAsync(
useStore("readonly", (store) => {
store.transaction.abort();
})
).toBeRejectedWith(null));

it("should not commit the transaction if the main callback throws", async () => {
const errorMessage = "oops";
await expectAsync(
useStore("readwrite", (store) => {
store.add(value, key);
throw new Error(errorMessage);
})
).toBeRejectedWithError(errorMessage);
await useStore("readonly", (store) => {
const countRequest = store.count();
countRequest.onsuccess = () => {
expect(countRequest.result).toBe(0);
};
});
});

const otherValue = "other IndexedDB value";
const otherKey = "other IndexedDB key";

it("should not commit the transaction if an illegal operation is attempted", async () => {
await useStore("readwrite", (store) => {
store.put(value, key);
});
await expectAsync(
useStore("readwrite", (store) => {
store.add(otherValue, otherKey);
// add requires that the given key not already exist.
store.add(otherValue, key);
})
).toBeRejectedWith(
jasmine.objectContaining({
constructor: DOMException,
name: "ConstraintError",
})
);
await useStore("readonly", (store) => {
const retrievalRequest = store.get(otherKey);
retrievalRequest.onsuccess = () => {
expect(retrievalRequest.result).toBeUndefined();
};
});
});

it("should commit the transaction if an error is recovered from", async () => {
await useStore("readwrite", (store) => {
store.put(value, key);
});
await useStore("readwrite", (store) => {
store.add(otherValue, otherKey);
// add requires that the given key not already exist.
const badRequest = store.add(otherValue, key);
badRequest.onsuccess = fail;
badRequest.onerror = (event) => {
assertInstance(badRequest.error, DOMException);
expect(badRequest.error.name).toBe("ConstraintError");
event.preventDefault();
};
});
await useStore("readonly", (store) => {
const retrievalRequest = store.get(otherKey);
retrievalRequest.onsuccess = () => {
expect(retrievalRequest.result).toBe(otherValue);
};
});
});
});
2 changes: 1 addition & 1 deletion lib/public_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export interface Ad {
/**
* The amount that the buyer is willing to pay in order to have this ad
* selected. The ad with the highest price is selected; in case of a tie, an
* ad with the highest price is selected arbitrarily (based on idb-keyval
* ad with the highest price is selected arbitrarily (based on IndexedDB
* implementation details).
*
* This is used by our temporary hardcoded auction logic and will not exist
Expand Down
12 changes: 11 additions & 1 deletion lib/shared/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,16 @@ export function isArray(value: unknown): value is readonly unknown[] {
return Array.isArray(value);
}

/**
* Returns an error appropriate for when an assertion fails.
*
* Due to the generic error message, this should be used only in test code or
* when it is believed (but not proven) impossible for the check to fail.
*/
export function assertionError(): TypeError {
return new TypeError("Assertion failure");
}

/**
* Throws if the given condition is false.
*
Expand All @@ -49,7 +59,7 @@ export function isArray(value: unknown): value is readonly unknown[] {
*/
export function assert(condition: boolean): asserts condition {
if (!condition) {
throw new TypeError("Assertion failure");
throw assertionError();
}
}

Expand Down
9 changes: 9 additions & 0 deletions lib/shared/types_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import "jasmine";
import {
assert,
assertInstance,
assertionError,
assertType,
isArray,
isKeyValueObject,
Expand Down Expand Up @@ -66,6 +67,14 @@ describe("isArray", () => {
});
});

describe("assertionError", () => {
it("should return a different error on each call", () => {
const error1 = assertionError();
const error2 = assertionError();
expect(error1.stack).not.toBe(error2.stack);
});
});

describe("assert", () => {
it("should do nothing on a true condition", () => {
assert(true);
Expand Down
Loading

0 comments on commit 2fca9a4

Please sign in to comment.