diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 7287e9b285..6120155334 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -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, diff --git a/apps/browser/src/platform/background/service-factories/derived-state-provider.factory.ts b/apps/browser/src/platform/background/service-factories/derived-state-provider.factory.ts index 3c3900144b..2c5f8f2419 100644 --- a/apps/browser/src/platform/background/service-factories/derived-state-provider.factory.ts +++ b/apps/browser/src/platform/background/service-factories/derived-state-provider.factory.ts @@ -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 { - return factory( - cache, - "derivedStateProvider", - opts, - async () => new BackgroundDerivedStateProvider(), - ); + return factory(cache, "derivedStateProvider", opts, async () => new InlineDerivedStateProvider()); } diff --git a/apps/browser/src/platform/state/background-derived-state.provider.ts b/apps/browser/src/platform/state/background-derived-state.provider.ts deleted file mode 100644 index cbc5a34b37..0000000000 --- a/apps/browser/src/platform/state/background-derived-state.provider.ts +++ /dev/null @@ -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( - parentState$: Observable, - deriveDefinition: DeriveDefinition, - dependencies: TDeps, - ): DerivedState { - return new BackgroundDerivedState( - parentState$, - deriveDefinition, - deriveDefinition.buildCacheKey(), - dependencies, - ); - } -} diff --git a/apps/browser/src/platform/state/background-derived-state.ts b/apps/browser/src/platform/state/background-derived-state.ts deleted file mode 100644 index 61768cb970..0000000000 --- a/apps/browser/src/platform/state/background-derived-state.ts +++ /dev/null @@ -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 { - private portSubscriptions: Map = new Map(); - - constructor( - parentState$: Observable, - deriveDefinition: DeriveDefinition, - 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; - 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, - 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, - port: chrome.runtime.Port, - ) { - port.postMessage({ - ...message, - originator: "background", - }); - } -} diff --git a/apps/browser/src/platform/state/derived-state-interactions.spec.ts b/apps/browser/src/platform/state/derived-state-interactions.spec.ts deleted file mode 100644 index 823c071a4c..0000000000 --- a/apps/browser/src/platform/state/derived-state-interactions.spec.ts +++ /dev/null @@ -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; - let background: BackgroundDerivedState>; - let parentState$: Subject; - const initialParent = "2020-01-01"; - const ngZone = mock(); - const portName = "testPort"; - - beforeEach(() => { - mockPorts(); - parentState$ = new Subject(); - - 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); - }); - }); -}); diff --git a/apps/browser/src/platform/state/foreground-derived-state.provider.ts b/apps/browser/src/platform/state/foreground-derived-state.provider.ts deleted file mode 100644 index 8b8d82b914..0000000000 --- a/apps/browser/src/platform/state/foreground-derived-state.provider.ts +++ /dev/null @@ -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( - _parentState$: Observable, - deriveDefinition: DeriveDefinition, - _dependencies: TDeps, - ): DerivedState { - return new ForegroundDerivedState( - deriveDefinition, - deriveDefinition.buildCacheKey(), - this.ngZone, - ); - } -} diff --git a/apps/browser/src/platform/state/foreground-derived-state.spec.ts b/apps/browser/src/platform/state/foreground-derived-state.spec.ts deleted file mode 100644 index ee224540c1..0000000000 --- a/apps/browser/src/platform/state/foreground-derived-state.spec.ts +++ /dev/null @@ -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; - const portName = "testPort"; - const ngZone = mock(); - - 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(); - }); -}); diff --git a/apps/browser/src/platform/state/foreground-derived-state.ts b/apps/browser/src/platform/state/foreground-derived-state.ts deleted file mode 100644 index 6abe363876..0000000000 --- a/apps/browser/src/platform/state/foreground-derived-state.ts +++ /dev/null @@ -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 implements DerivedState { - private port: chrome.runtime.Port; - private backgroundResponses$: Observable; - state$: Observable; - - constructor( - private deriveDefinition: DeriveDefinition, - 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; - return this.deriveDefinition.deserialize(json); - }), - ); - }; - - this.state$ = defer(() => of(this.initializePort())).pipe( - switchMap(() => latestValueFromPort$(this.port)), - share({ - connector: () => new ReplaySubject(1), - resetOnRefCountZero: () => - timer(this.deriveDefinition.cleanupDelayMs).pipe(tap(() => this.tearDownPort())), - }), - runInsideAngular(this.ngZone), - ); - } - - async forceValue(value: TTo): Promise { - 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 { - 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) { - this.port.postMessage({ - ...message, - originator: "foreground", - }); - } - - private tearDownPort() { - if (this.port == null) { - return; - } - - this.port.disconnect(); - this.port = null; - this.backgroundResponses$ = null; - } -} diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index de2fc72747..2313833ab8 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -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, diff --git a/libs/common/src/platform/state/implementations/inline-derived-state.spec.ts b/libs/common/src/platform/state/implementations/inline-derived-state.spec.ts new file mode 100644 index 0000000000..28a1a701b0 --- /dev/null +++ b/libs/common/src/platform/state/implementations/inline-derived-state.spec.ts @@ -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>( + new StateDefinition("test", "disk"), + "test", + { + derive: (value, deps) => !value, + deserializer: (value) => value, + }, + ); + + const asyncDeriveDefinition = new DeriveDefinition>( + new StateDefinition("test", "disk"), + "test", + { + derive: async (value, deps) => Promise.resolve(!value), + deserializer: (value) => value, + }, + ); + + const parentState = new Subject(); + + 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); + }); + }); +}); diff --git a/libs/common/src/platform/state/implementations/inline-derived-state.ts b/libs/common/src/platform/state/implementations/inline-derived-state.ts new file mode 100644 index 0000000000..79b2c92100 --- /dev/null +++ b/libs/common/src/platform/state/implementations/inline-derived-state.ts @@ -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( + parentState$: Observable, + deriveDefinition: DeriveDefinition, + dependencies: TDeps, + ): DerivedState { + return new InlineDerivedState(parentState$, deriveDefinition, dependencies); + } +} + +export class InlineDerivedState + implements DerivedState +{ + constructor( + parentState$: Observable, + deriveDefinition: DeriveDefinition, + dependencies: TDeps, + ) { + this.state$ = parentState$.pipe( + concatMap(async (value) => await deriveDefinition.derive(value, dependencies)), + ); + } + + state$: Observable; + + forceValue(value: TTo): Promise { + // No need to force anything, we don't keep a cache + return Promise.resolve(value); + } +}