[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
This commit is contained in:
Matt Gibson 2022-12-06 16:26:42 -06:00 committed by GitHub
parent 200dd0a1fb
commit 750ff736cd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 38 additions and 19 deletions

View File

@ -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 { mock, MockProxy } from "jest-mock-extended";
import { BehaviorSubject, ReplaySubject } from "rxjs"; import { BehaviorSubject, ReplaySubject } from "rxjs";
@ -30,6 +30,7 @@ describe("session syncer", () => {
}); });
stateService = mock<BrowserStateService>(); stateService = mock<BrowserStateService>();
stateService.hasInSessionMemory.mockResolvedValue(false);
sut = new SessionSyncer(behaviorSubject, stateService, metaData); sut = new SessionSyncer(behaviorSubject, stateService, metaData);
}); });
@ -101,24 +102,26 @@ describe("session syncer", () => {
expect(sut["ignoreNUpdates"]).toBe(1); 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); stateService.hasInSessionMemory.mockResolvedValue(true);
//Block a call to update //Block a call to update
const updateSpy = jest.spyOn(sut as any, "update").mockImplementation(); const updateSpy = jest.spyOn(sut as any, "update").mockImplementation();
sut.init(); sut.init();
await awaitAsync();
expect(updateSpy).toHaveBeenCalledWith(); 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); stateService.hasInSessionMemory.mockResolvedValue(false);
//Block a call to update //Block a call to update
const updateSpy = jest.spyOn(sut as any, "update").mockImplementation(); const updateSpy = jest.spyOn(sut as any, "update").mockImplementation();
sut.init(); sut.init();
await awaitAsync();
expect(updateSpy).toHaveBeenCalledWith(); expect(updateSpy).not.toHaveBeenCalled();
}); });
}); });

View File

@ -42,9 +42,12 @@ export class SessionSyncer {
} }
this.observe(); this.observe();
if (this.stateService.hasInSessionMemory(this.metaData.sessionKey)) { // must be synchronous
this.update(); this.stateService.hasInSessionMemory(this.metaData.sessionKey).then((hasInSessionMemory) => {
} if (hasInSessionMemory) {
this.update();
}
});
this.listenForUpdates(); this.listenForUpdates();
} }

View File

@ -28,6 +28,11 @@ export class BrowserStateService
protected activeAccountSubject: BehaviorSubject<string>; protected activeAccountSubject: BehaviorSubject<string>;
@sessionSync({ ctor: Boolean }) @sessionSync({ ctor: Boolean })
protected activeAccountUnlockedSubject: BehaviorSubject<boolean>; protected activeAccountUnlockedSubject: BehaviorSubject<boolean>;
@sessionSync({
initializer: Account.fromJSON as any, // TODO: Remove this any when all any types are removed from Account
initializeAs: "record",
})
protected accountDiskCache: BehaviorSubject<Record<string, Account>>;
protected accountDeserializer = Account.fromJSON; protected accountDeserializer = Account.fromJSON;

View File

@ -18,7 +18,7 @@ export class MemoryStorageService
} }
async has(key: string): Promise<boolean> { async has(key: string): Promise<boolean> {
return this.get(key) != null; return (await this.get(key)) != null;
} }
save(key: string, obj: any): Promise<any> { save(key: string, obj: any): Promise<any> {

View File

@ -79,7 +79,7 @@ export class StateService<
private hasBeenInited = false; private hasBeenInited = false;
private isRecoveredSession = false; private isRecoveredSession = false;
private accountDiskCache = new Map<string, TAccount>(); protected accountDiskCache = new BehaviorSubject<Record<string, TAccount>>({});
// default account serializer, must be overridden by child class // default account serializer, must be overridden by child class
protected accountDeserializer = Account.fromJSON as (json: Jsonify<TAccount>) => TAccount; protected accountDeserializer = Account.fromJSON as (json: Jsonify<TAccount>) => TAccount;
@ -2383,7 +2383,7 @@ export class StateService<
} }
if (this.useAccountCache) { if (this.useAccountCache) {
const cachedAccount = this.accountDiskCache.get(options.userId); const cachedAccount = this.accountDiskCache.value[options.userId];
if (cachedAccount != null) { if (cachedAccount != null) {
return cachedAccount; return cachedAccount;
} }
@ -2397,9 +2397,7 @@ export class StateService<
)) ))
: await this.storageService.get<TAccount>(options.userId, options); : await this.storageService.get<TAccount>(options.userId, options);
if (this.useAccountCache) { this.setDiskCache(options.userId, account);
this.accountDiskCache.set(options.userId, account);
}
return account; return account;
} }
@ -2430,9 +2428,7 @@ export class StateService<
await storageLocation.save(`${options.userId}`, account, options); await storageLocation.save(`${options.userId}`, account, options);
if (this.useAccountCache) { this.deleteDiskCache(options.userId);
this.accountDiskCache.delete(options.userId);
}
} }
protected async saveAccountToMemory(account: TAccount): Promise<void> { protected async saveAccountToMemory(account: TAccount): Promise<void> {
@ -2643,9 +2639,7 @@ export class StateService<
userId = userId ?? state.activeUserId; userId = userId ?? state.activeUserId;
delete state.accounts[userId]; delete state.accounts[userId];
if (this.useAccountCache) { this.deleteDiskCache(userId);
this.accountDiskCache.delete(userId);
}
return state; return state;
}); });
@ -2770,6 +2764,20 @@ export class StateService<
await this.setState(updatedState); 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<T>( function withPrototypeForArrayMembers<T>(