1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-15 15:53:27 +00:00

Browser MV3: Default store values to session storage (#8844)

* Introduce browser large object storage location.

This location is encrypted and serialized to disk in order to allow for storage of uncountable things like vault items that take a significant amount of time to prepare, but are not guaranteed to fit within session storage.

however, limit the need to write to disk is a big benefit, so _most_ things are written to storage.session instead, where things specifically flagged as large will be moved to disk-backed memory

* Store derived values in large object store for browser

* Fix AbstractMemoryStorageService implementation
This commit is contained in:
Matt Gibson
2024-04-22 08:55:19 -04:00
committed by GitHub
parent f829cdd8a7
commit b5362ca1ce
20 changed files with 171 additions and 58 deletions

View File

@@ -171,8 +171,8 @@ export class DeriveDefinition<TFrom, TTo, TDeps extends DerivedStateDependencies
return this.options.clearOnCleanup ?? true;
}
buildCacheKey(): string {
return `derived_${this.stateDefinition.name}_${this.uniqueDerivationName}`;
buildCacheKey(location: string): string {
return `derived_${location}_${this.stateDefinition.name}_${this.uniqueDerivationName}`;
}
/**

View File

@@ -5,6 +5,7 @@ import {
AbstractStorageService,
ObservableStorageService,
} from "../../abstractions/storage.service";
import { StorageServiceProvider } from "../../services/storage-service.provider";
import { DeriveDefinition } from "../derive-definition";
import { DerivedState } from "../derived-state";
import { DerivedStateProvider } from "../derived-state.provider";
@@ -14,14 +15,18 @@ import { DefaultDerivedState } from "./default-derived-state";
export class DefaultDerivedStateProvider implements DerivedStateProvider {
private cache: Record<string, DerivedState<unknown>> = {};
constructor(protected memoryStorage: AbstractStorageService & ObservableStorageService) {}
constructor(protected storageServiceProvider: StorageServiceProvider) {}
get<TFrom, TTo, TDeps extends DerivedStateDependencies>(
parentState$: Observable<TFrom>,
deriveDefinition: DeriveDefinition<TFrom, TTo, TDeps>,
dependencies: TDeps,
): DerivedState<TTo> {
const cacheKey = deriveDefinition.buildCacheKey();
// TODO: we probably want to support optional normal memory storage for browser
const [location, storageService] = this.storageServiceProvider.get("memory", {
browser: "memory-large-object",
});
const cacheKey = deriveDefinition.buildCacheKey(location);
const existingDerivedState = this.cache[cacheKey];
if (existingDerivedState != null) {
// I have to cast out of the unknown generic but this should be safe if rules
@@ -29,7 +34,10 @@ export class DefaultDerivedStateProvider implements DerivedStateProvider {
return existingDerivedState as DefaultDerivedState<TFrom, TTo, TDeps>;
}
const newDerivedState = this.buildDerivedState(parentState$, deriveDefinition, dependencies);
const newDerivedState = this.buildDerivedState(parentState$, deriveDefinition, dependencies, [
location,
storageService,
]);
this.cache[cacheKey] = newDerivedState;
return newDerivedState;
}
@@ -38,11 +46,12 @@ export class DefaultDerivedStateProvider implements DerivedStateProvider {
parentState$: Observable<TFrom>,
deriveDefinition: DeriveDefinition<TFrom, TTo, TDeps>,
dependencies: TDeps,
storageLocation: [string, AbstractStorageService & ObservableStorageService],
): DerivedState<TTo> {
return new DefaultDerivedState<TFrom, TTo, TDeps>(
parentState$,
deriveDefinition,
this.memoryStorage,
storageLocation[1],
dependencies,
);
}

View File

@@ -72,12 +72,12 @@ describe("DefaultDerivedState", () => {
parentState$.next(dateString);
await awaitAsync();
expect(memoryStorage.internalStore[deriveDefinition.buildCacheKey()]).toEqual(
expect(memoryStorage.internalStore[deriveDefinition.storageKey]).toEqual(
derivedValue(new Date(dateString)),
);
const calls = memoryStorage.mock.save.mock.calls;
expect(calls.length).toBe(1);
expect(calls[0][0]).toBe(deriveDefinition.buildCacheKey());
expect(calls[0][0]).toBe(deriveDefinition.storageKey);
expect(calls[0][1]).toEqual(derivedValue(new Date(dateString)));
});
@@ -94,7 +94,7 @@ describe("DefaultDerivedState", () => {
it("should store the forced value", async () => {
await sut.forceValue(forced);
expect(memoryStorage.internalStore[deriveDefinition.buildCacheKey()]).toEqual(
expect(memoryStorage.internalStore[deriveDefinition.storageKey]).toEqual(
derivedValue(forced),
);
});
@@ -109,7 +109,7 @@ describe("DefaultDerivedState", () => {
it("should store the forced value", async () => {
await sut.forceValue(forced);
expect(memoryStorage.internalStore[deriveDefinition.buildCacheKey()]).toEqual(
expect(memoryStorage.internalStore[deriveDefinition.storageKey]).toEqual(
derivedValue(forced),
);
});
@@ -153,7 +153,7 @@ describe("DefaultDerivedState", () => {
parentState$.next(newDate);
await awaitAsync();
expect(memoryStorage.internalStore[deriveDefinition.buildCacheKey()]).toEqual(
expect(memoryStorage.internalStore[deriveDefinition.storageKey]).toEqual(
derivedValue(new Date(newDate)),
);
@@ -161,7 +161,7 @@ describe("DefaultDerivedState", () => {
// Wait for cleanup
await awaitAsync(cleanupDelayMs * 2);
expect(memoryStorage.internalStore[deriveDefinition.buildCacheKey()]).toBeUndefined();
expect(memoryStorage.internalStore[deriveDefinition.storageKey]).toBeUndefined();
});
it("should not clear state after cleanup if clearOnCleanup is false", async () => {
@@ -171,7 +171,7 @@ describe("DefaultDerivedState", () => {
parentState$.next(newDate);
await awaitAsync();
expect(memoryStorage.internalStore[deriveDefinition.buildCacheKey()]).toEqual(
expect(memoryStorage.internalStore[deriveDefinition.storageKey]).toEqual(
derivedValue(new Date(newDate)),
);
@@ -179,7 +179,7 @@ describe("DefaultDerivedState", () => {
// Wait for cleanup
await awaitAsync(cleanupDelayMs * 2);
expect(memoryStorage.internalStore[deriveDefinition.buildCacheKey()]).toEqual(
expect(memoryStorage.internalStore[deriveDefinition.storageKey]).toEqual(
derivedValue(new Date(newDate)),
);
});

View File

@@ -24,8 +24,10 @@ export type ClientLocations = {
web: StorageLocation | "disk-local";
/**
* Overriding storage location for browser clients.
*
* "memory-large-object" is used to store non-countable objects in memory. This exists due to limited persistent memory available to browser extensions.
*/
//browser: StorageLocation;
browser: StorageLocation | "memory-large-object";
/**
* Overriding storage location for desktop clients.
*/

View File

@@ -116,7 +116,9 @@ export const EVENT_COLLECTION_DISK = new StateDefinition("eventCollection", "dis
export const SEND_DISK = new StateDefinition("encryptedSend", "disk", {
web: "memory",
});
export const SEND_MEMORY = new StateDefinition("decryptedSend", "memory");
export const SEND_MEMORY = new StateDefinition("decryptedSend", "memory", {
browser: "memory-large-object",
});
// Vault
@@ -133,10 +135,16 @@ export const VAULT_ONBOARDING = new StateDefinition("vaultOnboarding", "disk", {
export const VAULT_SETTINGS_DISK = new StateDefinition("vaultSettings", "disk", {
web: "disk-local",
});
export const VAULT_BROWSER_MEMORY = new StateDefinition("vaultBrowser", "memory");
export const VAULT_SEARCH_MEMORY = new StateDefinition("vaultSearch", "memory");
export const VAULT_BROWSER_MEMORY = new StateDefinition("vaultBrowser", "memory", {
browser: "memory-large-object",
});
export const VAULT_SEARCH_MEMORY = new StateDefinition("vaultSearch", "memory", {
browser: "memory-large-object",
});
export const CIPHERS_DISK = new StateDefinition("ciphers", "disk", { web: "memory" });
export const CIPHERS_DISK_LOCAL = new StateDefinition("ciphersLocal", "disk", {
web: "disk-local",
});
export const CIPHERS_MEMORY = new StateDefinition("ciphersMemory", "memory");
export const CIPHERS_MEMORY = new StateDefinition("ciphersMemory", "memory", {
browser: "memory-large-object",
});