From 56f447bd14ca7a432e0958986954fe5146a46d04 Mon Sep 17 00:00:00 2001 From: Todd Schiller Date: Sun, 22 Sep 2024 19:48:43 -0400 Subject: [PATCH] Use mod instance in marketplace mod updater Remove test debugging statement --- src/background/deploymentUpdater.ts | 27 +++--- src/background/modUpdater.test.ts | 36 -------- src/background/modUpdater.ts | 96 ++++++++-------------- src/background/utils/deactivateMod.test.ts | 70 ++++------------ src/background/utils/deactivateMod.ts | 24 ++---- 5 files changed, 75 insertions(+), 178 deletions(-) diff --git a/src/background/deploymentUpdater.ts b/src/background/deploymentUpdater.ts index 7d4284efbe..083423610e 100644 --- a/src/background/deploymentUpdater.ts +++ b/src/background/deploymentUpdater.ts @@ -69,7 +69,10 @@ import deactivateModInstancesAndSaveState from "@/background/utils/deactivateMod import saveModComponentStateAndReloadTabs, { type ReloadOptions, } from "@/background/utils/saveModComponentStateAndReloadTabs"; -import { selectModInstances } from "@/store/modComponents/modInstanceSelectors"; +import { + selectModInstanceMap, + selectModInstances, +} from "@/store/modComponents/modInstanceSelectors"; // eslint-disable-next-line local-rules/persistBackgroundData -- Static const { reducer: modComponentReducer, actions: modComponentActions } = @@ -192,22 +195,26 @@ async function activateDeployment({ let _editorState = editorState; const { deployment, modDefinition } = activatableDeployment; - const modInstances = selectModInstances({ + const modInstanceMap = selectModInstanceMap({ options: _optionsState, }); - const isAlreadyActivated = modInstances.some( + // Check if the deployment is already activated, regardless of the package id + const isAlreadyActivated = [...modInstanceMap.values()].some( (modInstance) => modInstance.deploymentMetadata?.id === deployment.id, ); - // Deactivate any existing mod instance corresponding to the deployed package - const result = deactivateMod(deployment.package.package_id, { - modComponentState: _optionsState, - editorState: _editorState, - }); + // Deactivate any existing mod instance corresponding to the deployed package, regardless of deployment + const packageModInstance = modInstanceMap.get(deployment.package.package_id); + if (packageModInstance) { + const result = deactivateMod(packageModInstance, { + modComponentState: _optionsState, + editorState: _editorState, + }); - _optionsState = result.modComponentState; - _editorState = result.editorState; + _optionsState = result.modComponentState; + _editorState = result.editorState; + } // Activate the deployed mod with the service definition _optionsState = modComponentReducer( diff --git a/src/background/modUpdater.test.ts b/src/background/modUpdater.test.ts index 348a7d34cc..58c27779f1 100644 --- a/src/background/modUpdater.test.ts +++ b/src/background/modUpdater.test.ts @@ -143,42 +143,6 @@ describe("getActivatedMarketplaceModVersions function", () => { }, ]); }); - - it("reports error if multiple mod component versions activated for same mod", async () => { - const sameMod = modMetadataFactory({ - sharing: publicSharingDefinitionFactory(), - }); - - const onePublicActivatedMod = activatedModComponentFactory({ - _recipe: sameMod, - }); - - const sameModDifferentVersion = modMetadataFactory({ - ...sameMod, - version: "2.0.0" as SemVerString, - }); - - const anotherPublicActivatedMod = activatedModComponentFactory({ - _recipe: sameModDifferentVersion, - }); - - await saveModComponentState({ - activatedModComponents: [ - onePublicActivatedMod, - anotherPublicActivatedMod, - ], - }); - - const result = await getActivatedMarketplaceModVersions(); - - expect(result).toEqual([ - { - name: onePublicActivatedMod!._recipe!.id, - version: onePublicActivatedMod!._recipe!.version, - }, - ]); - expect(reportError).toHaveBeenCalled(); - }); }); describe("fetchModUpdates function", () => { diff --git a/src/background/modUpdater.ts b/src/background/modUpdater.ts index 52a4c737a2..e7df114f72 100644 --- a/src/background/modUpdater.ts +++ b/src/background/modUpdater.ts @@ -25,20 +25,21 @@ import { import type { RegistryId, SemVerString } from "@/types/registryTypes"; import type { ModDefinition } from "@/types/modDefinitionTypes"; import modComponentSlice from "@/store/modComponents/modComponentSlice"; -import { groupBy, isEmpty, uniq } from "lodash"; +import { isEmpty } from "lodash"; import { queueReloadModEveryTab } from "@/contentScript/messenger/api"; import { getEditorState, saveEditorState } from "@/store/editorStorage"; import type { EditorState } from "@/pageEditor/store/editor/pageEditorTypes"; -import type { ActivatedModComponent } from "@/types/modComponentTypes"; -import { collectModOptions } from "@/store/modComponents/modComponentUtils"; import type { ModComponentState } from "@/store/modComponents/modComponentTypes"; import { uninstallContextMenu } from "@/background/contextMenus/uninstallContextMenu"; -import collectExistingConfiguredDependenciesForMod from "@/integrations/util/collectExistingConfiguredDependenciesForMod"; import { flagOn } from "@/auth/featureFlagStorage"; import { assertNotNullish } from "@/utils/nullishUtils"; import { FeatureFlags } from "@/auth/featureFlags"; import { API_PATHS } from "@/data/service/urlPaths"; import deactivateMod from "@/background/utils/deactivateMod"; +import { + selectModInstanceMap, + selectModInstances, +} from "@/store/modComponents/modInstanceSelectors"; const UPDATE_INTERVAL_MS = 10 * 60 * 1000; @@ -62,41 +63,21 @@ type PackageVersionPair = { name: RegistryId; version: SemVerString }; export async function getActivatedMarketplaceModVersions(): Promise< PackageVersionPair[] > { - const { activatedModComponents } = await getModComponentState(); + const modInstances = selectModInstances({ + options: await getModComponentState(), + }); // Typically most Marketplace mods would not be a deployment. If this happens to be the case, // the deployment updater will handle the updates. - const mods: Array = activatedModComponents - .filter((mod) => mod._recipe?.sharing?.public && !mod._deployment) - .map((mod) => mod._recipe); - - const modVersions: PackageVersionPair[] = []; - - for (const [name, modComponents] of Object.entries( - groupBy(mods, "id"), - ) as Array<[RegistryId, Array]>) { - const uniqueModVersions: SemVerString[] = uniq( - modComponents - .map((modComponent) => modComponent?.version) - .filter((x) => x != null), - ); - - if (uniqueModVersions.length > 1) { - reportError( - new Error( - `Found multiple mod component versions activated for the same mod: ${name} (${uniqueModVersions.join( - ", ", - )})`, - ), - ); - } - - assertNotNullish(uniqueModVersions[0], "Mod component version is required"); - - modVersions.push({ name, version: uniqueModVersions[0] }); - } + const marketplaceModInstances = modInstances.filter( + (mod) => mod.definition.sharing.public && !mod.deploymentMetadata, + ); - return modVersions; + return marketplaceModInstances.map(({ definition }) => { + const { id, version } = definition.metadata; + assertNotNullish(version, "Mod component version is required"); + return { name: id, version }; + }); } /** @@ -146,14 +127,23 @@ export async function fetchModUpdates(): Promise { * * The ModComponents will have new UUIDs. * - * @param modDefinition the mod to update + * @param newModDefinition the mod to update * @param reduxState the current state of the modComponent and editor redux stores * @returns new redux state with the mod updated */ export function updateMod( - modDefinition: ModDefinition, + newModDefinition: ModDefinition, { options: modComponentState, editor: editorState }: ActivatedModState, ): ActivatedModState { + const modInstanceMap = selectModInstanceMap({ + options: modComponentState, + }); + + const modInstance = modInstanceMap.get(newModDefinition.metadata.id); + + // Must be present because updateMod is only called when there is an update available for an existing mod + assertNotNullish(modInstance, "Mod instance not found"); + console.log({ modComponentState: JSON.stringify(modComponentState, null, 2), }); @@ -161,40 +151,26 @@ export function updateMod( const { modComponentState: nextModComponentState, editorState: nextEditorState, - // This type is weird, please ignore it for now, we need to clean up a lot of stuff with these - // mod component types. These "deactivated" components are not passed anywhere else, or put into - // redux, or anything like that. They are only used to collect the configured dependencies and the - // mod options in order to re-install the mod (see the calls to collectExistingConfiguredDependenciesForMod - // and collectRecipeOptions immediately following this code). - deactivatedModComponents, - } = deactivateMod(modDefinition.metadata.id, { + } = deactivateMod(modInstance, { modComponentState, editorState, }); - for (const deactivatedModComponent of deactivatedModComponents) { + for (const modComponentId of modInstance.modComponentIds) { // Remove the menu item UI from all mods. We must explicitly remove context menu items because otherwise the user // will see duplicate menu items because the old/new mod components have different UUIDs. // `updateMods` calls `queueReloadModEveryTab`. Therefore, if the user clicks on a tab where the new version of the // mod component is not loaded yet, they'll get a notification to reload the page. - void uninstallContextMenu({ modComponentId: deactivatedModComponent.id }); + void uninstallContextMenu({ modComponentId }); } - const configuredDependencies = collectExistingConfiguredDependenciesForMod( - modDefinition, - deactivatedModComponents, - ); - - const optionsArgs = collectModOptions( - deactivatedModComponents.filter((modComponent) => modComponent.optionsArgs), - ); - const finalModComponentState = modComponentSlice.reducer( nextModComponentState, modComponentSlice.actions.activateMod({ - modDefinition, - configuredDependencies, - optionsArgs, + modDefinition: newModDefinition, + // Activate using the same options/configuration + configuredDependencies: modInstance.integrationsArgs, + optionsArgs: modInstance.optionsArgs, screen: "background", isReactivate: true, }), @@ -210,8 +186,8 @@ async function updateMods(modUpdates: BackwardsCompatibleUpdate[]) { let newOptionsState = await getModComponentState(); let newEditorState = await getEditorState(); - for (const { backwards_compatible: update } of modUpdates) { - const { options, editor } = updateMod(update, { + for (const { backwards_compatible: updatedDefinition } of modUpdates) { + const { options, editor } = updateMod(updatedDefinition, { options: newOptionsState, editor: newEditorState, }); diff --git a/src/background/utils/deactivateMod.test.ts b/src/background/utils/deactivateMod.test.ts index b87fac7644..28f6dd113a 100644 --- a/src/background/utils/deactivateMod.test.ts +++ b/src/background/utils/deactivateMod.test.ts @@ -25,25 +25,20 @@ import { modMetadataFactory, activatedModComponentFactory, } from "@/testUtils/factories/modComponentFactories"; -import { starterBrickDefinitionFactory } from "@/testUtils/factories/modDefinitionFactories"; -import { type ActivatedModComponent } from "@/types/modComponentTypes"; -import { type RegistryId } from "@/types/registryTypes"; +import { type ModInstance } from "@/types/modInstanceTypes"; +import { modInstanceFactory } from "@/testUtils/factories/modInstanceFactories"; +import { mapModInstanceToActivatedModComponents } from "@/store/modComponents/modInstanceUtils"; describe("deactivateMod", () => { - let modToDeactivate: ActivatedModComponent["_recipe"]; + let modToDeactivate: ModInstance; beforeEach(async () => { - modToDeactivate = modMetadataFactory({}); - const anotherMod = modMetadataFactory({}); + modToDeactivate = modInstanceFactory(); + const anotherMod = modMetadataFactory(); await saveModComponentState({ activatedModComponents: [ - activatedModComponentFactory({ - _recipe: modToDeactivate, - }), - activatedModComponentFactory({ - _recipe: modToDeactivate, - }), + ...mapModInstanceToActivatedModComponents(modToDeactivate), activatedModComponentFactory({ _recipe: anotherMod, }), @@ -55,50 +50,17 @@ describe("deactivateMod", () => { const modComponentState = await getModComponentState(); const editorState = await getEditorState(); - const { - modComponentState: nextModComponentState, - deactivatedModComponents, - } = deactivateMod(modToDeactivate!.id, { - modComponentState, - editorState, - }); - - expect(deactivatedModComponents).toHaveLength(2); - expect(deactivatedModComponents[0]!._recipe!.id).toEqual( - modToDeactivate!.id, - ); - expect(deactivatedModComponents[1]!._recipe!.id).toEqual( - modToDeactivate!.id, + const { modComponentState: nextModComponentState } = deactivateMod( + modToDeactivate, + { + modComponentState, + editorState, + }, ); expect(nextModComponentState.activatedModComponents).toHaveLength(1); - }); - - it("should do nothing if mod id does not have any activated mod components", async () => { - const starterBrick = starterBrickDefinitionFactory(); - const modComponent = activatedModComponentFactory({ - extensionPointId: starterBrick.metadata!.id, - _recipe: modMetadataFactory({}), - }); - - await saveModComponentState({ - activatedModComponents: [modComponent], - }); - - const modComponentState = await getModComponentState(); - const editorState = await getEditorState(); - - const { - modComponentState: nextModComponentState, - deactivatedModComponents, - } = deactivateMod("@test/id-doesnt-exist" as RegistryId, { - modComponentState, - editorState, - }); - - expect(deactivatedModComponents).toEqual([]); - expect(nextModComponentState.activatedModComponents).toEqual( - modComponentState.activatedModComponents, - ); + expect( + nextModComponentState.activatedModComponents[0]!._recipe!.id, + ).not.toEqual(modToDeactivate.definition.metadata.id); }); }); diff --git a/src/background/utils/deactivateMod.ts b/src/background/utils/deactivateMod.ts index 81ee01702f..082827ef48 100644 --- a/src/background/utils/deactivateMod.ts +++ b/src/background/utils/deactivateMod.ts @@ -16,22 +16,20 @@ */ import deactivateModComponent from "@/background/utils/deactivateModComponent"; -import getModComponentsForMod from "@/mods/util/getModComponentsForMod"; import { type EditorState } from "@/pageEditor/store/editor/pageEditorTypes"; import { type ModComponentState } from "@/store/modComponents/modComponentTypes"; -import { type ActivatedModComponent } from "@/types/modComponentTypes"; -import { type RegistryId } from "@/types/registryTypes"; +import { type ModInstance } from "@/types/modInstanceTypes"; /** - * Deactivates all mod components with the given mod id. Does not remove the mod UI from existing tabs. + * Returns the Redux state that excludes the mod. NOTE: does not remove the mod UI from existing tabs. * - * @param modId the mod registry id + * @param modInstance the active mod to deactivate * @param reduxState the current state of the modComponent and editor redux stores * @returns new redux state with the mod components deactivated * and the mod components that were deactivated */ function deactivateMod( - modId: RegistryId, + modInstance: ModInstance, { editorState, modComponentState, @@ -42,36 +40,26 @@ function deactivateMod( ): { modComponentState: ModComponentState; editorState: EditorState | undefined; - deactivatedModComponents: ActivatedModComponent[]; } { - const activatedModComponents = getModComponentsForMod( - modId, - modComponentState, - ); - const deactivatedModComponents: ActivatedModComponent[] = []; - let _nextModComponentState = modComponentState; let _nextEditorState = editorState; - for (const activatedModComponent of activatedModComponents) { + for (const modComponentId of modInstance.modComponentIds) { const { modComponentState: nextModComponentState, editorState: nextEditorState, - } = deactivateModComponent(activatedModComponent.id, { + } = deactivateModComponent(modComponentId, { modComponentState: _nextModComponentState, editorState: _nextEditorState, }); _nextModComponentState = nextModComponentState; _nextEditorState = nextEditorState; - - deactivatedModComponents.push(activatedModComponent); } return { modComponentState: _nextModComponentState, editorState: _nextEditorState, - deactivatedModComponents, }; }