Skip to content

Commit

Permalink
[PM-6404] Initial Clear Events Code (#8029)
Browse files Browse the repository at this point in the history
* Add New KeyDefinitionOption

* Add New Services

* Add WebStorageServiceProvider Tests

* Update Error Message

* Add `UserKeyDefinition`

* Fix Deserialization Helpers

* Fix KeyDefinition

* Add `UserKeyDefinition`

* Fix Deserialization Helpers

* Fix KeyDefinition

* Move `ClearEvent`

* Cleanup

* Fix Imports

* Remove `updateMock`

* Call Super in Web Implementation

* Use Better Type to Avoid Casting

* Better Error Docs

* Move StorageKey Creation to Function

* Throw Aggregated Error for Failures
  • Loading branch information
justindbaur authored Feb 27, 2024
1 parent 929b5eb commit 87c75e5
Show file tree
Hide file tree
Showing 14 changed files with 553 additions and 10 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// eslint-disable-next-line import/no-restricted-paths
import { StateEventRegistrarService } from "@bitwarden/common/platform/state/state-event-registrar.service";

import { CachedServices, FactoryOptions, factory } from "./factory-options";
import {
GlobalStateProviderInitOptions,
globalStateProviderFactory,
} from "./global-state-provider.factory";
import {
StorageServiceProviderInitOptions,
storageServiceProviderFactory,
} from "./storage-service-provider.factory";

type StateEventRegistrarServiceFactoryOptions = FactoryOptions;

export type StateEventRegistrarServiceInitOptions = StateEventRegistrarServiceFactoryOptions &
GlobalStateProviderInitOptions &
StorageServiceProviderInitOptions;

export async function stateEventRegistrarServiceFactory(
cache: {
stateEventRegistrarService?: StateEventRegistrarService;
} & CachedServices,
opts: StateEventRegistrarServiceInitOptions,
): Promise<StateEventRegistrarService> {
return factory(
cache,
"stateEventRegistrarService",
opts,
async () =>
new StateEventRegistrarService(
await globalStateProviderFactory(cache, opts),
await storageServiceProviderFactory(cache, opts),
),
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { StorageServiceProvider } from "@bitwarden/common/platform/services/storage-service.provider";

import { CachedServices, FactoryOptions, factory } from "./factory-options";
import {
DiskStorageServiceInitOptions,
MemoryStorageServiceInitOptions,
observableDiskStorageServiceFactory,
observableMemoryStorageServiceFactory,
} from "./storage-service.factory";

type StorageServiceProviderFactoryOptions = FactoryOptions;

export type StorageServiceProviderInitOptions = StorageServiceProviderFactoryOptions &
MemoryStorageServiceInitOptions &
DiskStorageServiceInitOptions;

export async function storageServiceProviderFactory(
cache: {
storageServiceProvider?: StorageServiceProvider;
} & CachedServices,
opts: StorageServiceProviderInitOptions,
): Promise<StorageServiceProvider> {
return factory(
cache,
"storageServiceProvider",
opts,
async () =>
new StorageServiceProvider(
await observableDiskStorageServiceFactory(cache, opts),
await observableMemoryStorageServiceFactory(cache, opts),
),
);
}
63 changes: 63 additions & 0 deletions apps/web/src/app/platform/web-storage-service.provider.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { mock } from "jest-mock-extended";

import {
AbstractStorageService,
ObservableStorageService,
} from "@bitwarden/common/platform/abstractions/storage.service";
import { PossibleLocation } from "@bitwarden/common/platform/services/storage-service.provider";
import {
ClientLocations,
StorageLocation,
// eslint-disable-next-line import/no-restricted-paths
} from "@bitwarden/common/platform/state/state-definition";

import { WebStorageServiceProvider } from "./web-storage-service.provider";

describe("WebStorageServiceProvider", () => {
const mockDiskStorage = mock<AbstractStorageService & ObservableStorageService>();
const mockMemoryStorage = mock<AbstractStorageService & ObservableStorageService>();
const mockDiskLocalStorage = mock<AbstractStorageService & ObservableStorageService>();

const sut = new WebStorageServiceProvider(
mockDiskStorage,
mockMemoryStorage,
mockDiskLocalStorage,
);

describe("get", () => {
const getTests = [
{
input: { default: "disk", overrides: {} },
expected: "disk",
},
{
input: { default: "memory", overrides: {} },
expected: "memory",
},
{
input: { default: "disk", overrides: { web: "disk-local" } },
expected: "disk-local",
},
{
input: { default: "disk", overrides: { web: "memory" } },
expected: "memory",
},
{
input: { default: "memory", overrides: { web: "disk" } },
expected: "disk",
},
] satisfies {
input: { default: StorageLocation; overrides: Partial<ClientLocations> };
expected: PossibleLocation;
}[];

it.each(getTests)("computes properly based on %s", ({ input, expected: expectedLocation }) => {
const [actualLocation] = sut.get(input.default, input.overrides);
expect(actualLocation).toStrictEqual(expectedLocation);
});

it("throws on unsupported option", () => {
expect(() => sut.get("blah" as any, {})).toThrow();
});
});
});
37 changes: 37 additions & 0 deletions apps/web/src/app/platform/web-storage-service.provider.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import {
AbstractStorageService,
ObservableStorageService,
} from "@bitwarden/common/platform/abstractions/storage.service";
import {
PossibleLocation,
StorageServiceProvider,
} from "@bitwarden/common/platform/services/storage-service.provider";
import {
ClientLocations,
// eslint-disable-next-line import/no-restricted-paths
} from "@bitwarden/common/platform/state/state-definition";

export class WebStorageServiceProvider extends StorageServiceProvider {
constructor(
diskStorageService: AbstractStorageService & ObservableStorageService,
memoryStorageService: AbstractStorageService & ObservableStorageService,
private readonly diskLocalStorageService: AbstractStorageService & ObservableStorageService,
) {
super(diskStorageService, memoryStorageService);
}

override get(
defaultLocation: PossibleLocation,
overrides: Partial<ClientLocations>,
): [location: PossibleLocation, service: AbstractStorageService & ObservableStorageService] {
const location = overrides["web"] ?? defaultLocation;
switch (location) {
case "disk-local":
return ["disk-local", this.diskLocalStorageService];
default:
// Pass in computed location to super because they could have
// overriden default "disk" with web "memory".
return super.get(location, overrides);
}
}
}
12 changes: 3 additions & 9 deletions libs/common/spec/fake-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ export class FakeGlobalState<T> implements GlobalState<T> {
this.stateSubject.next(initialValue ?? null);
}

update: <TCombine>(
async update<TCombine>(
configureState: (state: T, dependency: TCombine) => T,
options?: StateUpdateOptions<T, TCombine>,
) => Promise<T> = jest.fn(async (configureState, options) => {
): Promise<T> {
options = populateOptionsWithDefault(options);
if (this.stateSubject["_buffer"].length == 0) {
// throw a more helpful not initialized error
Expand All @@ -64,9 +64,7 @@ export class FakeGlobalState<T> implements GlobalState<T> {
this.stateSubject.next(newState);
this.nextMock(newState);
return newState;
});

updateMock = this.update as jest.MockedFunction<typeof this.update>;
}
/** Tracks update values resolved by `FakeState.update` */
nextMock = jest.fn<void, [T]>();

Expand Down Expand Up @@ -128,8 +126,6 @@ export class FakeSingleUserState<T> implements SingleUserState<T> {
return newState;
}

updateMock = this.update as jest.MockedFunction<typeof this.update>;

/** Tracks update values resolved by `FakeState.update` */
nextMock = jest.fn<void, [T]>();
private _keyDefinition: UserKeyDefinition<T> | null = null;
Expand Down Expand Up @@ -191,8 +187,6 @@ export class FakeActiveUserState<T> implements ActiveUserState<T> {
return [this.userId, newState];
}

updateMock = this.update as jest.MockedFunction<typeof this.update>;

/** Tracks update values resolved by `FakeState.update` */
nextMock = jest.fn<void, [[UserId, T]]>();

Expand Down
28 changes: 28 additions & 0 deletions libs/common/src/platform/services/storage-service.provider.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { mock } from "jest-mock-extended";

import { AbstractStorageService, ObservableStorageService } from "../abstractions/storage.service";

import { StorageServiceProvider } from "./storage-service.provider";

describe("StorageServiceProvider", () => {
const mockDiskStorage = mock<AbstractStorageService & ObservableStorageService>();
const mockMemoryStorage = mock<AbstractStorageService & ObservableStorageService>();

const sut = new StorageServiceProvider(mockDiskStorage, mockMemoryStorage);

describe("get", () => {
it("gets disk service when default location is disk", () => {
const [computedLocation, computedService] = sut.get("disk", {});

expect(computedLocation).toBe("disk");
expect(computedService).toStrictEqual(mockDiskStorage);
});

it("gets memory service when default location is memory", () => {
const [computedLocation, computedService] = sut.get("memory", {});

expect(computedLocation).toBe("memory");
expect(computedService).toStrictEqual(mockMemoryStorage);
});
});
});
39 changes: 39 additions & 0 deletions libs/common/src/platform/services/storage-service.provider.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { AbstractStorageService, ObservableStorageService } from "../abstractions/storage.service";
// eslint-disable-next-line import/no-restricted-paths
import { ClientLocations, StorageLocation } from "../state/state-definition";

export type PossibleLocation = StorageLocation | ClientLocations[keyof ClientLocations];

/**
* A provider for getting client specific computed storage locations and services.
*/
export class StorageServiceProvider {
constructor(
protected readonly diskStorageService: AbstractStorageService & ObservableStorageService,
protected readonly memoryStorageService: AbstractStorageService & ObservableStorageService,
) {}

/**
* Computes the location and corresponding service for a given client.
*
* **NOTE** The default implementation does not respect client overrides and if clients
* have special overrides they are responsible for implementing this service.
* @param defaultLocation The default location to use if no client specific override is preferred.
* @param overrides Client specific overrides
* @returns The computed storage location and corresponding storage service to use to get/store state.
* @throws If there is no configured storage service for the given inputs.
*/
get(
defaultLocation: PossibleLocation,
overrides: Partial<ClientLocations>,
): [location: PossibleLocation, service: AbstractStorageService & ObservableStorageService] {
switch (defaultLocation) {
case "disk":
return [defaultLocation, this.diskStorageService];
case "memory":
return [defaultLocation, this.memoryStorageService];
default:
throw new Error(`Unexpected location: ${defaultLocation}`);
}
}
}
1 change: 1 addition & 0 deletions libs/common/src/platform/state/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ export { ActiveUserStateProvider, SingleUserStateProvider } from "./user-state.p
export { KeyDefinition } from "./key-definition";
export { StateUpdateOptions } from "./state-update-options";
export { UserKeyDefinition } from "./user-key-definition";
export { StateEventRunnerService } from "./state-event-runner.service";

export * from "./state-definitions";
2 changes: 2 additions & 0 deletions libs/common/src/platform/state/state-definitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ export const VAULT_FILTER_DISK = new StateDefinition("vaultFilter", "disk", {
web: "disk-local",
});

export const CLEAR_EVENT_DISK = new StateDefinition("clearEvent", "disk");

export const NEW_WEB_LAYOUT_BANNER_DISK = new StateDefinition("newWebLayoutBanner", "disk", {
web: "disk-local",
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import { mock } from "jest-mock-extended";

import { FakeGlobalStateProvider } from "../../../spec";
import { AbstractStorageService, ObservableStorageService } from "../abstractions/storage.service";
import { StorageServiceProvider } from "../services/storage-service.provider";

import { StateDefinition } from "./state-definition";
import { STATE_LOCK_EVENT, StateEventRegistrarService } from "./state-event-registrar.service";
import { UserKeyDefinition } from "./user-key-definition";

describe("StateEventRegistrarService", () => {
const globalStateProvider = new FakeGlobalStateProvider();
const lockState = globalStateProvider.getFake(STATE_LOCK_EVENT);
const storageServiceProvider = mock<StorageServiceProvider>();

const sut = new StateEventRegistrarService(globalStateProvider, storageServiceProvider);

describe("registerEvents", () => {
const fakeKeyDefinition = new UserKeyDefinition<boolean>(
new StateDefinition("fakeState", "disk"),
"fakeKey",
{
deserializer: (s) => s,
clearOn: ["lock"],
},
);

beforeEach(() => {
jest.resetAllMocks();
});

it("adds event on null storage", async () => {
storageServiceProvider.get.mockReturnValue([
"disk",
mock<AbstractStorageService & ObservableStorageService>(),
]);

await sut.registerEvents(fakeKeyDefinition);

expect(lockState.nextMock).toHaveBeenCalledWith([
{
key: "fakeKey",
location: "disk",
state: "fakeState",
},
]);
});

it("adds event on empty array in storage", async () => {
lockState.stateSubject.next([]);
storageServiceProvider.get.mockReturnValue([
"disk",
mock<AbstractStorageService & ObservableStorageService>(),
]);

await sut.registerEvents(fakeKeyDefinition);

expect(lockState.nextMock).toHaveBeenCalledWith([
{
key: "fakeKey",
location: "disk",
state: "fakeState",
},
]);
});

it("doesn't add a duplicate", async () => {
lockState.stateSubject.next([
{
key: "fakeKey",
location: "disk",
state: "fakeState",
},
]);
storageServiceProvider.get.mockReturnValue([
"disk",
mock<AbstractStorageService & ObservableStorageService>(),
]);

await sut.registerEvents(fakeKeyDefinition);

expect(lockState.nextMock).not.toHaveBeenCalled();
});
});
});
Loading

0 comments on commit 87c75e5

Please sign in to comment.