diff --git a/apps/browser/src/decorators/session-sync-observable/browser-session.decorator.spec.ts b/apps/browser/src/decorators/session-sync-observable/browser-session.decorator.spec.ts index cc8a5618760..92c5dfb0170 100644 --- a/apps/browser/src/decorators/session-sync-observable/browser-session.decorator.spec.ts +++ b/apps/browser/src/decorators/session-sync-observable/browser-session.decorator.spec.ts @@ -1,6 +1,6 @@ import { BehaviorSubject } from "rxjs"; -import { StateService } from "../../services/state.service"; +import { BrowserStateService } from "../../services/browser-state.service"; import { browserSession } from "./browser-session.decorator"; import { SessionStorable } from "./session-storable"; @@ -22,25 +22,25 @@ describe("browserSession decorator", () => { }); it("should create if StateService is a constructor argument", () => { - const stateService = Object.create(StateService.prototype, {}); + const stateService = Object.create(BrowserStateService.prototype, {}); @browserSession class TestClass { - constructor(private stateService: StateService) {} + constructor(private stateService: BrowserStateService) {} } expect(new TestClass(stateService)).toBeDefined(); }); describe("interaction with @sessionSync decorator", () => { - let stateService: StateService; + let stateService: BrowserStateService; @browserSession class TestClass { @sessionSync({ initializer: (s: string) => s }) private behaviorSubject = new BehaviorSubject(""); - constructor(private stateService: StateService) {} + constructor(private stateService: BrowserStateService) {} fromJSON(json: any) { this.behaviorSubject.next(json); @@ -48,7 +48,7 @@ describe("browserSession decorator", () => { } beforeEach(() => { - stateService = Object.create(StateService.prototype, {}) as StateService; + stateService = Object.create(BrowserStateService.prototype, {}) as BrowserStateService; }); it("should create a session syncer", () => { diff --git a/apps/browser/src/decorators/session-sync-observable/browser-session.decorator.ts b/apps/browser/src/decorators/session-sync-observable/browser-session.decorator.ts index 73cdf767357..351343910f1 100644 --- a/apps/browser/src/decorators/session-sync-observable/browser-session.decorator.ts +++ b/apps/browser/src/decorators/session-sync-observable/browser-session.decorator.ts @@ -1,6 +1,6 @@ import { Constructor } from "type-fest"; -import { StateService } from "../../services/state.service"; +import { BrowserStateService } from "../../services/browser-state.service"; import { SessionStorable } from "./session-storable"; import { SessionSyncer } from "./session-syncer"; @@ -22,7 +22,9 @@ export function browserSession>(constructor: TCto super(...args); // Require state service to be injected - const stateService = args.find((arg) => arg instanceof StateService); + const stateService: BrowserStateService = [this as any] + .concat(args) + .find((arg) => typeof arg.setInSessionMemory === "function"); if (!stateService) { throw new Error( `Cannot decorate ${constructor.name} with browserSession, Browser's StateService must be injected` @@ -38,7 +40,7 @@ export function browserSession>(constructor: TCto ); } - buildSyncer(metadata: SyncedItemMetadata, stateService: StateService) { + buildSyncer(metadata: SyncedItemMetadata, stateService: BrowserStateService) { const syncer = new SessionSyncer((this as any)[metadata.propertyKey], stateService, metadata); syncer.init(); return syncer; 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 df0764528f7..243fbcc02c7 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 @@ -2,10 +2,11 @@ import { Jsonify } from "type-fest"; import { SessionStorable } from "./session-storable"; -class BuildOptions { +class BuildOptions> { ctor?: new () => T; - initializer?: (keyValuePair: Jsonify) => T; + initializer?: (keyValuePair: TJson) => T; initializeAsArray? = false; + initializeAsRecord? = false; } /** @@ -47,6 +48,7 @@ export function sessionSync(buildOptions: BuildOptions) { ctor: buildOptions.ctor, initializer: buildOptions.initializer, initializeAsArray: buildOptions.initializeAsArray, + initializeAsRecord: buildOptions.initializeAsRecord, }); }; } 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 5286cece1bb..6d8e5c2c495 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 @@ -1,5 +1,6 @@ +import { awaitAsync as flushAsyncObservables } from "@bitwarden/angular/../test-utils"; import { mock, MockProxy } from "jest-mock-extended"; -import { BehaviorSubject } from "rxjs"; +import { BehaviorSubject, ReplaySubject } from "rxjs"; import { BrowserApi } from "../../browser/browserApi"; import { StateService } from "../../services/abstractions/state.service"; @@ -34,10 +35,10 @@ describe("session syncer", () => { }); describe("constructor", () => { - it("should throw if behaviorSubject is not an instance of BehaviorSubject", () => { + it("should throw if subject is not an instance of Subject", () => { expect(() => { new SessionSyncer({} as any, stateService, null); - }).toThrowError("behaviorSubject must be an instance of BehaviorSubject"); + }).toThrowError("subject must inherit from Subject"); }); it("should create if either ctor or initializer is provided", () => { @@ -59,28 +60,50 @@ describe("session syncer", () => { }); }); - describe("manifest v2 init", () => { - let observeSpy: jest.SpyInstance; - let listenForUpdatesSpy: jest.SpyInstance; - - beforeEach(() => { - observeSpy = jest.spyOn(behaviorSubject, "subscribe").mockReturnThis(); - listenForUpdatesSpy = jest.spyOn(BrowserApi, "messageListener").mockReturnValue(); - jest.spyOn(chrome.runtime, "getManifest").mockReturnValue({ - name: "bitwarden-test", - version: "0.0.0", - manifest_version: 2, - }); + describe("init", () => { + it("should ignore all updates currently in a ReplaySubject's buffer", () => { + const replaySubject = new ReplaySubject(Infinity); + replaySubject.next("1"); + replaySubject.next("2"); + replaySubject.next("3"); + sut = new SessionSyncer(replaySubject, stateService, metaData); + // block observing the subject + jest.spyOn(sut as any, "observe").mockImplementation(); sut.init(); + + expect(sut["ignoreNUpdates"]).toBe(3); }); - it("should not start observing", () => { - expect(observeSpy).not.toHaveBeenCalled(); + it("should ignore BehaviorSubject's initial value", () => { + const behaviorSubject = new BehaviorSubject("initial"); + sut = new SessionSyncer(behaviorSubject, stateService, metaData); + // block observing the subject + jest.spyOn(sut as any, "observe").mockImplementation(); + + sut.init(); + + expect(sut["ignoreNUpdates"]).toBe(1); }); - it("should not start listening", () => { - expect(listenForUpdatesSpy).not.toHaveBeenCalled(); + it("should grab an initial value from storage if it exists", () => { + stateService.hasInSessionMemory.mockResolvedValue(true); + //Block a call to update + const updateSpy = jest.spyOn(sut as any, "update").mockImplementation(); + + sut.init(); + + expect(updateSpy).toHaveBeenCalledWith(); + }); + + it("should not grab an initial value from storage if it does not exist", () => { + stateService.hasInSessionMemory.mockResolvedValue(false); + //Block a call to update + const updateSpy = jest.spyOn(sut as any, "update").mockImplementation(); + + sut.init(); + + expect(updateSpy).toHaveBeenCalledWith(); }); }); @@ -146,6 +169,7 @@ describe("session syncer", () => { stateService.getFromSessionMemory.mockResolvedValue("test"); await sut.updateFromMessage({ command: `${sessionKey}_update`, id: "different_id" }); + await flushAsyncObservables(); expect(stateService.getFromSessionMemory).toHaveBeenCalledTimes(1); expect(stateService.getFromSessionMemory).toHaveBeenCalledWith(sessionKey, builder); 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 2acfed2954f..c379f0047b0 100644 --- a/apps/browser/src/decorators/session-sync-observable/session-syncer.ts +++ b/apps/browser/src/decorators/session-sync-observable/session-syncer.ts @@ -1,4 +1,4 @@ -import { BehaviorSubject, concatMap, Subscription } from "rxjs"; +import { BehaviorSubject, concatMap, ReplaySubject, Subject, Subscription } from "rxjs"; import { Utils } from "@bitwarden/common/misc/utils"; @@ -11,16 +11,16 @@ export class SessionSyncer { subscription: Subscription; id = Utils.newGuid(); - // everyone gets the same initial values - private ignoreNextUpdate = true; + // ignore initial values + private ignoreNUpdates = 0; constructor( - private behaviorSubject: BehaviorSubject, + private subject: Subject, private stateService: StateService, private metaData: SyncedItemMetadata ) { - if (!(behaviorSubject instanceof BehaviorSubject)) { - throw new Error("behaviorSubject must be an instance of BehaviorSubject"); + if (!(subject instanceof Subject)) { + throw new Error("subject must inherit from Subject"); } if (metaData.ctor == null && metaData.initializer == null) { @@ -29,11 +29,23 @@ export class SessionSyncer { } init() { - if (BrowserApi.manifestVersion !== 3) { - return; + switch (this.subject.constructor) { + case ReplaySubject: + // ignore all updates currently in the buffer + this.ignoreNUpdates = (this.subject as any)._buffer.length; + break; + case BehaviorSubject: + this.ignoreNUpdates = 1; + break; + default: + break; } this.observe(); + if (this.stateService.hasInSessionMemory(this.metaData.sessionKey)) { + this.update(); + } + this.listenForUpdates(); } @@ -41,11 +53,11 @@ export class SessionSyncer { // This may be a memory leak. // There is no good time to unsubscribe from this observable. Hopefully Manifest V3 clears memory from temporary // contexts. If so, this is handled by destruction of the context. - this.subscription = this.behaviorSubject + this.subscription = this.subject .pipe( concatMap(async (next) => { - if (this.ignoreNextUpdate) { - this.ignoreNextUpdate = false; + if (this.ignoreNUpdates > 0) { + this.ignoreNUpdates -= 1; return; } await this.updateSession(next); @@ -66,10 +78,14 @@ export class SessionSyncer { if (message.command != this.updateMessageCommand || message.id === this.id) { return; } + this.update(); + } + + async update() { const builder = SyncedItemMetadata.builder(this.metaData); const value = await this.stateService.getFromSessionMemory(this.metaData.sessionKey, builder); - this.ignoreNextUpdate = true; - this.behaviorSubject.next(value); + this.ignoreNUpdates = 1; + this.subject.next(value); } private async updateSession(value: any) { 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 2b3f4715d46..db6b9c59c91 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 @@ -4,14 +4,27 @@ export class SyncedItemMetadata { ctor?: new () => any; initializer?: (keyValuePair: any) => any; initializeAsArray?: boolean; + initializeAsRecord?: boolean; static builder(metadata: SyncedItemMetadata): (o: any) => any { + if (metadata.initializeAsArray && metadata.initializeAsRecord) { + throw new Error("initializeAsArray and initializeAsRecord cannot both be true"); + } + const itemBuilder = metadata.initializer != null ? metadata.initializer : (o: any) => Object.assign(new metadata.ctor(), o); if (metadata.initializeAsArray) { return (keyValuePair: any) => keyValuePair.map((o: any) => itemBuilder(o)); + } else if (metadata.initializeAsRecord) { + return (keyValuePair: any) => { + const record: Record = {}; + for (const key in keyValuePair) { + record[key] = itemBuilder(keyValuePair[key]); + } + return record; + }; } else { return (keyValuePair: any) => itemBuilder(keyValuePair); } 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 5cd869a5b67..58efe7307f5 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 @@ -36,4 +36,16 @@ describe("builder", () => { expect(builder([{}])).toBeInstanceOf(Array); expect(builder([{}])[0]).toBe("used initializer"); }); + + it("should honor initialize as record", () => { + const metadata = { + propertyKey, + sessionKey: key, + initializer: initializer, + initializeAsRecord: true, + }; + const builder = SyncedItemMetadata.builder(metadata); + expect(builder({ key: "" })).toBeInstanceOf(Object); + expect(builder({ key: "" })).toStrictEqual({ key: "used initializer" }); + }); }); diff --git a/apps/browser/src/services/abstractions/state.service.ts b/apps/browser/src/services/abstractions/state.service.ts index 294bc814e61..348732a4063 100644 --- a/apps/browser/src/services/abstractions/state.service.ts +++ b/apps/browser/src/services/abstractions/state.service.ts @@ -9,6 +9,7 @@ import { BrowserGroupingsComponentState } from "../../models/browserGroupingsCom import { BrowserSendComponentState } from "../../models/browserSendComponentState"; export abstract class StateService extends BaseStateServiceAbstraction { + abstract hasInSessionMemory(key: string): Promise; abstract getFromSessionMemory(key: string, deserializer?: (obj: Jsonify) => T): Promise; abstract setInSessionMemory(key: string, value: any): Promise; getBrowserGroupingComponentState: ( diff --git a/libs/angular/test-utils.ts b/libs/angular/test-utils.ts new file mode 100644 index 00000000000..a2422e698fd --- /dev/null +++ b/libs/angular/test-utils.ts @@ -0,0 +1,3 @@ +export async function awaitAsync(ms = 0) { + await new Promise((resolve) => setTimeout(resolve, ms)); +}