From 750ff736cd80973056824e03acc876934b9af76e Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 6 Dec 2022 16:26:42 -0600 Subject: [PATCH] [ps-2003] Ps/ps 1854 fix pin (#4193) * Await in `has` calls. * Add disk cache to browser synced items Note: `Map` doesn't serialize nicely so it's easier to swap over to a `Record` object for out cache * Mock and await init promises in tests * Remove redundant settings checks --- .../session-syncer.spec.ts | 11 ++++--- .../session-sync-observable/session-syncer.ts | 9 ++++-- .../src/services/browser-state.service.ts | 5 ++++ .../src/services/memoryStorage.service.ts | 2 +- libs/common/src/services/state.service.ts | 30 ++++++++++++------- 5 files changed, 38 insertions(+), 19 deletions(-) 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 00a0da433a5..c37b640f3f9 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,4 +1,4 @@ -import { awaitAsync as flushAsyncObservables } from "@bitwarden/angular/../test-utils"; +import { awaitAsync, awaitAsync as flushAsyncObservables } from "@bitwarden/angular/../test-utils"; import { mock, MockProxy } from "jest-mock-extended"; import { BehaviorSubject, ReplaySubject } from "rxjs"; @@ -30,6 +30,7 @@ describe("session syncer", () => { }); stateService = mock(); + stateService.hasInSessionMemory.mockResolvedValue(false); sut = new SessionSyncer(behaviorSubject, stateService, metaData); }); @@ -101,24 +102,26 @@ describe("session syncer", () => { expect(sut["ignoreNUpdates"]).toBe(1); }); - it("should grab an initial value from storage if it exists", () => { + it("should grab an initial value from storage if it exists", async () => { stateService.hasInSessionMemory.mockResolvedValue(true); //Block a call to update const updateSpy = jest.spyOn(sut as any, "update").mockImplementation(); sut.init(); + await awaitAsync(); expect(updateSpy).toHaveBeenCalledWith(); }); - it("should not grab an initial value from storage if it does not exist", () => { + it("should not grab an initial value from storage if it does not exist", async () => { stateService.hasInSessionMemory.mockResolvedValue(false); //Block a call to update const updateSpy = jest.spyOn(sut as any, "update").mockImplementation(); sut.init(); + await awaitAsync(); - expect(updateSpy).toHaveBeenCalledWith(); + expect(updateSpy).not.toHaveBeenCalled(); }); }); 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 68294b68c3d..91b371ef817 100644 --- a/apps/browser/src/decorators/session-sync-observable/session-syncer.ts +++ b/apps/browser/src/decorators/session-sync-observable/session-syncer.ts @@ -42,9 +42,12 @@ export class SessionSyncer { } this.observe(); - if (this.stateService.hasInSessionMemory(this.metaData.sessionKey)) { - this.update(); - } + // must be synchronous + this.stateService.hasInSessionMemory(this.metaData.sessionKey).then((hasInSessionMemory) => { + if (hasInSessionMemory) { + this.update(); + } + }); this.listenForUpdates(); } diff --git a/apps/browser/src/services/browser-state.service.ts b/apps/browser/src/services/browser-state.service.ts index 24630dcbaef..57873f23073 100644 --- a/apps/browser/src/services/browser-state.service.ts +++ b/apps/browser/src/services/browser-state.service.ts @@ -28,6 +28,11 @@ export class BrowserStateService protected activeAccountSubject: BehaviorSubject; @sessionSync({ ctor: Boolean }) protected activeAccountUnlockedSubject: BehaviorSubject; + @sessionSync({ + initializer: Account.fromJSON as any, // TODO: Remove this any when all any types are removed from Account + initializeAs: "record", + }) + protected accountDiskCache: BehaviorSubject>; protected accountDeserializer = Account.fromJSON; diff --git a/libs/common/src/services/memoryStorage.service.ts b/libs/common/src/services/memoryStorage.service.ts index 0911992c3cb..cfb94193e5c 100644 --- a/libs/common/src/services/memoryStorage.service.ts +++ b/libs/common/src/services/memoryStorage.service.ts @@ -18,7 +18,7 @@ export class MemoryStorageService } async has(key: string): Promise { - return this.get(key) != null; + return (await this.get(key)) != null; } save(key: string, obj: any): Promise { diff --git a/libs/common/src/services/state.service.ts b/libs/common/src/services/state.service.ts index fe787f21c0e..2a2ce53d622 100644 --- a/libs/common/src/services/state.service.ts +++ b/libs/common/src/services/state.service.ts @@ -79,7 +79,7 @@ export class StateService< private hasBeenInited = false; private isRecoveredSession = false; - private accountDiskCache = new Map(); + protected accountDiskCache = new BehaviorSubject>({}); // default account serializer, must be overridden by child class protected accountDeserializer = Account.fromJSON as (json: Jsonify) => TAccount; @@ -2383,7 +2383,7 @@ export class StateService< } if (this.useAccountCache) { - const cachedAccount = this.accountDiskCache.get(options.userId); + const cachedAccount = this.accountDiskCache.value[options.userId]; if (cachedAccount != null) { return cachedAccount; } @@ -2397,9 +2397,7 @@ export class StateService< )) : await this.storageService.get(options.userId, options); - if (this.useAccountCache) { - this.accountDiskCache.set(options.userId, account); - } + this.setDiskCache(options.userId, account); return account; } @@ -2430,9 +2428,7 @@ export class StateService< await storageLocation.save(`${options.userId}`, account, options); - if (this.useAccountCache) { - this.accountDiskCache.delete(options.userId); - } + this.deleteDiskCache(options.userId); } protected async saveAccountToMemory(account: TAccount): Promise { @@ -2643,9 +2639,7 @@ export class StateService< userId = userId ?? state.activeUserId; delete state.accounts[userId]; - if (this.useAccountCache) { - this.accountDiskCache.delete(userId); - } + this.deleteDiskCache(userId); return state; }); @@ -2770,6 +2764,20 @@ export class StateService< await this.setState(updatedState); }); } + + private setDiskCache(key: string, value: TAccount, options?: StorageOptions) { + if (this.useAccountCache) { + this.accountDiskCache.value[key] = value; + this.accountDiskCache.next(this.accountDiskCache.value); + } + } + + private deleteDiskCache(key: string) { + if (this.useAccountCache) { + delete this.accountDiskCache.value[key]; + this.accountDiskCache.next(this.accountDiskCache.value); + } + } } function withPrototypeForArrayMembers(