Skip to content

Commit

Permalink
Split storage ops from serialization (#2023)
Browse files Browse the repository at this point in the history
* Split storage ops from serialization

This rearranges the storage (was: `userNumber`) module to improve
maintainability.

In particular, the localStorage-specific operations are extracted into
dedicated localStorage functions, and the actual
pruning/migrating/updating is also split out. This gives three layers:
localStorage, pruning/migrating/updating, and top-level functions (like
`getAnchors`) used by the rest of the codebase.

The storage (was: `userNumber`) test suite is also updated to clarify
that the test data is related to localStorage.

* Add migration test
  • Loading branch information
nmattia authored Nov 9, 2023
1 parent 9afc7f2 commit 7e78f7a
Show file tree
Hide file tree
Showing 9 changed files with 256 additions and 192 deletions.
7 changes: 2 additions & 5 deletions src/frontend/src/components/authenticateBox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { usePin } from "$src/flows/pin/usePin";
import { useRecovery } from "$src/flows/recovery/useRecovery";
import { getRegisterFlowOpts, registerFlow } from "$src/flows/register";
import { I18n } from "$src/i18n";
import { getAnchors, setAnchorUsed } from "$src/storage";
import {
LoginData,
LoginFlowError,
Expand All @@ -23,11 +24,7 @@ import {
bufferEqual,
} from "$src/utils/iiConnection";
import { TemplateElement, withRef } from "$src/utils/lit-html";
import {
getAnchors,
parseUserNumber,
setAnchorUsed,
} from "$src/utils/userNumber";
import { parseUserNumber } from "$src/utils/userNumber";
import { NonEmptyArray, isNonEmptyArray, unreachable } from "$src/utils/utils";
import { isNullish, nonNullish } from "@dfinity/utils";
import { TemplateResult, html, render } from "lit-html";
Expand Down
2 changes: 1 addition & 1 deletion src/frontend/src/flows/addDevice/manage/addFIDODevice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import { DeviceData } from "$generated/internet_identity_types";
import { displayError } from "$src/components/displayError";
import { withLoader } from "$src/components/loader";
import { inferPasskeyAlias, loadUAParser } from "$src/flows/register";
import { setAnchorUsed } from "$src/storage";
import { authenticatorAttachmentToKeyType } from "$src/utils/authenticatorAttachment";
import {
AuthenticatedConnection,
creationOptions,
} from "$src/utils/iiConnection";
import { setAnchorUsed } from "$src/utils/userNumber";
import {
displayCancelError,
displayDuplicateDeviceError,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import {
import { displayError } from "$src/components/displayError";
import { withLoader } from "$src/components/loader";
import { inferPasskeyAlias, loadUAParser } from "$src/flows/register";
import { setAnchorUsed } from "$src/storage";
import { authenticatorAttachmentToKeyType } from "$src/utils/authenticatorAttachment";
import { Connection, creationOptions } from "$src/utils/iiConnection";
import { setAnchorUsed } from "$src/utils/userNumber";
import { unknownToString, unreachable, unreachableLax } from "$src/utils/utils";
import {
displayCancelError,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import {
import { displayError } from "$src/components/displayError";
import { mainWindow } from "$src/components/mainWindow";
import { toast } from "$src/components/toast";
import { setAnchorUsed } from "$src/storage";
import { AsyncCountdown } from "$src/utils/countdown";
import { Connection } from "$src/utils/iiConnection";
import { renderPage } from "$src/utils/lit-html";
import { setAnchorUsed } from "$src/utils/userNumber";
import { delayMillis, unknownToString } from "$src/utils/utils";
import { nonNullish } from "@dfinity/utils";
import { html } from "lit-html";
Expand Down
2 changes: 1 addition & 1 deletion src/frontend/src/flows/recovery/useRecovery.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { promptDeviceAlias } from "$src/components/alias";
import { displayError } from "$src/components/displayError";
import { withLoader } from "$src/components/loader";
import { setAnchorUsed } from "$src/storage";
import { authenticatorAttachmentToKeyType } from "$src/utils/authenticatorAttachment";
import { LoginFlowResult } from "$src/utils/flowResult";
import { AuthenticatedConnection, Connection } from "$src/utils/iiConnection";
import { setAnchorUsed } from "$src/utils/userNumber";
import { unknownToString, unreachableLax } from "$src/utils/utils";
import { constructIdentity } from "$src/utils/webAuthn";
import {
Expand Down
2 changes: 1 addition & 1 deletion src/frontend/src/flows/register/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { pinStepper } from "$src/flows/pin/stepper";
import { registerStepper } from "$src/flows/register/stepper";
import { registerDisabled } from "$src/flows/registerDisabled";
import { I18n } from "$src/i18n";
import { setAnchorUsed } from "$src/storage";
import { authenticatorAttachmentToKeyType } from "$src/utils/authenticatorAttachment";
import { LoginFlowCanceled } from "$src/utils/flowResult";
import {
Expand All @@ -23,7 +24,6 @@ import {
IIWebAuthnIdentity,
RegisterResult,
} from "$src/utils/iiConnection";
import { setAnchorUsed } from "$src/utils/userNumber";
import { SignIdentity } from "@dfinity/agent";
import { ECDSAKeyIdentity } from "@dfinity/identity";
import { nonNullish } from "@dfinity/utils";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,45 @@
import { nonNullish } from "@dfinity/utils";
import { vi } from "vitest";
import { getAnchors, MAX_SAVED_ANCHORS, setAnchorUsed } from "./userNumber";
import { getAnchors, MAX_SAVED_ANCHORS, setAnchorUsed } from ".";

testLocalStorage("anchors default to nothing", () => {
testStorage("anchors default to nothing", () => {
expect(getAnchors()).toStrictEqual([]);
});

testLocalStorage(
testStorage(
"old userNumber is recovered",
() => {
expect(getAnchors()).toStrictEqual([BigInt(123456)]);
},
{ before: { userNumber: "123456" } }
{ localStorage: { before: { userNumber: "123456" } } }
);

testLocalStorage("one anchor can be stored", () => {
testStorage(
"reading old userNumber migrates anchors",
() => {
vi.useFakeTimers().setSystemTime(new Date(20));
getAnchors();
vi.useRealTimers();
},
{
localStorage: {
before: { userNumber: "123456" },
after: {
anchors: JSON.stringify({
"123456": { lastUsedTimestamp: 20 },
}),
userNumber: "123456",
},
},
}
);

testStorage("one anchor can be stored", () => {
setAnchorUsed(BigInt(10000));
expect(getAnchors()).toStrictEqual([BigInt(10000)]);
});

testLocalStorage("multiple anchors can be stored", () => {
testStorage("multiple anchors can be stored", () => {
setAnchorUsed(BigInt(10000));
setAnchorUsed(BigInt(10001));
setAnchorUsed(BigInt(10003));
Expand All @@ -28,7 +48,7 @@ testLocalStorage("multiple anchors can be stored", () => {
expect(getAnchors()).toContain(BigInt(10003));
});

testLocalStorage("anchors are sorted", () => {
testStorage("anchors are sorted", () => {
const anchors = [BigInt(10400), BigInt(10001), BigInt(1011003)];
for (const anchor of anchors) {
setAnchorUsed(anchor);
Expand All @@ -37,14 +57,14 @@ testLocalStorage("anchors are sorted", () => {
expect(getAnchors()).toStrictEqual(anchors);
});

testLocalStorage("only N anchors are stored", () => {
testStorage("only N anchors are stored", () => {
for (let i = 0; i < MAX_SAVED_ANCHORS + 5; i++) {
setAnchorUsed(BigInt(i));
}
expect(getAnchors().length).toStrictEqual(MAX_SAVED_ANCHORS);
});

testLocalStorage("old anchors are dropped", () => {
testStorage("old anchors are dropped", () => {
vi.useFakeTimers().setSystemTime(new Date(0));
setAnchorUsed(BigInt(10000));
vi.useFakeTimers().setSystemTime(new Date(1));
Expand All @@ -58,23 +78,25 @@ testLocalStorage("old anchors are dropped", () => {
vi.useRealTimers();
});

testLocalStorage(
testStorage(
"unknown fields are not dropped",
() => {
vi.useFakeTimers().setSystemTime(new Date(20));
setAnchorUsed(BigInt(10000));
vi.useRealTimers();
},
{
before: {
anchors: JSON.stringify({
"10000": { lastUsedTimestamp: 10, hello: "world" },
}),
},
after: {
anchors: JSON.stringify({
"10000": { lastUsedTimestamp: 20, hello: "world" },
}),
localStorage: {
before: {
anchors: JSON.stringify({
"10000": { lastUsedTimestamp: 10, hello: "world" },
}),
},
after: {
anchors: JSON.stringify({
"10000": { lastUsedTimestamp: 20, hello: "world" },
}),
},
},
}
);
Expand All @@ -85,21 +107,22 @@ testLocalStorage(
* If `after` is specified, the content of local storage are checked against `after` after the
* test is run and before local storage is cleared.
*/
function testLocalStorage(
function testStorage(
name: string,
fn: () => void,
opts?: { before?: LocalStorage; after?: LocalStorage }
opts?: { localStorage?: { before?: LocalStorage; after?: LocalStorage } }
) {
test(name, () => {
localStorage.clear();
const before = opts?.before;
const before = opts?.localStorage?.before;
if (nonNullish(before)) {
setLocalStorage(before);
}
fn();
if (nonNullish(opts) && nonNullish(opts?.after)) {
const after = opts?.localStorage?.after;
if (nonNullish(after)) {
const actual: LocalStorage = readLocalStorage();
const expected: LocalStorage = opts.after;
const expected: LocalStorage = after;
expect(actual).toStrictEqual(expected);
}

Expand Down
Loading

0 comments on commit 7e78f7a

Please sign in to comment.