mirror of
https://github.com/bitwarden/browser.git
synced 2025-01-22 21:21:35 +01:00
Remove StateService
useAccountCache
(#8882)
* Remove Account Cache from StateService * Remove Extra Change * Fix Desktop Build
This commit is contained in:
parent
b7957d6e28
commit
94fe9bd053
@ -30,7 +30,6 @@ import {
|
|||||||
|
|
||||||
type StateServiceFactoryOptions = FactoryOptions & {
|
type StateServiceFactoryOptions = FactoryOptions & {
|
||||||
stateServiceOptions: {
|
stateServiceOptions: {
|
||||||
useAccountCache?: boolean;
|
|
||||||
stateFactory: StateFactory<GlobalState, Account>;
|
stateFactory: StateFactory<GlobalState, Account>;
|
||||||
};
|
};
|
||||||
};
|
};
|
||||||
@ -64,7 +63,6 @@ export async function stateServiceFactory(
|
|||||||
await environmentServiceFactory(cache, opts),
|
await environmentServiceFactory(cache, opts),
|
||||||
await tokenServiceFactory(cache, opts),
|
await tokenServiceFactory(cache, opts),
|
||||||
await migrationRunnerFactory(cache, opts),
|
await migrationRunnerFactory(cache, opts),
|
||||||
opts.stateServiceOptions.useAccountCache,
|
|
||||||
),
|
),
|
||||||
);
|
);
|
||||||
// TODO: If we run migration through a chrome installed/updated event we can turn off running migrations
|
// TODO: If we run migration through a chrome installed/updated event we can turn off running migrations
|
||||||
|
@ -27,7 +27,6 @@ describe("Browser State Service", () => {
|
|||||||
let diskStorageService: MockProxy<AbstractStorageService>;
|
let diskStorageService: MockProxy<AbstractStorageService>;
|
||||||
let logService: MockProxy<LogService>;
|
let logService: MockProxy<LogService>;
|
||||||
let stateFactory: MockProxy<StateFactory<GlobalState, Account>>;
|
let stateFactory: MockProxy<StateFactory<GlobalState, Account>>;
|
||||||
let useAccountCache: boolean;
|
|
||||||
let environmentService: MockProxy<EnvironmentService>;
|
let environmentService: MockProxy<EnvironmentService>;
|
||||||
let tokenService: MockProxy<TokenService>;
|
let tokenService: MockProxy<TokenService>;
|
||||||
let migrationRunner: MockProxy<MigrationRunner>;
|
let migrationRunner: MockProxy<MigrationRunner>;
|
||||||
@ -46,8 +45,6 @@ describe("Browser State Service", () => {
|
|||||||
environmentService = mock();
|
environmentService = mock();
|
||||||
tokenService = mock();
|
tokenService = mock();
|
||||||
migrationRunner = mock();
|
migrationRunner = mock();
|
||||||
// turn off account cache for tests
|
|
||||||
useAccountCache = false;
|
|
||||||
|
|
||||||
state = new State(new GlobalState());
|
state = new State(new GlobalState());
|
||||||
state.accounts[userId] = new Account({
|
state.accounts[userId] = new Account({
|
||||||
@ -78,7 +75,6 @@ describe("Browser State Service", () => {
|
|||||||
environmentService,
|
environmentService,
|
||||||
tokenService,
|
tokenService,
|
||||||
migrationRunner,
|
migrationRunner,
|
||||||
useAccountCache,
|
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
@ -15,7 +15,6 @@ import { MigrationRunner } from "@bitwarden/common/platform/services/migration-r
|
|||||||
import { StateService as BaseStateService } from "@bitwarden/common/platform/services/state.service";
|
import { StateService as BaseStateService } from "@bitwarden/common/platform/services/state.service";
|
||||||
|
|
||||||
import { Account } from "../../models/account";
|
import { Account } from "../../models/account";
|
||||||
import { BrowserApi } from "../browser/browser-api";
|
|
||||||
import { browserSession, sessionSync } from "../decorators/session-sync-observable";
|
import { browserSession, sessionSync } from "../decorators/session-sync-observable";
|
||||||
|
|
||||||
import { BrowserStateService } from "./abstractions/browser-state.service";
|
import { BrowserStateService } from "./abstractions/browser-state.service";
|
||||||
@ -45,7 +44,6 @@ export class DefaultBrowserStateService
|
|||||||
environmentService: EnvironmentService,
|
environmentService: EnvironmentService,
|
||||||
tokenService: TokenService,
|
tokenService: TokenService,
|
||||||
migrationRunner: MigrationRunner,
|
migrationRunner: MigrationRunner,
|
||||||
useAccountCache = true,
|
|
||||||
) {
|
) {
|
||||||
super(
|
super(
|
||||||
storageService,
|
storageService,
|
||||||
@ -57,45 +55,7 @@ export class DefaultBrowserStateService
|
|||||||
environmentService,
|
environmentService,
|
||||||
tokenService,
|
tokenService,
|
||||||
migrationRunner,
|
migrationRunner,
|
||||||
useAccountCache,
|
|
||||||
);
|
);
|
||||||
|
|
||||||
// TODO: This is a hack to fix having a disk cache on both the popup and
|
|
||||||
// the background page that can get out of sync. We need to work out the
|
|
||||||
// best way to handle caching with multiple instances of the state service.
|
|
||||||
if (useAccountCache) {
|
|
||||||
BrowserApi.storageChangeListener((changes, namespace) => {
|
|
||||||
if (namespace === "local") {
|
|
||||||
for (const key of Object.keys(changes)) {
|
|
||||||
if (key !== "accountActivity" && this.accountDiskCache.value[key]) {
|
|
||||||
this.deleteDiskCache(key);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
});
|
|
||||||
|
|
||||||
BrowserApi.addListener(
|
|
||||||
chrome.runtime.onMessage,
|
|
||||||
(message: { command: string }, _, respond) => {
|
|
||||||
if (message.command === "initializeDiskCache") {
|
|
||||||
respond(JSON.stringify(this.accountDiskCache.value));
|
|
||||||
}
|
|
||||||
},
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
override async initAccountState(): Promise<void> {
|
|
||||||
if (this.isRecoveredSession && this.useAccountCache) {
|
|
||||||
// request cache initialization
|
|
||||||
|
|
||||||
const response = await BrowserApi.sendMessageWithResponse<string>("initializeDiskCache");
|
|
||||||
this.accountDiskCache.next(JSON.parse(response));
|
|
||||||
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
await super.initAccountState();
|
|
||||||
}
|
}
|
||||||
|
|
||||||
async addAccount(account: Account) {
|
async addAccount(account: Account) {
|
||||||
|
@ -4,7 +4,6 @@ import { Subject, merge } from "rxjs";
|
|||||||
import { SafeProvider, safeProvider } from "@bitwarden/angular/platform/utils/safe-provider";
|
import { SafeProvider, safeProvider } from "@bitwarden/angular/platform/utils/safe-provider";
|
||||||
import {
|
import {
|
||||||
SECURE_STORAGE,
|
SECURE_STORAGE,
|
||||||
STATE_SERVICE_USE_CACHE,
|
|
||||||
LOCALES_DIRECTORY,
|
LOCALES_DIRECTORY,
|
||||||
SYSTEM_LANGUAGE,
|
SYSTEM_LANGUAGE,
|
||||||
MEMORY_STORAGE,
|
MEMORY_STORAGE,
|
||||||
@ -205,7 +204,6 @@ const safeProviders: SafeProvider[] = [
|
|||||||
EnvironmentService,
|
EnvironmentService,
|
||||||
TokenService,
|
TokenService,
|
||||||
MigrationRunner,
|
MigrationRunner,
|
||||||
STATE_SERVICE_USE_CACHE,
|
|
||||||
],
|
],
|
||||||
}),
|
}),
|
||||||
safeProvider({
|
safeProvider({
|
||||||
|
@ -205,7 +205,6 @@ export class Main {
|
|||||||
this.environmentService,
|
this.environmentService,
|
||||||
this.tokenService,
|
this.tokenService,
|
||||||
this.migrationRunner,
|
this.migrationRunner,
|
||||||
false, // Do not use disk caching because this will get out of sync with the renderer service
|
|
||||||
);
|
);
|
||||||
|
|
||||||
this.desktopSettingsService = new DesktopSettingsService(stateProvider);
|
this.desktopSettingsService = new DesktopSettingsService(stateProvider);
|
||||||
|
@ -5,7 +5,6 @@ import { SafeProvider, safeProvider } from "@bitwarden/angular/platform/utils/sa
|
|||||||
import {
|
import {
|
||||||
SECURE_STORAGE,
|
SECURE_STORAGE,
|
||||||
STATE_FACTORY,
|
STATE_FACTORY,
|
||||||
STATE_SERVICE_USE_CACHE,
|
|
||||||
LOCALES_DIRECTORY,
|
LOCALES_DIRECTORY,
|
||||||
SYSTEM_LANGUAGE,
|
SYSTEM_LANGUAGE,
|
||||||
MEMORY_STORAGE,
|
MEMORY_STORAGE,
|
||||||
@ -78,10 +77,6 @@ const safeProviders: SafeProvider[] = [
|
|||||||
provide: STATE_FACTORY,
|
provide: STATE_FACTORY,
|
||||||
useValue: new StateFactory(GlobalState, Account),
|
useValue: new StateFactory(GlobalState, Account),
|
||||||
}),
|
}),
|
||||||
safeProvider({
|
|
||||||
provide: STATE_SERVICE_USE_CACHE,
|
|
||||||
useValue: false,
|
|
||||||
}),
|
|
||||||
safeProvider({
|
safeProvider({
|
||||||
provide: I18nServiceAbstraction,
|
provide: I18nServiceAbstraction,
|
||||||
useClass: I18nService,
|
useClass: I18nService,
|
||||||
|
@ -4,7 +4,6 @@ import {
|
|||||||
MEMORY_STORAGE,
|
MEMORY_STORAGE,
|
||||||
SECURE_STORAGE,
|
SECURE_STORAGE,
|
||||||
STATE_FACTORY,
|
STATE_FACTORY,
|
||||||
STATE_SERVICE_USE_CACHE,
|
|
||||||
} from "@bitwarden/angular/services/injection-tokens";
|
} from "@bitwarden/angular/services/injection-tokens";
|
||||||
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
|
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
|
||||||
import { TokenService } from "@bitwarden/common/auth/abstractions/token.service";
|
import { TokenService } from "@bitwarden/common/auth/abstractions/token.service";
|
||||||
@ -34,7 +33,6 @@ export class StateService extends BaseStateService<GlobalState, Account> {
|
|||||||
environmentService: EnvironmentService,
|
environmentService: EnvironmentService,
|
||||||
tokenService: TokenService,
|
tokenService: TokenService,
|
||||||
migrationRunner: MigrationRunner,
|
migrationRunner: MigrationRunner,
|
||||||
@Inject(STATE_SERVICE_USE_CACHE) useAccountCache = true,
|
|
||||||
) {
|
) {
|
||||||
super(
|
super(
|
||||||
storageService,
|
storageService,
|
||||||
@ -46,7 +44,6 @@ export class StateService extends BaseStateService<GlobalState, Account> {
|
|||||||
environmentService,
|
environmentService,
|
||||||
tokenService,
|
tokenService,
|
||||||
migrationRunner,
|
migrationRunner,
|
||||||
useAccountCache,
|
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -36,7 +36,6 @@ export const MEMORY_STORAGE = new SafeInjectionToken<AbstractMemoryStorageServic
|
|||||||
);
|
);
|
||||||
export const SECURE_STORAGE = new SafeInjectionToken<AbstractStorageService>("SECURE_STORAGE");
|
export const SECURE_STORAGE = new SafeInjectionToken<AbstractStorageService>("SECURE_STORAGE");
|
||||||
export const STATE_FACTORY = new SafeInjectionToken<StateFactory>("STATE_FACTORY");
|
export const STATE_FACTORY = new SafeInjectionToken<StateFactory>("STATE_FACTORY");
|
||||||
export const STATE_SERVICE_USE_CACHE = new SafeInjectionToken<boolean>("STATE_SERVICE_USE_CACHE");
|
|
||||||
export const LOGOUT_CALLBACK = new SafeInjectionToken<
|
export const LOGOUT_CALLBACK = new SafeInjectionToken<
|
||||||
(expired: boolean, userId?: string) => Promise<void>
|
(expired: boolean, userId?: string) => Promise<void>
|
||||||
>("LOGOUT_CALLBACK");
|
>("LOGOUT_CALLBACK");
|
||||||
|
@ -269,7 +269,6 @@ import {
|
|||||||
SafeInjectionToken,
|
SafeInjectionToken,
|
||||||
SECURE_STORAGE,
|
SECURE_STORAGE,
|
||||||
STATE_FACTORY,
|
STATE_FACTORY,
|
||||||
STATE_SERVICE_USE_CACHE,
|
|
||||||
SUPPORTS_SECURE_STORAGE,
|
SUPPORTS_SECURE_STORAGE,
|
||||||
SYSTEM_LANGUAGE,
|
SYSTEM_LANGUAGE,
|
||||||
SYSTEM_THEME_OBSERVABLE,
|
SYSTEM_THEME_OBSERVABLE,
|
||||||
@ -313,10 +312,6 @@ const safeProviders: SafeProvider[] = [
|
|||||||
provide: STATE_FACTORY,
|
provide: STATE_FACTORY,
|
||||||
useValue: new StateFactory(GlobalState, Account),
|
useValue: new StateFactory(GlobalState, Account),
|
||||||
}),
|
}),
|
||||||
safeProvider({
|
|
||||||
provide: STATE_SERVICE_USE_CACHE,
|
|
||||||
useValue: true,
|
|
||||||
}),
|
|
||||||
safeProvider({
|
safeProvider({
|
||||||
provide: LOGOUT_CALLBACK,
|
provide: LOGOUT_CALLBACK,
|
||||||
useFactory:
|
useFactory:
|
||||||
@ -690,7 +685,6 @@ const safeProviders: SafeProvider[] = [
|
|||||||
EnvironmentService,
|
EnvironmentService,
|
||||||
TokenServiceAbstraction,
|
TokenServiceAbstraction,
|
||||||
MigrationRunner,
|
MigrationRunner,
|
||||||
STATE_SERVICE_USE_CACHE,
|
|
||||||
],
|
],
|
||||||
}),
|
}),
|
||||||
safeProvider({
|
safeProvider({
|
||||||
|
@ -65,8 +65,6 @@ export class StateService<
|
|||||||
private hasBeenInited = false;
|
private hasBeenInited = false;
|
||||||
protected isRecoveredSession = false;
|
protected isRecoveredSession = false;
|
||||||
|
|
||||||
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;
|
||||||
|
|
||||||
@ -80,7 +78,6 @@ export class StateService<
|
|||||||
protected environmentService: EnvironmentService,
|
protected environmentService: EnvironmentService,
|
||||||
protected tokenService: TokenService,
|
protected tokenService: TokenService,
|
||||||
private migrationRunner: MigrationRunner,
|
private migrationRunner: MigrationRunner,
|
||||||
protected useAccountCache: boolean = true,
|
|
||||||
) {}
|
) {}
|
||||||
|
|
||||||
async init(initOptions: InitOptions = {}): Promise<void> {
|
async init(initOptions: InitOptions = {}): Promise<void> {
|
||||||
@ -995,13 +992,6 @@ export class StateService<
|
|||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (this.useAccountCache) {
|
|
||||||
const cachedAccount = this.accountDiskCache.value[options.userId];
|
|
||||||
if (cachedAccount != null) {
|
|
||||||
return cachedAccount;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
const account = options?.useSecureStorage
|
const account = options?.useSecureStorage
|
||||||
? (await this.secureStorageService.get<TAccount>(options.userId, options)) ??
|
? (await this.secureStorageService.get<TAccount>(options.userId, options)) ??
|
||||||
(await this.storageService.get<TAccount>(
|
(await this.storageService.get<TAccount>(
|
||||||
@ -1009,8 +999,6 @@ export class StateService<
|
|||||||
this.reconcileOptions(options, { htmlStorageLocation: HtmlStorageLocation.Local }),
|
this.reconcileOptions(options, { htmlStorageLocation: HtmlStorageLocation.Local }),
|
||||||
))
|
))
|
||||||
: await this.storageService.get<TAccount>(options.userId, options);
|
: await this.storageService.get<TAccount>(options.userId, options);
|
||||||
|
|
||||||
this.setDiskCache(options.userId, account);
|
|
||||||
return account;
|
return account;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -1040,8 +1028,6 @@ export class StateService<
|
|||||||
: this.storageService;
|
: this.storageService;
|
||||||
|
|
||||||
await storageLocation.save(`${options.userId}`, account, options);
|
await storageLocation.save(`${options.userId}`, account, options);
|
||||||
|
|
||||||
this.deleteDiskCache(options.userId);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
protected async saveAccountToMemory(account: TAccount): Promise<void> {
|
protected async saveAccountToMemory(account: TAccount): Promise<void> {
|
||||||
@ -1241,9 +1227,6 @@ export class StateService<
|
|||||||
await this.updateState(async (state) => {
|
await this.updateState(async (state) => {
|
||||||
userId = userId ?? state.activeUserId;
|
userId = userId ?? state.activeUserId;
|
||||||
delete state.accounts[userId];
|
delete state.accounts[userId];
|
||||||
|
|
||||||
this.deleteDiskCache(userId);
|
|
||||||
|
|
||||||
return state;
|
return state;
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
@ -1357,20 +1340,6 @@ export class StateService<
|
|||||||
return await this.setState(updatedState);
|
return 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);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
protected deleteDiskCache(key: string) {
|
|
||||||
if (this.useAccountCache) {
|
|
||||||
delete this.accountDiskCache.value[key];
|
|
||||||
this.accountDiskCache.next(this.accountDiskCache.value);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
function withPrototypeForArrayMembers<T>(
|
function withPrototypeForArrayMembers<T>(
|
||||||
|
Loading…
Reference in New Issue
Block a user