From 6c18ab46e10c30450bcc4afe4246e232d3d15dc9 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Wed, 10 Jan 2024 13:26:45 -0500 Subject: [PATCH 01/15] feat(plugins): make install command additive, remove type during uninstall Signed-off-by: Trae Yelovich --- .../imperative/src/config/src/ConfigSchema.ts | 3 +- .../__tests__/plugins/__resources__/schema.ts | 19 +++ .../npm-interface/install.unit.test.ts | 120 +++++++++++++++++- .../npm-interface/uninstall.unit.test.ts | 76 ++++++++++- .../utilities/npm-interface/install.ts | 84 +++++++++++- .../utilities/npm-interface/uninstall.ts | 69 ++++++++++ 6 files changed, 359 insertions(+), 12 deletions(-) create mode 100644 packages/imperative/src/imperative/__tests__/plugins/__resources__/schema.ts diff --git a/packages/imperative/src/config/src/ConfigSchema.ts b/packages/imperative/src/config/src/ConfigSchema.ts index 6a04334475..17a8cd77cb 100644 --- a/packages/imperative/src/config/src/ConfigSchema.ts +++ b/packages/imperative/src/config/src/ConfigSchema.ts @@ -104,8 +104,9 @@ export class ConfigSchema { * Transform a JSON schema to an Imperative profile schema. * @param schema The JSON schema for profile properties * @returns Imperative profile schema + * @internal */ - private static parseSchema(schema: any): IProfileSchema { + public static parseSchema(schema: any): IProfileSchema { const properties: { [key: string]: IProfileProperty } = {}; for (const [k, v] of Object.entries((schema.properties.properties || {}) as { [key: string]: any })) { properties[k] = { type: v.type }; diff --git a/packages/imperative/src/imperative/__tests__/plugins/__resources__/schema.ts b/packages/imperative/src/imperative/__tests__/plugins/__resources__/schema.ts new file mode 100644 index 0000000000..db39b96ce1 --- /dev/null +++ b/packages/imperative/src/imperative/__tests__/plugins/__resources__/schema.ts @@ -0,0 +1,19 @@ +import { IProfileTypeConfiguration } from "../../../.."; + +const mockSchema: IProfileTypeConfiguration = { + type: "test-type", + schema: { + title: "test-type", + description: "A test type profile", + type: "object", + required: [], + properties: { + host: { + type: "string", + secure: false + } + } + } +}; + +export default mockSchema; \ No newline at end of file diff --git a/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/install.unit.test.ts b/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/install.unit.test.ts index 8c7ff5b02f..e738341c7b 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/install.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/install.unit.test.ts @@ -58,7 +58,9 @@ import { ConfigurationLoader } from "../../../../src/ConfigurationLoader"; import { UpdateImpConfig } from "../../../../src/UpdateImpConfig"; import * as fs from "fs"; import * as path from "path"; - +import { gt as versionGreaterThan } from "semver"; +import { ProfileInfo } from "../../../../../config"; +import mockSchema from "../../__resources__/schema"; function setResolve(toResolve: string, resolveTo?: string) { expectedVal = toResolve; @@ -78,7 +80,12 @@ describe("PMF: Install Interface", () => { PMF_requirePluginModuleCallback: pmfI.requirePluginModuleCallback as Mock, ConfigurationLoader_load: ConfigurationLoader.load as Mock, UpdateImpConfig_addProfiles: UpdateImpConfig.addProfiles as Mock, - path: path as unknown as Mock + path: path as unknown as Mock, + ConfigSchema_loadSchema: jest.spyOn(ConfigSchema, "loadSchema"), + ProfileInfo: { + readExtendersJsonFromDisk: jest.spyOn(ProfileInfo, "readExtendersJsonFromDisk"), + writeExtendersJson: jest.spyOn(ProfileInfo, "writeExtendersJson") + } }; const packageName = "a"; @@ -101,7 +108,16 @@ describe("PMF: Install Interface", () => { mocks.sync.mockReturnValue("fake_find-up_sync_result" as any); jest.spyOn(path, "dirname").mockReturnValue("fake-dirname"); jest.spyOn(path, "join").mockReturnValue("/fake/join/path"); - mocks.ConfigurationLoader_load.mockReturnValue({ profiles: ["fake"] } as any); + mocks.ProfileInfo.readExtendersJsonFromDisk.mockReturnValue({ + profileTypes: { + "zosmf": { + from: ["Zowe CLI"] + } + } + }); + mocks.ProfileInfo.writeExtendersJson.mockImplementation(); + mocks.ConfigSchema_loadSchema.mockReturnValue([mockSchema]); + mocks.ConfigurationLoader_load.mockReturnValue({ profiles: [mockSchema] } as any); }); afterAll(() => { @@ -130,7 +146,7 @@ describe("PMF: Install Interface", () => { if (shouldUpdate) { expect(mocks.UpdateImpConfig_addProfiles).toHaveBeenCalledTimes(1); expect(mocks.ConfigSchema_updateSchema).toHaveBeenCalledTimes(1); - expect(mocks.ConfigSchema_updateSchema).toHaveBeenCalledWith({ layer: "global" }); + expect(mocks.ConfigSchema_updateSchema).toHaveBeenCalledWith(expect.objectContaining({ layer: "global" })); } else { expect(mocks.UpdateImpConfig_addProfiles).not.toHaveBeenCalled(); expect(mocks.ConfigSchema_updateSchema).not.toHaveBeenCalled(); @@ -165,7 +181,7 @@ describe("PMF: Install Interface", () => { describe("Basic install", () => { beforeEach(() => { mocks.getPackageInfo.mockResolvedValue({ name: packageName, version: packageVersion } as never); - jest.spyOn(fs, "existsSync").mockReturnValueOnce(true); + jest.spyOn(fs, "existsSync").mockReturnValue(true); jest.spyOn(path, "normalize").mockReturnValue("testing"); jest.spyOn(fs, "lstatSync").mockReturnValue({ isSymbolicLink: jest.fn().mockReturnValue(true) @@ -326,7 +342,7 @@ describe("PMF: Install Interface", () => { }); }); - it("should merge contents of previous json file", async () => { + it("should merge contents of previous plugins.json file", async () => { // value for our previous plugins.json const oneOldPlugin: IPluginJson = { plugin1: { @@ -355,6 +371,98 @@ describe("PMF: Install Interface", () => { }); }); + describe("Updating the global schema", () => { + const expectTestSchemaMgmt = async (opts: { + schemaExists: boolean; + newProfileType: boolean; + version?: string; + lastVersion?: string; + }) => { + const oneOldPlugin: IPluginJson = { + plugin1: { + package: "plugin1", + registry: packageRegistry, + version: "1.2.3" + } + }; + if (opts.newProfileType) { + const schema = { ...mockSchema, schemaVersion: opts.version }; + mocks.ConfigurationLoader_load.mockReturnValue({ + profiles: [ + schema + ] + } as any); + } + + mocks.getPackageInfo.mockResolvedValue({ name: packageName, version: packageVersion } as never); + jest.spyOn(fs, "existsSync").mockReturnValueOnce(true).mockReturnValueOnce(opts.schemaExists); + jest.spyOn(path, "normalize").mockReturnValue("testing"); + jest.spyOn(fs, "lstatSync").mockReturnValue({ + isSymbolicLink: jest.fn().mockReturnValue(true) + } as any); + mocks.readFileSync.mockReturnValue(oneOldPlugin as any); + + if (opts.lastVersion) { + mocks.ProfileInfo.readExtendersJsonFromDisk.mockReturnValueOnce({ + profileTypes: { + "test-type": { + from: [oneOldPlugin.plugin1.package], + version: opts.lastVersion, + latestFrom: oneOldPlugin.plugin1.package + } + } + }); + } + + setResolve(packageName); + await install(packageName, packageRegistry); + if (opts.schemaExists) { + expect(mocks.ConfigSchema_updateSchema).toHaveBeenCalled(); + } else { + expect(mocks.ConfigSchema_updateSchema).not.toHaveBeenCalled(); + } + + if (opts.version && opts.lastVersion) { + if (versionGreaterThan(opts.version, opts.lastVersion)) { + expect(mocks.ProfileInfo.writeExtendersJson).toHaveBeenCalled(); + } else { + expect(mocks.ProfileInfo.writeExtendersJson).not.toHaveBeenCalled(); + } + } + }; + it("should update the schema to contain the new profile type", async () => { + expectTestSchemaMgmt({ + schemaExists: true, + newProfileType: true + }); + }); + + it("should not update the schema if it doesn't exist", async () => { + expectTestSchemaMgmt({ + schemaExists: false, + newProfileType: true + }); + }); + + it("updates the schema with a newer schema version than the one present", () => { + expectTestSchemaMgmt({ + schemaExists: true, + newProfileType: true, + version: "2.0.0", + lastVersion: "1.0.0" + }); + }); + + it("doesn't update the schema with an older schema version than the one present", () => { + expectTestSchemaMgmt({ + schemaExists: true, + newProfileType: true, + version: "1.0.0", + lastVersion: "2.0.0" + }); + }); + }); + it("should throw errors", async () => { // Create a placeholder error object that should be set after the call to install let expectedError: ImperativeError = new ImperativeError({ diff --git a/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/uninstall.unit.test.ts b/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/uninstall.unit.test.ts index 28204bd208..e1459d02cd 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/uninstall.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/uninstall.unit.test.ts @@ -20,6 +20,7 @@ jest.mock("../../../../../cmd/src/response/CommandResponse"); jest.mock("../../../../../cmd/src/response/HandlerResponse"); import * as fs from "fs"; +import * as jsonfile from "jsonfile"; import { Console } from "../../../../../console"; import { sync } from "cross-spawn"; import { ImperativeError } from "../../../../../error"; @@ -29,7 +30,9 @@ import { PMFConstants } from "../../../../src/plugins/utilities/PMFConstants"; import { readFileSync, writeFileSync } from "jsonfile"; import { findNpmOnPath } from "../../../../src/plugins/utilities/NpmFunctions"; import { uninstall } from "../../../../src/plugins/utilities/npm-interface"; - +import { ConfigSchema, ProfileInfo } from "../../../../../config"; +import mockSchema from "../../__resources__/schema"; +import { ExecUtils } from "../../../../../utilities"; describe("PMF: Uninstall Interface", () => { // Objects created so types are correct. @@ -202,4 +205,75 @@ describe("PMF: Uninstall Interface", () => { expect(caughtError.message).toContain("Failed to uninstall plugin, install folder still exists"); }); }); + + describe("Schema management", () => { + const getBlockMocks = () => { + jest.spyOn(fs, "existsSync").mockRestore(); + return { + ConfigSchema: { + buildSchema: jest.spyOn(ConfigSchema, "buildSchema").mockImplementation(), + loadSchema: jest.spyOn(ConfigSchema, "loadSchema").mockReturnValueOnce([mockSchema]), + updateSchema: jest.spyOn(ConfigSchema, "updateSchema").mockImplementation() + }, + fs: { + existsSync: jest.spyOn(fs, "existsSync").mockReturnValueOnce(false) + }, + jsonfile: { + // avoid throwing error during plugin uninstall by marking plug-in folder as non-existent + writeFileSync: jest.spyOn(jsonfile, "writeFileSync").mockImplementation() + }, + ExecUtils: { + spawnAndGetOutput: jest.spyOn(ExecUtils, "spawnAndGetOutput").mockImplementation() + } + }; + }; + + const expectTestSchemaMgmt = (opts: { schemaUpdated?: boolean }) => { + const pluginJsonFile: IPluginJson = { + a: { + package: "a", + registry: packageRegistry, + version: "3.2.1" + }, + plugin2: { + package: "plugin1", + registry: packageRegistry, + version: "1.2.3" + } + }; + + mocks.readFileSync.mockReturnValue(pluginJsonFile as any); + const blockMocks = getBlockMocks(); + if (opts.schemaUpdated) { + blockMocks.fs.existsSync.mockReturnValueOnce(true); + jest.spyOn(ProfileInfo, "readExtendersJsonFromDisk").mockReturnValue({ + profileTypes: { + "test-type": { + from: ["a"], + } + } + }); + } + uninstall(packageName); + + // Check that schema was updated, if it was supposed to update + if (opts.schemaUpdated) { + expect(blockMocks.ConfigSchema.buildSchema).toHaveBeenCalled(); + expect(blockMocks.ConfigSchema.updateSchema).toHaveBeenCalled(); + expect(blockMocks.jsonfile.writeFileSync).toHaveBeenCalled(); + } else { + expect(blockMocks.ConfigSchema.buildSchema).not.toHaveBeenCalled(); + expect(blockMocks.ConfigSchema.updateSchema).not.toHaveBeenCalled(); + expect(blockMocks.jsonfile.writeFileSync).not.toHaveBeenCalledTimes(2); + } + }; + + it("Removes a type from the schema if the plug-in is the last source", () => { + expectTestSchemaMgmt({ schemaUpdated: true }); + }); + + it("Does not modify the schema if another source contributes to that profile type", () => { + expectTestSchemaMgmt({ schemaUpdated: false }); + }); + }); }); diff --git a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts index f76354c168..d9867d584a 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts @@ -12,6 +12,7 @@ import { PMFConstants } from "../PMFConstants"; import * as path from "path"; import * as fs from "fs"; +import * as jsonfile from "jsonfile"; import { readFileSync, writeFileSync } from "jsonfile"; import { IPluginJson } from "../../doc/IPluginJson"; import { Logger } from "../../../../../logger"; @@ -24,6 +25,10 @@ import { PluginManagementFacility } from "../../PluginManagementFacility"; import { ConfigurationLoader } from "../../../ConfigurationLoader"; import { UpdateImpConfig } from "../../../UpdateImpConfig"; import { CredentialManagerOverride, ICredentialManagerNameMap } from "../../../../../security"; +import { fileURLToPath, pathToFileURL } from "url"; +import { IProfileTypeConfiguration } from "../../../../../profiles"; +import * as semver from "semver"; +import { ProfileInfo } from "../../../../../config"; /** * Common function that abstracts the install process. This function should be called for each @@ -134,14 +139,85 @@ export async function install(packageLocation: string, registry: string, install const pluginImpConfig = ConfigurationLoader.load(null, packageInfo, requirerFunction); iConsole.debug(`Checking for global team configuration files to update.`); - if (PMFConstants.instance.PLUGIN_USING_CONFIG && - PMFConstants.instance.PLUGIN_CONFIG.layers.filter((layer) => layer.global && layer.exists).length > 0) + if (PMFConstants.instance.PLUGIN_USING_CONFIG) { // Update the Imperative Configuration to add the profiles introduced by the recently installed plugin // This might be needed outside of PLUGIN_USING_CONFIG scenarios, but we haven't had issues with other APIs before - if (Array.isArray(pluginImpConfig.profiles)) { + const globalLayer = PMFConstants.instance.PLUGIN_CONFIG.layers.find((layer) => layer.global && layer.exists); + if (globalLayer && Array.isArray(pluginImpConfig.profiles)) { UpdateImpConfig.addProfiles(pluginImpConfig.profiles); - ConfigSchema.updateSchema({ layer: "global" }); + const schemaUri = new URL(globalLayer.properties.$schema, pathToFileURL(globalLayer.path)); + const schemaPath = fileURLToPath(schemaUri); + if (fs.existsSync(schemaPath)) { + let loadedSchema: IProfileTypeConfiguration[]; + try { + // load schema from disk to prevent removal of profile types from other applications + loadedSchema = ConfigSchema.loadSchema(jsonfile.readFileSync(schemaPath)); + } catch (err) { + iConsole.error("Error when adding new profile type for plugin %s: failed to parse schema", newPlugin.package); + } + + // Only update global schema if we were able to load it from disk + if (loadedSchema != null) { + const existingTypes = loadedSchema.map((obj) => obj.type); + const extendersJson = ProfileInfo.readExtendersJsonFromDisk(); + + // Helper function to update extenders.json object during plugin install. + // Returns true if the object was updated, and false otherwise + const updateExtendersJson = (profile: IProfileTypeConfiguration): boolean => { + if (!(profile.type in extendersJson.profileTypes)) { + // If the type doesn't exist, add it to extenders.json and return + extendersJson.profileTypes[profile.type] = { + from: [packageInfo.name], + version: profile.schemaVersion + }; + return true; + } + + // Otherwise, only update extenders.json if the schema version is newer + const existingTypeInfo = extendersJson.profileTypes[profile.type]; + if (semver.valid(existingTypeInfo.version)) { + if (profile.schemaVersion && semver.lt(profile.schemaVersion, existingTypeInfo.version)) { + return false; + } + } + + extendersJson.profileTypes[profile.type] = { + from: [packageInfo.name], + version: profile.schemaVersion + }; + return true; + }; + + // Determine new profile types to add to schema + let shouldUpdate = false; + for (const profile of pluginImpConfig.profiles) { + if (!(profile.type in existingTypes)) { + loadedSchema.push(profile); + } else { + const existingType = loadedSchema.find((obj) => obj.type === profile.type); + if (semver.valid(existingType.schemaVersion)) { + if (semver.gt(profile.schemaVersion, existingType.schemaVersion)) { + existingType.schema = profile.schema; + existingType.schemaVersion = profile.schemaVersion; + } + } else { + existingType.schema = profile.schema; + existingType.schemaVersion = profile.schemaVersion; + } + } + shouldUpdate = shouldUpdate || updateExtendersJson(profile); + } + + if (shouldUpdate) { + // Update extenders.json (if necessary) after installing the plugin + ProfileInfo.writeExtendersJson(extendersJson); + } + const schema = ConfigSchema.buildSchema(loadedSchema); + ConfigSchema.updateSchema({ layer: "global", schema }); + jsonfile.writeFileSync(schemaPath, schema, { spaces: 4 }); + } + } } } diff --git a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts index 4239da8dda..4b22b478f0 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts @@ -10,6 +10,7 @@ */ import * as fs from "fs"; +import * as jsonfile from "jsonfile"; import * as path from "path"; import { PMFConstants } from "../PMFConstants"; import { readFileSync, writeFileSync } from "jsonfile"; @@ -19,6 +20,9 @@ import { ImperativeError } from "../../../../../error"; import { ExecUtils, TextUtils } from "../../../../../utilities"; import { StdioOptions } from "child_process"; import { findNpmOnPath } from "../NpmFunctions"; +import { ConfigSchema, ProfileInfo } from "../../../../../config"; +import { fileURLToPath, pathToFileURL } from "url"; +import { IProfileTypeConfiguration } from "../../../../../profiles"; const npmCmd = findNpmOnPath(); /** @@ -84,6 +88,71 @@ export function uninstall(packageName: string): void { throw new Error("Failed to uninstall plugin, install folder still exists:\n " + installFolder); } + if (PMFConstants.instance.PLUGIN_USING_CONFIG) { + // Update the Imperative Configuration to add the profiles introduced by the recently installed plugin + // This might be needed outside of PLUGIN_USING_CONFIG scenarios, but we haven't had issues with other APIs before + const globalLayer = PMFConstants.instance.PLUGIN_CONFIG.layers.find((layer) => layer.global && layer.exists); + if (globalLayer) { + const schemaUri = new URL(globalLayer.properties.$schema, pathToFileURL(globalLayer.path)); + const schemaPath = fileURLToPath(schemaUri); + if (fs.existsSync(schemaPath)) { + const extendersJson = ProfileInfo.readExtendersJsonFromDisk(); + const pluginTypes = Object.keys(extendersJson.profileTypes) + .filter((type) => type in extendersJson.profileTypes && + extendersJson.profileTypes[type].from.includes(npmPackage)); + const typesToRemove: string[] = []; + if (pluginTypes.length > 0) { + // Only remove a profile type contributed by this plugin if its the single source for that type. + for (const profileType of pluginTypes) { + const typeInfo = extendersJson.profileTypes[profileType]; + if (typeInfo.from.length > 1) { + // If there are other sources, remove the version for that type if this plugin provides the + // latest version. This will allow the next source to contribute a different schema version. + if (typeInfo.latestFrom === npmPackage) { + extendersJson.profileTypes[profileType] = { + ...typeInfo, + from: typeInfo.from.filter((v) => v !== npmPackage), + latestFrom: undefined, + version: undefined + }; + } else { + extendersJson.profileTypes[profileType] = { + ...typeInfo, + from: typeInfo.from.filter((v) => v !== npmPackage) + }; + } + } else { + extendersJson.profileTypes[profileType] = { + ...typeInfo, + from: typeInfo.from.filter((v) => v !== npmPackage) + }; + typesToRemove.push(profileType); + } + } + ProfileInfo.writeExtendersJson(extendersJson); + } + + let loadedSchema: IProfileTypeConfiguration[]; + try { + // load schema from disk to prevent removal of profile types from other applications + loadedSchema = ConfigSchema.loadSchema(jsonfile.readFileSync(schemaPath)); + } catch (err) { + iConsole.error("Error when removing profile type for plugin %s: failed to parse schema", npmPackage); + } + + // Only update global schema if we were able to load it from disk + if (loadedSchema != null) { + if (typesToRemove.length > 0) { + loadedSchema = loadedSchema.filter((typeCfg) => !typesToRemove.includes(typeCfg.type)); + const schema = ConfigSchema.buildSchema(loadedSchema); + ConfigSchema.updateSchema({ layer: "global", schema }); + jsonfile.writeFileSync(schemaPath, schema, { spaces: 4 }); + } + } + } + } + } + iConsole.info("Uninstall complete"); writeFileSync(PMFConstants.instance.PLUGIN_JSON, updatedInstalledPlugins, { From 84f71ad9d2587f7ab8d45d513845d92d8ae6429b Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Wed, 10 Jan 2024 15:18:32 -0500 Subject: [PATCH 02/15] fix: add missing license, resolve SonarCloud issue Signed-off-by: Trae Yelovich --- .../__tests__/plugins/__resources__/schema.ts | 11 +++++++++++ .../src/plugins/utilities/npm-interface/install.ts | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/imperative/src/imperative/__tests__/plugins/__resources__/schema.ts b/packages/imperative/src/imperative/__tests__/plugins/__resources__/schema.ts index db39b96ce1..1495ada4c0 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/__resources__/schema.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/__resources__/schema.ts @@ -1,3 +1,14 @@ +/* +* This program and the accompanying materials are made available under the terms of the +* Eclipse Public License v2.0 which accompanies this distribution, and is available at +* https://www.eclipse.org/legal/epl-v20.html +* +* SPDX-License-Identifier: EPL-2.0 +* +* Copyright Contributors to the Zowe Project. +* +*/ + import { IProfileTypeConfiguration } from "../../../.."; const mockSchema: IProfileTypeConfiguration = { diff --git a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts index d9867d584a..4a99e71d01 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts @@ -192,7 +192,7 @@ export async function install(packageLocation: string, registry: string, install // Determine new profile types to add to schema let shouldUpdate = false; for (const profile of pluginImpConfig.profiles) { - if (!(profile.type in existingTypes)) { + if (!existingTypes.includes(profile.type)) { loadedSchema.push(profile); } else { const existingType = loadedSchema.find((obj) => obj.type === profile.type); From 13c1e94132a75fd7e50e0be322891d251d59b62b Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Wed, 10 Jan 2024 15:58:13 -0500 Subject: [PATCH 03/15] fix: avoid OR short-circuit when updating schema Signed-off-by: Trae Yelovich --- .../imperative/src/plugins/utilities/npm-interface/install.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts index 4a99e71d01..fa6ab98440 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts @@ -206,7 +206,7 @@ export async function install(packageLocation: string, registry: string, install existingType.schemaVersion = profile.schemaVersion; } } - shouldUpdate = shouldUpdate || updateExtendersJson(profile); + shouldUpdate = updateExtendersJson(profile) || shouldUpdate; } if (shouldUpdate) { From b412e4dd2fb6d54a8856f90814235c0114470279 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Wed, 10 Jan 2024 15:59:38 -0500 Subject: [PATCH 04/15] chore: remove duplicated import Signed-off-by: Trae Yelovich --- .../src/plugins/utilities/npm-interface/install.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts index fa6ab98440..9114c79bfd 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts @@ -12,7 +12,6 @@ import { PMFConstants } from "../PMFConstants"; import * as path from "path"; import * as fs from "fs"; -import * as jsonfile from "jsonfile"; import { readFileSync, writeFileSync } from "jsonfile"; import { IPluginJson } from "../../doc/IPluginJson"; import { Logger } from "../../../../../logger"; @@ -152,7 +151,7 @@ export async function install(packageLocation: string, registry: string, install let loadedSchema: IProfileTypeConfiguration[]; try { // load schema from disk to prevent removal of profile types from other applications - loadedSchema = ConfigSchema.loadSchema(jsonfile.readFileSync(schemaPath)); + loadedSchema = ConfigSchema.loadSchema(readFileSync(schemaPath)); } catch (err) { iConsole.error("Error when adding new profile type for plugin %s: failed to parse schema", newPlugin.package); } @@ -215,7 +214,7 @@ export async function install(packageLocation: string, registry: string, install } const schema = ConfigSchema.buildSchema(loadedSchema); ConfigSchema.updateSchema({ layer: "global", schema }); - jsonfile.writeFileSync(schemaPath, schema, { spaces: 4 }); + writeFileSync(schemaPath, schema, { spaces: 4 }); } } } From 98ab4acdee94276cca5105e3bf80ad6b0b090a30 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Wed, 10 Jan 2024 16:30:08 -0500 Subject: [PATCH 05/15] fix: remove redundant write calls for schema Signed-off-by: Trae Yelovich --- .../imperative/src/plugins/utilities/npm-interface/install.ts | 1 - .../src/plugins/utilities/npm-interface/uninstall.ts | 4 +--- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts index 9114c79bfd..f19130f058 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts @@ -214,7 +214,6 @@ export async function install(packageLocation: string, registry: string, install } const schema = ConfigSchema.buildSchema(loadedSchema); ConfigSchema.updateSchema({ layer: "global", schema }); - writeFileSync(schemaPath, schema, { spaces: 4 }); } } } diff --git a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts index 4b22b478f0..bf7e2ded2e 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts @@ -10,7 +10,6 @@ */ import * as fs from "fs"; -import * as jsonfile from "jsonfile"; import * as path from "path"; import { PMFConstants } from "../PMFConstants"; import { readFileSync, writeFileSync } from "jsonfile"; @@ -135,7 +134,7 @@ export function uninstall(packageName: string): void { let loadedSchema: IProfileTypeConfiguration[]; try { // load schema from disk to prevent removal of profile types from other applications - loadedSchema = ConfigSchema.loadSchema(jsonfile.readFileSync(schemaPath)); + loadedSchema = ConfigSchema.loadSchema(readFileSync(schemaPath)); } catch (err) { iConsole.error("Error when removing profile type for plugin %s: failed to parse schema", npmPackage); } @@ -146,7 +145,6 @@ export function uninstall(packageName: string): void { loadedSchema = loadedSchema.filter((typeCfg) => !typesToRemove.includes(typeCfg.type)); const schema = ConfigSchema.buildSchema(loadedSchema); ConfigSchema.updateSchema({ layer: "global", schema }); - jsonfile.writeFileSync(schemaPath, schema, { spaces: 4 }); } } } From 3f20c4855ab91cea8ccaa1840fb9dc6d2600703a Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Wed, 10 Jan 2024 17:56:47 -0500 Subject: [PATCH 06/15] fix(plugins): Do not update schema if using web URL Signed-off-by: Trae Yelovich --- .../imperative/src/plugins/utilities/npm-interface/install.ts | 4 ++-- .../src/plugins/utilities/npm-interface/uninstall.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts index f19130f058..97da69b93b 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts @@ -146,8 +146,8 @@ export async function install(packageLocation: string, registry: string, install if (globalLayer && Array.isArray(pluginImpConfig.profiles)) { UpdateImpConfig.addProfiles(pluginImpConfig.profiles); const schemaUri = new URL(globalLayer.properties.$schema, pathToFileURL(globalLayer.path)); - const schemaPath = fileURLToPath(schemaUri); - if (fs.existsSync(schemaPath)) { + const schemaPath = schemaUri.protocol === "file:" ? fileURLToPath(schemaUri) : undefined; + if (schemaPath && fs.existsSync(schemaPath)) { let loadedSchema: IProfileTypeConfiguration[]; try { // load schema from disk to prevent removal of profile types from other applications diff --git a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts index bf7e2ded2e..49e24c179a 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts @@ -93,8 +93,8 @@ export function uninstall(packageName: string): void { const globalLayer = PMFConstants.instance.PLUGIN_CONFIG.layers.find((layer) => layer.global && layer.exists); if (globalLayer) { const schemaUri = new URL(globalLayer.properties.$schema, pathToFileURL(globalLayer.path)); - const schemaPath = fileURLToPath(schemaUri); - if (fs.existsSync(schemaPath)) { + const schemaPath = schemaUri.protocol === "file:" ? fileURLToPath(schemaUri) : undefined; + if (schemaPath && fs.existsSync(schemaPath)) { const extendersJson = ProfileInfo.readExtendersJsonFromDisk(); const pluginTypes = Object.keys(extendersJson.profileTypes) .filter((type) => type in extendersJson.profileTypes && From be3717973f31a7da6ecb268eb0d0e495d51ddb36 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Thu, 11 Jan 2024 10:30:42 -0500 Subject: [PATCH 07/15] feat: move extenders.json logic into separate fn, add unit tests Signed-off-by: Trae Yelovich --- .../npm-interface/uninstall.unit.test.ts | 104 ++++++++++++++++++ .../utilities/npm-interface/uninstall.ts | 93 ++++++++-------- 2 files changed, 153 insertions(+), 44 deletions(-) diff --git a/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/uninstall.unit.test.ts b/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/uninstall.unit.test.ts index e1459d02cd..a94c0bb420 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/uninstall.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/uninstall.unit.test.ts @@ -33,6 +33,8 @@ import { uninstall } from "../../../../src/plugins/utilities/npm-interface"; import { ConfigSchema, ProfileInfo } from "../../../../../config"; import mockSchema from "../../__resources__/schema"; import { ExecUtils } from "../../../../../utilities"; +import { IExtendersJsonOpts } from "../../../../../config/src/doc/IExtenderOpts"; +import { updateAndGetRemovedTypes } from "../../../../src/plugins/utilities/npm-interface/uninstall"; describe("PMF: Uninstall Interface", () => { // Objects created so types are correct. @@ -276,4 +278,106 @@ describe("PMF: Uninstall Interface", () => { expectTestSchemaMgmt({ schemaUpdated: false }); }); }); + + describe("updateAndGetRemovedTypes", () => { + const getBlockMocks = () => { + const profileInfo = { + readExtendersJsonFromDisk: jest.spyOn(ProfileInfo, "readExtendersJsonFromDisk"), + writeExtendersJson: jest.spyOn(ProfileInfo, "writeExtendersJson").mockImplementation(), + }; + + return { + profileInfo, + }; + }; + + const expectUpdateExtendersJson = (shouldUpdate: { + extJson: boolean; + schema?: boolean; + }, extendersJson: IExtendersJsonOpts) => { + const blockMocks = getBlockMocks(); + blockMocks.profileInfo.readExtendersJsonFromDisk.mockReturnValue(extendersJson); + + const hasMultipleSources = extendersJson.profileTypes["some-type"].from.length > 1; + const wasLatestSource = extendersJson.profileTypes["some-type"].latestFrom === "aPluginPackage"; + + const typesToRemove = updateAndGetRemovedTypes("aPluginPackage"); + if (shouldUpdate.extJson) { + expect(blockMocks.profileInfo.writeExtendersJson).toHaveBeenCalled(); + } else { + expect(blockMocks.profileInfo.writeExtendersJson).not.toHaveBeenCalled(); + return; + } + + const newExtendersObj = blockMocks.profileInfo.writeExtendersJson.mock.calls[0][0]; + + if (hasMultipleSources) { + expect(blockMocks.profileInfo.writeExtendersJson).not.toHaveBeenCalledWith( + expect.objectContaining({ + profileTypes: { + "some-type": { + latestFrom: undefined + } + } + }) + ); + + const newFrom = newExtendersObj.profileTypes["some-type"].from; + expect(newFrom).not.toContain("aPluginPackage"); + } else { + expect("some-type" in newExtendersObj.profileTypes).toBe(false); + } + + if (wasLatestSource && hasMultipleSources) { + expect(newExtendersObj.profileTypes["some-type"].latestFrom).toBeUndefined(); + expect(newExtendersObj.profileTypes["some-type"].version).toBeUndefined(); + } + + expect(typesToRemove.length > 0).toBe(shouldUpdate.schema ?? false); + }; + + it("package is only source for profile type", () => { + expectUpdateExtendersJson({ extJson: true, schema: true }, { + profileTypes: { + "some-type": { + from: ["aPluginPackage"], + } + } + }); + }); + + it("package is latest source of profile type", () => { + expectUpdateExtendersJson({ extJson: true }, { + profileTypes: { + "some-type": { + from: ["aPluginPackage", "someOtherPlugin"], + latestFrom: "aPluginPackage" + } + } + }); + }); + + it("profile type has multiple sources", () => { + expectUpdateExtendersJson({ extJson: true }, { + profileTypes: { + "some-type": { + from: ["aPluginPackage", "someOtherPlugin"], + } + } + }); + }); + + it("returns an empty list when package does not contribute any profile types", () => { + const blockMocks = getBlockMocks(); + blockMocks.profileInfo.readExtendersJsonFromDisk.mockReturnValue({ + profileTypes: { + "some-type": { + from: ["anotherPkg"] + } + } + }); + expect(updateAndGetRemovedTypes("aPluginPackage").length).toBe(0); + }); + }); }); + diff --git a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts index 49e24c179a..1676e03e89 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts @@ -24,6 +24,47 @@ import { fileURLToPath, pathToFileURL } from "url"; import { IProfileTypeConfiguration } from "../../../../../profiles"; const npmCmd = findNpmOnPath(); +/** + * Updates `extenders.json` and returns a list of types to remove from the schema, if applicable. + * @param npmPackage The package name for the plug-in that's being uninstalled + * @returns A list of types to remove from the schema + */ +export function updateAndGetRemovedTypes(npmPackage: string): string[] { + const extendersJson = ProfileInfo.readExtendersJsonFromDisk(); + const pluginTypes = Object.keys(extendersJson.profileTypes) + .filter((type) => extendersJson.profileTypes[type].from.includes(npmPackage)); + const typesToRemove: string[] = []; + if (pluginTypes.length > 0) { + // Only remove a profile type contributed by this plugin if its the single source for that type. + for (const profileType of pluginTypes) { + const typeInfo = extendersJson.profileTypes[profileType]; + if (typeInfo.from.length > 1) { + // If there are other sources, remove the version for that type if this plugin provides the + // latest version. This will allow the next source to contribute a different schema version. + if (typeInfo.latestFrom === npmPackage) { + extendersJson.profileTypes[profileType] = { + ...typeInfo, + from: typeInfo.from.filter((v) => v !== npmPackage), + latestFrom: undefined, + version: undefined + }; + } else { + extendersJson.profileTypes[profileType] = { + ...typeInfo, + from: typeInfo.from.filter((v) => v !== npmPackage) + }; + } + } else { + delete extendersJson.profileTypes[profileType]; + typesToRemove.push(profileType); + } + } + ProfileInfo.writeExtendersJson(extendersJson); + } + + return typesToRemove; +} + /** * @TODO - allow multiple packages to be uninstalled? * Common function that abstracts the uninstall process. @@ -95,42 +136,6 @@ export function uninstall(packageName: string): void { const schemaUri = new URL(globalLayer.properties.$schema, pathToFileURL(globalLayer.path)); const schemaPath = schemaUri.protocol === "file:" ? fileURLToPath(schemaUri) : undefined; if (schemaPath && fs.existsSync(schemaPath)) { - const extendersJson = ProfileInfo.readExtendersJsonFromDisk(); - const pluginTypes = Object.keys(extendersJson.profileTypes) - .filter((type) => type in extendersJson.profileTypes && - extendersJson.profileTypes[type].from.includes(npmPackage)); - const typesToRemove: string[] = []; - if (pluginTypes.length > 0) { - // Only remove a profile type contributed by this plugin if its the single source for that type. - for (const profileType of pluginTypes) { - const typeInfo = extendersJson.profileTypes[profileType]; - if (typeInfo.from.length > 1) { - // If there are other sources, remove the version for that type if this plugin provides the - // latest version. This will allow the next source to contribute a different schema version. - if (typeInfo.latestFrom === npmPackage) { - extendersJson.profileTypes[profileType] = { - ...typeInfo, - from: typeInfo.from.filter((v) => v !== npmPackage), - latestFrom: undefined, - version: undefined - }; - } else { - extendersJson.profileTypes[profileType] = { - ...typeInfo, - from: typeInfo.from.filter((v) => v !== npmPackage) - }; - } - } else { - extendersJson.profileTypes[profileType] = { - ...typeInfo, - from: typeInfo.from.filter((v) => v !== npmPackage) - }; - typesToRemove.push(profileType); - } - } - ProfileInfo.writeExtendersJson(extendersJson); - } - let loadedSchema: IProfileTypeConfiguration[]; try { // load schema from disk to prevent removal of profile types from other applications @@ -138,14 +143,14 @@ export function uninstall(packageName: string): void { } catch (err) { iConsole.error("Error when removing profile type for plugin %s: failed to parse schema", npmPackage); } - - // Only update global schema if we were able to load it from disk - if (loadedSchema != null) { - if (typesToRemove.length > 0) { - loadedSchema = loadedSchema.filter((typeCfg) => !typesToRemove.includes(typeCfg.type)); - const schema = ConfigSchema.buildSchema(loadedSchema); - ConfigSchema.updateSchema({ layer: "global", schema }); - } + // update extenders.json with any removed types - function returns the list of types to remove + const typesToRemove = updateAndGetRemovedTypes(npmPackage); + + // Only update global schema if there are types to remove and accessible from disk + if (loadedSchema != null && typesToRemove.length > 0) { + loadedSchema = loadedSchema.filter((typeCfg) => !typesToRemove.includes(typeCfg.type)); + const schema = ConfigSchema.buildSchema(loadedSchema); + ConfigSchema.updateSchema({ layer: "global", schema }); } } } From 60c7730269cc43f07fd381734de42e018da78a78 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Thu, 11 Jan 2024 13:38:12 -0500 Subject: [PATCH 08/15] fix: check for valid schemaVersion during install Signed-off-by: Trae Yelovich --- .../imperative/src/plugins/utilities/npm-interface/install.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts index 97da69b93b..1e7fa4eca5 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts @@ -196,7 +196,7 @@ export async function install(packageLocation: string, registry: string, install } else { const existingType = loadedSchema.find((obj) => obj.type === profile.type); if (semver.valid(existingType.schemaVersion)) { - if (semver.gt(profile.schemaVersion, existingType.schemaVersion)) { + if (semver.valid(profile.schemaVersion) && semver.gt(profile.schemaVersion, existingType.schemaVersion)) { existingType.schema = profile.schema; existingType.schemaVersion = profile.schemaVersion; } From 6a189e9b05518f5d4f4743096e6a1e9c69f19c6d Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Fri, 12 Jan 2024 14:47:58 -0500 Subject: [PATCH 09/15] fix: update install logic for relocated version property Signed-off-by: Trae Yelovich --- .../src/plugins/utilities/npm-interface/install.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts index 1e7fa4eca5..b1eee7f6d8 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts @@ -168,7 +168,7 @@ export async function install(packageLocation: string, registry: string, install // If the type doesn't exist, add it to extenders.json and return extendersJson.profileTypes[profile.type] = { from: [packageInfo.name], - version: profile.schemaVersion + version: profile.schema.version }; return true; } @@ -176,14 +176,14 @@ export async function install(packageLocation: string, registry: string, install // Otherwise, only update extenders.json if the schema version is newer const existingTypeInfo = extendersJson.profileTypes[profile.type]; if (semver.valid(existingTypeInfo.version)) { - if (profile.schemaVersion && semver.lt(profile.schemaVersion, existingTypeInfo.version)) { + if (profile.schema.version && semver.lt(profile.schema.version, existingTypeInfo.version)) { return false; } } extendersJson.profileTypes[profile.type] = { from: [packageInfo.name], - version: profile.schemaVersion + version: profile.schema.version }; return true; }; @@ -195,14 +195,14 @@ export async function install(packageLocation: string, registry: string, install loadedSchema.push(profile); } else { const existingType = loadedSchema.find((obj) => obj.type === profile.type); - if (semver.valid(existingType.schemaVersion)) { - if (semver.valid(profile.schemaVersion) && semver.gt(profile.schemaVersion, existingType.schemaVersion)) { + if (semver.valid(existingType.schema.version)) { + if (semver.valid(profile.schema.version) && semver.gt(profile.schema.version, existingType.schema.version)) { existingType.schema = profile.schema; - existingType.schemaVersion = profile.schemaVersion; + existingType.schema.version = profile.schema.version; } } else { existingType.schema = profile.schema; - existingType.schemaVersion = profile.schemaVersion; + existingType.schema.version = profile.schema.version; } } shouldUpdate = updateExtendersJson(profile) || shouldUpdate; From 697fedd4984cd787a914bbc1c1ca6552fb81edfa Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Tue, 16 Jan 2024 08:55:35 -0500 Subject: [PATCH 10/15] fix: adjust tests for relocated version, rename resource Signed-off-by: Trae Yelovich --- .../__resources__/{schema.ts => typeConfiguration.ts} | 4 ++-- .../plugins/utilities/npm-interface/install.unit.test.ts | 8 ++++---- .../utilities/npm-interface/uninstall.unit.test.ts | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) rename packages/imperative/src/imperative/__tests__/plugins/__resources__/{schema.ts => typeConfiguration.ts} (89%) diff --git a/packages/imperative/src/imperative/__tests__/plugins/__resources__/schema.ts b/packages/imperative/src/imperative/__tests__/plugins/__resources__/typeConfiguration.ts similarity index 89% rename from packages/imperative/src/imperative/__tests__/plugins/__resources__/schema.ts rename to packages/imperative/src/imperative/__tests__/plugins/__resources__/typeConfiguration.ts index 1495ada4c0..c9a7b79830 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/__resources__/schema.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/__resources__/typeConfiguration.ts @@ -11,7 +11,7 @@ import { IProfileTypeConfiguration } from "../../../.."; -const mockSchema: IProfileTypeConfiguration = { +const mockTypeConfig: IProfileTypeConfiguration = { type: "test-type", schema: { title: "test-type", @@ -27,4 +27,4 @@ const mockSchema: IProfileTypeConfiguration = { } }; -export default mockSchema; \ No newline at end of file +export default mockTypeConfig; \ No newline at end of file diff --git a/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/install.unit.test.ts b/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/install.unit.test.ts index e738341c7b..2687462043 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/install.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/install.unit.test.ts @@ -60,7 +60,7 @@ import * as fs from "fs"; import * as path from "path"; import { gt as versionGreaterThan } from "semver"; import { ProfileInfo } from "../../../../../config"; -import mockSchema from "../../__resources__/schema"; +import mockTypeConfig from "../../__resources__/typeConfiguration"; function setResolve(toResolve: string, resolveTo?: string) { expectedVal = toResolve; @@ -116,8 +116,8 @@ describe("PMF: Install Interface", () => { } }); mocks.ProfileInfo.writeExtendersJson.mockImplementation(); - mocks.ConfigSchema_loadSchema.mockReturnValue([mockSchema]); - mocks.ConfigurationLoader_load.mockReturnValue({ profiles: [mockSchema] } as any); + mocks.ConfigSchema_loadSchema.mockReturnValue([mockTypeConfig]); + mocks.ConfigurationLoader_load.mockReturnValue({ profiles: [mockTypeConfig] } as any); }); afterAll(() => { @@ -386,7 +386,7 @@ describe("PMF: Install Interface", () => { } }; if (opts.newProfileType) { - const schema = { ...mockSchema, schemaVersion: opts.version }; + const schema = { ...mockTypeConfig, schema: { ...mockTypeConfig.schema, version: opts.version } }; mocks.ConfigurationLoader_load.mockReturnValue({ profiles: [ schema diff --git a/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/uninstall.unit.test.ts b/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/uninstall.unit.test.ts index a94c0bb420..1a0230e89f 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/uninstall.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/uninstall.unit.test.ts @@ -31,7 +31,7 @@ import { readFileSync, writeFileSync } from "jsonfile"; import { findNpmOnPath } from "../../../../src/plugins/utilities/NpmFunctions"; import { uninstall } from "../../../../src/plugins/utilities/npm-interface"; import { ConfigSchema, ProfileInfo } from "../../../../../config"; -import mockSchema from "../../__resources__/schema"; +import mockTypeConfig from "../../__resources__/typeConfiguration"; import { ExecUtils } from "../../../../../utilities"; import { IExtendersJsonOpts } from "../../../../../config/src/doc/IExtenderOpts"; import { updateAndGetRemovedTypes } from "../../../../src/plugins/utilities/npm-interface/uninstall"; @@ -214,7 +214,7 @@ describe("PMF: Uninstall Interface", () => { return { ConfigSchema: { buildSchema: jest.spyOn(ConfigSchema, "buildSchema").mockImplementation(), - loadSchema: jest.spyOn(ConfigSchema, "loadSchema").mockReturnValueOnce([mockSchema]), + loadSchema: jest.spyOn(ConfigSchema, "loadSchema").mockReturnValueOnce([mockTypeConfig]), updateSchema: jest.spyOn(ConfigSchema, "updateSchema").mockImplementation() }, fs: { From 945b255e41f9a88a902df58392ba54c8d452bff4 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Tue, 16 Jan 2024 11:10:31 -0500 Subject: [PATCH 11/15] refactor: move updateExtendersJson out of install function Signed-off-by: Trae Yelovich --- .../utilities/npm-interface/install.ts | 60 ++++++++++--------- 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts index b1eee7f6d8..bb33b3bec6 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts @@ -28,6 +28,37 @@ import { fileURLToPath, pathToFileURL } from "url"; import { IProfileTypeConfiguration } from "../../../../../profiles"; import * as semver from "semver"; import { ProfileInfo } from "../../../../../config"; +import { IExtendersJsonOpts } from "../../../../../config/src/doc/IExtenderOpts"; + +// Helper function to update extenders.json object during plugin install. +// Returns true if the object was updated, and false otherwise +const updateExtendersJson = ( + extendersJson: IExtendersJsonOpts, + packageInfo: { name: string; version: string; }, + profile: IProfileTypeConfiguration): boolean => { + if (!(profile.type in extendersJson.profileTypes)) { + // If the type doesn't exist, add it to extenders.json and return + extendersJson.profileTypes[profile.type] = { + from: [packageInfo.name], + version: profile.schema.version + }; + return true; + } + + // Otherwise, only update extenders.json if the schema version is newer + const existingTypeInfo = extendersJson.profileTypes[profile.type]; + if (semver.valid(existingTypeInfo.version)) { + if (profile.schema.version && semver.lt(profile.schema.version, existingTypeInfo.version)) { + return false; + } + } + + extendersJson.profileTypes[profile.type] = { + from: [packageInfo.name], + version: profile.schema.version + }; + return true; +}; /** * Common function that abstracts the install process. This function should be called for each @@ -161,33 +192,6 @@ export async function install(packageLocation: string, registry: string, install const existingTypes = loadedSchema.map((obj) => obj.type); const extendersJson = ProfileInfo.readExtendersJsonFromDisk(); - // Helper function to update extenders.json object during plugin install. - // Returns true if the object was updated, and false otherwise - const updateExtendersJson = (profile: IProfileTypeConfiguration): boolean => { - if (!(profile.type in extendersJson.profileTypes)) { - // If the type doesn't exist, add it to extenders.json and return - extendersJson.profileTypes[profile.type] = { - from: [packageInfo.name], - version: profile.schema.version - }; - return true; - } - - // Otherwise, only update extenders.json if the schema version is newer - const existingTypeInfo = extendersJson.profileTypes[profile.type]; - if (semver.valid(existingTypeInfo.version)) { - if (profile.schema.version && semver.lt(profile.schema.version, existingTypeInfo.version)) { - return false; - } - } - - extendersJson.profileTypes[profile.type] = { - from: [packageInfo.name], - version: profile.schema.version - }; - return true; - }; - // Determine new profile types to add to schema let shouldUpdate = false; for (const profile of pluginImpConfig.profiles) { @@ -205,7 +209,7 @@ export async function install(packageLocation: string, registry: string, install existingType.schema.version = profile.schema.version; } } - shouldUpdate = updateExtendersJson(profile) || shouldUpdate; + shouldUpdate = updateExtendersJson(extendersJson, packageInfo, profile) || shouldUpdate; } if (shouldUpdate) { From b339ce06dca285d5c463faf9b4fa9b2fadbba639 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Tue, 16 Jan 2024 11:47:30 -0500 Subject: [PATCH 12/15] refactor: use Config.getSchemaInfo during install/uninstall Signed-off-by: Trae Yelovich --- packages/imperative/src/config/src/__mocks__/Config.ts | 9 ++++++++- .../src/plugins/utilities/npm-interface/install.ts | 8 +++----- .../src/plugins/utilities/npm-interface/uninstall.ts | 7 +++---- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/imperative/src/config/src/__mocks__/Config.ts b/packages/imperative/src/config/src/__mocks__/Config.ts index 0d3a961ad2..b302f9b08c 100644 --- a/packages/imperative/src/config/src/__mocks__/Config.ts +++ b/packages/imperative/src/config/src/__mocks__/Config.ts @@ -9,7 +9,7 @@ * */ -import { IConfigOpts } from "../.."; +import { IConfigOpts, IConfigSchemaInfo } from "../.."; import { IConfigLayer } from "../../src/doc/IConfigLayer"; export class Config { @@ -54,4 +54,11 @@ export class Config { return config; } + public getSchemaInfo(): IConfigSchemaInfo { + return { + local: true, + resolved: "/some/path/to/schema.json", + original: "/some/path/to/schema.json" + }; + } } \ No newline at end of file diff --git a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts index bb33b3bec6..8532fcf7e9 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts @@ -24,7 +24,6 @@ import { PluginManagementFacility } from "../../PluginManagementFacility"; import { ConfigurationLoader } from "../../../ConfigurationLoader"; import { UpdateImpConfig } from "../../../UpdateImpConfig"; import { CredentialManagerOverride, ICredentialManagerNameMap } from "../../../../../security"; -import { fileURLToPath, pathToFileURL } from "url"; import { IProfileTypeConfiguration } from "../../../../../profiles"; import * as semver from "semver"; import { ProfileInfo } from "../../../../../config"; @@ -176,13 +175,12 @@ export async function install(packageLocation: string, registry: string, install const globalLayer = PMFConstants.instance.PLUGIN_CONFIG.layers.find((layer) => layer.global && layer.exists); if (globalLayer && Array.isArray(pluginImpConfig.profiles)) { UpdateImpConfig.addProfiles(pluginImpConfig.profiles); - const schemaUri = new URL(globalLayer.properties.$schema, pathToFileURL(globalLayer.path)); - const schemaPath = schemaUri.protocol === "file:" ? fileURLToPath(schemaUri) : undefined; - if (schemaPath && fs.existsSync(schemaPath)) { + const schemaInfo = PMFConstants.instance.PLUGIN_CONFIG.getSchemaInfo(); + if (schemaInfo.local && fs.existsSync(schemaInfo.resolved)) { let loadedSchema: IProfileTypeConfiguration[]; try { // load schema from disk to prevent removal of profile types from other applications - loadedSchema = ConfigSchema.loadSchema(readFileSync(schemaPath)); + loadedSchema = ConfigSchema.loadSchema(readFileSync(schemaInfo.resolved)); } catch (err) { iConsole.error("Error when adding new profile type for plugin %s: failed to parse schema", newPlugin.package); } diff --git a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts index 1676e03e89..25d9bd5a78 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts @@ -133,13 +133,12 @@ export function uninstall(packageName: string): void { // This might be needed outside of PLUGIN_USING_CONFIG scenarios, but we haven't had issues with other APIs before const globalLayer = PMFConstants.instance.PLUGIN_CONFIG.layers.find((layer) => layer.global && layer.exists); if (globalLayer) { - const schemaUri = new URL(globalLayer.properties.$schema, pathToFileURL(globalLayer.path)); - const schemaPath = schemaUri.protocol === "file:" ? fileURLToPath(schemaUri) : undefined; - if (schemaPath && fs.existsSync(schemaPath)) { + const schemaInfo = PMFConstants.instance.PLUGIN_CONFIG.getSchemaInfo(); + if (schemaInfo.local && fs.existsSync(schemaInfo.resolved)) { let loadedSchema: IProfileTypeConfiguration[]; try { // load schema from disk to prevent removal of profile types from other applications - loadedSchema = ConfigSchema.loadSchema(readFileSync(schemaPath)); + loadedSchema = ConfigSchema.loadSchema(readFileSync(schemaInfo.resolved)); } catch (err) { iConsole.error("Error when removing profile type for plugin %s: failed to parse schema", npmPackage); } From 1a2909a3b28de5fe7ddd890682ae3852bd156659 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Tue, 16 Jan 2024 13:23:41 -0500 Subject: [PATCH 13/15] chore: remove unused imports from URL module Signed-off-by: Trae Yelovich --- .../imperative/src/plugins/utilities/npm-interface/uninstall.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts index 25d9bd5a78..ec9e033747 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts @@ -20,7 +20,6 @@ import { ExecUtils, TextUtils } from "../../../../../utilities"; import { StdioOptions } from "child_process"; import { findNpmOnPath } from "../NpmFunctions"; import { ConfigSchema, ProfileInfo } from "../../../../../config"; -import { fileURLToPath, pathToFileURL } from "url"; import { IProfileTypeConfiguration } from "../../../../../profiles"; const npmCmd = findNpmOnPath(); From 3ad578600506e73b34a2ebb9e51f9c2380337a36 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Fri, 19 Jan 2024 08:03:23 -0500 Subject: [PATCH 14/15] style: Use arrow function for updateAndGetRemovedTypes Signed-off-by: Trae Yelovich --- .../imperative/src/plugins/utilities/npm-interface/uninstall.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts index ec9e033747..f3661a7687 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/uninstall.ts @@ -28,7 +28,7 @@ const npmCmd = findNpmOnPath(); * @param npmPackage The package name for the plug-in that's being uninstalled * @returns A list of types to remove from the schema */ -export function updateAndGetRemovedTypes(npmPackage: string): string[] { +export const updateAndGetRemovedTypes = (npmPackage: string): string[] => { const extendersJson = ProfileInfo.readExtendersJsonFromDisk(); const pluginTypes = Object.keys(extendersJson.profileTypes) .filter((type) => extendersJson.profileTypes[type].from.includes(npmPackage)); From e2b026d715ebce31858747a3b8605ebba0891fc7 Mon Sep 17 00:00:00 2001 From: Trae Yelovich Date: Fri, 19 Jan 2024 08:26:48 -0500 Subject: [PATCH 15/15] tests: updateExtendersJson Signed-off-by: Trae Yelovich --- .../npm-interface/install.unit.test.ts | 26 +++++++++++++++++++ .../utilities/npm-interface/install.ts | 2 +- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/install.unit.test.ts b/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/install.unit.test.ts index 2687462043..6d07a1a448 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/install.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/install.unit.test.ts @@ -61,6 +61,8 @@ import * as path from "path"; import { gt as versionGreaterThan } from "semver"; import { ProfileInfo } from "../../../../../config"; import mockTypeConfig from "../../__resources__/typeConfiguration"; +import { updateExtendersJson } from "../../../../src/plugins/utilities/npm-interface/install"; +import { IExtendersJsonOpts } from "../../../../../config/src/doc/IExtenderOpts"; function setResolve(toResolve: string, resolveTo?: string) { expectedVal = toResolve; @@ -463,6 +465,30 @@ describe("PMF: Install Interface", () => { }); }); + describe("updating extenders.json", () => { + it("adds a new profile type if it doesn't exist", () => { + const extendersJson = { profileTypes: {} } as IExtendersJsonOpts; + updateExtendersJson(extendersJson, { name: "aPkg", version: "1.0.0" }, mockTypeConfig); + expect(extendersJson.profileTypes["test-type"]).not.toBeUndefined(); + }); + + it("replaces a profile type with a newer schema version", () => { + const extendersJson = { profileTypes: { "test-type": { from: ["Zowe Client App"], version: "0.9.0" } } }; + updateExtendersJson(extendersJson, { name: "aPkg", version: "1.0.0" }, + { ...mockTypeConfig, schema: { ...mockTypeConfig.schema, version: "1.0.0" } }); + expect(extendersJson.profileTypes["test-type"]).not.toBeUndefined(); + expect(extendersJson.profileTypes["test-type"].version).toBe("1.0.0"); + }); + + it("does not change the schema version if older", () => { + const extendersJson = { profileTypes: { "test-type": { from: ["Zowe Client App"], version: "1.2.0" } } }; + updateExtendersJson(extendersJson, { name: "aPkg", version: "1.0.0" }, + { ...mockTypeConfig, schema: { ...mockTypeConfig.schema, version: "1.0.0" } }); + expect(extendersJson.profileTypes["test-type"]).not.toBeUndefined(); + expect(extendersJson.profileTypes["test-type"].version).toBe("1.2.0"); + }); + }); + it("should throw errors", async () => { // Create a placeholder error object that should be set after the call to install let expectedError: ImperativeError = new ImperativeError({ diff --git a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts index 8532fcf7e9..c627ac4dcc 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/install.ts @@ -31,7 +31,7 @@ import { IExtendersJsonOpts } from "../../../../../config/src/doc/IExtenderOpts" // Helper function to update extenders.json object during plugin install. // Returns true if the object was updated, and false otherwise -const updateExtendersJson = ( +export const updateExtendersJson = ( extendersJson: IExtendersJsonOpts, packageInfo: { name: string; version: string; }, profile: IProfileTypeConfiguration): boolean => {