From f64092cc90eb9ab9ecabbf4a83769412429f7c9c Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Tue, 6 Feb 2024 12:01:12 -0500 Subject: [PATCH] [PM-6032] Run migrations in main process (#7795) * Run Migrations in Desktop Main Process * Add `waitForMigrations` method * Add `InitOptions` * Fix Destructuring --- apps/desktop/src/app/services/init.service.ts | 2 +- apps/desktop/src/main.ts | 7 ++- .../platform/abstractions/state.service.ts | 16 ++++++- .../src/platform/services/state.service.ts | 18 ++++++-- libs/common/src/state-migrations/migrate.ts | 43 +++++++++++++++++++ 5 files changed, 79 insertions(+), 7 deletions(-) diff --git a/apps/desktop/src/app/services/init.service.ts b/apps/desktop/src/app/services/init.service.ts index 27fb212821..9bbf0b2e37 100644 --- a/apps/desktop/src/app/services/init.service.ts +++ b/apps/desktop/src/app/services/init.service.ts @@ -43,7 +43,7 @@ export class InitService { init() { return async () => { this.nativeMessagingService.init(); - await this.stateService.init(); + await this.stateService.init({ runMigrations: false }); // Desktop will run them in main process await this.environmentService.setUrlsFromStorage(); // Workaround to ignore stateService.activeAccount until URLs are set // TODO: Remove this when implementing ticket PM-2637 diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index 8c5304c987..cfe6116b85 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -16,7 +16,8 @@ import { DefaultGlobalStateProvider } from "@bitwarden/common/platform/state/imp import { DefaultSingleUserStateProvider } from "@bitwarden/common/platform/state/implementations/default-single-user-state.provider"; import { DefaultStateProvider } from "@bitwarden/common/platform/state/implementations/default-state.provider"; import { MemoryStorageService as MemoryStorageServiceForStateProviders } from "@bitwarden/common/platform/state/storage/memory-storage.service"; -/*/ eslint-enable import/no-restricted-paths */ +/* eslint-enable import/no-restricted-paths */ +import { migrate } from "@bitwarden/common/state-migrations"; import { MenuMain } from "./main/menu/menu.main"; import { MessagingMain } from "./main/messaging.main"; @@ -191,8 +192,10 @@ export class Main { bootstrap() { this.desktopCredentialStorageListener.init(); - this.windowMain.init().then( + // Run migrations first, then other things + migrate(this.storageService, this.logService).then( async () => { + await this.windowMain.init(); const locale = await this.stateService.getLocale(); await this.i18nService.init(locale != null ? locale : app.getLocale()); this.messagingMain.init(); diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index 63f0bdf37a..618ab5f007 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -36,6 +36,20 @@ import { EncString } from "../models/domain/enc-string"; import { StorageOptions } from "../models/domain/storage-options"; import { SymmetricCryptoKey } from "../models/domain/symmetric-crypto-key"; +/** + * Options for customizing the initiation behavior. + */ +export type InitOptions = { + /** + * Whether or not to run state migrations as part of the init process. Defaults to true. + * + * If false, the init method will instead wait for migrations to complete before doing its + * other init operations. Make sure migrations have either already completed, or will complete + * before calling {@link StateService.init} with `runMigrations: false`. + */ + runMigrations?: boolean; +}; + export abstract class StateService { accounts$: Observable<{ [userId: string]: T }>; activeAccount$: Observable; @@ -44,7 +58,7 @@ export abstract class StateService { addAccount: (account: T) => Promise; setActiveUser: (userId: string) => Promise; clean: (options?: StorageOptions) => Promise; - init: () => Promise; + init: (initOptions?: InitOptions) => Promise; getAccessToken: (options?: StorageOptions) => Promise; setAccessToken: (value: string, options?: StorageOptions) => Promise; diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index 34ac0aabe6..c50ee4ef88 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -16,6 +16,7 @@ import { VaultTimeoutAction } from "../../enums/vault-timeout-action.enum"; import { EventData } from "../../models/data/event.data"; import { WindowState } from "../../models/domain/window-state"; import { migrate } from "../../state-migrations"; +import { waitForMigrations } from "../../state-migrations/migrate"; import { GeneratorOptions } from "../../tools/generator/generator-options"; import { GeneratedPasswordHistory, PasswordGeneratorOptions } from "../../tools/generator/password"; import { UsernameGeneratorOptions } from "../../tools/generator/username"; @@ -33,7 +34,10 @@ import { CollectionView } from "../../vault/models/view/collection.view"; import { AddEditCipherInfo } from "../../vault/types/add-edit-cipher-info"; import { EnvironmentService } from "../abstractions/environment.service"; import { LogService } from "../abstractions/log.service"; -import { StateService as StateServiceAbstraction } from "../abstractions/state.service"; +import { + InitOptions, + StateService as StateServiceAbstraction, +} from "../abstractions/state.service"; import { AbstractMemoryStorageService, AbstractStorageService, @@ -126,12 +130,20 @@ export class StateService< .subscribe(); } - async init(): Promise { + async init(initOptions: InitOptions = {}): Promise { + // Deconstruct and apply defaults + const { runMigrations = true } = initOptions; if (this.hasBeenInited) { return; } - await migrate(this.storageService, this.logService); + if (runMigrations) { + await migrate(this.storageService, this.logService); + } else { + // It may have been requested to not run the migrations but we should defensively not + // continue this method until migrations have a chance to be completed elsewhere. + await waitForMigrations(this.storageService, this.logService); + } await this.state().then(async (state) => { if (state == null) { diff --git a/libs/common/src/state-migrations/migrate.ts b/libs/common/src/state-migrations/migrate.ts index 170317a665..ffea50afe2 100644 --- a/libs/common/src/state-migrations/migrate.ts +++ b/libs/common/src/state-migrations/migrate.ts @@ -71,3 +71,46 @@ export async function currentVersion( logService.info(`State version: ${state}`); return state; } + +/** + * Waits for migrations to have a chance to run and will resolve the promise once they are. + * + * @param storageService Disk storage where the `stateVersion` will or is already saved in. + * @param logService Log service + */ +export async function waitForMigrations( + storageService: AbstractStorageService, + logService: LogService, +) { + const isReady = async () => { + const version = await currentVersion(storageService, logService); + // The saved version is what we consider the latest + // migrations should be complete + return version === CURRENT_VERSION; + }; + + const wait = async (time: number) => { + // Wait exponentially + const nextTime = time * 2; + if (nextTime > 8192) { + // Don't wait longer than ~8 seconds in a single wait, + // if the migrations still haven't happened. They aren't + // likely to. + return; + } + return new Promise((resolve) => { + setTimeout(async () => { + if (!(await isReady())) { + logService.info(`Waiting for migrations to finish, waiting for ${nextTime}ms`); + await wait(nextTime); + } + resolve(); + }, time); + }); + }; + + if (!(await isReady())) { + // Wait for 2ms to start with + await wait(2); + } +}