[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 <jsnider@bitwarden.com>

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);
   });
 });

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<LogService>;
   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<MigrationHelper> {
   const logService: MockProxy<LogService> = mock();
   const storage: MockProxy<AbstractStorageService> = 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<MigrationHelper>();
   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") {

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;
   }

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);
+        });
       });
     });
   });