Add a `checkedLocalStore` implementation (#523)

Summary:
This provides some extra checking around `LocalStore` calls. In
particular, it fails fast on the nasty bug where storing a `Map`
actually stores the empty object (`JSON.stringify(new Map()) === "{}"`).
Similarly, retrieving a value that was stored as `undefined` will raise
an error, because `JSON.parse(JSON.stringify(undefined))` raises an
error.

This should have negligible performance impact—local storage access
should never be on a critical path. We can choose to elide this in
production if we want.

Test Plan:
Unit tests added. Manual testing of the cred explorer yields no errors.

wchargin-branch: checked-local-store

src/app/checkedLocalStore.js

@@ -0,0 +1,49 @@
+// @flow
+
+import deepEqual from "lodash.isequal";
+
+import {LocalStore} from "./localStore";
+
+/*
+ * A wrapper for a `LocalStore` that adds contract checking. This
+ * implementation verifies that all provided keys are strings and all
+ * provided values are plain old JSON values.
+ */
+export default class CheckedLocalStore implements LocalStore {
+  _base: LocalStore;
+
+  constructor(base: LocalStore): void {
+    this._base = base;
+  }
+
+  get<T>(key: string, whenUnavailable: T): T {
+    this._validateKey(key);
+    return this._base.get(key, whenUnavailable);
+  }
+
+  set(key: string, data: mixed): void {
+    this._validateKey(key);
+    this._validateValue(data);
+    return this._base.set(key, data);
+  }
+
+  del(key: string): void {
+    this._validateKey(key);
+    return this._base.del(key);
+  }
+
+  _validateKey(key: string) {
+    if (typeof key !== "string") {
+      throw new Error(`bad key (${typeof key}): ${key}`);
+    }
+  }
+
+  _validateValue(data: any) {
+    try {
+      if (deepEqual(data, JSON.parse(JSON.stringify(data)))) {
+        return;
+      }
+    } catch (_) {}
+    throw new Error(`bad value: ${data}`);
+  }
+}

src/app/checkedLocalStore.test.js

@@ -0,0 +1,89 @@
+// @flow
+
+import {LocalStore} from "./localStore";
+import CheckedLocalStore from "./checkedLocalStore";
+
+describe("src/app/checkedLocalStore", () => {
+  function makeBase(): LocalStore {
+    return {
+      get: jest.fn(),
+      set: jest.fn(),
+      del: jest.fn(),
+    };
+  }
+
+  it("forwards valid `get`", () => {
+    const base = makeBase();
+    const cls = new CheckedLocalStore(base);
+    const whenUnavailable = Symbol("whenUnavailable");
+    const result = {key: "lime"};
+    base.get.mockReturnValueOnce(result);
+    expect(cls.get("quay", whenUnavailable)).toBe(result);
+    expect(base.get).toHaveBeenCalledWith("quay", whenUnavailable);
+    expect(base.get).toHaveBeenCalledTimes(1);
+    expect(base.set).toHaveBeenCalledTimes(0);
+    expect(base.del).toHaveBeenCalledTimes(0);
+  });
+
+  it("forwards valid `set`", () => {
+    const base = makeBase();
+    const cls = new CheckedLocalStore(base);
+    expect(cls.set("quay", {key: "lime"})).toBe(undefined);
+    expect(base.set).toHaveBeenCalledWith("quay", {key: "lime"});
+    expect(base.get).toHaveBeenCalledTimes(0);
+    expect(base.set).toHaveBeenCalledTimes(1);
+    expect(base.del).toHaveBeenCalledTimes(0);
+  });
+
+  it("forwards valid `del`", () => {
+    const base = makeBase();
+    const cls = new CheckedLocalStore(base);
+    expect(cls.del("quay")).toBe(undefined);
+    expect(base.del).toHaveBeenCalledWith("quay");
+    expect(base.get).toHaveBeenCalledTimes(0);
+    expect(base.set).toHaveBeenCalledTimes(0);
+    expect(base.del).toHaveBeenCalledTimes(1);
+  });
+
+  function checkErrorCase(consumeLocalStore: (LocalStore) => void) {
+    const base = makeBase();
+    const cls = new CheckedLocalStore(base);
+    consumeLocalStore(cls);
+    expect(base.get).not.toHaveBeenCalled();
+    expect(base.set).not.toHaveBeenCalled();
+    expect(base.del).not.toHaveBeenCalled();
+  }
+
+  it("errors on non-string keys with `get`", () => {
+    checkErrorCase((cls) => {
+      // $ExpectFlowError
+      expect(() => cls.get(12)).toThrow("bad key (number): 12");
+    });
+  });
+
+  it("errors on non-string keys with `set`", () => {
+    checkErrorCase((cls) => {
+      // $ExpectFlowError
+      expect(() => cls.set(12, "twelve")).toThrow("bad key (number): 12");
+    });
+  });
+
+  it("errors on non-string keys with `del`", () => {
+    checkErrorCase((cls) => {
+      // $ExpectFlowError
+      expect(() => cls.del(12)).toThrow("bad key (number): 12");
+    });
+  });
+
+  it("errors on setting ES6 `Map` values", () => {
+    checkErrorCase((cls) => {
+      expect(() => cls.set("a", new Map())).toThrow("bad value: [object Map]");
+    });
+  });
+
+  it("errors on setting `undefined`", () => {
+    checkErrorCase((cls) => {
+      expect(() => cls.set("a", undefined)).toThrow("bad value: undefined");
+    });
+  });
+});