Upstream
/
bitwarden-browser
mirror of https://github.com/bitwarden/browser.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

[PM-7985] Add & Use InlineDerivedStateProvider (#9131) 

* Add & Use InlineDerivedStateProvider

* Remove Comment

* Delete Foreground & Background Derived State
This commit is contained in:
Justin Baur 2024-05-13 07:42:29 -04:00 committed by GitHub
parent c0216e191a
commit 657807c96a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 108 additions and 462 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
apps/browser/src/background/main.background.ts
View File

@ -125,6 +125,7 @@ import { DefaultActiveUserStateProvider } from "@bitwarden/common/platform/state
import { DefaultGlobalStateProvider } from "@bitwarden/common/platform/state/implementations/default-global-state.provider";
import { DefaultSingleUserStateProvider } from "@bitwarden/common/platform/state/implementations/default-single-user-state.provider";
import { DefaultStateProvider } from "@bitwarden/common/platform/state/implementations/default-state.provider";
import { InlineDerivedStateProvider } from "@bitwarden/common/platform/state/implementations/inline-derived-state";
import { StateEventRegistrarService } from "@bitwarden/common/platform/state/state-event-registrar.service";
/* eslint-enable import/no-restricted-paths */
import { DefaultThemeStateService } from "@bitwarden/common/platform/theming/theme-state.service";
@ -224,7 +225,6 @@ import I18nService from "../platform/services/i18n.service";
import { LocalBackedSessionStorageService } from "../platform/services/local-backed-session-storage.service";
import { BackgroundPlatformUtilsService } from "../platform/services/platform-utils/background-platform-utils.service";
import { BrowserPlatformUtilsService } from "../platform/services/platform-utils/browser-platform-utils.service";
import { BackgroundDerivedStateProvider } from "../platform/state/background-derived-state.provider";
import { BackgroundMemoryStorageService } from "../platform/storage/background-memory-storage.service";
import { BrowserStorageServiceProvider } from "../platform/storage/browser-storage-service.provider";
import { ForegroundMemoryStorageService } from "../platform/storage/foreground-memory-storage.service";
@ -495,7 +495,7 @@ export default class MainBackground {
this.accountService,
this.singleUserStateProvider,
);
this.derivedStateProvider = new BackgroundDerivedStateProvider();
this.derivedStateProvider = new InlineDerivedStateProvider();
this.stateProvider = new DefaultStateProvider(
this.activeUserStateProvider,
this.singleUserStateProvider,

11
apps/browser/src/platform/background/service-factories/derived-state-provider.factory.ts
View File

@ -1,6 +1,6 @@
import { DerivedStateProvider } from "@bitwarden/common/platform/state";
import { BackgroundDerivedStateProvider } from "../../state/background-derived-state.provider";
// eslint-disable-next-line import/no-restricted-paths -- For dependency creation
import { InlineDerivedStateProvider } from "@bitwarden/common/platform/state/implementations/inline-derived-state";
import { CachedServices, FactoryOptions, factory } from "./factory-options";
@ -12,10 +12,5 @@ export async function derivedStateProviderFactory(
cache: { derivedStateProvider?: DerivedStateProvider } & CachedServices,
opts: DerivedStateProviderInitOptions,
): Promise<DerivedStateProvider> {
return factory(
cache,
"derivedStateProvider",
opts,
async () => new BackgroundDerivedStateProvider(),
);
return factory(cache, "derivedStateProvider", opts, async () => new InlineDerivedStateProvider());
}

23
apps/browser/src/platform/state/background-derived-state.provider.ts
View File

@ -1,23 +0,0 @@
import { Observable } from "rxjs";
import { DeriveDefinition, DerivedState } from "@bitwarden/common/platform/state";
// eslint-disable-next-line import/no-restricted-paths -- extending this class for this client
import { DefaultDerivedStateProvider } from "@bitwarden/common/platform/state/implementations/default-derived-state.provider";
import { DerivedStateDependencies } from "@bitwarden/common/src/types/state";
import { BackgroundDerivedState } from "./background-derived-state";
export class BackgroundDerivedStateProvider extends DefaultDerivedStateProvider {
override buildDerivedState<TFrom, TTo, TDeps extends DerivedStateDependencies>(
parentState$: Observable<TFrom>,
deriveDefinition: DeriveDefinition<TFrom, TTo, TDeps>,
dependencies: TDeps,
): DerivedState<TTo> {
return new BackgroundDerivedState(
parentState$,
deriveDefinition,
deriveDefinition.buildCacheKey(),
dependencies,
);
}
}

107
apps/browser/src/platform/state/background-derived-state.ts
View File

@ -1,107 +0,0 @@
import { Observable, Subscription, concatMap } from "rxjs";
import { Jsonify } from "type-fest";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { DeriveDefinition } from "@bitwarden/common/platform/state";
// eslint-disable-next-line import/no-restricted-paths -- extending this class for this client
import { DefaultDerivedState } from "@bitwarden/common/platform/state/implementations/default-derived-state";
import { DerivedStateDependencies } from "@bitwarden/common/types/state";
import { BrowserApi } from "../browser/browser-api";
export class BackgroundDerivedState<
TFrom,
TTo,
TDeps extends DerivedStateDependencies,
> extends DefaultDerivedState<TFrom, TTo, TDeps> {
private portSubscriptions: Map<chrome.runtime.Port, Subscription> = new Map();
constructor(
parentState$: Observable<TFrom>,
deriveDefinition: DeriveDefinition<TFrom, TTo, TDeps>,
portName: string,
dependencies: TDeps,
) {
super(parentState$, deriveDefinition, dependencies);
// listen for foreground derived states to connect
BrowserApi.addListener(chrome.runtime.onConnect, (port) => {
if (port.name !== portName) {
return;
}
const listenerCallback = this.onMessageFromForeground.bind(this);
port.onDisconnect.addListener(() => {
this.portSubscriptions.get(port)?.unsubscribe();
this.portSubscriptions.delete(port);
port.onMessage.removeListener(listenerCallback);
});
port.onMessage.addListener(listenerCallback);
const stateSubscription = this.state$
.pipe(
concatMap(async (state) => {
await this.sendMessage(
{
action: "nextState",
data: JSON.stringify(state),
id: Utils.newGuid(),
},
port,
);
}),
)
.subscribe();
this.portSubscriptions.set(port, stateSubscription);
});
}
private async onMessageFromForeground(message: DerivedStateMessage, port: chrome.runtime.Port) {
if (message.originator === "background") {
return;
}
switch (message.action) {
case "nextState": {
const dataObj = JSON.parse(message.data) as Jsonify<TTo>;
const data = this.deriveDefinition.deserialize(dataObj);
await this.forceValue(data);
await this.sendResponse(
message,
{
action: "resolve",
},
port,
);
break;
}
}
}
private async sendResponse(
originalMessage: DerivedStateMessage,
response: Omit<DerivedStateMessage, "originator" | "id">,
port: chrome.runtime.Port,
) {
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.sendMessage(
{
...response,
id: originalMessage.id,
},
port,
);
}
private async sendMessage(
message: Omit<DerivedStateMessage, "originator">,
port: chrome.runtime.Port,
) {
port.postMessage({
...message,
originator: "background",
});
}
}

117
apps/browser/src/platform/state/derived-state-interactions.spec.ts
View File

@ -1,117 +0,0 @@
/**
* need to update test environment so structuredClone works appropriately
* @jest-environment ../../libs/shared/test.environment.ts
*/
import { NgZone } from "@angular/core";
import { mock } from "jest-mock-extended";
import { Subject, firstValueFrom } from "rxjs";
import { DeriveDefinition } from "@bitwarden/common/platform/state";
// eslint-disable-next-line import/no-restricted-paths -- needed to define a derive definition
import { StateDefinition } from "@bitwarden/common/platform/state/state-definition";
import { awaitAsync, trackEmissions, ObservableTracker } from "@bitwarden/common/spec";
import { mockPorts } from "../../../spec/mock-port.spec-util";
import { BackgroundDerivedState } from "./background-derived-state";
import { ForegroundDerivedState } from "./foreground-derived-state";
const stateDefinition = new StateDefinition("test", "memory");
const deriveDefinition = new DeriveDefinition(stateDefinition, "test", {
derive: (dateString: string) => (dateString == null ? null : new Date(dateString)),
deserializer: (dateString: string) => (dateString == null ? null : new Date(dateString)),
cleanupDelayMs: 1000,
});
// Mock out the runInsideAngular operator so we don't have to deal with zone.js
jest.mock("../browser/run-inside-angular.operator", () => {
return {
runInsideAngular: (ngZone: any) => (source: any) => source,
};
});
describe("foreground background derived state interactions", () => {
let foreground: ForegroundDerivedState<Date>;
let background: BackgroundDerivedState<string, Date, Record<string, unknown>>;
let parentState$: Subject<string>;
const initialParent = "2020-01-01";
const ngZone = mock<NgZone>();
const portName = "testPort";
beforeEach(() => {
mockPorts();
parentState$ = new Subject<string>();
background = new BackgroundDerivedState(parentState$, deriveDefinition, portName, {});
foreground = new ForegroundDerivedState(deriveDefinition, portName, ngZone);
});
afterEach(() => {
parentState$.complete();
jest.resetAllMocks();
});
it("should connect between foreground and background", async () => {
const foregroundEmissions = trackEmissions(foreground.state$);
const backgroundEmissions = trackEmissions(background.state$);
parentState$.next(initialParent);
await awaitAsync(10);
expect(backgroundEmissions).toEqual([new Date(initialParent)]);
expect(foregroundEmissions).toEqual([new Date(initialParent)]);
});
it("should initialize a late-connected foreground", async () => {
const newForeground = new ForegroundDerivedState(deriveDefinition, portName, ngZone);
const backgroundTracker = new ObservableTracker(background.state$);
parentState$.next(initialParent);
const foregroundTracker = new ObservableTracker(newForeground.state$);
expect(await backgroundTracker.expectEmission()).toEqual(new Date(initialParent));
expect(await foregroundTracker.expectEmission()).toEqual(new Date(initialParent));
});
describe("forceValue", () => {
it("should force the value to the background", async () => {
const dateString = "2020-12-12";
const emissions = trackEmissions(background.state$);
await foreground.forceValue(new Date(dateString));
await awaitAsync();
expect(emissions).toEqual([new Date(dateString)]);
});
it("should not create new ports if already connected", async () => {
// establish port with subscription
trackEmissions(foreground.state$);
const connectMock = chrome.runtime.connect as jest.Mock;
const initialConnectCalls = connectMock.mock.calls.length;
expect(foreground["port"]).toBeDefined();
const newDate = new Date();
await foreground.forceValue(newDate);
await awaitAsync();
expect(connectMock.mock.calls.length).toBe(initialConnectCalls);
expect(await firstValueFrom(background.state$)).toEqual(newDate);
});
it("should create a port if not connected", async () => {
const connectMock = chrome.runtime.connect as jest.Mock;
const initialConnectCalls = connectMock.mock.calls.length;
expect(foreground["port"]).toBeUndefined();
const newDate = new Date();
await foreground.forceValue(newDate);
await awaitAsync();
expect(connectMock.mock.calls.length).toBe(initialConnectCalls + 1);
expect(foreground["port"]).toBeNull();
expect(await firstValueFrom(background.state$)).toEqual(newDate);
});
});
});

26
apps/browser/src/platform/state/foreground-derived-state.provider.ts
View File

@ -1,26 +0,0 @@
import { NgZone } from "@angular/core";
import { Observable } from "rxjs";
import { DeriveDefinition, DerivedState } from "@bitwarden/common/platform/state";
// eslint-disable-next-line import/no-restricted-paths -- extending this class for this client
import { DefaultDerivedStateProvider } from "@bitwarden/common/platform/state/implementations/default-derived-state.provider";
import { DerivedStateDependencies } from "@bitwarden/common/src/types/state";
import { ForegroundDerivedState } from "./foreground-derived-state";
export class ForegroundDerivedStateProvider extends DefaultDerivedStateProvider {
constructor(private ngZone: NgZone) {
super();
}
override buildDerivedState<TFrom, TTo, TDeps extends DerivedStateDependencies>(
_parentState$: Observable<TFrom>,
deriveDefinition: DeriveDefinition<TFrom, TTo, TDeps>,
_dependencies: TDeps,
): DerivedState<TTo> {
return new ForegroundDerivedState(
deriveDefinition,
deriveDefinition.buildCacheKey(),
this.ngZone,
);
}
}

61
apps/browser/src/platform/state/foreground-derived-state.spec.ts
View File

@ -1,61 +0,0 @@
import { NgZone } from "@angular/core";
import { awaitAsync } from "@bitwarden/common/../spec";
import { mock } from "jest-mock-extended";
import { DeriveDefinition } from "@bitwarden/common/platform/state";
// eslint-disable-next-line import/no-restricted-paths -- needed to define a derive definition
import { StateDefinition } from "@bitwarden/common/platform/state/state-definition";
import { mockPorts } from "../../../spec/mock-port.spec-util";
import { ForegroundDerivedState } from "./foreground-derived-state";
const stateDefinition = new StateDefinition("test", "memory");
const deriveDefinition = new DeriveDefinition(stateDefinition, "test", {
derive: (dateString: string) => (dateString == null ? null : new Date(dateString)),
deserializer: (dateString: string) => (dateString == null ? null : new Date(dateString)),
cleanupDelayMs: 1,
});
// Mock out the runInsideAngular operator so we don't have to deal with zone.js
jest.mock("../browser/run-inside-angular.operator", () => {
return {
runInsideAngular: (ngZone: any) => (source: any) => source,
};
});
describe("ForegroundDerivedState", () => {
let sut: ForegroundDerivedState<Date>;
const portName = "testPort";
const ngZone = mock<NgZone>();
beforeEach(() => {
mockPorts();
sut = new ForegroundDerivedState(deriveDefinition, portName, ngZone);
});
afterEach(() => {
jest.resetAllMocks();
});
it("should not connect a port until subscribed", async () => {
expect(sut["port"]).toBeUndefined();
const subscription = sut.state$.subscribe();
expect(sut["port"]).toBeDefined();
subscription.unsubscribe();
});
it("should disconnect its port when unsubscribed", async () => {
const subscription = sut.state$.subscribe();
expect(sut["port"]).toBeDefined();
const disconnectSpy = jest.spyOn(sut["port"], "disconnect");
subscription.unsubscribe();
// wait for the cleanup delay
await awaitAsync(deriveDefinition.cleanupDelayMs * 2);
expect(disconnectSpy).toHaveBeenCalled();
expect(sut["port"]).toBeNull();
});
});

115
apps/browser/src/platform/state/foreground-derived-state.ts
View File

@ -1,115 +0,0 @@
import { NgZone } from "@angular/core";
import {
Observable,
ReplaySubject,
defer,
filter,
firstValueFrom,
map,
of,
share,
switchMap,
tap,
timer,
} from "rxjs";
import { Jsonify } from "type-fest";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { DeriveDefinition, DerivedState } from "@bitwarden/common/platform/state";
import { DerivedStateDependencies } from "@bitwarden/common/types/state";
import { fromChromeEvent } from "../browser/from-chrome-event";
import { runInsideAngular } from "../browser/run-inside-angular.operator";
export class ForegroundDerivedState<TTo> implements DerivedState<TTo> {
private port: chrome.runtime.Port;
private backgroundResponses$: Observable<DerivedStateMessage>;
state$: Observable<TTo>;
constructor(
private deriveDefinition: DeriveDefinition<unknown, TTo, DerivedStateDependencies>,
private portName: string,
private ngZone: NgZone,
) {
const latestValueFromPort$ = (port: chrome.runtime.Port) => {
return fromChromeEvent(port.onMessage).pipe(
map(([message]) => message as DerivedStateMessage),
filter((message) => message.originator === "background" && message.action === "nextState"),
map((message) => {
const json = JSON.parse(message.data) as Jsonify<TTo>;
return this.deriveDefinition.deserialize(json);
}),
);
};
this.state$ = defer(() => of(this.initializePort())).pipe(
switchMap(() => latestValueFromPort$(this.port)),
share({
connector: () => new ReplaySubject<TTo>(1),
resetOnRefCountZero: () =>
timer(this.deriveDefinition.cleanupDelayMs).pipe(tap(() => this.tearDownPort())),
}),
runInsideAngular(this.ngZone),
);
}
async forceValue(value: TTo): Promise<TTo> {
let cleanPort = false;
if (this.port == null) {
this.initializePort();
cleanPort = true;
}
await this.delegateToBackground("nextState", value);
if (cleanPort) {
this.tearDownPort();
}
return value;
}
private initializePort() {
if (this.port != null) {
return;
}
this.port = chrome.runtime.connect({ name: this.portName });
this.backgroundResponses$ = fromChromeEvent(this.port.onMessage).pipe(
map(([message]) => message as DerivedStateMessage),
filter((message) => message.originator === "background"),
);
return this.backgroundResponses$;
}
private async delegateToBackground(action: DerivedStateActions, data: TTo): Promise<void> {
const id = Utils.newGuid();
// listen for response before request
const response = firstValueFrom(
this.backgroundResponses$.pipe(filter((message) => message.id === id)),
);
this.sendMessage({
id,
action,
data: JSON.stringify(data),
});
await response;
}
private sendMessage(message: Omit<DerivedStateMessage, "originator">) {
this.port.postMessage({
...message,
originator: "foreground",
});
}
private tearDownPort() {
if (this.port == null) {
return;
}
this.port.disconnect();
this.port = null;
this.backgroundResponses$ = null;
}
}

7
apps/browser/src/popup/services/services.module.ts
View File

@ -78,6 +78,8 @@ import {
GlobalStateProvider,
StateProvider,
} from "@bitwarden/common/platform/state";
// eslint-disable-next-line import/no-restricted-paths -- Used for dependency injection
import { InlineDerivedStateProvider } from "@bitwarden/common/platform/state/implementations/inline-derived-state";
import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/password";
import { UsernameGenerationServiceAbstraction } from "@bitwarden/common/tools/generator/username";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
@ -112,7 +114,6 @@ import { BrowserScriptInjectorService } from "../../platform/services/browser-sc
import { DefaultBrowserStateService } from "../../platform/services/default-browser-state.service";
import I18nService from "../../platform/services/i18n.service";
import { ForegroundPlatformUtilsService } from "../../platform/services/platform-utils/foreground-platform-utils.service";
import { ForegroundDerivedStateProvider } from "../../platform/state/foreground-derived-state.provider";
import { BrowserStorageServiceProvider } from "../../platform/storage/browser-storage-service.provider";
import { ForegroundMemoryStorageService } from "../../platform/storage/foreground-memory-storage.service";
import { fromChromeRuntimeMessaging } from "../../platform/utils/from-chrome-runtime-messaging";
@ -512,8 +513,8 @@ const safeProviders: SafeProvider[] = [
}),
safeProvider({
provide: DerivedStateProvider,
useClass: ForegroundDerivedStateProvider,
deps: [NgZone],
useClass: InlineDerivedStateProvider,
deps: [],
}),
safeProvider({
provide: AutofillSettingsServiceAbstraction,

62
libs/common/src/platform/state/implementations/inline-derived-state.spec.ts Normal file
View File

@ -0,0 +1,62 @@
import { Subject, firstValueFrom } from "rxjs";
import { DeriveDefinition } from "../derive-definition";
import { StateDefinition } from "../state-definition";
import { InlineDerivedState } from "./inline-derived-state";
describe("InlineDerivedState", () => {
const syncDeriveDefinition = new DeriveDefinition<boolean, boolean, Record<string, unknown>>(
new StateDefinition("test", "disk"),
"test",
{
derive: (value, deps) => !value,
deserializer: (value) => value,
},
);
const asyncDeriveDefinition = new DeriveDefinition<boolean, boolean, Record<string, unknown>>(
new StateDefinition("test", "disk"),
"test",
{
derive: async (value, deps) => Promise.resolve(!value),
deserializer: (value) => value,
},
);
const parentState = new Subject<boolean>();
describe("state", () => {
const cases = [
{
it: "works when derive function is sync",
definition: syncDeriveDefinition,
},
{
it: "works when derive function is async",
definition: asyncDeriveDefinition,
},
];
it.each(cases)("$it", async ({ definition }) => {
const sut = new InlineDerivedState(parentState.asObservable(), definition, {});
const valuePromise = firstValueFrom(sut.state$);
parentState.next(true);
const value = await valuePromise;
expect(value).toBe(false);
});
});
describe("forceValue", () => {
it("returns the force value back to the caller", async () => {
const sut = new InlineDerivedState(parentState.asObservable(), syncDeriveDefinition, {});
const value = await sut.forceValue(true);
expect(value).toBe(true);
});
});
});

37
libs/common/src/platform/state/implementations/inline-derived-state.ts Normal file
View File

@ -0,0 +1,37 @@
import { Observable, concatMap } from "rxjs";
import { DerivedStateDependencies } from "../../../types/state";
import { DeriveDefinition } from "../derive-definition";
import { DerivedState } from "../derived-state";
import { DerivedStateProvider } from "../derived-state.provider";
export class InlineDerivedStateProvider implements DerivedStateProvider {
get<TFrom, TTo, TDeps extends DerivedStateDependencies>(
parentState$: Observable<TFrom>,
deriveDefinition: DeriveDefinition<TFrom, TTo, TDeps>,
dependencies: TDeps,
): DerivedState<TTo> {
return new InlineDerivedState(parentState$, deriveDefinition, dependencies);
}
}
export class InlineDerivedState<TFrom, TTo, TDeps extends DerivedStateDependencies>
implements DerivedState<TTo>
{
constructor(
parentState$: Observable<TFrom>,
deriveDefinition: DeriveDefinition<TFrom, TTo, TDeps>,
dependencies: TDeps,
) {
this.state$ = parentState$.pipe(
concatMap(async (value) => await deriveDefinition.derive(value, dependencies)),
);
}
state$: Observable<TTo>;
forceValue(value: TTo): Promise<TTo> {
// No need to force anything, we don't keep a cache
return Promise.resolve(value);
}
}