diff --git a/apps/browser/src/browser/browserApi.ts b/apps/browser/src/browser/browserApi.ts index 60666d23fe..68afc22dcf 100644 --- a/apps/browser/src/browser/browserApi.ts +++ b/apps/browser/src/browser/browserApi.ts @@ -115,6 +115,10 @@ export class BrowserApi { return chrome.extension.getBackgroundPage(); } + static isBackgroundPage(window: Window & typeof globalThis): boolean { + return window === chrome.extension.getBackgroundPage(); + } + static getApplicationVersion(): string { return chrome.runtime.getManifest().version; } diff --git a/apps/browser/src/decorators/session-sync-observable/session-sync.decorator.spec.ts b/apps/browser/src/decorators/session-sync-observable/session-sync.decorator.spec.ts index c6d46b28fd..7a6e726608 100644 --- a/apps/browser/src/decorators/session-sync-observable/session-sync.decorator.spec.ts +++ b/apps/browser/src/decorators/session-sync-observable/session-sync.decorator.spec.ts @@ -4,11 +4,10 @@ import { sessionSync } from "./session-sync.decorator"; describe("sessionSync decorator", () => { const initializer = (s: string) => "test"; - const ctor = String; class TestClass { - @sessionSync({ ctor: ctor, initializer: initializer }) + @sessionSync({ initializer: initializer }) private testProperty = new BehaviorSubject(""); - @sessionSync({ ctor: ctor, initializer: initializer, initializeAs: "array" }) + @sessionSync({ initializer: initializer, initializeAs: "array" }) private secondTestProperty = new BehaviorSubject(""); complete() { @@ -23,13 +22,11 @@ describe("sessionSync decorator", () => { expect.objectContaining({ propertyKey: "testProperty", sessionKey: "testProperty_0", - ctor: ctor, initializer: initializer, }), expect.objectContaining({ propertyKey: "secondTestProperty", sessionKey: "secondTestProperty_1", - ctor: ctor, initializer: initializer, initializeAs: "array", }), @@ -38,7 +35,7 @@ describe("sessionSync decorator", () => { }); class TestClass2 { - @sessionSync({ ctor: ctor, initializer: initializer }) + @sessionSync({ initializer: initializer }) private testProperty = new BehaviorSubject(""); complete() { @@ -52,7 +49,6 @@ describe("sessionSync decorator", () => { expect.objectContaining({ propertyKey: "testProperty", sessionKey: "testProperty_2", - ctor: ctor, initializer: initializer, }), ]); diff --git a/apps/browser/src/decorators/session-sync-observable/session-sync.decorator.ts b/apps/browser/src/decorators/session-sync-observable/session-sync.decorator.ts index 6a6b3d33fa..e439cea45a 100644 --- a/apps/browser/src/decorators/session-sync-observable/session-sync.decorator.ts +++ b/apps/browser/src/decorators/session-sync-observable/session-sync.decorator.ts @@ -4,7 +4,6 @@ import { SessionStorable } from "./session-storable"; import { InitializeOptions } from "./sync-item-metadata"; class BuildOptions> { - ctor?: new () => T; initializer?: (keyValuePair: TJson) => T; initializeAs?: InitializeOptions; } @@ -48,7 +47,6 @@ export function sessionSync(buildOptions: BuildOptions) { p.__syncedItemMetadata.push({ propertyKey, sessionKey: `${propertyKey}_${index++}`, - ctor: buildOptions.ctor, initializer: buildOptions.initializer, initializeAs: buildOptions.initializeAs ?? "object", }); diff --git a/apps/browser/src/decorators/session-sync-observable/session-syncer.spec.ts b/apps/browser/src/decorators/session-sync-observable/session-syncer.spec.ts index 2de87b551d..6331237629 100644 --- a/apps/browser/src/decorators/session-sync-observable/session-syncer.spec.ts +++ b/apps/browser/src/decorators/session-sync-observable/session-syncer.spec.ts @@ -53,8 +53,8 @@ describe("session syncer", () => { new SessionSyncer(behaviorSubject, storageService, { propertyKey, sessionKey, - ctor: String, initializeAs: "object", + initializer: () => null, }) ).toBeDefined(); expect( @@ -72,8 +72,9 @@ describe("session syncer", () => { propertyKey, sessionKey, initializeAs: "object", + initializer: null, }); - }).toThrowError("ctor or initializer must be provided"); + }).toThrowError("initializer must be provided"); }); }); @@ -106,7 +107,7 @@ describe("session syncer", () => { it("should grab an initial value from storage if it exists", async () => { storageService.has.mockResolvedValue(true); //Block a call to update - const updateSpy = jest.spyOn(sut as any, "update").mockImplementation(); + const updateSpy = jest.spyOn(sut as any, "updateFromMemory").mockImplementation(); sut.init(); await awaitAsync(); @@ -128,20 +129,15 @@ describe("session syncer", () => { describe("a value is emitted on the observable", () => { let sendMessageSpy: jest.SpyInstance; + const value = "test"; + const serializedValue = JSON.stringify(value); beforeEach(() => { sendMessageSpy = jest.spyOn(BrowserApi, "sendMessage"); sut.init(); - behaviorSubject.next("test"); - }); - - it("should update the session memory", async () => { - // await finishing of fire-and-forget operation - await new Promise((resolve) => setTimeout(resolve, 100)); - expect(storageService.save).toHaveBeenCalledTimes(1); - expect(storageService.save).toHaveBeenCalledWith(sessionKey, "test"); + behaviorSubject.next(value); }); it("should update sessionSyncers in other contexts", async () => { @@ -149,7 +145,10 @@ describe("session syncer", () => { await new Promise((resolve) => setTimeout(resolve, 100)); expect(sendMessageSpy).toHaveBeenCalledTimes(1); - expect(sendMessageSpy).toHaveBeenCalledWith(`${sessionKey}_update`, { id: sut.id }); + expect(sendMessageSpy).toHaveBeenCalledWith(`${sessionKey}_update`, { + id: sut.id, + serializedValue, + }); }); }); @@ -185,26 +184,104 @@ describe("session syncer", () => { it("should update from message on emit from another instance", async () => { const builder = jest.fn(); jest.spyOn(SyncedItemMetadata, "builder").mockReturnValue(builder); - storageService.getBypassCache.mockResolvedValue("test"); + const value = "test"; + const serializedValue = JSON.stringify(value); + builder.mockReturnValue(value); // Expect no circular messaging await awaitAsync(); expect(sendMessageSpy).toHaveBeenCalledTimes(0); - await sut.updateFromMessage({ command: `${sessionKey}_update`, id: "different_id" }); + await sut.updateFromMessage({ + command: `${sessionKey}_update`, + id: "different_id", + serializedValue, + }); await awaitAsync(); - expect(storageService.getBypassCache).toHaveBeenCalledTimes(1); - expect(storageService.getBypassCache).toHaveBeenCalledWith(sessionKey, { - deserializer: builder, - }); + expect(storageService.getBypassCache).toHaveBeenCalledTimes(0); expect(nextSpy).toHaveBeenCalledTimes(1); - expect(nextSpy).toHaveBeenCalledWith("test"); - expect(behaviorSubject.value).toBe("test"); + expect(nextSpy).toHaveBeenCalledWith(value); + expect(behaviorSubject.value).toBe(value); // Expect no circular messaging expect(sendMessageSpy).toHaveBeenCalledTimes(0); }); }); + + describe("memory storage", () => { + const value = "test"; + const serializedValue = JSON.stringify(value); + let saveSpy: jest.SpyInstance; + const builder = jest.fn().mockReturnValue(value); + const manifestVersionSpy = jest.spyOn(BrowserApi, "manifestVersion", "get"); + const isBackgroundPageSpy = jest.spyOn(BrowserApi, "isBackgroundPage"); + + beforeEach(async () => { + jest.spyOn(SyncedItemMetadata, "builder").mockReturnValue(builder); + saveSpy = jest.spyOn(storageService, "save"); + + sut.init(); + await awaitAsync(); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + it("should always store on observed next for manifest version 3", async () => { + manifestVersionSpy.mockReturnValue(3); + isBackgroundPageSpy.mockReturnValueOnce(true).mockReturnValueOnce(false); + behaviorSubject.next(value); + await awaitAsync(); + behaviorSubject.next(value); + await awaitAsync(); + + expect(saveSpy).toHaveBeenCalledTimes(2); + }); + + it("should not store on message receive for manifest version 3", async () => { + manifestVersionSpy.mockReturnValue(3); + isBackgroundPageSpy.mockReturnValueOnce(true).mockReturnValueOnce(false); + await sut.updateFromMessage({ + command: `${sessionKey}_update`, + id: "different_id", + serializedValue, + }); + await awaitAsync(); + + expect(saveSpy).toHaveBeenCalledTimes(0); + }); + + it("should store on message receive for manifest version 2 for background page only", async () => { + manifestVersionSpy.mockReturnValue(2); + isBackgroundPageSpy.mockReturnValueOnce(true).mockReturnValueOnce(false); + await sut.updateFromMessage({ + command: `${sessionKey}_update`, + id: "different_id", + serializedValue, + }); + await awaitAsync(); + await sut.updateFromMessage({ + command: `${sessionKey}_update`, + id: "different_id", + serializedValue, + }); + await awaitAsync(); + + expect(saveSpy).toHaveBeenCalledTimes(1); + }); + + it("should store on observed next for manifest version 2 for background page only", async () => { + manifestVersionSpy.mockReturnValue(2); + isBackgroundPageSpy.mockReturnValueOnce(true).mockReturnValueOnce(false); + behaviorSubject.next(value); + await awaitAsync(); + behaviorSubject.next(value); + await awaitAsync(); + + expect(saveSpy).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/apps/browser/src/decorators/session-sync-observable/session-syncer.ts b/apps/browser/src/decorators/session-sync-observable/session-syncer.ts index 536e8a4871..8751a12d1d 100644 --- a/apps/browser/src/decorators/session-sync-observable/session-syncer.ts +++ b/apps/browser/src/decorators/session-sync-observable/session-syncer.ts @@ -23,8 +23,8 @@ export class SessionSyncer { throw new Error("subject must inherit from Subject"); } - if (metaData.ctor == null && metaData.initializer == null) { - throw new Error("ctor or initializer must be provided"); + if (metaData.initializer == null) { + throw new Error("initializer must be provided"); } } @@ -45,7 +45,7 @@ export class SessionSyncer { // must be synchronous const hasInSessionMemory = await this.memoryStorageService.has(this.metaData.sessionKey); if (hasInSessionMemory) { - await this.update(); + await this.updateFromMemory(); } this.listenForUpdates(); @@ -83,21 +83,31 @@ export class SessionSyncer { if (message.command != this.updateMessageCommand || message.id === this.id) { return; } - this.update(); + await this.update(message.serializedValue); } - async update() { + async updateFromMemory() { + const value = await this.memoryStorageService.getBypassCache(this.metaData.sessionKey); + await this.update(value); + } + + async update(serializedValue: any) { + const unBuiltValue = JSON.parse(serializedValue); + if (BrowserApi.manifestVersion !== 3 && BrowserApi.isBackgroundPage(self)) { + await this.memoryStorageService.save(this.metaData.sessionKey, serializedValue); + } const builder = SyncedItemMetadata.builder(this.metaData); - const value = await this.memoryStorageService.getBypassCache(this.metaData.sessionKey, { - deserializer: builder, - }); + const value = builder(unBuiltValue); this.ignoreNUpdates = 1; this.subject.next(value); } private async updateSession(value: any) { - await this.memoryStorageService.save(this.metaData.sessionKey, value); - await BrowserApi.sendMessage(this.updateMessageCommand, { id: this.id }); + const serializedValue = JSON.stringify(value); + if (BrowserApi.manifestVersion === 3 || BrowserApi.isBackgroundPage(self)) { + await this.memoryStorageService.save(this.metaData.sessionKey, serializedValue); + } + await BrowserApi.sendMessage(this.updateMessageCommand, { id: this.id, serializedValue }); } private get updateMessageCommand() { diff --git a/apps/browser/src/decorators/session-sync-observable/sync-item-metadata.ts b/apps/browser/src/decorators/session-sync-observable/sync-item-metadata.ts index facfda32fd..fe2b393923 100644 --- a/apps/browser/src/decorators/session-sync-observable/sync-item-metadata.ts +++ b/apps/browser/src/decorators/session-sync-observable/sync-item-metadata.ts @@ -3,15 +3,11 @@ export type InitializeOptions = "array" | "record" | "object"; export class SyncedItemMetadata { propertyKey: string; sessionKey: string; - ctor?: new () => any; - initializer?: (keyValuePair: any) => any; + initializer: (keyValuePair: any) => any; initializeAs: InitializeOptions; static builder(metadata: SyncedItemMetadata): (o: any) => any { - const itemBuilder = - metadata.initializer != null - ? metadata.initializer - : (o: any) => Object.assign(new metadata.ctor(), o); + const itemBuilder = metadata.initializer; if (metadata.initializeAs === "array") { return (keyValuePair: any) => keyValuePair.map((o: any) => itemBuilder(o)); } else if (metadata.initializeAs === "record") { diff --git a/apps/browser/src/decorators/session-sync-observable/synced-item-metadata.spec.ts b/apps/browser/src/decorators/session-sync-observable/synced-item-metadata.spec.ts index 12b0b57d52..61eb63eaac 100644 --- a/apps/browser/src/decorators/session-sync-observable/synced-item-metadata.spec.ts +++ b/apps/browser/src/decorators/session-sync-observable/synced-item-metadata.spec.ts @@ -4,10 +4,8 @@ describe("builder", () => { const propertyKey = "propertyKey"; const key = "key"; const initializer = (s: any) => "used initializer"; - class TestClass {} - const ctor = TestClass; - it("should use initializer if provided", () => { + it("should use initializer", () => { const metadata: SyncedItemMetadata = { propertyKey, sessionKey: key, @@ -18,29 +16,6 @@ describe("builder", () => { expect(builder({})).toBe("used initializer"); }); - it("should use ctor if initializer is not provided", () => { - const metadata: SyncedItemMetadata = { - propertyKey, - sessionKey: key, - ctor, - initializeAs: "object", - }; - const builder = SyncedItemMetadata.builder(metadata); - expect(builder({})).toBeInstanceOf(TestClass); - }); - - it("should prefer initializer over ctor", () => { - const metadata: SyncedItemMetadata = { - propertyKey, - sessionKey: key, - ctor, - initializer, - initializeAs: "object", - }; - const builder = SyncedItemMetadata.builder(metadata); - expect(builder({})).toBe("used initializer"); - }); - it("should honor initialize as array", () => { const metadata: SyncedItemMetadata = { propertyKey, diff --git a/apps/browser/src/services/browser-i18n.service.ts b/apps/browser/src/services/browser-i18n.service.ts index 1af1e54502..3d2f244ab5 100644 --- a/apps/browser/src/services/browser-i18n.service.ts +++ b/apps/browser/src/services/browser-i18n.service.ts @@ -8,7 +8,7 @@ import I18nService from "./i18n.service"; @browserSession export class BrowserI18nService extends I18nService { - @sessionSync({ ctor: String }) + @sessionSync({ initializer: (s: string) => s }) protected _locale: ReplaySubject; constructor(systemLanguage: string, private stateService: StateService) { diff --git a/apps/browser/src/services/browser-policy.service.ts b/apps/browser/src/services/browser-policy.service.ts index 613e5c39cf..19bd14dbbe 100644 --- a/apps/browser/src/services/browser-policy.service.ts +++ b/apps/browser/src/services/browser-policy.service.ts @@ -1,4 +1,5 @@ import { BehaviorSubject } from "rxjs"; +import { Jsonify } from "type-fest"; import { Policy } from "@bitwarden/common/models/domain/policy"; import { PolicyService } from "@bitwarden/common/services/policy/policy.service"; @@ -7,6 +8,9 @@ import { browserSession, sessionSync } from "../decorators/session-sync-observab @browserSession export class BrowserPolicyService extends PolicyService { - @sessionSync({ ctor: Policy, initializeAs: "array" }) + @sessionSync({ + initializer: (obj: Jsonify) => Object.assign(new Policy(), obj), + initializeAs: "array", + }) protected _policies: BehaviorSubject; } diff --git a/apps/browser/src/services/browser-state.service.ts b/apps/browser/src/services/browser-state.service.ts index 5de24a2a4b..3efd6e2edc 100644 --- a/apps/browser/src/services/browser-state.service.ts +++ b/apps/browser/src/services/browser-state.service.ts @@ -22,9 +22,9 @@ export class BrowserStateService initializeAs: "record", }) protected accountsSubject: BehaviorSubject<{ [userId: string]: Account }>; - @sessionSync({ ctor: String }) + @sessionSync({ initializer: (s: string) => s }) protected activeAccountSubject: BehaviorSubject; - @sessionSync({ ctor: Boolean }) + @sessionSync({ initializer: (b: boolean) => b }) protected activeAccountUnlockedSubject: BehaviorSubject; @sessionSync({ initializer: Account.fromJSON as any, // TODO: Remove this any when all any types are removed from Account