From 3caa6cb635878df24c4c06577a9d406e4a699958 Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Mon, 29 Apr 2024 07:28:58 -0400 Subject: [PATCH] [PM-7766] Add `clientType` to MigrationHelper (#8945) * Add `clientType` to MigrationHelper * PM-7766 - Fix migration builder tests to take new clientType into account. Co-authored-by: Justin Baur <19896123+justindbaur@users.noreply.github.com> * PM-7766 - Add client type to migration builder tests. * PM-7766 - Fix migration-helper.spec tests. * PM-7766 - Fix migrator.spec.ts --------- Co-authored-by: Jared Snider --- .../browser/src/background/main.background.ts | 2 + .../migration-runner.factory.ts | 2 + .../src/popup/services/services.module.ts | 6 + apps/cli/src/bw.ts | 1 + .../src/app/services/services.module.ts | 6 + apps/desktop/src/main.ts | 2 + apps/web/src/app/core/core.module.ts | 6 + .../src/app/platform/web-migration-runner.ts | 5 +- libs/angular/src/services/injection-tokens.ts | 2 + .../src/services/jslib-services.module.ts | 3 +- .../migration-builder.service.spec.ts | 52 +++-- .../services/migration-runner.spec.ts | 3 +- .../src/platform/services/migration-runner.ts | 3 + .../migration-builder.spec.ts | 119 +++++----- .../state-migrations/migration-helper.spec.ts | 208 ++++++++++-------- .../src/state-migrations/migration-helper.ts | 3 + .../src/state-migrations/migrator.spec.ts | 82 +++---- 17 files changed, 295 insertions(+), 210 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 67b763e82d..5aec6e01a4 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -71,6 +71,7 @@ import { } from "@bitwarden/common/autofill/services/user-notification-settings.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { DefaultBillingAccountProfileStateService } from "@bitwarden/common/billing/services/account/billing-account-profile-state.service"; +import { ClientType } from "@bitwarden/common/enums"; import { AppIdService as AppIdServiceAbstraction } from "@bitwarden/common/platform/abstractions/app-id.service"; import { ConfigApiServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config-api.service.abstraction"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; @@ -520,6 +521,7 @@ export default class MainBackground { this.storageService, this.logService, new MigrationBuilderService(), + ClientType.Browser, ); this.stateService = new DefaultBrowserStateService( diff --git a/apps/browser/src/platform/background/service-factories/migration-runner.factory.ts b/apps/browser/src/platform/background/service-factories/migration-runner.factory.ts index a49699a615..090531f7cf 100644 --- a/apps/browser/src/platform/background/service-factories/migration-runner.factory.ts +++ b/apps/browser/src/platform/background/service-factories/migration-runner.factory.ts @@ -1,3 +1,4 @@ +import { ClientType } from "@bitwarden/common/enums"; import { MigrationBuilderService } from "@bitwarden/common/platform/services/migration-builder.service"; import { MigrationRunner } from "@bitwarden/common/platform/services/migration-runner"; @@ -27,6 +28,7 @@ export async function migrationRunnerFactory( await diskStorageServiceFactory(cache, opts), await logServiceFactory(cache, opts), new MigrationBuilderService(), + ClientType.Browser, ), ); } diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 5944783232..bec278aeeb 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -13,6 +13,7 @@ import { SYSTEM_THEME_OBSERVABLE, SafeInjectionToken, INTRAPROCESS_MESSAGING_SUBJECT, + CLIENT_TYPE, } from "@bitwarden/angular/services/injection-tokens"; import { JslibServicesModule } from "@bitwarden/angular/services/jslib-services.module"; import { AuthRequestServiceAbstraction } from "@bitwarden/auth/common"; @@ -45,6 +46,7 @@ import { UserNotificationSettingsServiceAbstraction, } from "@bitwarden/common/autofill/services/user-notification-settings.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; +import { ClientType } from "@bitwarden/common/enums"; import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service"; import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service"; import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; @@ -558,6 +560,10 @@ const safeProviders: SafeProvider[] = [ OBSERVABLE_LARGE_OBJECT_MEMORY_STORAGE, ], }), + safeProvider({ + provide: CLIENT_TYPE, + useValue: ClientType.Browser, + }), ]; @NgModule({ diff --git a/apps/cli/src/bw.ts b/apps/cli/src/bw.ts index 85fba27089..b3fb68fe63 100644 --- a/apps/cli/src/bw.ts +++ b/apps/cli/src/bw.ts @@ -344,6 +344,7 @@ export class Main { this.storageService, this.logService, new MigrationBuilderService(), + ClientType.Cli, ); this.stateService = new StateService( diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index b888df8013..1e3a7fdfa5 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -15,6 +15,7 @@ import { SafeInjectionToken, STATE_FACTORY, INTRAPROCESS_MESSAGING_SUBJECT, + CLIENT_TYPE, } from "@bitwarden/angular/services/injection-tokens"; import { JslibServicesModule } from "@bitwarden/angular/services/jslib-services.module"; import { VaultTimeoutSettingsService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout-settings.service"; @@ -25,6 +26,7 @@ import { KdfConfigService as KdfConfigServiceAbstraction } from "@bitwarden/comm import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service"; +import { ClientType } from "@bitwarden/common/enums"; import { CryptoFunctionService as CryptoFunctionServiceAbstraction } from "@bitwarden/common/platform/abstractions/crypto-function.service"; import { CryptoService as CryptoServiceAbstraction } from "@bitwarden/common/platform/abstractions/crypto.service"; import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; @@ -275,6 +277,10 @@ const safeProviders: SafeProvider[] = [ useClass: NativeMessagingManifestService, deps: [], }), + safeProvider({ + provide: CLIENT_TYPE, + useValue: ClientType.Desktop, + }), ]; @NgModule({ diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index 0766af90b6..d11fceeacc 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -6,6 +6,7 @@ import { Subject, firstValueFrom } from "rxjs"; import { TokenService as TokenServiceAbstraction } from "@bitwarden/common/auth/abstractions/token.service"; import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service"; import { TokenService } from "@bitwarden/common/auth/services/token.service"; +import { ClientType } from "@bitwarden/common/enums"; import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; import { KeyGenerationService as KeyGenerationServiceAbstraction } from "@bitwarden/common/platform/abstractions/key-generation.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; @@ -190,6 +191,7 @@ export class Main { this.storageService, this.logService, new MigrationBuilderService(), + ClientType.Desktop, ); // TODO: this state service will have access to on disk storage, but not in memory storage. diff --git a/apps/web/src/app/core/core.module.ts b/apps/web/src/app/core/core.module.ts index 7a95650039..c60280014c 100644 --- a/apps/web/src/app/core/core.module.ts +++ b/apps/web/src/app/core/core.module.ts @@ -13,10 +13,12 @@ import { OBSERVABLE_DISK_LOCAL_STORAGE, WINDOW, SafeInjectionToken, + CLIENT_TYPE, } from "@bitwarden/angular/services/injection-tokens"; import { JslibServicesModule } from "@bitwarden/angular/services/jslib-services.module"; import { ModalService as ModalServiceAbstraction } from "@bitwarden/angular/services/modal.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { ClientType } from "@bitwarden/common/enums"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { FileDownloadService } from "@bitwarden/common/platform/abstractions/file-download/file-download.service"; import { I18nService as I18nServiceAbstraction } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -157,6 +159,10 @@ const safeProviders: SafeProvider[] = [ new DefaultThemeStateService(globalStateProvider, ThemeType.Light), deps: [GlobalStateProvider], }), + safeProvider({ + provide: CLIENT_TYPE, + useValue: ClientType.Web, + }), ]; @NgModule({ diff --git a/apps/web/src/app/platform/web-migration-runner.ts b/apps/web/src/app/platform/web-migration-runner.ts index 4bd1d2d4b5..392eeeae04 100644 --- a/apps/web/src/app/platform/web-migration-runner.ts +++ b/apps/web/src/app/platform/web-migration-runner.ts @@ -1,3 +1,4 @@ +import { ClientType } from "@bitwarden/common/enums"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { AbstractStorageService } from "@bitwarden/common/platform/abstractions/storage.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; @@ -14,7 +15,7 @@ export class WebMigrationRunner extends MigrationRunner { migrationBuilderService: MigrationBuilderService, private diskLocalStorage: WindowStorageService, ) { - super(diskStorage, logService, migrationBuilderService); + super(diskStorage, logService, migrationBuilderService, ClientType.Web); } override async run(): Promise { @@ -46,7 +47,7 @@ class WebMigrationHelper extends MigrationHelper { storageService: WindowStorageService, logService: LogService, ) { - super(currentVersion, storageService, logService, "web-disk-local"); + super(currentVersion, storageService, logService, "web-disk-local", ClientType.Web); this.diskLocalStorageService = storageService; } diff --git a/libs/angular/src/services/injection-tokens.ts b/libs/angular/src/services/injection-tokens.ts index 413fc5b530..b7989e7f32 100644 --- a/libs/angular/src/services/injection-tokens.ts +++ b/libs/angular/src/services/injection-tokens.ts @@ -1,6 +1,7 @@ import { InjectionToken } from "@angular/core"; import { Observable, Subject } from "rxjs"; +import { ClientType } from "@bitwarden/common/enums"; import { AbstractMemoryStorageService, AbstractStorageService, @@ -52,3 +53,4 @@ export const SYSTEM_THEME_OBSERVABLE = new SafeInjectionToken>>( "INTRAPROCESS_MESSAGING_SUBJECT", ); +export const CLIENT_TYPE = new SafeInjectionToken("CLIENT_TYPE"); diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index b28e475cb2..2f6167d676 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -276,6 +276,7 @@ import { SYSTEM_THEME_OBSERVABLE, WINDOW, INTRAPROCESS_MESSAGING_SUBJECT, + CLIENT_TYPE, } from "./injection-tokens"; import { ModalService } from "./modal.service"; @@ -1099,7 +1100,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: MigrationRunner, useClass: MigrationRunner, - deps: [AbstractStorageService, LogService, MigrationBuilderService], + deps: [AbstractStorageService, LogService, MigrationBuilderService, CLIENT_TYPE], }), safeProvider({ provide: MigrationBuilderService, diff --git a/libs/common/src/platform/services/migration-builder.service.spec.ts b/libs/common/src/platform/services/migration-builder.service.spec.ts index 1330ea07a4..ee9508e8b1 100644 --- a/libs/common/src/platform/services/migration-builder.service.spec.ts +++ b/libs/common/src/platform/services/migration-builder.service.spec.ts @@ -1,6 +1,7 @@ import { mock } from "jest-mock-extended"; import { FakeStorageService } from "../../../spec/fake-storage.service"; +import { ClientType } from "../../enums"; import { MigrationHelper } from "../../state-migrations/migration-helper"; import { MigrationBuilderService } from "./migration-builder.service"; @@ -66,25 +67,38 @@ describe("MigrationBuilderService", () => { global: {}, }; - it.each([ - noAccounts, - nullAndUndefinedAccounts, - emptyAccountObject, - nullCommonAccountProperties, - emptyCommonAccountProperties, - nullGlobal, - undefinedGlobal, - emptyGlobalObject, - ])("should not produce migrations that throw when given data: %s", async (startingState) => { - const sut = new MigrationBuilderService(); + const startingStates = [ + { data: noAccounts, description: "No Accounts" }, + { data: nullAndUndefinedAccounts, description: "Null and Undefined Accounts" }, + { data: emptyAccountObject, description: "Empty Account Object" }, + { data: nullCommonAccountProperties, description: "Null Common Account Properties" }, + { data: emptyCommonAccountProperties, description: "Empty Common Account Properties" }, + { data: nullGlobal, description: "Null Global" }, + { data: undefinedGlobal, description: "Undefined Global" }, + { data: emptyGlobalObject, description: "Empty Global Object" }, + ]; - const helper = new MigrationHelper( - startingStateVersion, - new FakeStorageService(startingState), - mock(), - "general", - ); + const clientTypes = Object.values(ClientType); - await sut.build().migrate(helper); - }); + // Generate all possible test cases + const testCases = startingStates.flatMap((startingState) => + clientTypes.map((clientType) => ({ startingState, clientType })), + ); + + it.each(testCases)( + "should not produce migrations that throw when given $startingState.description for client $clientType", + async ({ startingState, clientType }) => { + const sut = new MigrationBuilderService(); + + const helper = new MigrationHelper( + startingStateVersion, + new FakeStorageService(startingState), + mock(), + "general", + clientType, + ); + + await sut.build().migrate(helper); + }, + ); }); diff --git a/libs/common/src/platform/services/migration-runner.spec.ts b/libs/common/src/platform/services/migration-runner.spec.ts index 3934137f66..fc0d98bc56 100644 --- a/libs/common/src/platform/services/migration-runner.spec.ts +++ b/libs/common/src/platform/services/migration-runner.spec.ts @@ -1,6 +1,7 @@ import { mock } from "jest-mock-extended"; import { awaitAsync } from "../../../spec"; +import { ClientType } from "../../enums"; import { CURRENT_VERSION } from "../../state-migrations"; import { MigrationBuilder } from "../../state-migrations/migration-builder"; import { LogService } from "../abstractions/log.service"; @@ -17,7 +18,7 @@ describe("MigrationRunner", () => { migrationBuilderService.build.mockReturnValue(mockMigrationBuilder); - const sut = new MigrationRunner(storage, logService, migrationBuilderService); + const sut = new MigrationRunner(storage, logService, migrationBuilderService, ClientType.Web); describe("migrate", () => { it("should not run migrations if state is empty", async () => { diff --git a/libs/common/src/platform/services/migration-runner.ts b/libs/common/src/platform/services/migration-runner.ts index 006031f7e5..9e3a6118af 100644 --- a/libs/common/src/platform/services/migration-runner.ts +++ b/libs/common/src/platform/services/migration-runner.ts @@ -1,3 +1,4 @@ +import { ClientType } from "../../enums"; import { waitForMigrations } from "../../state-migrations"; import { CURRENT_VERSION, currentVersion } from "../../state-migrations/migrate"; import { MigrationHelper } from "../../state-migrations/migration-helper"; @@ -11,6 +12,7 @@ export class MigrationRunner { protected diskStorage: AbstractStorageService, protected logService: LogService, protected migrationBuilderService: MigrationBuilderService, + private clientType: ClientType, ) {} async run(): Promise { @@ -19,6 +21,7 @@ export class MigrationRunner { this.diskStorage, this.logService, "general", + this.clientType, ); if (migrationHelper.currentVersion < 0) { diff --git a/libs/common/src/state-migrations/migration-builder.spec.ts b/libs/common/src/state-migrations/migration-builder.spec.ts index 6a4ff8e6d4..59d85609e0 100644 --- a/libs/common/src/state-migrations/migration-builder.spec.ts +++ b/libs/common/src/state-migrations/migration-builder.spec.ts @@ -1,5 +1,8 @@ import { mock } from "jest-mock-extended"; +// eslint-disable-next-line import/no-restricted-paths +import { ClientType } from "../enums"; + import { MigrationBuilder } from "./migration-builder"; import { MigrationHelper } from "./migration-helper"; import { Migrator } from "./migrator"; @@ -72,65 +75,69 @@ describe("MigrationBuilder", () => { expect(migrations[1]).toMatchObject({ migrator: expect.any(TestMigrator), direction: "down" }); }); - describe("migrate", () => { - let migrator: TestMigrator; - let rollback_migrator: TestMigrator; + const clientTypes = Object.values(ClientType); - beforeEach(() => { - sut = sut.with(TestMigrator, 0, 1).rollback(TestMigrator, 1, 0); - migrator = (sut as any).migrations[0].migrator; - rollback_migrator = (sut as any).migrations[1].migrator; + describe.each(clientTypes)("for client %s", (clientType) => { + describe("migrate", () => { + let migrator: TestMigrator; + let rollback_migrator: TestMigrator; + + beforeEach(() => { + sut = sut.with(TestMigrator, 0, 1).rollback(TestMigrator, 1, 0); + migrator = (sut as any).migrations[0].migrator; + rollback_migrator = (sut as any).migrations[1].migrator; + }); + + it("should migrate", async () => { + const helper = new MigrationHelper(0, mock(), mock(), "general", clientType); + const spy = jest.spyOn(migrator, "migrate"); + await sut.migrate(helper); + expect(spy).toBeCalledWith(helper); + }); + + it("should rollback", async () => { + const helper = new MigrationHelper(1, mock(), mock(), "general", clientType); + const spy = jest.spyOn(rollback_migrator, "rollback"); + await sut.migrate(helper); + expect(spy).toBeCalledWith(helper); + }); + + it("should update version on migrate", async () => { + const helper = new MigrationHelper(0, mock(), mock(), "general", clientType); + const spy = jest.spyOn(migrator, "updateVersion"); + await sut.migrate(helper); + expect(spy).toBeCalledWith(helper, "up"); + }); + + it("should update version on rollback", async () => { + const helper = new MigrationHelper(1, mock(), mock(), "general", clientType); + const spy = jest.spyOn(rollback_migrator, "updateVersion"); + await sut.migrate(helper); + expect(spy).toBeCalledWith(helper, "down"); + }); + + it("should not run the migrator if the current version does not match the from version", async () => { + const helper = new MigrationHelper(3, mock(), mock(), "general", clientType); + const migrate = jest.spyOn(migrator, "migrate"); + const rollback = jest.spyOn(rollback_migrator, "rollback"); + await sut.migrate(helper); + expect(migrate).not.toBeCalled(); + expect(rollback).not.toBeCalled(); + }); + + it("should not update version if the current version does not match the from version", async () => { + const helper = new MigrationHelper(3, mock(), mock(), "general", clientType); + const migrate = jest.spyOn(migrator, "updateVersion"); + const rollback = jest.spyOn(rollback_migrator, "updateVersion"); + await sut.migrate(helper); + expect(migrate).not.toBeCalled(); + expect(rollback).not.toBeCalled(); + }); }); - it("should migrate", async () => { - const helper = new MigrationHelper(0, mock(), mock(), "general"); - const spy = jest.spyOn(migrator, "migrate"); - await sut.migrate(helper); - expect(spy).toBeCalledWith(helper); + it("should be able to call instance methods", async () => { + const helper = new MigrationHelper(0, mock(), mock(), "general", clientType); + await sut.with(TestMigratorWithInstanceMethod, 0, 1).migrate(helper); }); - - it("should rollback", async () => { - const helper = new MigrationHelper(1, mock(), mock(), "general"); - const spy = jest.spyOn(rollback_migrator, "rollback"); - await sut.migrate(helper); - expect(spy).toBeCalledWith(helper); - }); - - it("should update version on migrate", async () => { - const helper = new MigrationHelper(0, mock(), mock(), "general"); - const spy = jest.spyOn(migrator, "updateVersion"); - await sut.migrate(helper); - expect(spy).toBeCalledWith(helper, "up"); - }); - - it("should update version on rollback", async () => { - const helper = new MigrationHelper(1, mock(), mock(), "general"); - const spy = jest.spyOn(rollback_migrator, "updateVersion"); - await sut.migrate(helper); - expect(spy).toBeCalledWith(helper, "down"); - }); - - it("should not run the migrator if the current version does not match the from version", async () => { - const helper = new MigrationHelper(3, mock(), mock(), "general"); - const migrate = jest.spyOn(migrator, "migrate"); - const rollback = jest.spyOn(rollback_migrator, "rollback"); - await sut.migrate(helper); - expect(migrate).not.toBeCalled(); - expect(rollback).not.toBeCalled(); - }); - - it("should not update version if the current version does not match the from version", async () => { - const helper = new MigrationHelper(3, mock(), mock(), "general"); - const migrate = jest.spyOn(migrator, "updateVersion"); - const rollback = jest.spyOn(rollback_migrator, "updateVersion"); - await sut.migrate(helper); - expect(migrate).not.toBeCalled(); - expect(rollback).not.toBeCalled(); - }); - }); - - it("should be able to call instance methods", async () => { - const helper = new MigrationHelper(0, mock(), mock(), "general"); - await sut.with(TestMigratorWithInstanceMethod, 0, 1).migrate(helper); }); }); diff --git a/libs/common/src/state-migrations/migration-helper.spec.ts b/libs/common/src/state-migrations/migration-helper.spec.ts index f86cac8768..5f366f2597 100644 --- a/libs/common/src/state-migrations/migration-helper.spec.ts +++ b/libs/common/src/state-migrations/migration-helper.spec.ts @@ -2,6 +2,8 @@ import { MockProxy, mock } from "jest-mock-extended"; // eslint-disable-next-line import/no-restricted-paths -- Needed to print log messages import { FakeStorageService } from "../../spec/fake-storage.service"; +// eslint-disable-next-line import/no-restricted-paths -- Needed client type enum +import { ClientType } from "../enums"; // eslint-disable-next-line import/no-restricted-paths -- Needed to print log messages import { LogService } from "../platform/abstractions/log.service"; // eslint-disable-next-line import/no-restricted-paths -- Needed to interface with storage locations @@ -32,116 +34,129 @@ describe("RemoveLegacyEtmKeyMigrator", () => { let logService: MockProxy; let sut: MigrationHelper; - beforeEach(() => { - logService = mock(); - storage = mock(); - storage.get.mockImplementation((key) => (exampleJSON as any)[key]); + const clientTypes = Object.values(ClientType); - sut = new MigrationHelper(0, storage, logService, "general"); - }); + describe.each(clientTypes)("for client %s", (clientType) => { + beforeEach(() => { + logService = mock(); + storage = mock(); + storage.get.mockImplementation((key) => (exampleJSON as any)[key]); - describe("get", () => { - it("should delegate to storage.get", async () => { - await sut.get("key"); - expect(storage.get).toHaveBeenCalledWith("key"); - }); - }); - - describe("set", () => { - it("should delegate to storage.save", async () => { - await sut.set("key", "value"); - expect(storage.save).toHaveBeenCalledWith("key", "value"); - }); - }); - - describe("getAccounts", () => { - it("should return all accounts", async () => { - const accounts = await sut.getAccounts(); - expect(accounts).toEqual([ - { userId: "c493ed01-4e08-4e88-abc7-332f380ca760", account: { otherStuff: "otherStuff1" } }, - { userId: "23e61a5f-2ece-4f5e-b499-f0bc489482a9", account: { otherStuff: "otherStuff2" } }, - ]); + sut = new MigrationHelper(0, storage, logService, "general", clientType); }); - it("should handle missing authenticatedAccounts", async () => { - storage.get.mockImplementation((key) => - key === "authenticatedAccounts" ? undefined : (exampleJSON as any)[key], - ); - const accounts = await sut.getAccounts(); - expect(accounts).toEqual([]); - }); - }); - - describe("getFromGlobal", () => { - it("should return the correct value", async () => { - sut.currentVersion = 9; - const value = await sut.getFromGlobal({ - stateDefinition: { name: "serviceName" }, - key: "key", + describe("get", () => { + it("should delegate to storage.get", async () => { + await sut.get("key"); + expect(storage.get).toHaveBeenCalledWith("key"); }); - expect(value).toEqual("global_serviceName_key"); }); - it("should throw if the current version is less than 9", () => { - expect(() => - sut.getFromGlobal({ stateDefinition: { name: "serviceName" }, key: "key" }), - ).toThrowError("No key builder should be used for versions prior to 9."); - }); - }); - - describe("setToGlobal", () => { - it("should set the correct value", async () => { - sut.currentVersion = 9; - await sut.setToGlobal({ stateDefinition: { name: "serviceName" }, key: "key" }, "new_value"); - expect(storage.save).toHaveBeenCalledWith("global_serviceName_key", "new_value"); + describe("set", () => { + it("should delegate to storage.save", async () => { + await sut.set("key", "value"); + expect(storage.save).toHaveBeenCalledWith("key", "value"); + }); }); - it("should throw if the current version is less than 9", () => { - expect(() => - sut.setToGlobal( + describe("getAccounts", () => { + it("should return all accounts", async () => { + const accounts = await sut.getAccounts(); + expect(accounts).toEqual([ + { + userId: "c493ed01-4e08-4e88-abc7-332f380ca760", + account: { otherStuff: "otherStuff1" }, + }, + { + userId: "23e61a5f-2ece-4f5e-b499-f0bc489482a9", + account: { otherStuff: "otherStuff2" }, + }, + ]); + }); + + it("should handle missing authenticatedAccounts", async () => { + storage.get.mockImplementation((key) => + key === "authenticatedAccounts" ? undefined : (exampleJSON as any)[key], + ); + const accounts = await sut.getAccounts(); + expect(accounts).toEqual([]); + }); + }); + + describe("getFromGlobal", () => { + it("should return the correct value", async () => { + sut.currentVersion = 9; + const value = await sut.getFromGlobal({ + stateDefinition: { name: "serviceName" }, + key: "key", + }); + expect(value).toEqual("global_serviceName_key"); + }); + + it("should throw if the current version is less than 9", () => { + expect(() => + sut.getFromGlobal({ stateDefinition: { name: "serviceName" }, key: "key" }), + ).toThrowError("No key builder should be used for versions prior to 9."); + }); + }); + + describe("setToGlobal", () => { + it("should set the correct value", async () => { + sut.currentVersion = 9; + await sut.setToGlobal( { stateDefinition: { name: "serviceName" }, key: "key" }, - "global_serviceName_key", - ), - ).toThrowError("No key builder should be used for versions prior to 9."); - }); - }); - - describe("getFromUser", () => { - it("should return the correct value", async () => { - sut.currentVersion = 9; - const value = await sut.getFromUser("userId", { - stateDefinition: { name: "serviceName" }, - key: "key", + "new_value", + ); + expect(storage.save).toHaveBeenCalledWith("global_serviceName_key", "new_value"); + }); + + it("should throw if the current version is less than 9", () => { + expect(() => + sut.setToGlobal( + { stateDefinition: { name: "serviceName" }, key: "key" }, + "global_serviceName_key", + ), + ).toThrowError("No key builder should be used for versions prior to 9."); }); - expect(value).toEqual("user_userId_serviceName_key"); }); - it("should throw if the current version is less than 9", () => { - expect(() => - sut.getFromUser("userId", { stateDefinition: { name: "serviceName" }, key: "key" }), - ).toThrowError("No key builder should be used for versions prior to 9."); - }); - }); + describe("getFromUser", () => { + it("should return the correct value", async () => { + sut.currentVersion = 9; + const value = await sut.getFromUser("userId", { + stateDefinition: { name: "serviceName" }, + key: "key", + }); + expect(value).toEqual("user_userId_serviceName_key"); + }); - describe("setToUser", () => { - it("should set the correct value", async () => { - sut.currentVersion = 9; - await sut.setToUser( - "userId", - { stateDefinition: { name: "serviceName" }, key: "key" }, - "new_value", - ); - expect(storage.save).toHaveBeenCalledWith("user_userId_serviceName_key", "new_value"); + it("should throw if the current version is less than 9", () => { + expect(() => + sut.getFromUser("userId", { stateDefinition: { name: "serviceName" }, key: "key" }), + ).toThrowError("No key builder should be used for versions prior to 9."); + }); }); - it("should throw if the current version is less than 9", () => { - expect(() => - sut.setToUser( + describe("setToUser", () => { + it("should set the correct value", async () => { + sut.currentVersion = 9; + await sut.setToUser( "userId", { stateDefinition: { name: "serviceName" }, key: "key" }, "new_value", - ), - ).toThrowError("No key builder should be used for versions prior to 9."); + ); + expect(storage.save).toHaveBeenCalledWith("user_userId_serviceName_key", "new_value"); + }); + + it("should throw if the current version is less than 9", () => { + expect(() => + sut.setToUser( + "userId", + { stateDefinition: { name: "serviceName" }, key: "key" }, + "new_value", + ), + ).toThrowError("No key builder should be used for versions prior to 9."); + }); }); }); }); @@ -151,6 +166,7 @@ export function mockMigrationHelper( storageJson: any, stateVersion = 0, type: MigrationHelperType = "general", + clientType: ClientType = ClientType.Web, ): MockProxy { const logService: MockProxy = mock(); const storage: MockProxy = mock(); @@ -158,7 +174,7 @@ export function mockMigrationHelper( storage.save.mockImplementation(async (key, value) => { (storageJson as any)[key] = value; }); - const helper = new MigrationHelper(stateVersion, storage, logService, type); + const helper = new MigrationHelper(stateVersion, storage, logService, type, clientType); const mockHelper = mock(); mockHelper.get.mockImplementation((key) => helper.get(key)); @@ -295,7 +311,13 @@ export async function runMigrator< const allInjectedData = injectData(initalData, []); const fakeStorageService = new FakeStorageService(initalData); - const helper = new MigrationHelper(migrator.fromVersion, fakeStorageService, mock(), "general"); + const helper = new MigrationHelper( + migrator.fromVersion, + fakeStorageService, + mock(), + "general", + ClientType.Web, + ); // Run their migrations if (direction === "rollback") { diff --git a/libs/common/src/state-migrations/migration-helper.ts b/libs/common/src/state-migrations/migration-helper.ts index aab0a7e9c8..2505e2b264 100644 --- a/libs/common/src/state-migrations/migration-helper.ts +++ b/libs/common/src/state-migrations/migration-helper.ts @@ -1,3 +1,5 @@ +// eslint-disable-next-line import/no-restricted-paths -- Needed to provide client type to migrations +import { ClientType } from "../enums"; // eslint-disable-next-line import/no-restricted-paths -- Needed to print log messages import { LogService } from "../platform/abstractions/log.service"; // eslint-disable-next-line import/no-restricted-paths -- Needed to interface with storage locations @@ -17,6 +19,7 @@ export class MigrationHelper { private storageService: AbstractStorageService, public logService: LogService, type: MigrationHelperType, + public clientType: ClientType, ) { this.type = type; } diff --git a/libs/common/src/state-migrations/migrator.spec.ts b/libs/common/src/state-migrations/migrator.spec.ts index d1189c25ea..4079dc3fda 100644 --- a/libs/common/src/state-migrations/migrator.spec.ts +++ b/libs/common/src/state-migrations/migrator.spec.ts @@ -1,5 +1,7 @@ import { mock, MockProxy } from "jest-mock-extended"; +// eslint-disable-next-line import/no-restricted-paths -- Needed client type enum +import { ClientType } from "../enums"; // eslint-disable-next-line import/no-restricted-paths -- Needed to print log messages import { LogService } from "../platform/abstractions/log.service"; // eslint-disable-next-line import/no-restricted-paths -- Needed to interface with storage locations @@ -23,52 +25,56 @@ describe("migrator default methods", () => { let helper: MigrationHelper; let sut: TestMigrator; - beforeEach(() => { - storage = mock(); - logService = mock(); - helper = new MigrationHelper(0, storage, logService, "general"); - sut = new TestMigrator(0, 1); - }); + const clientTypes = Object.values(ClientType); - describe("shouldMigrate", () => { - describe("up", () => { - it("should return true if the current version equals the from version", async () => { - expect(await sut.shouldMigrate(helper, "up")).toBe(true); + describe.each(clientTypes)("for client %s", (clientType) => { + beforeEach(() => { + storage = mock(); + logService = mock(); + helper = new MigrationHelper(0, storage, logService, "general", clientType); + sut = new TestMigrator(0, 1); + }); + + describe("shouldMigrate", () => { + describe("up", () => { + it("should return true if the current version equals the from version", async () => { + expect(await sut.shouldMigrate(helper, "up")).toBe(true); + }); + + it("should return false if the current version does not equal the from version", async () => { + helper.currentVersion = 1; + expect(await sut.shouldMigrate(helper, "up")).toBe(false); + }); }); - it("should return false if the current version does not equal the from version", async () => { - helper.currentVersion = 1; - expect(await sut.shouldMigrate(helper, "up")).toBe(false); + describe("down", () => { + it("should return true if the current version equals the to version", async () => { + helper.currentVersion = 1; + expect(await sut.shouldMigrate(helper, "down")).toBe(true); + }); + + it("should return false if the current version does not equal the to version", async () => { + expect(await sut.shouldMigrate(helper, "down")).toBe(false); + }); }); }); - describe("down", () => { - it("should return true if the current version equals the to version", async () => { - helper.currentVersion = 1; - expect(await sut.shouldMigrate(helper, "down")).toBe(true); + describe("updateVersion", () => { + describe("up", () => { + it("should update the version", async () => { + await sut.updateVersion(helper, "up"); + expect(storage.save).toBeCalledWith("stateVersion", 1); + expect(helper.currentVersion).toBe(1); + }); }); - it("should return false if the current version does not equal the to version", async () => { - expect(await sut.shouldMigrate(helper, "down")).toBe(false); - }); - }); - }); - - describe("updateVersion", () => { - describe("up", () => { - it("should update the version", async () => { - await sut.updateVersion(helper, "up"); - expect(storage.save).toBeCalledWith("stateVersion", 1); - expect(helper.currentVersion).toBe(1); - }); - }); - - describe("down", () => { - it("should update the version", async () => { - helper.currentVersion = 1; - await sut.updateVersion(helper, "down"); - expect(storage.save).toBeCalledWith("stateVersion", 0); - expect(helper.currentVersion).toBe(0); + describe("down", () => { + it("should update the version", async () => { + helper.currentVersion = 1; + await sut.updateVersion(helper, "down"); + expect(storage.save).toBeCalledWith("stateVersion", 0); + expect(helper.currentVersion).toBe(0); + }); }); }); });