From 2fa6c2693200e953f484dc0b941c4197ee2d2090 Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Wed, 23 Oct 2024 08:28:32 -0400 Subject: [PATCH 01/10] Fix handling of scoped registry on plugin install Signed-off-by: Timothy Johnson --- packages/imperative/CHANGELOG.md | 4 + .../cmd/install/install.handler.unit.test.ts | 46 ++-- .../utilities/NpmFunctions.unit.test.ts | 2 +- .../npm-interface/install.unit.test.ts | 4 +- .../npm-interface/update.unit.test.ts | 4 +- .../plugins/cmd/install/install.handler.ts | 210 +++++------------- .../src/plugins/doc/INpmInstallArgs.ts | 30 +++ .../src/plugins/utilities/NpmFunctions.ts | 28 +-- .../utilities/npm-interface/install.ts | 6 +- .../plugins/utilities/npm-interface/update.ts | 6 +- 10 files changed, 131 insertions(+), 209 deletions(-) create mode 100644 packages/imperative/src/imperative/src/plugins/doc/INpmInstallArgs.ts diff --git a/packages/imperative/CHANGELOG.md b/packages/imperative/CHANGELOG.md index 9893f8b278..d155d3a4ac 100644 --- a/packages/imperative/CHANGELOG.md +++ b/packages/imperative/CHANGELOG.md @@ -2,6 +2,10 @@ All notable changes to the Imperative package will be documented in this file. +## Recent Changes + +- BugFix: Fixed an issue where `plugins install` command could fail when installing scoped package because scoped registry was used to fetch all dependencies. [#2317](https://github.com/zowe/zowe-cli/issues/2317) + ## `8.2.0` - Enhancement: Use the new SDK method `ConfigUtils.hasTokenExpired` to check whether a given JSON web token has expired. [#2298](https://github.com/zowe/zowe-cli/pull/2298) diff --git a/packages/imperative/src/imperative/__tests__/plugins/cmd/install/install.handler.unit.test.ts b/packages/imperative/src/imperative/__tests__/plugins/cmd/install/install.handler.unit.test.ts index 1cbba51d8a..5d0ba5ce9e 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/cmd/install/install.handler.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/cmd/install/install.handler.unit.test.ts @@ -53,7 +53,6 @@ import { PMFConstants } from "../../../../src/plugins/utilities/PMFConstants"; import { TextUtils } from "../../../../../utilities"; import { getRegistry, getScopeRegistry, npmLogin } from "../../../../src/plugins/utilities/NpmFunctions"; import * as spawn from "cross-spawn"; -import { IO } from "../../../../../io"; describe("Plugin Management Facility install handler", () => { @@ -381,28 +380,27 @@ describe("Plugin Management Facility install handler", () => { expect(expectedError.additionalDetails).toContain("Command failed"); expect(expectedError.additionalDetails).toContain("npm"); }); - it("should handle installed plugins via public scope", async () => { - const handler = new InstallHandler(); + // it("should handle installed plugins via public scope", async () => { + // const handler = new InstallHandler(); - const params = getIHandlerParametersObject(); - params.arguments.plugin = ["@public/sample1"]; + // const params = getIHandlerParametersObject(); + // params.arguments.plugin = ["@public/sample1"]; - mocks.getScopeRegistry.mockReturnValueOnce("publicRegistryUrl" as any); + // mocks.getScopeRegistry.mockReturnValueOnce("publicRegistryUrl" as any); - try { - await handler.process(params); - } catch (e) { - expect(e).toBeUndefined(); - } + // try { + // await handler.process(params); + // } catch (e) { + // expect(e).toBeUndefined(); + // } - expect(mocks.install).toHaveBeenCalledWith("@public/sample1","publicRegistryUrl"); - }); + // expect(mocks.install).toHaveBeenCalledWith("@public/sample1","publicRegistryUrl"); + // }); it("should handle installed plugins via project/directory", async () => { const handler = new InstallHandler(); const params = getIHandlerParametersObject(); params.arguments.plugin = ["path/to/dir"]; - jest.spyOn(IO, 'isDir').mockReturnValue(true); try{ await handler.process(params); @@ -411,7 +409,7 @@ describe("Plugin Management Facility install handler", () => { expect(e).toBeUndefined(); } - expect(mocks.install).toHaveBeenCalledWith("path/to/dir","path/to/dir"); + expect(mocks.install).toHaveBeenCalledWith("path/to/dir", packageRegistry); }); it("should handle installed plugins via tarball file", async () => { const handler = new InstallHandler(); @@ -426,16 +424,14 @@ describe("Plugin Management Facility install handler", () => { expect(e).toBeUndefined(); } - expect(mocks.install).toHaveBeenCalledWith("path/to/dir/file.tgz","path/to/dir/file.tgz"); + expect(mocks.install).toHaveBeenCalledWith("path/to/dir/file.tgz", packageRegistry); }); - it("should handle multiple installed plugins via tarball, director, public registry, and private registry", async () => { + it("should handle multiple installed plugins via tarball, directory, and registry", async () => { const handler = new InstallHandler(); const params = getIHandlerParametersObject(); - params.arguments.plugin = ["@public/sample1","@private/sample1","path/to/dir","path/to/dir/file.tgz"]; - mocks.getScopeRegistry.mockReturnValueOnce("publicRegistryUrl" as any); - mocks.getScopeRegistry.mockReturnValueOnce("privateRegistryUrl" as any); - jest.spyOn(IO, 'isDir').mockReturnValue(true); + params.arguments.plugin = ["@public/sample1", "@private/sample1", "path/to/dir", "path/to/dir/file.tgz"]; + mocks.getRegistry.mockReturnValueOnce(packageRegistry2 as any); try{ await handler.process(params); @@ -444,10 +440,10 @@ describe("Plugin Management Facility install handler", () => { expect(e).toBeUndefined(); } - expect(mocks.install).toHaveBeenCalledWith("@public/sample1","publicRegistryUrl"); - expect(mocks.install).toHaveBeenCalledWith("@private/sample1","privateRegistryUrl"); - expect(mocks.install).toHaveBeenCalledWith("path/to/dir","path/to/dir"); - expect(mocks.install).toHaveBeenCalledWith("path/to/dir/file.tgz","path/to/dir/file.tgz"); + expect(mocks.install).toHaveBeenCalledWith("@public/sample1", packageRegistry2); + expect(mocks.install).toHaveBeenCalledWith("@private/sample1", packageRegistry2); + expect(mocks.install).toHaveBeenCalledWith("path/to/dir", packageRegistry2); + expect(mocks.install).toHaveBeenCalledWith("path/to/dir/file.tgz", packageRegistry2); }); }); diff --git a/packages/imperative/src/imperative/__tests__/plugins/utilities/NpmFunctions.unit.test.ts b/packages/imperative/src/imperative/__tests__/plugins/utilities/NpmFunctions.unit.test.ts index a710f1df2c..a0a28b47c1 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/utilities/NpmFunctions.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/utilities/NpmFunctions.unit.test.ts @@ -40,7 +40,7 @@ describe("NpmFunctions", () => { status: 0, stdout: stdoutBuffer } as any); - const result = npmFunctions.installPackages("fakePrefix", fakeRegistry, "samplePlugin"); + const result = npmFunctions.installPackages("samplePlugin", { prefix: "fakePrefix", registry: fakeRegistry }); expect(spawnSyncSpy.mock.calls[0][0]).toBe(npmCmd); expect(spawnSyncSpy.mock.calls[0][1]).toEqual(expect.arrayContaining(["install", "samplePlugin"])); expect(spawnSyncSpy.mock.calls[0][1]).toEqual(expect.arrayContaining(["--prefix", "fakePrefix"])); 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 9fc5aace6a..281c8932dc 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 @@ -133,8 +133,8 @@ describe("PMF: Install Interface", () => { * @param {string} expectedRegistry The registry that should be sent to npm install */ const wasNpmInstallCallValid = (expectedPackage: string, expectedRegistry: string, updateSchema?: boolean) => { - expect(mocks.installPackages).toHaveBeenCalledWith(PMFConstants.instance.PLUGIN_INSTALL_LOCATION, - expectedRegistry, expectedPackage); + expect(mocks.installPackages).toHaveBeenCalledWith(expectedPackage, + { prefix: PMFConstants.instance.PLUGIN_INSTALL_LOCATION, registry: expectedRegistry }); shouldUpdateSchema(updateSchema ?? true); }; diff --git a/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/update.unit.test.ts b/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/update.unit.test.ts index f873f360e7..37c6500de6 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/update.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/update.unit.test.ts @@ -65,8 +65,8 @@ describe("PMF: update Interface", () => { * the pipe sent to spawnSync stdio option. */ const wasNpmInstallCallValid = (expectedPackage: string, expectedRegistry: string) => { - expect(mocks.installPackages).toHaveBeenCalledWith(PMFConstants.instance.PLUGIN_INSTALL_LOCATION, - expectedRegistry, expectedPackage); + expect(mocks.installPackages).toHaveBeenCalledWith(expectedPackage, + { prefix: PMFConstants.instance.PLUGIN_INSTALL_LOCATION, registry: expectedRegistry }); }; describe("Basic update", () => { diff --git a/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts b/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts index 9f36fa32c7..7c002b4767 100644 --- a/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts +++ b/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts @@ -20,17 +20,14 @@ import { readFileSync } from "jsonfile"; import { ImperativeConfig, TextUtils } from "../../../../../utilities"; import { ImperativeError } from "../../../../../error"; import { runValidatePlugin } from "../../utilities/runValidatePlugin"; -import { - getRegistry, - getScopeRegistry, - npmLogin, -} from "../../utilities/NpmFunctions"; -import { IO } from "../../../../../io"; +import { getRegistry, npmLogin } from "../../utilities/NpmFunctions"; + /** * The install command handler for cli plugin install. * * @see {installDefinition} - */ export default class InstallHandler implements ICommandHandler { + */ +export default class InstallHandler implements ICommandHandler { /** * A logger for this class * @@ -72,90 +69,57 @@ import { IO } from "../../../../../io"; * * @throws {ImperativeError} */ - - private locationTypeTest(plugin: string){ - let isDirTest: boolean; - let installRegistry = getRegistry(); - try { - isDirTest = IO.isDir(plugin); - } catch (e) { - isDirTest = false; - } - - if (plugin.startsWith("@")) { - installRegistry = getScopeRegistry( - plugin.split("/")[0].substring(1) - ).replace("\n", ""); - } else if ( - plugin.substring(plugin.lastIndexOf(".") + 1) === "tgz" || - isDirTest - ) { - installRegistry = plugin; - } - return installRegistry; - } - public async process(params: IHandlerParameters): Promise { const chalk = TextUtils.chalk; - this.console.debug( - `Root Directory: ${PMFConstants.instance.PLUGIN_INSTALL_LOCATION}` - ); + this.console.debug(`Root Directory: ${PMFConstants.instance.PLUGIN_INSTALL_LOCATION}`); - if ( - params.arguments.plugin != null && - params.arguments.plugin.length > 0 && - typeof params.arguments.file !== "undefined" - ) { + if (params.arguments.plugin != null && params.arguments.plugin.length > 0 && typeof params.arguments.file !== "undefined") { throw new ImperativeError({ - msg: - `Option ${chalk.yellow.bold( - "--file" - )} can not be specified if positional ${chalk.yellow.bold( - "package..." - )} is as well. ` + `They are mutually exclusive.`, + msg: `Option ${chalk.yellow.bold("--file")} can not be specified if positional ${chalk.yellow.bold("package...")} is as well. ` + + `They are mutually exclusive.`, }); } else { try { - let installRegistry = - params.arguments.registry ?? - getRegistry().replace("\n", ""); + let installRegistry: any; + + // Get the registry to install to + if (typeof params.arguments.registry === "undefined") { + installRegistry = getRegistry().replace("\n", ""); + } else { + installRegistry = params.arguments.registry; + if (params.arguments.login) { + npmLogin(installRegistry); + } + } + + params.response.console.log( + "Plug-ins within the Imperative CLI Framework can legitimately gain\n" + + `control of the ${ImperativeConfig.instance.rootCommandName} CLI application ` + + "during the execution of every command.\n" + + "Install 3rd party plug-ins at your own risk.\n" + ); + + params.response.console.log("Location = " + installRegistry); // This section determines which npm logic needs to take place - if ( - params.arguments.plugin == null || - params.arguments.plugin.length === 0 - ) { - const configFile = - typeof params.arguments.file === "undefined" - ? PMFConstants.instance.PLUGIN_JSON - : resolve(params.arguments.file); + if (params.arguments.plugin == null || params.arguments.plugin.length === 0) { + const configFile = typeof params.arguments.file === "undefined" ? + PMFConstants.instance.PLUGIN_JSON : resolve(params.arguments.file); - this.console.debug( - "Need to install using plugins.json file" - ); + this.console.debug("Need to install using plugins.json file"); this.console.debug(`Using config file: ${configFile}`); // Attempt to load that file and formulate the corresponding package const packageJson: IPluginJson = readFileSync(configFile); if (Object.keys(packageJson).length === 0) { - params.response.console.log( - "No packages were found in " + - configFile + - ", so no plugins were installed." - ); + params.response.console.log("No packages were found in " + configFile + ", so no plugins were installed."); return; } for (const packageName in packageJson) { - if ( - Object.prototype.hasOwnProperty.call( - packageJson, - packageName - ) - ) { - const packageInfo: IPluginJsonObject = - packageJson[packageName]; + if (Object.prototype.hasOwnProperty.call(packageJson, packageName)) { + const packageInfo: IPluginJsonObject = packageJson[packageName]; // Registry is typed as optional in the doc but the function expects it // to be passed. So we'll always set it if it hasn't been done yet. @@ -163,89 +127,33 @@ import { IO } from "../../../../../io"; packageInfo.location = installRegistry; } - installRegistry = this.locationTypeTest(packageInfo.location); - - this.console.debug( - `Installing plugin: ${packageName}` - ); - this.console.debug( - `Package: ${packageInfo.package}` - ); - this.console.debug( - `Location: ${packageInfo.location}` - ); - this.console.debug( - `Version : ${packageInfo.version}` - ); + this.console.debug(`Installing plugin: ${packageName}`); + this.console.debug(`Package: ${packageInfo.package}`); + this.console.debug(`Location: ${packageInfo.location}`); + this.console.debug(`Version : ${packageInfo.version}`); // Get the argument to the install command // For simplicity a / or \ indicates that we are not dealing with an npm package - const packageArgument = - packageInfo.package === packageName - ? `${packageInfo.package}@${packageInfo.version}` - : packageInfo.package; + const packageArgument = packageInfo.package === packageName ? + `${packageInfo.package}@${packageInfo.version}` : packageInfo.package; this.console.debug(`Package: ${packageArgument}`); - params.response.console.log( - "Plug-ins within the Imperative CLI Framework can legitimately gain\n" + - `control of the ${ImperativeConfig.instance.rootCommandName} CLI application ` + - "during the execution of every command.\n" + - "Install 3rd party plug-ins at your own risk.\n" - ); - params.response.console.log( - "Location = " + installRegistry - ); - - params.response.console.log( - "\n_______________________________________________________________" - ); - const pluginName = await install( - packageArgument, - packageInfo.location, - true - ); - params.response.console.log( - "Installed plugin name = '" + pluginName + "'" - ); - params.response.console.log( - runValidatePlugin(pluginName) - ); + params.response.console.log("\n_______________________________________________________________"); + const pluginName = await install(packageArgument, packageInfo.location, true); + params.response.console.log("Installed plugin name = '" + pluginName + "'"); + params.response.console.log(runValidatePlugin(pluginName)); } } - } - for (const plugin of params.arguments.plugin ?? []) { - // Get the registry to install to - if (typeof params.arguments.registry === "undefined") { - installRegistry = this.locationTypeTest(plugin); - } else { - installRegistry = params.arguments.registry; - if (params.arguments.login) { - npmLogin(installRegistry); - } + // write the json file when done if not the plugin json file + } else { + for (const packageString of params.arguments.plugin) { + params.response.console.log("\n_______________________________________________________________"); + const pluginName = await install(packageString, installRegistry); + params.response.console.log("Installed plugin name = '" + pluginName + "'"); + params.response.console.log(runValidatePlugin(pluginName)); } - params.response.console.log( - "Plug-ins within the Imperative CLI Framework can legitimately gain\n" + - `control of the ${ImperativeConfig.instance.rootCommandName} CLI application ` + - "during the execution of every command.\n" + - "Install 3rd party plug-ins at your own risk.\n" - ); - params.response.console.log( - "Location = " + installRegistry - ); - - params.response.console.log( - "\n_______________________________________________________________" - ); - const pluginName = await install( - `${plugin}`, - installRegistry - ); - params.response.console.log( - "Installed plugin name = '" + pluginName + "'" - ); - params.response.console.log(runValidatePlugin(pluginName)); } } catch (e) { let installResultMsg = "Install Failed"; @@ -253,19 +161,13 @@ import { IO } from "../../../../../io"; * give a special message, as per UX request. */ if (e.mMessage) { - const matchArray = e.mMessage.match( - /The intended symlink.*already exists and is not a symbolic link/ - ); + const matchArray = e.mMessage.match(/The intended symlink.*already exists and is not a symbolic link/); if (matchArray !== null) { - installResultMsg = - "Installation completed. However, the plugin incorrectly contains\nits own copy of " + + installResultMsg = "Installation completed. However, the plugin incorrectly contains\nits own copy of " + `${PMFConstants.instance.CLI_CORE_PKG_NAME} or ${PMFConstants.instance.IMPERATIVE_PKG_NAME}.\n` + "Some plugin operations may not work correctly."; - } else if ( - e.mMessage.includes("Failed to create symbolic link") - ) { - installResultMsg = - "Installation completed. However, due to the following error, the plugin will not operate correctly."; + } else if (e.mMessage.includes("Failed to create symbolic link")) { + installResultMsg = "Installation completed. However, due to the following error, the plugin will not operate correctly."; } } throw new ImperativeError({ diff --git a/packages/imperative/src/imperative/src/plugins/doc/INpmInstallArgs.ts b/packages/imperative/src/imperative/src/plugins/doc/INpmInstallArgs.ts new file mode 100644 index 0000000000..c364afa9e7 --- /dev/null +++ b/packages/imperative/src/imperative/src/plugins/doc/INpmInstallArgs.ts @@ -0,0 +1,30 @@ +/* +* 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. +* +*/ + +/** + * Npm config options passed to the install command. + */ +export interface INpmInstallArgs { + /** + * The location to install global packages + */ + prefix: string; + + /** + * The base URL of the npm package registry + */ + registry: string; + + /** + * Allows us to handle scoped registries in the future + */ + [key: string]: string; +} diff --git a/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts b/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts index 0acb75ac1a..d3e26d9874 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts @@ -17,7 +17,7 @@ import { readFileSync } from "jsonfile"; import * as npmPackageArg from "npm-package-arg"; import * as pacote from "pacote"; import { ExecUtils } from "../../../../utilities"; -import { IO } from "../../../../io"; +import { INpmInstallArgs } from "../doc/INpmInstallArgs"; const npmCmd = findNpmOnPath(); /** @@ -40,29 +40,12 @@ export function findNpmOnPath(): string { * @return {string} command response * */ -export function installPackages(prefix: string, registry: string, npmPackage: string): string { +export function installPackages(npmPackage: string, npmArgs: INpmInstallArgs): string { const pipe: StdioOptions = ["pipe", "pipe", process.stderr]; - - const args = [ - "install", npmPackage, - "--prefix", prefix, - "-g" - ]; - let isDirTest: boolean; - - try{ - isDirTest = IO.isDir(registry); - } - catch(e){ - isDirTest = false; - } - - if (!(registry.substring(registry.lastIndexOf(".") + 1) === "tgz") && !isDirTest) { - args.push("--registry",registry); + const args = ["install", npmPackage, "-g", "--legacy-peer-deps"]; + for (const [k, v] of Object.entries(npmArgs)) { + args.push(`--${k}`, v); } - - args.push("--legacy-peer-deps"); - const execOutput = ExecUtils.spawnAndGetOutput(npmCmd, args, { cwd: PMFConstants.instance.PMF_ROOT, stdio: pipe @@ -71,7 +54,6 @@ export function installPackages(prefix: string, registry: string, npmPackage: st return execOutput.toString(); } - /** * Get the registry to install to. * 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 ea6d4030fd..63367fda63 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,7 @@ import { IProfileTypeConfiguration } from "../../../../../profiles"; import * as semver from "semver"; import { ConfigUtils } from "../../../../../config"; import { IExtendersJsonOpts } from "../../../../../config/src/doc/IExtenderOpts"; +import { JsUtils } from "../../../../../utilities"; // Helper function to update extenders.json object during plugin install. // Returns true if the object was updated, and false otherwise @@ -122,7 +123,10 @@ export async function install(packageLocation: string, registry: string, install // Perform the npm install. iConsole.info("Installing packages...this may take some time."); - installPackages(PMFConstants.instance.PLUGIN_INSTALL_LOCATION, registry, npmPackage); + installPackages(npmPackage, { + prefix: PMFConstants.instance.PLUGIN_INSTALL_LOCATION, + registry: JsUtils.isUrl(registry) ? registry : undefined, + }); // We fetch the package name and version of newly installed plugin const packageInfo = await getPackageInfo(npmPackage); diff --git a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/update.ts b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/update.ts index 837f92bf92..c9c4c8f8fc 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/update.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/update.ts @@ -12,6 +12,7 @@ import { PMFConstants } from "../PMFConstants"; import { Logger } from "../../../../../logger"; import { getPackageInfo, installPackages } from "../NpmFunctions"; +import { JsUtils } from "../../../../../utilities"; /** * @TODO - allow multiple packages to be updated? @@ -31,7 +32,10 @@ export async function update(packageName: string, registry: string) { // NOTE: Using npm install in order to retrieve the version which may be updated iConsole.info("updating package...this may take some time."); - installPackages(PMFConstants.instance.PLUGIN_INSTALL_LOCATION, registry, npmPackage); + installPackages(npmPackage, { + prefix: PMFConstants.instance.PLUGIN_INSTALL_LOCATION, + registry: JsUtils.isUrl(registry) ? registry : undefined, + }); // We fetch the package version of newly installed plugin const packageInfo = await getPackageInfo(npmPackage); From f9d08277920026c1d36b5a5399fe08d48ab7d307 Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Wed, 23 Oct 2024 12:29:47 -0400 Subject: [PATCH 02/10] WIP Add NpmRegistryInfo class to handle package scopes Signed-off-by: Timothy Johnson --- .../plugins/cmd/install/install.handler.ts | 25 ++--- .../src/plugins/cmd/update/update.handler.ts | 16 ++- .../src/plugins/doc/INpmInstallArgs.ts | 2 +- .../src/plugins/utilities/NpmFunctions.ts | 99 ++++++++++++------- .../utilities/npm-interface/install.ts | 15 ++- .../plugins/utilities/npm-interface/update.ts | 9 +- 6 files changed, 92 insertions(+), 74 deletions(-) diff --git a/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts b/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts index 7c002b4767..0d3f43885c 100644 --- a/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts +++ b/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts @@ -20,7 +20,7 @@ import { readFileSync } from "jsonfile"; import { ImperativeConfig, TextUtils } from "../../../../../utilities"; import { ImperativeError } from "../../../../../error"; import { runValidatePlugin } from "../../utilities/runValidatePlugin"; -import { getRegistry, npmLogin } from "../../utilities/NpmFunctions"; +import { NpmRegistryInfo } from "../../utilities/NpmFunctions"; /** * The install command handler for cli plugin install. @@ -80,16 +80,11 @@ export default class InstallHandler implements ICommandHandler { }); } else { try { - let installRegistry: any; + const registryInfo = new NpmRegistryInfo(params.arguments.registry); // Get the registry to install to - if (typeof params.arguments.registry === "undefined") { - installRegistry = getRegistry().replace("\n", ""); - } else { - installRegistry = params.arguments.registry; - if (params.arguments.login) { - npmLogin(installRegistry); - } + if (params.arguments.registry != null && params.arguments.login) { + registryInfo.npmLogin(); } params.response.console.log( @@ -99,7 +94,7 @@ export default class InstallHandler implements ICommandHandler { "Install 3rd party plug-ins at your own risk.\n" ); - params.response.console.log("Location = " + installRegistry); + params.response.console.log("Location = " + registryInfo.location); // This section determines which npm logic needs to take place if (params.arguments.plugin == null || params.arguments.plugin.length === 0) { @@ -123,9 +118,8 @@ export default class InstallHandler implements ICommandHandler { // Registry is typed as optional in the doc but the function expects it // to be passed. So we'll always set it if it hasn't been done yet. - if (!packageInfo.location) { - packageInfo.location = installRegistry; - } + registryInfo.setPackage(packageInfo); + packageInfo.location ??= registryInfo.location; this.console.debug(`Installing plugin: ${packageName}`); this.console.debug(`Package: ${packageInfo.package}`); @@ -140,7 +134,7 @@ export default class InstallHandler implements ICommandHandler { this.console.debug(`Package: ${packageArgument}`); params.response.console.log("\n_______________________________________________________________"); - const pluginName = await install(packageArgument, packageInfo.location, true); + const pluginName = await install(packageArgument, registryInfo, true); params.response.console.log("Installed plugin name = '" + pluginName + "'"); params.response.console.log(runValidatePlugin(pluginName)); } @@ -150,7 +144,8 @@ export default class InstallHandler implements ICommandHandler { } else { for (const packageString of params.arguments.plugin) { params.response.console.log("\n_______________________________________________________________"); - const pluginName = await install(packageString, installRegistry); + registryInfo.setPackage(packageString); + const pluginName = await install(packageString, registryInfo); params.response.console.log("Installed plugin name = '" + pluginName + "'"); params.response.console.log(runValidatePlugin(pluginName)); } diff --git a/packages/imperative/src/imperative/src/plugins/cmd/update/update.handler.ts b/packages/imperative/src/imperative/src/plugins/cmd/update/update.handler.ts index 551af090b4..ae0cfc5c12 100644 --- a/packages/imperative/src/imperative/src/plugins/cmd/update/update.handler.ts +++ b/packages/imperative/src/imperative/src/plugins/cmd/update/update.handler.ts @@ -17,7 +17,7 @@ import { ImperativeError } from "../../../../../error"; import { TextUtils } from "../../../../../utilities"; import { IPluginJson } from "../../doc/IPluginJson"; import { readFileSync, writeFileSync } from "jsonfile"; -import { npmLogin } from "../../utilities/NpmFunctions"; +import { NpmRegistryInfo } from "../../utilities/NpmFunctions"; /** * The update command handler for cli plugin install. @@ -52,7 +52,7 @@ export default class UpdateHandler implements ICommandHandler { this.console.debug(`Root Directory: ${PMFConstants.instance.PLUGIN_INSTALL_LOCATION}`); const plugin: string = params.arguments.plugin; - let registry = params.arguments.registry; + const registryInfo = new NpmRegistryInfo(params.arguments.registry); if (params.arguments.plugin == null || params.arguments.plugin.length === 0) { throw new ImperativeError({ @@ -63,8 +63,8 @@ export default class UpdateHandler implements ICommandHandler { iConsole.debug("Reading in the current configuration."); const installedPlugins: IPluginJson = readFileSync(PMFConstants.instance.PLUGIN_JSON); - if (params.arguments.login) { - npmLogin(registry); + if (params.arguments.registry != null && params.arguments.login) { + registryInfo.npmLogin(); } if (Object.prototype.hasOwnProperty.call(installedPlugins, plugin)) { @@ -76,12 +76,10 @@ export default class UpdateHandler implements ICommandHandler { // as package may not match the plugin value. This is true for plugins installed by // folder location. Example: plugin 'imperative-sample-plugin' installed from ../imperative-plugins packageName = installedPlugins[pluginName].package; - if (registry === undefined) { - registry = installedPlugins[pluginName].location; - } + registryInfo.setPackage(installedPlugins[pluginName]); // Call update which returns the plugin's version so plugins.json can be updated - installedPlugins[pluginName].version = await update(packageName, registry); - installedPlugins[pluginName].location = registry; // update in case it changed + installedPlugins[pluginName].version = await update(packageName, registryInfo); + installedPlugins[pluginName].location = registryInfo.location; // update in case it changed writeFileSync(PMFConstants.instance.PLUGIN_JSON, installedPlugins, { spaces: 2 diff --git a/packages/imperative/src/imperative/src/plugins/doc/INpmInstallArgs.ts b/packages/imperative/src/imperative/src/plugins/doc/INpmInstallArgs.ts index c364afa9e7..873740303e 100644 --- a/packages/imperative/src/imperative/src/plugins/doc/INpmInstallArgs.ts +++ b/packages/imperative/src/imperative/src/plugins/doc/INpmInstallArgs.ts @@ -21,7 +21,7 @@ export interface INpmInstallArgs { /** * The base URL of the npm package registry */ - registry: string; + registry?: string; /** * Allows us to handle scoped registries in the future diff --git a/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts b/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts index d3e26d9874..78c0ca4377 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts @@ -16,8 +16,9 @@ import { StdioOptions } from "child_process"; import { readFileSync } from "jsonfile"; import * as npmPackageArg from "npm-package-arg"; import * as pacote from "pacote"; -import { ExecUtils } from "../../../../utilities"; +import { ExecUtils, JsUtils } from "../../../../utilities"; import { INpmInstallArgs } from "../doc/INpmInstallArgs"; +import { IPluginJsonObject } from "../doc/IPluginJsonObject"; const npmCmd = findNpmOnPath(); /** @@ -44,7 +45,9 @@ export function installPackages(npmPackage: string, npmArgs: INpmInstallArgs): s const pipe: StdioOptions = ["pipe", "pipe", process.stderr]; const args = ["install", npmPackage, "-g", "--legacy-peer-deps"]; for (const [k, v] of Object.entries(npmArgs)) { - args.push(`--${k}`, v); + if (v != null) { + args.push(`--${k}`, v); + } } const execOutput = ExecUtils.spawnAndGetOutput(npmCmd, args, { cwd: PMFConstants.instance.PMF_ROOT, @@ -54,39 +57,6 @@ export function installPackages(npmPackage: string, npmArgs: INpmInstallArgs): s return execOutput.toString(); } -/** - * Get the registry to install to. - * - * @return {string} - */ -export function getRegistry(): string { - const execOutput = ExecUtils.spawnAndGetOutput(npmCmd, [ "config", "get", "registry" ]); - return execOutput.toString(); -} - -export function getScopeRegistry(scope: string): string { - const execOutput = ExecUtils.spawnAndGetOutput(npmCmd, [ "config", "get", `@${scope}:registry` ]); - if(execOutput.toString().trim() === 'undefined') return getRegistry(); - return execOutput.toString(); -} - -/** - * NPM login to be able to install from secure registry - * @param {string} registry The npm registry to install from. - */ -export function npmLogin(registry: string) { - ExecUtils.spawnAndGetOutput(npmCmd, - [ - "login", - "--registry", registry, - "--always-auth", - "--auth-type=legacy" - ], { - stdio: [0,1,2] - } - ); -} - /** * Fetch name and version of NPM package that was installed * @param pkgSpec The package name as specified on NPM install @@ -101,3 +71,62 @@ export async function getPackageInfo(pkgSpec: string): Promise<{ name: string, v return pacote.manifest(pkgSpec); } } + +export class NpmRegistryInfo { + private defaultRegistry?: string; + private defaultRegistryScope?: string; + + constructor(private customRegistry?: string) { + if (customRegistry == null) { + this.defaultRegistry = this.getRegistry(); + } + } + + private getRegistry(): string { + const execOutput = ExecUtils.spawnAndGetOutput(npmCmd, ["config", "get", "registry"]); + return execOutput.toString().replace("\n", ""); + } + + private getScopeRegistry(scope?: string): string { + const execOutput = ExecUtils.spawnAndGetOutput(npmCmd, ["config", "get", `${scope ?? this.defaultRegistryScope}:registry`]); + if (execOutput.toString().trim() === "undefined") return this.getRegistry(); + return execOutput.toString().replace("\n", ""); + } + + public get location(): string { + return this.customRegistry ?? this.defaultRegistry; + } + + /** + * NPM login to be able to install from secure registry + * @param {string} registry The npm registry to install from. + */ + public npmLogin() { + ExecUtils.spawnAndGetOutput(npmCmd, + [ + "login", + "--registry", this.customRegistry, + "--always-auth", + "--auth-type=legacy" + ], { + stdio: [0,1,2] + } + ); + } + + public buildRegistryArgs(): Partial { + const registrySpec = this.defaultRegistryScope ? `${this.defaultRegistryScope}:registry` : "registry"; + return this.customRegistry != null ? { [registrySpec]: this.location } : {}; + } + + public setPackage(packageInfo: string | IPluginJsonObject) { + const packageName = typeof packageInfo === "string" ? packageInfo : packageInfo.package; + const packageScope = packageName.startsWith("@") ? packageName.split("/")[0] : undefined; + if (typeof packageInfo === "string" || packageInfo.location == null) { + this.defaultRegistry = packageScope != null ? this.getScopeRegistry(packageScope) : this.getRegistry(); + } else { + this.defaultRegistry = JsUtils.isUrl(packageInfo.location) ? packageInfo.location : undefined; + this.defaultRegistryScope = packageScope; + } + } +} 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 63367fda63..bb7f22cd44 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 @@ -18,7 +18,7 @@ import { Logger } from "../../../../../logger"; import { IImperativeConfig } from "../../../doc/IImperativeConfig"; import { ImperativeError } from "../../../../../error"; import { IPluginJsonObject } from "../../doc/IPluginJsonObject"; -import { getPackageInfo, installPackages } from "../NpmFunctions"; +import { getPackageInfo, installPackages, NpmRegistryInfo } from "../NpmFunctions"; import { ConfigSchema } from "../../../../../config/src/ConfigSchema"; import { PluginManagementFacility } from "../../PluginManagementFacility"; import { ConfigurationLoader } from "../../../ConfigurationLoader"; @@ -28,7 +28,6 @@ import { IProfileTypeConfiguration } from "../../../../../profiles"; import * as semver from "semver"; import { ConfigUtils } from "../../../../../config"; import { IExtendersJsonOpts } from "../../../../../config/src/doc/IExtenderOpts"; -import { JsUtils } from "../../../../../utilities"; // Helper function to update extenders.json object during plugin install. // Returns true if the object was updated, and false otherwise @@ -75,9 +74,7 @@ export const updateExtendersJson = ( * be converted to an absolute path prior to being passed to the * `npm install` command. * - * @param {string} registry The npm registry to use, this is expected to be passed by every caller - * so if calling functions don't have a registry available, they need - * to get it from npm. + * @param {NpmRegistryInfo} registryInfo The npm registry to use. * * @param {boolean} [installFromFile=false] If installing from a file, the package location is * automatically interpreted as an absolute location. @@ -88,7 +85,7 @@ export const updateExtendersJson = ( * it. * @returns {string} The name of the plugin. */ -export async function install(packageLocation: string, registry: string, installFromFile = false) { +export async function install(packageLocation: string, registryInfo: NpmRegistryInfo, installFromFile = false) { const iConsole = Logger.getImperativeLogger(); let npmPackage = packageLocation; @@ -118,14 +115,14 @@ export async function install(packageLocation: string, registry: string, install } try { - iConsole.debug(`Installing from registry ${registry}`); + iConsole.debug(`Installing from registry ${registryInfo.location}`); // Perform the npm install. iConsole.info("Installing packages...this may take some time."); installPackages(npmPackage, { prefix: PMFConstants.instance.PLUGIN_INSTALL_LOCATION, - registry: JsUtils.isUrl(registry) ? registry : undefined, + ...registryInfo.buildRegistryArgs(), }); // We fetch the package name and version of newly installed plugin @@ -154,7 +151,7 @@ export async function install(packageLocation: string, registry: string, install const newPlugin: IPluginJsonObject = { package: npmPackage, - location: registry, + location: registryInfo.location, version: packageVersion }; iConsole.debug("Updating the current configuration with new plugin:\n" + diff --git a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/update.ts b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/update.ts index c9c4c8f8fc..af4ae19de3 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/update.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/update.ts @@ -11,8 +11,7 @@ import { PMFConstants } from "../PMFConstants"; import { Logger } from "../../../../../logger"; -import { getPackageInfo, installPackages } from "../NpmFunctions"; -import { JsUtils } from "../../../../../utilities"; +import { getPackageInfo, installPackages, NpmRegistryInfo } from "../NpmFunctions"; /** * @TODO - allow multiple packages to be updated? @@ -20,10 +19,10 @@ import { JsUtils } from "../../../../../utilities"; * * @param {string} packageName A package name. This value is a valid npm package name. * - * @param {string} registry The npm registry. + * @param {NpmRegistryInfo} registryInfo The npm registry to use. * */ -export async function update(packageName: string, registry: string) { +export async function update(packageName: string, registryInfo: NpmRegistryInfo) { const iConsole = Logger.getImperativeLogger(); const npmPackage = packageName; @@ -34,7 +33,7 @@ export async function update(packageName: string, registry: string) { installPackages(npmPackage, { prefix: PMFConstants.instance.PLUGIN_INSTALL_LOCATION, - registry: JsUtils.isUrl(registry) ? registry : undefined, + ...registryInfo.buildRegistryArgs(), }); // We fetch the package version of newly installed plugin From fd256669ff952eff58e1806724f2298a5b066f89 Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Wed, 23 Oct 2024 15:37:04 -0400 Subject: [PATCH 03/10] Start fixing unit tests that broke Signed-off-by: Timothy Johnson --- .../npm-interface/install.unit.test.ts | 158 ++++++++---------- .../npm-interface/uninstall.unit.test.ts | 24 +-- .../npm-interface/update.unit.test.ts | 76 ++++----- .../src/plugins/utilities/NpmFunctions.ts | 6 +- 4 files changed, 123 insertions(+), 141 deletions(-) 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 281c8932dc..3162751b83 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 @@ -10,35 +10,12 @@ */ /* eslint-disable jest/expect-expect */ -import Mock = jest.Mock; let expectedVal: any; let returnedVal: any; jest.mock("cross-spawn"); jest.mock("jsonfile"); -jest.mock("find-up"); jest.mock("../../../../src/plugins/utilities/PMFConstants"); -jest.mock("../../../../src/plugins/PluginManagementFacility"); -jest.mock("../../../../src/ConfigurationLoader"); -jest.mock("../../../../src/UpdateImpConfig"); -jest.mock("../../../../../config/src/ConfigSchema"); -jest.mock("../../../../../logger"); -jest.mock("../../../../../cmd/src/response/CommandResponse"); -jest.mock("../../../../../cmd/src/response/HandlerResponse"); -jest.mock("../../../../src/plugins/utilities/NpmFunctions"); -jest.doMock("path", () => { - const originalPath = jest.requireActual("path"); - return { - ...originalPath, - resolve: (...path: string[]) => { - if (path[0] == expectedVal) { - return returnedVal ? returnedVal : expectedVal; - } else { - return originalPath.resolve(...path); - } - } - }; -}); import { Console } from "../../../../../console"; import { ImperativeError } from "../../../../../error"; @@ -48,9 +25,10 @@ import { IPluginJson } from "../../../../src/plugins/doc/IPluginJson"; import { IPluginJsonObject } from "../../../../src/plugins/doc/IPluginJsonObject"; import { Logger } from "../../../../../logger"; import { PMFConstants } from "../../../../src/plugins/utilities/PMFConstants"; -import { readFileSync, writeFileSync } from "jsonfile"; -import { sync } from "find-up"; -import { getPackageInfo, installPackages } from "../../../../src/plugins/utilities/NpmFunctions"; +import * as crossSpawn from "cross-spawn"; +import * as jsonfile from "jsonfile"; +import * as findUp from "find-up"; +import * as npmFns from "../../../../src/plugins/utilities/NpmFunctions"; import { ConfigSchema } from "../../../../../config/src/ConfigSchema"; import { PluginManagementFacility } from "../../../../src/plugins/PluginManagementFacility"; import { AbstractPluginLifeCycle } from "../../../../src/plugins/AbstractPluginLifeCycle"; @@ -73,17 +51,19 @@ describe("PMF: Install Interface", () => { // Objects created so types are correct. const pmfI = PluginManagementFacility.instance; const mocks = { - installPackages: installPackages as unknown as Mock, - readFileSync: readFileSync as Mock, - writeFileSync: writeFileSync as Mock, - sync: sync as unknown as Mock, - getPackageInfo: getPackageInfo as unknown as Mock, - ConfigSchema_updateSchema: ConfigSchema.updateSchema as unknown as Mock, - PMF_requirePluginModuleCallback: pmfI.requirePluginModuleCallback as Mock, - ConfigurationLoader_load: ConfigurationLoader.load as Mock, - UpdateImpConfig_addProfiles: UpdateImpConfig.addProfiles as Mock, - path: path as unknown as Mock, - ConfigSchema_loadSchema: jest.spyOn(ConfigSchema, "loadSchema"), + installPackages: jest.spyOn(npmFns, "installPackages"), + readFileSync: jest.spyOn(jsonfile, "readFileSync"), + writeFileSync: jest.spyOn(jsonfile, "writeFileSync"), + findUpSync: jest.spyOn(findUp, "sync"), + getPackageInfo: jest.spyOn(npmFns, "getPackageInfo"), + requirePluginModuleCallback: jest.spyOn(pmfI, "requirePluginModuleCallback"), + loadConfiguration: jest.spyOn(ConfigurationLoader, "load"), + addProfiles: jest.spyOn(UpdateImpConfig, "addProfiles"), + spawnSync: jest.spyOn(crossSpawn, "sync"), + ConfigSchema: { + loadSchema: jest.spyOn(ConfigSchema, "loadSchema"), + updateSchema: jest.spyOn(ConfigSchema, "updateSchema") + }, ConfigUtils: { readExtendersJson: jest.spyOn(ConfigUtils, "readExtendersJson"), writeExtendersJson: jest.spyOn(ConfigUtils, "writeExtendersJson") @@ -101,15 +81,13 @@ describe("PMF: Install Interface", () => { returnedVal = undefined; // This needs to be mocked before running install - (Logger.getImperativeLogger as unknown as Mock).mockReturnValue(new Logger(new Console()) as any); + jest.spyOn(Logger, "getImperativeLogger").mockReturnValue(new Logger(new Console())); /* Since install() adds new plugins into the value returned from - * readFileSyc(plugins.json), we must reset readFileSync to return an empty set before each test. + * readFileSync(plugins.json), we must reset readFileSync to return an empty set before each test. */ - mocks.readFileSync.mockReturnValue({} as any); - 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.readFileSync.mockReturnValue({}); + mocks.findUpSync.mockReturnValue("fake_find-up_sync_result"); mocks.ConfigUtils.readExtendersJson.mockReturnValue({ profileTypes: { "zosmf": { @@ -118,8 +96,12 @@ describe("PMF: Install Interface", () => { } }); mocks.ConfigUtils.writeExtendersJson.mockImplementation(); - mocks.ConfigSchema_loadSchema.mockReturnValue([mockTypeConfig]); - mocks.ConfigurationLoader_load.mockReturnValue({ profiles: [mockTypeConfig] } as any); + mocks.ConfigSchema.loadSchema.mockReturnValue([mockTypeConfig]); + mocks.loadConfiguration.mockReturnValue({ profiles: [mockTypeConfig] }); + mocks.spawnSync.mockReturnValue({ + status: 0, + stdout: Buffer.from(`+ ${packageName}`) + } as any); }); afterAll(() => { @@ -142,16 +124,16 @@ describe("PMF: Install Interface", () => { * Validates that plugins install call updates the global schema. */ const shouldUpdateSchema = (shouldUpdate: boolean) => { - expect(mocks.PMF_requirePluginModuleCallback).toHaveBeenCalledTimes(1); - expect(mocks.ConfigurationLoader_load).toHaveBeenCalledTimes(1); + expect(mocks.requirePluginModuleCallback).toHaveBeenCalledTimes(1); + expect(mocks.loadConfiguration).toHaveBeenCalledTimes(1); if (shouldUpdate) { - expect(mocks.UpdateImpConfig_addProfiles).toHaveBeenCalledTimes(1); - expect(mocks.ConfigSchema_updateSchema).toHaveBeenCalledTimes(1); - expect(mocks.ConfigSchema_updateSchema).toHaveBeenCalledWith(expect.objectContaining({ layer: "global" })); + expect(mocks.addProfiles).toHaveBeenCalledTimes(1); + expect(mocks.ConfigSchema.updateSchema).toHaveBeenCalledTimes(1); + expect(mocks.ConfigSchema.updateSchema).toHaveBeenCalledWith(expect.objectContaining({ layer: "global" })); } else { - expect(mocks.UpdateImpConfig_addProfiles).not.toHaveBeenCalled(); - expect(mocks.ConfigSchema_updateSchema).not.toHaveBeenCalled(); + expect(mocks.addProfiles).not.toHaveBeenCalled(); + expect(mocks.ConfigSchema.updateSchema).not.toHaveBeenCalled(); } }; @@ -182,9 +164,17 @@ describe("PMF: Install Interface", () => { describe("Basic install", () => { beforeEach(() => { - mocks.getPackageInfo.mockResolvedValue({ name: packageName, version: packageVersion } as never); + mocks.getPackageInfo.mockResolvedValue({ name: packageName, version: packageVersion }); + mocks.ConfigSchema.updateSchema.mockImplementation(); jest.spyOn(fs, "existsSync").mockReturnValue(true); - jest.spyOn(path, "normalize").mockReturnValue("testing"); + // jest.spyOn(path, "normalize").mockReturnValue("testing"); + jest.spyOn(path, "resolve").mockImplementation((...paths) => { + if (paths[0] == expectedVal) { + return returnedVal ? returnedVal : expectedVal; + } else { + return jest.requireActual("path").resolve(...paths); + } + }); jest.spyOn(fs, "lstatSync").mockReturnValue({ isSymbolicLink: jest.fn().mockReturnValue(true) } as any); @@ -192,7 +182,7 @@ describe("PMF: Install Interface", () => { it("should install from the npm registry", async () => { setResolve(packageName); - await install(packageName, packageRegistry); + await install(packageName, new npmFns.NpmRegistryInfo(packageRegistry)); // Validate the install wasNpmInstallCallValid(packageName, packageRegistry); @@ -208,7 +198,7 @@ describe("PMF: Install Interface", () => { jest.spyOn(path, "isAbsolute").mockReturnValueOnce(true); setResolve(rootFile); - await install(rootFile, packageRegistry); + await install(rootFile, new npmFns.NpmRegistryInfo(packageRegistry)); // Validate the install wasNpmInstallCallValid(rootFile, packageRegistry); @@ -224,7 +214,7 @@ describe("PMF: Install Interface", () => { jest.spyOn(path, "isAbsolute").mockReturnValueOnce(true); setResolve(rootFile); - await install(rootFile, packageRegistry); + await install(rootFile, new npmFns.NpmRegistryInfo(packageRegistry)); // Validate the install wasNpmInstallCallValid(rootFile, packageRegistry); @@ -251,7 +241,7 @@ describe("PMF: Install Interface", () => { // Call the install function setResolve(relativePath, absolutePath); - await install(relativePath, packageRegistry); + await install(relativePath, new npmFns.NpmRegistryInfo(packageRegistry)); // Validate results wasNpmInstallCallValid(absolutePath, packageRegistry); @@ -269,7 +259,7 @@ describe("PMF: Install Interface", () => { // mocks.isUrl.mockReturnValue(true); - await install(installUrl, packageRegistry); + await install(installUrl, new npmFns.NpmRegistryInfo(packageRegistry)); wasNpmInstallCallValid(installUrl, packageRegistry); wasWriteFileSyncCallValid({}, packageName, { @@ -280,9 +270,9 @@ describe("PMF: Install Interface", () => { }); it("should install plugin that does not define profiles", async () => { - mocks.ConfigurationLoader_load.mockReturnValueOnce({} as any); + mocks.loadConfiguration.mockReturnValueOnce({}); setResolve(packageName); - await install(packageName, packageRegistry); + await install(packageName, new npmFns.NpmRegistryInfo(packageRegistry)); // Validate the install wasNpmInstallCallValid(packageName, packageRegistry, false); @@ -295,20 +285,20 @@ describe("PMF: Install Interface", () => { }); // end Basic install describe("Advanced install", () => { - it("should write even when install from file is true", async () => { + fit("should write even when install from file is true", async () => { // This test is constructed in such a way that all if conditions with installFromFile // are validated to have been called or not. const location = "/this/should/not/change"; jest.spyOn(path, "isAbsolute").mockReturnValueOnce(false); - jest.spyOn(fs, "existsSync").mockReturnValueOnce(true); - mocks.getPackageInfo.mockResolvedValue({ name: packageName, version: packageVersion } as never); - jest.spyOn(path, "normalize").mockReturnValue("testing"); + jest.spyOn(fs, "existsSync").mockReturnValue(true); + mocks.getPackageInfo.mockResolvedValue({ name: packageName, version: packageVersion }); + // jest.spyOn(path, "normalize").mockReturnValue("testing"); jest.spyOn(fs, "lstatSync").mockReturnValue({ isSymbolicLink: jest.fn().mockReturnValue(true) } as any); - await install(location, packageRegistry, true); + await install(location, new npmFns.NpmRegistryInfo(packageRegistry), true); wasNpmInstallCallValid(location, packageRegistry); expect(mocks.writeFileSync).toHaveBeenCalled(); @@ -319,7 +309,7 @@ describe("PMF: Install Interface", () => { const semverPackage = `${packageName}@${semverVersion}`; jest.spyOn(fs, "existsSync").mockReturnValueOnce(true); - jest.spyOn(path, "normalize").mockReturnValue("testing"); + // jest.spyOn(path, "normalize").mockReturnValue("testing"); jest.spyOn(fs, "lstatSync").mockReturnValue({ isSymbolicLink: jest.fn().mockReturnValue(true) } as any); @@ -329,11 +319,11 @@ describe("PMF: Install Interface", () => { jest.spyOn(path, "isAbsolute").mockReturnValueOnce(true); // This is valid under semver ^1.5.2 - mocks.getPackageInfo.mockResolvedValue({ name: packageName, version: "1.5.16" } as never); + mocks.getPackageInfo.mockResolvedValue({ name: packageName, version: "1.5.16" }); // Call the install setResolve(semverPackage); - await install(semverPackage, packageRegistry); + await install(semverPackage, new npmFns.NpmRegistryInfo(packageRegistry)); // Test that shit happened wasNpmInstallCallValid(semverPackage, packageRegistry); @@ -354,16 +344,16 @@ describe("PMF: Install Interface", () => { } }; - mocks.getPackageInfo.mockResolvedValue({ name: packageName, version: packageVersion } as never); + mocks.getPackageInfo.mockResolvedValue({ name: packageName, version: packageVersion }); jest.spyOn(fs, "existsSync").mockReturnValueOnce(true); - jest.spyOn(path, "normalize").mockReturnValue("testing"); + // jest.spyOn(path, "normalize").mockReturnValue("testing"); jest.spyOn(fs, "lstatSync").mockReturnValue({ isSymbolicLink: jest.fn().mockReturnValue(true) } as any); - mocks.readFileSync.mockReturnValue(oneOldPlugin as any); + mocks.readFileSync.mockReturnValue(oneOldPlugin); setResolve(packageName); - await install(packageName, packageRegistry); + await install(packageName, new npmFns.NpmRegistryInfo(packageRegistry)); wasNpmInstallCallValid(packageName, packageRegistry); wasWriteFileSyncCallValid(oneOldPlugin, packageName, { @@ -373,7 +363,7 @@ describe("PMF: Install Interface", () => { }); }); - describe("Updating the global schema", () => { + xdescribe("Updating the global schema", () => { const expectTestSchemaMgmt = async (opts: { schemaExists: boolean; newProfileType: boolean; @@ -389,20 +379,20 @@ describe("PMF: Install Interface", () => { }; if (opts.newProfileType) { const schema = { ...mockTypeConfig, schema: { ...mockTypeConfig.schema, version: opts.version } }; - mocks.ConfigurationLoader_load.mockReturnValue({ + mocks.loadConfiguration.mockReturnValue({ profiles: [ schema ] - } as any); + }); } - mocks.getPackageInfo.mockResolvedValue({ name: packageName, version: packageVersion } as never); + mocks.getPackageInfo.mockResolvedValue({ name: packageName, version: packageVersion }); jest.spyOn(fs, "existsSync").mockReturnValueOnce(true).mockReturnValueOnce(opts.schemaExists); - jest.spyOn(path, "normalize").mockReturnValue("testing"); + // jest.spyOn(path, "normalize").mockReturnValue("testing"); jest.spyOn(fs, "lstatSync").mockReturnValue({ isSymbolicLink: jest.fn().mockReturnValue(true) } as any); - mocks.readFileSync.mockReturnValue(oneOldPlugin as any); + mocks.readFileSync.mockReturnValue(oneOldPlugin); if (opts.lastVersion) { mocks.ConfigUtils.readExtendersJson.mockReturnValueOnce({ @@ -417,11 +407,11 @@ describe("PMF: Install Interface", () => { } setResolve(packageName); - await install(packageName, packageRegistry); + await install(packageName, new npmFns.NpmRegistryInfo(packageRegistry)); if (opts.schemaExists) { - expect(mocks.ConfigSchema_updateSchema).toHaveBeenCalled(); + expect(mocks.ConfigSchema.updateSchema).toHaveBeenCalled(); } else { - expect(mocks.ConfigSchema_updateSchema).not.toHaveBeenCalled(); + expect(mocks.ConfigSchema.updateSchema).not.toHaveBeenCalled(); } if (opts.version && opts.lastVersion) { @@ -501,7 +491,7 @@ describe("PMF: Install Interface", () => { }); try { - await install("test", "http://www.example.com"); + await install("test", new npmFns.NpmRegistryInfo("http://www.example.com")); } catch (e) { expectedError = e; } @@ -629,7 +619,7 @@ describe("PMF: Install Interface", () => { } catch (err) { thrownErr = err; } - expect(requirePluginModuleCallbackSpy as any).toHaveBeenCalledTimes(1); + expect(requirePluginModuleCallbackSpy).toHaveBeenCalledTimes(1); expect(thrownErr).not.toBeDefined(); expect(postInstallWorked).toBe(true); }); 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 49714cfa17..bc7846c0a3 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 @@ -10,24 +10,18 @@ */ /* eslint-disable jest/expect-expect */ -import Mock = jest.Mock; - jest.mock("cross-spawn"); jest.mock("jsonfile"); jest.mock("../../../../src/plugins/utilities/PMFConstants"); -jest.mock("../../../../../logger"); -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 * as crossSpawn from "cross-spawn"; import { ImperativeError } from "../../../../../error"; import { IPluginJson } from "../../../../src/plugins/doc/IPluginJson"; import { Logger } from "../../../../../logger"; 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, ConfigUtils } from "../../../../../config"; @@ -39,9 +33,9 @@ import { updateAndGetRemovedTypes } from "../../../../src/plugins/utilities/npm- describe("PMF: Uninstall Interface", () => { // Objects created so types are correct. const mocks = { - spawnSync: sync as unknown as Mock, - readFileSync: readFileSync as Mock, - writeFileSync: writeFileSync as Mock + spawnSync: jest.spyOn(crossSpawn, "sync"), + readFileSync: jest.spyOn(jsonfile, "readFileSync"), + writeFileSync: jest.spyOn(jsonfile, "writeFileSync") }; const samplePackageName = "imperative-sample-plugin"; @@ -54,7 +48,7 @@ describe("PMF: Uninstall Interface", () => { jest.resetAllMocks(); // This needs to be mocked before running uninstall - (Logger.getImperativeLogger as unknown as Mock).mockReturnValue(new Logger(new Console()) as any); + jest.spyOn(Logger, "getImperativeLogger").mockReturnValue(new Logger(new Console())); }); afterAll(() => { @@ -127,7 +121,7 @@ describe("PMF: Uninstall Interface", () => { } }; - mocks.readFileSync.mockReturnValue(pluginJsonFile as any); + mocks.readFileSync.mockReturnValue(pluginJsonFile); uninstall(packageName); @@ -151,7 +145,7 @@ describe("PMF: Uninstall Interface", () => { } }; - mocks.readFileSync.mockReturnValue(pluginJsonFile as any); + mocks.readFileSync.mockReturnValue(pluginJsonFile); uninstall(samplePackageName); @@ -192,7 +186,7 @@ describe("PMF: Uninstall Interface", () => { } }; - mocks.readFileSync.mockReturnValue(pluginJsonFile as any); + mocks.readFileSync.mockReturnValue(pluginJsonFile); jest.spyOn(fs, "existsSync").mockReturnValueOnce(true); let caughtError; @@ -244,7 +238,7 @@ describe("PMF: Uninstall Interface", () => { } }; - mocks.readFileSync.mockReturnValue(pluginJsonFile as any); + mocks.readFileSync.mockReturnValue(pluginJsonFile); const blockMocks = getBlockMocks(); if (opts.schemaUpdated) { blockMocks.fs.existsSync.mockReturnValueOnce(true); diff --git a/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/update.unit.test.ts b/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/update.unit.test.ts index 37c6500de6..ef36b5e971 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/update.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/update.unit.test.ts @@ -9,30 +9,24 @@ * */ -import Mock = jest.Mock; - jest.mock("cross-spawn"); jest.mock("jsonfile"); jest.mock("../../../../src/plugins/utilities/PMFConstants"); -jest.mock("../../../../../logger"); -jest.mock("../../../../../cmd/src/response/CommandResponse"); -jest.mock("../../../../../cmd/src/response/HandlerResponse"); -jest.mock("../../../../src/plugins/utilities/NpmFunctions"); import { Console } from "../../../../../console"; import { IPluginJson } from "../../../../src/plugins/doc/IPluginJson"; import { Logger } from "../../../../../logger"; import { PMFConstants } from "../../../../src/plugins/utilities/PMFConstants"; -import { readFileSync } from "jsonfile"; +import * as jsonfile from "jsonfile"; import { update } from "../../../../src/plugins/utilities/npm-interface"; -import { getPackageInfo, installPackages } from "../../../../src/plugins/utilities/NpmFunctions"; +import * as npmFns from "../../../../src/plugins/utilities/NpmFunctions"; describe("PMF: update Interface", () => { // Objects created so types are correct. const mocks = { - installPackages: installPackages as unknown as Mock, - readFileSync: readFileSync as Mock, - getPackageInfo: getPackageInfo as unknown as Mock + installPackages: jest.spyOn(npmFns, "installPackages"), + readFileSync: jest.spyOn(jsonfile, "readFileSync"), + getPackageInfo: jest.spyOn(npmFns, "getPackageInfo") }; const packageName = "pretty-format"; @@ -44,12 +38,12 @@ describe("PMF: update Interface", () => { jest.resetAllMocks(); // This needs to be mocked before running update - (Logger.getImperativeLogger as unknown as Mock).mockReturnValue(new Logger(new Console()) as any); + jest.spyOn(Logger, "getImperativeLogger").mockReturnValue(new Logger(new Console())); /* Since update() adds new plugins into the value returned from - * readFileSyc(plugins.json), we must reset readFileSync to return an empty set before each test. + * readFileSync(plugins.json), we must reset readFileSync to return an empty set before each test. */ - mocks.readFileSync.mockReturnValue({} as any); + mocks.readFileSync.mockReturnValue({}); }); afterAll(() => { @@ -64,51 +58,55 @@ describe("PMF: update Interface", () => { * @param {boolean} [updateFromFile=false] was the update from a file. This affects * the pipe sent to spawnSync stdio option. */ - const wasNpmInstallCallValid = (expectedPackage: string, expectedRegistry: string) => { + const wasNpmInstallCallValid = (expectedPackage: string, expectedRegistry: Record) => { expect(mocks.installPackages).toHaveBeenCalledWith(expectedPackage, - { prefix: PMFConstants.instance.PLUGIN_INSTALL_LOCATION, registry: expectedRegistry }); + { prefix: PMFConstants.instance.PLUGIN_INSTALL_LOCATION, ...expectedRegistry }); }; - describe("Basic update", () => { - it("should update from the npm registry", async () => { + it("should update from the npm registry", async () => { - // value for our plugins.json - const oneOldPlugin: IPluginJson = { - plugin1: { - package: packageName, - location: packageRegistry, - version: packageVersion - } - }; + // value for our plugins.json + const oneOldPlugin: IPluginJson = { + plugin1: { + package: packageName, + location: packageRegistry, + version: packageVersion + } + }; - mocks.getPackageInfo.mockResolvedValue({ name: packageName, version: packageVersion } as never); - mocks.readFileSync.mockReturnValue(oneOldPlugin as any); + mocks.getPackageInfo.mockResolvedValue({ name: packageName, version: packageVersion }); + mocks.readFileSync.mockReturnValue(oneOldPlugin); - const data = await update(packageName, packageRegistry); - expect(data).toEqual(packageVersion); + const registryInfo = new npmFns.NpmRegistryInfo(packageRegistry); + registryInfo.setPackage(oneOldPlugin.plugin1); + const data = await update(packageName, registryInfo); + expect(data).toEqual(packageVersion); - // Validate the update - wasNpmInstallCallValid(packageName, packageRegistry); - }); + // Validate the update + wasNpmInstallCallValid(packageName, { registry: packageRegistry }); }); - it("should update from the npm registry", async () => { + + it("should update from scoped npm registry", async () => { // value for our plugins.json + const scopedPackageName = `@org/${packageName}`; const oneOldPlugin: IPluginJson = { plugin1: { - package: packageName, + package: scopedPackageName, location: packageRegistry, version: packageVersion } }; - mocks.getPackageInfo.mockResolvedValue({ name: packageName, version: packageVersion } as never); - mocks.readFileSync.mockReturnValue(oneOldPlugin as any); + mocks.getPackageInfo.mockResolvedValue({ name: scopedPackageName, version: packageVersion }); + mocks.readFileSync.mockReturnValue(oneOldPlugin); - const data = await update(packageName, packageRegistry); + const registryInfo = new npmFns.NpmRegistryInfo(packageRegistry); + registryInfo.setPackage(oneOldPlugin.plugin1); + const data = await update(scopedPackageName, registryInfo); expect(data).toEqual(packageVersion); // Validate the update - wasNpmInstallCallValid(packageName, packageRegistry); + wasNpmInstallCallValid(scopedPackageName, { "@org:registry": packageRegistry }); }); }); diff --git a/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts b/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts index 78c0ca4377..6abcd92ab4 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts @@ -101,11 +101,11 @@ export class NpmRegistryInfo { * NPM login to be able to install from secure registry * @param {string} registry The npm registry to install from. */ - public npmLogin() { + public npmLogin(registry?: string) { ExecUtils.spawnAndGetOutput(npmCmd, [ "login", - "--registry", this.customRegistry, + "--registry", registry ?? this.customRegistry, "--always-auth", "--auth-type=legacy" ], { @@ -115,7 +115,7 @@ export class NpmRegistryInfo { } public buildRegistryArgs(): Partial { - const registrySpec = this.defaultRegistryScope ? `${this.defaultRegistryScope}:registry` : "registry"; + const registrySpec = this.defaultRegistryScope != null ? `${this.defaultRegistryScope}:registry` : "registry"; return this.customRegistry != null ? { [registrySpec]: this.location } : {}; } From b44a5efefae82826b1151145824be1c4014c8e70 Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Wed, 23 Oct 2024 23:03:55 -0400 Subject: [PATCH 04/10] Refactor NpmFunctions and fix more unit tests Signed-off-by: Timothy Johnson --- .../cmd/install/install.handler.unit.test.ts | 162 ++++++++++-------- .../cmd/list/list.handler.unit.test.ts | 5 +- .../uninstall/uninstall.handler.unit.test.ts | 2 +- .../cmd/update/update.handler.unit.test.ts | 67 ++++---- .../utilities/NpmFunctions.unit.test.ts | 8 +- .../npm-interface/install.unit.test.ts | 58 ++++--- .../npm-interface/uninstall.unit.test.ts | 4 +- .../npm-interface/update.unit.test.ts | 6 +- .../plugins/cmd/install/install.handler.ts | 12 +- .../src/plugins/cmd/update/update.handler.ts | 8 +- .../src/plugins/doc/INpmRegistryInfo.ts | 20 +++ .../src/plugins/utilities/NpmFunctions.ts | 67 ++++---- .../utilities/npm-interface/install.ts | 85 +++++---- .../utilities/npm-interface/uninstall.ts | 43 +++-- .../plugins/utilities/npm-interface/update.ts | 10 +- 15 files changed, 291 insertions(+), 266 deletions(-) create mode 100644 packages/imperative/src/imperative/src/plugins/doc/INpmRegistryInfo.ts diff --git a/packages/imperative/src/imperative/__tests__/plugins/cmd/install/install.handler.unit.test.ts b/packages/imperative/src/imperative/__tests__/plugins/cmd/install/install.handler.unit.test.ts index 5d0ba5ce9e..103cfd866a 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/cmd/install/install.handler.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/cmd/install/install.handler.unit.test.ts @@ -10,31 +10,22 @@ */ /* eslint-disable jest/expect-expect */ -import Mock = jest.Mock; - let expectedVal: unknown; let returnedVal: unknown; jest.mock("cross-spawn"); jest.mock("jsonfile"); -jest.mock("../../../../src/plugins/utilities/npm-interface/install"); -jest.mock("../../../../src/plugins/utilities/runValidatePlugin"); jest.mock("../../../../src/plugins/utilities/PMFConstants"); -jest.mock("../../../../../cmd/src/response/CommandResponse"); jest.mock("../../../../../cmd/src/response/HandlerResponse"); -jest.mock("../../../../../cmd/src/doc/handler/IHandlerParameters"); -jest.mock("../../../../../logger"); -jest.mock("../../../../src/Imperative"); -jest.mock("../../../../src/plugins/utilities/NpmFunctions"); jest.doMock("path", () => { const originalPath = jest.requireActual("path"); return { ...originalPath, - resolve: (...path: string[]) => { - if (path[0] == expectedVal) { + resolve: (...paths: string[]) => { + if (paths[0] === expectedVal) { return returnedVal ? returnedVal : expectedVal; } else { - return originalPath.resolve(...path); + return originalPath.resolve(...paths); } } }; @@ -43,28 +34,30 @@ jest.doMock("path", () => { import { HandlerResponse, IHandlerParameters } from "../../../../../cmd"; import { Console } from "../../../../../console"; import { ImperativeError } from "../../../../../error"; -import { install } from "../../../../src/plugins/utilities/npm-interface"; -import { runValidatePlugin } from "../../../../src/plugins/utilities/runValidatePlugin"; +import * as npmInterface from "../../../../src/plugins/utilities/npm-interface"; +import * as validatePlugin from "../../../../src/plugins/utilities/runValidatePlugin"; import InstallHandler from "../../../../src/plugins/cmd/install/install.handler"; import { IPluginJson } from "../../../../src/plugins/doc/IPluginJson"; import { Logger } from "../../../../../logger"; -import { readFileSync, writeFileSync } from "jsonfile"; +import * as jsonfile from "jsonfile"; import { PMFConstants } from "../../../../src/plugins/utilities/PMFConstants"; import { TextUtils } from "../../../../../utilities"; -import { getRegistry, getScopeRegistry, npmLogin } from "../../../../src/plugins/utilities/NpmFunctions"; +import { NpmRegistryUtils } from "../../../../src/plugins/utilities/NpmFunctions"; import * as spawn from "cross-spawn"; +import { INpmRegistryInfo } from "../../../../src/plugins/doc/INpmRegistryInfo"; describe("Plugin Management Facility install handler", () => { // Objects created so types are correct. + const origGetRegistry = jest.requireActual("../../../../src/plugins/utilities/NpmFunctions").NpmRegistryUtils.getRegistry; const mocks = { - npmLogin: npmLogin as Mock, - getRegistry: getRegistry as unknown as Mock, - getScopeRegistry: getScopeRegistry as unknown as Mock, - readFileSync: readFileSync as Mock, - writeFileSync: writeFileSync as Mock, - install: install as unknown as Mock, - runValidatePlugin: runValidatePlugin as unknown as Mock, + npmLogin: jest.spyOn(NpmRegistryUtils, "npmLogin"), + getRegistry: jest.spyOn(NpmRegistryUtils, "getRegistry"), + getScopeRegistry: jest.spyOn(NpmRegistryUtils as any, "getScopeRegistry"), + readFileSync: jest.spyOn(jsonfile, "readFileSync"), + writeFileSync: jest.spyOn(jsonfile, "writeFileSync"), + install: jest.spyOn(npmInterface, "install"), + runValidatePlugin: jest.spyOn(validatePlugin, "runValidatePlugin"), }; // two plugin set of values @@ -83,11 +76,11 @@ describe("Plugin Management Facility install handler", () => { jest.clearAllMocks(); // This needs to be mocked before running process function of uninstall handler - (Logger.getImperativeLogger as unknown as Mock).mockReturnValue(new Logger(new Console()) as any); - mocks.getRegistry.mockReturnValue(packageRegistry as any); - mocks.readFileSync.mockReturnValue({} as any); - npmLogin(packageRegistry); - mocks.runValidatePlugin.mockReturnValue(finalValidationMsg as any); + jest.spyOn(Logger, "getImperativeLogger").mockReturnValue(new Logger(new Console())); + mocks.getRegistry.mockReturnValue(packageRegistry); + mocks.readFileSync.mockReturnValue({}); + mocks.runValidatePlugin.mockReturnValue(finalValidationMsg); + mocks.install.mockImplementation(); expectedVal = undefined; returnedVal = undefined; }); @@ -139,7 +132,7 @@ describe("Plugin Management Facility install handler", () => { */ const wasInstallCallValid = ( packageLocation: string, - registry: string, + registry: INpmRegistryInfo, installFromFile = false ) => { if (installFromFile) { @@ -161,8 +154,9 @@ describe("Plugin Management Facility install handler", () => { */ const wasInstallSuccessful = (params: IHandlerParameters) => { // get the text of the last message that was displayed - const outputMsg = (params.response.console.log as Mock).mock.calls[(params.response.console.log as Mock).mock.calls.length - 1][0]; - expect(outputMsg).toContain(finalValidationMsg); + expect(params.response.console.log).toHaveBeenLastCalledWith( + expect.stringContaining(finalValidationMsg) + ); }; /** @@ -192,10 +186,10 @@ describe("Plugin Management Facility install handler", () => { }; // Override the return value for this test only - mocks.readFileSync.mockReturnValueOnce(fileJson as any); + mocks.readFileSync.mockReturnValueOnce(fileJson); mocks.install - .mockReturnValueOnce("a" as any) - .mockReturnValueOnce("plugin2" as any); + .mockResolvedValueOnce("a") + .mockResolvedValueOnce("plugin2"); const handler = new InstallHandler(); @@ -212,18 +206,17 @@ describe("Plugin Management Facility install handler", () => { // Validate the call to get the registry value wasGetRegistryCalled(); - // Validate the call to login - wasNpmLoginCallValid(packageRegistry); - expect(mocks.install).toHaveBeenCalledTimes(2); - wasInstallCallValid(`${fileJson.a.package}@${fileJson.a.version}`, packageRegistry, true); - wasInstallCallValid(fileJson.plugin2.package, packageRegistry2, true); + wasInstallCallValid(`${fileJson.a.package}@${fileJson.a.version}`, { location: packageRegistry, npmArgs: {} }, true); + wasInstallCallValid(fileJson.plugin2.package, { + location: packageRegistry2, + npmArgs: { registry: packageRegistry2 } + }, true); expect(mocks.runValidatePlugin).toHaveBeenCalledTimes(2); expect(mocks.runValidatePlugin).toHaveBeenCalledWith("a"); expect(mocks.runValidatePlugin).toHaveBeenCalledWith("plugin2"); - // Validate that the read was correct wasReadFileSyncCallValid(resolveVal); @@ -264,11 +257,8 @@ describe("Plugin Management Facility install handler", () => { // Validate the call to get the registry value wasGetRegistryCalled(); - // Validate the call to login - wasNpmLoginCallValid(packageRegistry); - // Check that install worked as expected - wasInstallCallValid(params.arguments.plugin[0], packageRegistry); + wasInstallCallValid(params.arguments.plugin[0], { location: packageRegistry, npmArgs: {} }); // Check that success is output wasInstallSuccessful(params); @@ -284,8 +274,35 @@ describe("Plugin Management Facility install handler", () => { await handler.process(params as IHandlerParameters); // Validate the call to install with specified plugin and registry - wasInstallCallValid(params.arguments.plugin[0], params.arguments.registry); + wasInstallCallValid(params.arguments.plugin[0], { + location: params.arguments.registry, + npmArgs: { registry: params.arguments.registry } + }); + + wasInstallSuccessful(params); + }); + + it("should install single package with registry and login specified", async () => { + mocks.npmLogin.mockReturnValueOnce(); + const handler = new InstallHandler(); + + const params = getIHandlerParametersObject(); + params.arguments.plugin = ["sample1"]; + params.arguments.registry = "http://localhost:4873/"; + params.arguments.login = true; + + await handler.process(params as IHandlerParameters); + + // Validate the call to login + wasNpmLoginCallValid(packageRegistry); + + // Check that install worked as expected + wasInstallCallValid(params.arguments.plugin[0], { + location: params.arguments.registry, + npmArgs: { registry: params.arguments.registry } + }); + // Check that success is output wasInstallSuccessful(params); }); @@ -300,14 +317,11 @@ describe("Plugin Management Facility install handler", () => { // Validate the install wasGetRegistryCalled(); - // Validate the call to login - wasNpmLoginCallValid(packageRegistry); - // Validate that install was called with each of these values expect(mocks.install).toHaveBeenCalledTimes(params.arguments.plugin.length); - wasInstallCallValid(params.arguments.plugin[0], packageRegistry); - wasInstallCallValid(params.arguments.plugin[1], packageRegistry); - wasInstallCallValid(params.arguments.plugin[2], packageRegistry); + wasInstallCallValid(params.arguments.plugin[0], { location: packageRegistry, npmArgs: {} }); + wasInstallCallValid(params.arguments.plugin[1], { location: packageRegistry, npmArgs: {} }); + wasInstallCallValid(params.arguments.plugin[2], { location: packageRegistry, npmArgs: {} }); wasInstallSuccessful(params); }); @@ -323,8 +337,7 @@ describe("Plugin Management Facility install handler", () => { // Validate the call to get the registry value wasGetRegistryCalled(); - // Validate the call to login - wasNpmLoginCallValid(packageRegistry); + // Validate that the read was correct wasReadFileSyncCallValid(PMFConstants.instance.PLUGIN_JSON); expect(params.response.console.log).toHaveBeenCalledWith("No packages were found in " + @@ -368,7 +381,7 @@ describe("Plugin Management Facility install handler", () => { }); jest.spyOn(spawn, "sync").mockReturnValueOnce({ status: 1 } as any); - mocks.getRegistry.mockImplementationOnce(jest.requireActual("../../../../src/plugins/utilities/NpmFunctions").getRegistry); + mocks.getRegistry.mockImplementationOnce(origGetRegistry); try { await handler.process(params); @@ -380,22 +393,22 @@ describe("Plugin Management Facility install handler", () => { expect(expectedError.additionalDetails).toContain("Command failed"); expect(expectedError.additionalDetails).toContain("npm"); }); - // it("should handle installed plugins via public scope", async () => { - // const handler = new InstallHandler(); + it("should handle installed plugins via package name", async () => { + const handler = new InstallHandler(); - // const params = getIHandlerParametersObject(); - // params.arguments.plugin = ["@public/sample1"]; + const params = getIHandlerParametersObject(); + params.arguments.plugin = ["@public/sample1"]; - // mocks.getScopeRegistry.mockReturnValueOnce("publicRegistryUrl" as any); + mocks.getScopeRegistry.mockReturnValueOnce("publicRegistryUrl"); - // try { - // await handler.process(params); - // } catch (e) { - // expect(e).toBeUndefined(); - // } + try { + await handler.process(params); + } catch (e) { + expect(e).toBeUndefined(); + } - // expect(mocks.install).toHaveBeenCalledWith("@public/sample1","publicRegistryUrl"); - // }); + wasInstallCallValid("@public/sample1", { location: "publicRegistryUrl", npmArgs: {} }); + }); it("should handle installed plugins via project/directory", async () => { const handler = new InstallHandler(); @@ -409,7 +422,7 @@ describe("Plugin Management Facility install handler", () => { expect(e).toBeUndefined(); } - expect(mocks.install).toHaveBeenCalledWith("path/to/dir", packageRegistry); + wasInstallCallValid("path/to/dir", { location: "path/to/dir", npmArgs: {} }); }); it("should handle installed plugins via tarball file", async () => { const handler = new InstallHandler(); @@ -424,14 +437,15 @@ describe("Plugin Management Facility install handler", () => { expect(e).toBeUndefined(); } - expect(mocks.install).toHaveBeenCalledWith("path/to/dir/file.tgz", packageRegistry); + wasInstallCallValid("path/to/dir/file.tgz", { location: "path/to/dir/file.tgz", npmArgs: {} }); }); it("should handle multiple installed plugins via tarball, directory, and registry", async () => { const handler = new InstallHandler(); const params = getIHandlerParametersObject(); params.arguments.plugin = ["@public/sample1", "@private/sample1", "path/to/dir", "path/to/dir/file.tgz"]; - mocks.getRegistry.mockReturnValueOnce(packageRegistry2 as any); + mocks.getScopeRegistry.mockReturnValueOnce("publicRegistryUrl"); + mocks.getScopeRegistry.mockReturnValueOnce("privateRegistryUrl"); try{ await handler.process(params); @@ -440,10 +454,10 @@ describe("Plugin Management Facility install handler", () => { expect(e).toBeUndefined(); } - expect(mocks.install).toHaveBeenCalledWith("@public/sample1", packageRegistry2); - expect(mocks.install).toHaveBeenCalledWith("@private/sample1", packageRegistry2); - expect(mocks.install).toHaveBeenCalledWith("path/to/dir", packageRegistry2); - expect(mocks.install).toHaveBeenCalledWith("path/to/dir/file.tgz", packageRegistry2); + expect(mocks.install).toHaveBeenCalledTimes(params.arguments.plugin.length); + wasInstallCallValid("@public/sample1", { location: "publicRegistryUrl", npmArgs: {} }); + wasInstallCallValid("@private/sample1", { location: "privateRegistryUrl", npmArgs: {} }); + wasInstallCallValid("path/to/dir", { location: "path/to/dir", npmArgs: {} }); + wasInstallCallValid("path/to/dir/file.tgz", { location: "path/to/dir/file.tgz", npmArgs: {} }); }); }); - diff --git a/packages/imperative/src/imperative/__tests__/plugins/cmd/list/list.handler.unit.test.ts b/packages/imperative/src/imperative/__tests__/plugins/cmd/list/list.handler.unit.test.ts index 8db628f52b..ecf7670ab0 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/cmd/list/list.handler.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/cmd/list/list.handler.unit.test.ts @@ -84,7 +84,7 @@ describe("Plugin Management Facility list handler", () => { mocks.readFileSync.mockReturnValue({} as any); }); - test("list packages", async () => { + it("list packages", async () => { // plugin definitions mocking unsorted file contents const fileJson: IPluginJson = { @@ -113,7 +113,7 @@ describe("Plugin Management Facility list handler", () => { }); - test("list packages short", async () => { + it("list packages short", async () => { // plugin definitions mocking unsorted file contents const fileJson: IPluginJson = { @@ -143,4 +143,3 @@ describe("Plugin Management Facility list handler", () => { }); }); - diff --git a/packages/imperative/src/imperative/__tests__/plugins/cmd/uninstall/uninstall.handler.unit.test.ts b/packages/imperative/src/imperative/__tests__/plugins/cmd/uninstall/uninstall.handler.unit.test.ts index 84a3592d04..6d1e5042e0 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/cmd/uninstall/uninstall.handler.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/cmd/uninstall/uninstall.handler.unit.test.ts @@ -109,7 +109,7 @@ describe("Plugin Management Facility uninstall handler", () => { expect(params.response.console.log).toHaveBeenCalledWith("Removal of the npm package(s) was successful.\n"); }; - test("uninstall specified package", async () => { + it("uninstall specified package", async () => { // plugin definitions mocking file contents const fileJson: IPluginJson = { a: { diff --git a/packages/imperative/src/imperative/__tests__/plugins/cmd/update/update.handler.unit.test.ts b/packages/imperative/src/imperative/__tests__/plugins/cmd/update/update.handler.unit.test.ts index d4c29e9c2c..4351cba397 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/cmd/update/update.handler.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/cmd/update/update.handler.unit.test.ts @@ -9,17 +9,10 @@ * */ -import Mock = jest.Mock; - jest.mock("child_process"); jest.mock("jsonfile"); -jest.mock("../../../../src/plugins/utilities/npm-interface/update"); jest.mock("../../../../src/plugins/utilities/PMFConstants"); -jest.mock("../../../../../cmd/src/doc/handler/IHandlerParameters"); -jest.mock("../../../../../cmd/src/response/CommandResponse"); jest.mock("../../../../../cmd/src/response/HandlerResponse"); -jest.mock("../../../../../logger"); -jest.mock("../../../../src/plugins/utilities/NpmFunctions"); import { HandlerResponse, IHandlerParameters } from "../../../../../cmd"; import { Console } from "../../../../../console"; @@ -27,21 +20,22 @@ import { IPluginJson } from "../../../../src/plugins/doc/IPluginJson"; import { Logger } from "../../../../../logger"; import { PMFConstants } from "../../../../src/plugins/utilities/PMFConstants"; import UpdateHandler from "../../../../src/plugins/cmd/update/update.handler"; -import * as NpmFunctions from "../../../../src/plugins/utilities/NpmFunctions"; -import * as ChildProcesses from "child_process"; -import * as JsonFile from "jsonfile"; -import * as NpmInterface from "../../../../src/plugins/utilities/npm-interface"; +import { NpmRegistryUtils } from "../../../../src/plugins/utilities/NpmFunctions"; +import * as childProcess from "child_process"; +import * as jsonfile from "jsonfile"; +import * as npmInterface from "../../../../src/plugins/utilities/npm-interface"; +import { INpmRegistryInfo } from "../../../../src/plugins/doc/INpmRegistryInfo"; describe("Plugin Management Facility update handler", () => { const resolveVal = "test/imperative-plugins"; const mocks = { - npmLoginSpy: jest.spyOn(NpmFunctions, 'npmLogin') as jest.SpyInstance, - execSyncSpy: jest.spyOn(ChildProcesses, 'execSync') as jest.SpyInstance, - readFileSyncSpy: jest.spyOn(JsonFile, 'readFileSync') as jest.SpyInstance, - writeFileSyncSpy: jest.spyOn(JsonFile, 'writeFileSync') as jest.SpyInstance, - updateSpy: jest.spyOn(NpmInterface, 'update') as jest.SpyInstance, + npmLoginSpy: jest.spyOn(NpmRegistryUtils, "npmLogin"), + execSyncSpy: jest.spyOn(childProcess, "execSync"), + readFileSyncSpy: jest.spyOn(jsonfile, "readFileSync"), + writeFileSyncSpy: jest.spyOn(jsonfile, "writeFileSync"), + updateSpy: jest.spyOn(npmInterface, "update"), }; // two plugin set of values @@ -56,14 +50,14 @@ describe("Plugin Management Facility update handler", () => { const pluginName = "imperative-sample-plugin"; beforeEach(() => { - // Mocks need cleared after every test for clean test runs + // Mocks need cleared after every test for clean test runs jest.resetAllMocks(); // This needs to be mocked before running process function of update handler - (Logger.getImperativeLogger as unknown as Mock).mockReturnValue(new Logger(new Console()) as any); - mocks.execSyncSpy.mockReturnValue(packageRegistry as any); - mocks.readFileSyncSpy.mockReturnValue({} as any); - NpmFunctions.npmLogin(packageRegistry); + jest.spyOn(Logger, "getImperativeLogger").mockReturnValue(new Logger(new Console())); + mocks.execSyncSpy.mockReturnValue(packageRegistry); + mocks.readFileSyncSpy.mockReturnValue({}); + NpmRegistryUtils.npmLogin(packageRegistry); }); /** @@ -93,7 +87,7 @@ describe("Plugin Management Facility update handler", () => { */ const wasUpdateCallValid = ( packageNameParm: string, - registry: string + registry: INpmRegistryInfo ) => { expect(mocks.updateSpy).toHaveBeenCalledWith( packageNameParm, registry @@ -107,7 +101,8 @@ describe("Plugin Management Facility update handler", () => { * process function. */ const wasUpdateSuccessful = (params: IHandlerParameters) => { - expect(params.response.console.log).toHaveBeenCalledWith(`Update of the npm package(${params.arguments.plugin}) was successful.\n`); + expect(params.response.console.log).toHaveBeenCalledWith( + `Update of the npm package(${resolveVal}) was successful.\n`); }; /** @@ -131,7 +126,7 @@ describe("Plugin Management Facility update handler", () => { ); }; - test("update specified plugin", async () => { + it("update specified plugin", async () => { // plugin definitions mocking file contents const fileJson: IPluginJson = { @@ -148,7 +143,7 @@ describe("Plugin Management Facility update handler", () => { }; // Override the return value for this test only - mocks.readFileSyncSpy.mockReturnValueOnce(fileJson as any); + mocks.readFileSyncSpy.mockReturnValueOnce(fileJson); const handler = new UpdateHandler(); @@ -161,13 +156,14 @@ describe("Plugin Management Facility update handler", () => { // Validate the call to login wasNpmLoginCallValid(packageRegistry); wasWriteFileSyncValid(PMFConstants.instance.PLUGIN_JSON, fileJson); - wasUpdateCallValid(packageName, packageRegistry); - - expect(params.response.console.log).toHaveBeenCalledWith( - `Update of the npm package(${resolveVal}) was successful.\n`); + wasUpdateCallValid(packageName, { + location: resolveVal, + npmArgs: { registry: packageRegistry } + }); + wasUpdateSuccessful(params); }); - test("update imperative-sample-plugin", async () => { + it("update imperative-sample-plugin", async () => { // plugin definitions mocking file contents const fileJson: IPluginJson = { @@ -179,7 +175,7 @@ describe("Plugin Management Facility update handler", () => { }; // Override the return value for this test only - mocks.readFileSyncSpy.mockReturnValueOnce(fileJson as any); + mocks.readFileSyncSpy.mockReturnValueOnce(fileJson); const handler = new UpdateHandler(); @@ -191,9 +187,10 @@ describe("Plugin Management Facility update handler", () => { // Validate the call to login wasNpmLoginCallValid(packageRegistry); wasWriteFileSyncValid(PMFConstants.instance.PLUGIN_JSON, fileJson); - wasUpdateCallValid(resolveVal, packageRegistry); - expect(params.response.console.log).toHaveBeenCalledWith( - `Update of the npm package(${resolveVal}) was successful.\n`); + wasUpdateCallValid(resolveVal, { + location: packageRegistry, + npmArgs: { registry: packageRegistry } + }); + wasUpdateSuccessful(params); }); }); - diff --git a/packages/imperative/src/imperative/__tests__/plugins/utilities/NpmFunctions.unit.test.ts b/packages/imperative/src/imperative/__tests__/plugins/utilities/NpmFunctions.unit.test.ts index a0a28b47c1..b0e39b5137 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/utilities/NpmFunctions.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/utilities/NpmFunctions.unit.test.ts @@ -54,7 +54,7 @@ describe("NpmFunctions", () => { status: 0, stdout: stdoutBuffer } as any); - const result = npmFunctions.getRegistry(); + const result = npmFunctions.NpmRegistryUtils.getRegistry(); expect(spawnSyncSpy.mock.calls[0][0]).toBe(npmCmd); expect(spawnSyncSpy.mock.calls[0][1]).toEqual(["config", "get", "registry"]); expect(result).toBe(stdoutBuffer.toString()); @@ -62,7 +62,7 @@ describe("NpmFunctions", () => { it("npmLogin should run npm login command", () => { const spawnSyncSpy = jest.spyOn(spawn, "sync").mockReturnValueOnce({ status: 0 } as any); - npmFunctions.npmLogin(fakeRegistry); + npmFunctions.NpmRegistryUtils.npmLogin(fakeRegistry); expect(spawnSyncSpy.mock.calls[0][0]).toBe(npmCmd); expect(spawnSyncSpy.mock.calls[0][1]).toContain("login"); expect(spawnSyncSpy.mock.calls[0][1]).toEqual(expect.arrayContaining(["--registry", fakeRegistry])); @@ -136,10 +136,10 @@ describe("NpmFunctions", () => { expect(pacote.manifest).toHaveBeenCalledTimes(1); }); - it("getScopeRegistry() should return registry for 'test' scope", async () => { + it("getScopeRegistry() should return registry for 'test' scope", () => { const spawnSpy = jest.spyOn(ExecUtils, "spawnAndGetOutput"); spawnSpy.mockReturnValueOnce("https://test123.com"); - const result = await npmFunctions.getScopeRegistry("test"); + const result = (npmFunctions.NpmRegistryUtils as any).getScopeRegistry("test"); expect(result).toBe("https://test123.com"); expect(spawnSpy).toHaveBeenCalledTimes(1); }); 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 3162751b83..2e15b6d88b 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 @@ -16,6 +16,19 @@ let returnedVal: any; jest.mock("cross-spawn"); jest.mock("jsonfile"); jest.mock("../../../../src/plugins/utilities/PMFConstants"); +jest.doMock("path", () => { + const originalPath = jest.requireActual("path"); + return { + ...originalPath, + resolve: (...paths: string[]) => { + if (paths[0] === expectedVal) { + return returnedVal ? returnedVal : expectedVal; + } else { + return originalPath.resolve(...paths); + } + } + }; +}); import { Console } from "../../../../../console"; import { ImperativeError } from "../../../../../error"; @@ -25,7 +38,7 @@ import { IPluginJson } from "../../../../src/plugins/doc/IPluginJson"; import { IPluginJsonObject } from "../../../../src/plugins/doc/IPluginJsonObject"; import { Logger } from "../../../../../logger"; import { PMFConstants } from "../../../../src/plugins/utilities/PMFConstants"; -import * as crossSpawn from "cross-spawn"; +import * as spawn from "cross-spawn"; import * as jsonfile from "jsonfile"; import * as findUp from "find-up"; import * as npmFns from "../../../../src/plugins/utilities/NpmFunctions"; @@ -59,7 +72,7 @@ describe("PMF: Install Interface", () => { requirePluginModuleCallback: jest.spyOn(pmfI, "requirePluginModuleCallback"), loadConfiguration: jest.spyOn(ConfigurationLoader, "load"), addProfiles: jest.spyOn(UpdateImpConfig, "addProfiles"), - spawnSync: jest.spyOn(crossSpawn, "sync"), + spawnSync: jest.spyOn(spawn, "sync"), ConfigSchema: { loadSchema: jest.spyOn(ConfigSchema, "loadSchema"), updateSchema: jest.spyOn(ConfigSchema, "updateSchema") @@ -73,6 +86,7 @@ describe("PMF: Install Interface", () => { const packageName = "a"; const packageVersion = "1.2.3"; const packageRegistry = "https://registry.npmjs.org/"; + const registryInfo = npmFns.NpmRegistryUtils.buildRegistryInfo(packageName, packageRegistry); beforeEach(() => { // Mocks need cleared after every test for clean test runs @@ -97,6 +111,7 @@ describe("PMF: Install Interface", () => { }); mocks.ConfigUtils.writeExtendersJson.mockImplementation(); mocks.ConfigSchema.loadSchema.mockReturnValue([mockTypeConfig]); + mocks.ConfigSchema.updateSchema.mockImplementation(); mocks.loadConfiguration.mockReturnValue({ profiles: [mockTypeConfig] }); mocks.spawnSync.mockReturnValue({ status: 0, @@ -165,16 +180,7 @@ describe("PMF: Install Interface", () => { describe("Basic install", () => { beforeEach(() => { mocks.getPackageInfo.mockResolvedValue({ name: packageName, version: packageVersion }); - mocks.ConfigSchema.updateSchema.mockImplementation(); jest.spyOn(fs, "existsSync").mockReturnValue(true); - // jest.spyOn(path, "normalize").mockReturnValue("testing"); - jest.spyOn(path, "resolve").mockImplementation((...paths) => { - if (paths[0] == expectedVal) { - return returnedVal ? returnedVal : expectedVal; - } else { - return jest.requireActual("path").resolve(...paths); - } - }); jest.spyOn(fs, "lstatSync").mockReturnValue({ isSymbolicLink: jest.fn().mockReturnValue(true) } as any); @@ -182,7 +188,7 @@ describe("PMF: Install Interface", () => { it("should install from the npm registry", async () => { setResolve(packageName); - await install(packageName, new npmFns.NpmRegistryInfo(packageRegistry)); + await install(packageName, registryInfo); // Validate the install wasNpmInstallCallValid(packageName, packageRegistry); @@ -198,7 +204,7 @@ describe("PMF: Install Interface", () => { jest.spyOn(path, "isAbsolute").mockReturnValueOnce(true); setResolve(rootFile); - await install(rootFile, new npmFns.NpmRegistryInfo(packageRegistry)); + await install(rootFile, registryInfo); // Validate the install wasNpmInstallCallValid(rootFile, packageRegistry); @@ -214,7 +220,7 @@ describe("PMF: Install Interface", () => { jest.spyOn(path, "isAbsolute").mockReturnValueOnce(true); setResolve(rootFile); - await install(rootFile, new npmFns.NpmRegistryInfo(packageRegistry)); + await install(rootFile, registryInfo); // Validate the install wasNpmInstallCallValid(rootFile, packageRegistry); @@ -241,7 +247,7 @@ describe("PMF: Install Interface", () => { // Call the install function setResolve(relativePath, absolutePath); - await install(relativePath, new npmFns.NpmRegistryInfo(packageRegistry)); + await install(relativePath, registryInfo); // Validate results wasNpmInstallCallValid(absolutePath, packageRegistry); @@ -259,7 +265,7 @@ describe("PMF: Install Interface", () => { // mocks.isUrl.mockReturnValue(true); - await install(installUrl, new npmFns.NpmRegistryInfo(packageRegistry)); + await install(installUrl, registryInfo); wasNpmInstallCallValid(installUrl, packageRegistry); wasWriteFileSyncCallValid({}, packageName, { @@ -272,7 +278,7 @@ describe("PMF: Install Interface", () => { it("should install plugin that does not define profiles", async () => { mocks.loadConfiguration.mockReturnValueOnce({}); setResolve(packageName); - await install(packageName, new npmFns.NpmRegistryInfo(packageRegistry)); + await install(packageName, registryInfo); // Validate the install wasNpmInstallCallValid(packageName, packageRegistry, false); @@ -285,7 +291,7 @@ describe("PMF: Install Interface", () => { }); // end Basic install describe("Advanced install", () => { - fit("should write even when install from file is true", async () => { + it("should write even when install from file is true", async () => { // This test is constructed in such a way that all if conditions with installFromFile // are validated to have been called or not. const location = "/this/should/not/change"; @@ -293,12 +299,11 @@ describe("PMF: Install Interface", () => { jest.spyOn(path, "isAbsolute").mockReturnValueOnce(false); jest.spyOn(fs, "existsSync").mockReturnValue(true); mocks.getPackageInfo.mockResolvedValue({ name: packageName, version: packageVersion }); - // jest.spyOn(path, "normalize").mockReturnValue("testing"); jest.spyOn(fs, "lstatSync").mockReturnValue({ isSymbolicLink: jest.fn().mockReturnValue(true) } as any); - await install(location, new npmFns.NpmRegistryInfo(packageRegistry), true); + await install(location, registryInfo, true); wasNpmInstallCallValid(location, packageRegistry); expect(mocks.writeFileSync).toHaveBeenCalled(); @@ -309,7 +314,6 @@ describe("PMF: Install Interface", () => { const semverPackage = `${packageName}@${semverVersion}`; jest.spyOn(fs, "existsSync").mockReturnValueOnce(true); - // jest.spyOn(path, "normalize").mockReturnValue("testing"); jest.spyOn(fs, "lstatSync").mockReturnValue({ isSymbolicLink: jest.fn().mockReturnValue(true) } as any); @@ -323,7 +327,7 @@ describe("PMF: Install Interface", () => { // Call the install setResolve(semverPackage); - await install(semverPackage, new npmFns.NpmRegistryInfo(packageRegistry)); + await install(semverPackage, registryInfo); // Test that shit happened wasNpmInstallCallValid(semverPackage, packageRegistry); @@ -346,14 +350,13 @@ describe("PMF: Install Interface", () => { mocks.getPackageInfo.mockResolvedValue({ name: packageName, version: packageVersion }); jest.spyOn(fs, "existsSync").mockReturnValueOnce(true); - // jest.spyOn(path, "normalize").mockReturnValue("testing"); jest.spyOn(fs, "lstatSync").mockReturnValue({ isSymbolicLink: jest.fn().mockReturnValue(true) } as any); mocks.readFileSync.mockReturnValue(oneOldPlugin); setResolve(packageName); - await install(packageName, new npmFns.NpmRegistryInfo(packageRegistry)); + await install(packageName, registryInfo); wasNpmInstallCallValid(packageName, packageRegistry); wasWriteFileSyncCallValid(oneOldPlugin, packageName, { @@ -363,7 +366,7 @@ describe("PMF: Install Interface", () => { }); }); - xdescribe("Updating the global schema", () => { + describe("Updating the global schema", () => { const expectTestSchemaMgmt = async (opts: { schemaExists: boolean; newProfileType: boolean; @@ -388,7 +391,6 @@ describe("PMF: Install Interface", () => { mocks.getPackageInfo.mockResolvedValue({ name: packageName, version: packageVersion }); 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); @@ -407,7 +409,7 @@ describe("PMF: Install Interface", () => { } setResolve(packageName); - await install(packageName, new npmFns.NpmRegistryInfo(packageRegistry)); + await install(packageName, registryInfo); if (opts.schemaExists) { expect(mocks.ConfigSchema.updateSchema).toHaveBeenCalled(); } else { @@ -491,7 +493,7 @@ describe("PMF: Install Interface", () => { }); try { - await install("test", new npmFns.NpmRegistryInfo("http://www.example.com")); + await install("test", registryInfo); } catch (e) { expectedError = e; } 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 bc7846c0a3..5c396ccdbb 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 @@ -17,7 +17,7 @@ jest.mock("../../../../src/plugins/utilities/PMFConstants"); import * as fs from "fs"; import * as jsonfile from "jsonfile"; import { Console } from "../../../../../console"; -import * as crossSpawn from "cross-spawn"; +import * as spawn from "cross-spawn"; import { ImperativeError } from "../../../../../error"; import { IPluginJson } from "../../../../src/plugins/doc/IPluginJson"; import { Logger } from "../../../../../logger"; @@ -33,7 +33,7 @@ import { updateAndGetRemovedTypes } from "../../../../src/plugins/utilities/npm- describe("PMF: Uninstall Interface", () => { // Objects created so types are correct. const mocks = { - spawnSync: jest.spyOn(crossSpawn, "sync"), + spawnSync: jest.spyOn(spawn, "sync"), readFileSync: jest.spyOn(jsonfile, "readFileSync"), writeFileSync: jest.spyOn(jsonfile, "writeFileSync") }; diff --git a/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/update.unit.test.ts b/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/update.unit.test.ts index ef36b5e971..2b34db20ff 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/update.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/update.unit.test.ts @@ -77,8 +77,7 @@ describe("PMF: update Interface", () => { mocks.getPackageInfo.mockResolvedValue({ name: packageName, version: packageVersion }); mocks.readFileSync.mockReturnValue(oneOldPlugin); - const registryInfo = new npmFns.NpmRegistryInfo(packageRegistry); - registryInfo.setPackage(oneOldPlugin.plugin1); + const registryInfo = npmFns.NpmRegistryUtils.buildRegistryInfo(oneOldPlugin.plugin1); const data = await update(packageName, registryInfo); expect(data).toEqual(packageVersion); @@ -101,8 +100,7 @@ describe("PMF: update Interface", () => { mocks.getPackageInfo.mockResolvedValue({ name: scopedPackageName, version: packageVersion }); mocks.readFileSync.mockReturnValue(oneOldPlugin); - const registryInfo = new npmFns.NpmRegistryInfo(packageRegistry); - registryInfo.setPackage(oneOldPlugin.plugin1); + const registryInfo = npmFns.NpmRegistryUtils.buildRegistryInfo(oneOldPlugin.plugin1); const data = await update(scopedPackageName, registryInfo); expect(data).toEqual(packageVersion); diff --git a/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts b/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts index 0d3f43885c..ad08ea59e9 100644 --- a/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts +++ b/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts @@ -20,7 +20,7 @@ import { readFileSync } from "jsonfile"; import { ImperativeConfig, TextUtils } from "../../../../../utilities"; import { ImperativeError } from "../../../../../error"; import { runValidatePlugin } from "../../utilities/runValidatePlugin"; -import { NpmRegistryInfo } from "../../utilities/NpmFunctions"; +import { NpmRegistryUtils } from "../../utilities/NpmFunctions"; /** * The install command handler for cli plugin install. @@ -80,11 +80,11 @@ export default class InstallHandler implements ICommandHandler { }); } else { try { - const registryInfo = new NpmRegistryInfo(params.arguments.registry); + const installRegistry = NpmRegistryUtils.getRegistry(params.arguments.registry); // Get the registry to install to if (params.arguments.registry != null && params.arguments.login) { - registryInfo.npmLogin(); + NpmRegistryUtils.npmLogin(installRegistry); } params.response.console.log( @@ -94,7 +94,7 @@ export default class InstallHandler implements ICommandHandler { "Install 3rd party plug-ins at your own risk.\n" ); - params.response.console.log("Location = " + registryInfo.location); + params.response.console.log("Location = " + installRegistry); // This section determines which npm logic needs to take place if (params.arguments.plugin == null || params.arguments.plugin.length === 0) { @@ -118,7 +118,7 @@ export default class InstallHandler implements ICommandHandler { // Registry is typed as optional in the doc but the function expects it // to be passed. So we'll always set it if it hasn't been done yet. - registryInfo.setPackage(packageInfo); + const registryInfo = NpmRegistryUtils.buildRegistryInfo(packageInfo, params.arguments.registry); packageInfo.location ??= registryInfo.location; this.console.debug(`Installing plugin: ${packageName}`); @@ -144,7 +144,7 @@ export default class InstallHandler implements ICommandHandler { } else { for (const packageString of params.arguments.plugin) { params.response.console.log("\n_______________________________________________________________"); - registryInfo.setPackage(packageString); + const registryInfo = NpmRegistryUtils.buildRegistryInfo(packageString, params.arguments.registry); const pluginName = await install(packageString, registryInfo); params.response.console.log("Installed plugin name = '" + pluginName + "'"); params.response.console.log(runValidatePlugin(pluginName)); diff --git a/packages/imperative/src/imperative/src/plugins/cmd/update/update.handler.ts b/packages/imperative/src/imperative/src/plugins/cmd/update/update.handler.ts index ae0cfc5c12..93b2a2b8f7 100644 --- a/packages/imperative/src/imperative/src/plugins/cmd/update/update.handler.ts +++ b/packages/imperative/src/imperative/src/plugins/cmd/update/update.handler.ts @@ -17,7 +17,7 @@ import { ImperativeError } from "../../../../../error"; import { TextUtils } from "../../../../../utilities"; import { IPluginJson } from "../../doc/IPluginJson"; import { readFileSync, writeFileSync } from "jsonfile"; -import { NpmRegistryInfo } from "../../utilities/NpmFunctions"; +import { NpmRegistryUtils } from "../../utilities/NpmFunctions"; /** * The update command handler for cli plugin install. @@ -52,7 +52,7 @@ export default class UpdateHandler implements ICommandHandler { this.console.debug(`Root Directory: ${PMFConstants.instance.PLUGIN_INSTALL_LOCATION}`); const plugin: string = params.arguments.plugin; - const registryInfo = new NpmRegistryInfo(params.arguments.registry); + const registry = NpmRegistryUtils.getRegistry(params.arguments.registry); if (params.arguments.plugin == null || params.arguments.plugin.length === 0) { throw new ImperativeError({ @@ -64,7 +64,7 @@ export default class UpdateHandler implements ICommandHandler { const installedPlugins: IPluginJson = readFileSync(PMFConstants.instance.PLUGIN_JSON); if (params.arguments.registry != null && params.arguments.login) { - registryInfo.npmLogin(); + NpmRegistryUtils.npmLogin(registry); } if (Object.prototype.hasOwnProperty.call(installedPlugins, plugin)) { @@ -76,7 +76,7 @@ export default class UpdateHandler implements ICommandHandler { // as package may not match the plugin value. This is true for plugins installed by // folder location. Example: plugin 'imperative-sample-plugin' installed from ../imperative-plugins packageName = installedPlugins[pluginName].package; - registryInfo.setPackage(installedPlugins[pluginName]); + const registryInfo = NpmRegistryUtils.buildRegistryInfo(installedPlugins[pluginName], params.arguments.registry); // Call update which returns the plugin's version so plugins.json can be updated installedPlugins[pluginName].version = await update(packageName, registryInfo); installedPlugins[pluginName].location = registryInfo.location; // update in case it changed diff --git a/packages/imperative/src/imperative/src/plugins/doc/INpmRegistryInfo.ts b/packages/imperative/src/imperative/src/plugins/doc/INpmRegistryInfo.ts new file mode 100644 index 0000000000..0e0ff9c3ee --- /dev/null +++ b/packages/imperative/src/imperative/src/plugins/doc/INpmRegistryInfo.ts @@ -0,0 +1,20 @@ +/* +* 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 { INpmInstallArgs } from "./INpmInstallArgs"; + +/** + * Location info for an npm package. + */ +export interface INpmRegistryInfo { + location: string; + npmArgs: Partial; +} diff --git a/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts b/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts index 6abcd92ab4..23d8f32b37 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts @@ -19,6 +19,7 @@ import * as pacote from "pacote"; import { ExecUtils, JsUtils } from "../../../../utilities"; import { INpmInstallArgs } from "../doc/INpmInstallArgs"; import { IPluginJsonObject } from "../doc/IPluginJsonObject"; +import { INpmRegistryInfo } from "../doc/INpmRegistryInfo"; const npmCmd = findNpmOnPath(); /** @@ -72,40 +73,22 @@ export async function getPackageInfo(pkgSpec: string): Promise<{ name: string, v } } -export class NpmRegistryInfo { - private defaultRegistry?: string; - private defaultRegistryScope?: string; - - constructor(private customRegistry?: string) { - if (customRegistry == null) { - this.defaultRegistry = this.getRegistry(); - } - } - - private getRegistry(): string { +export class NpmRegistryUtils { + public static getRegistry(userRegistry?: string): string { + if (userRegistry != null) return userRegistry; const execOutput = ExecUtils.spawnAndGetOutput(npmCmd, ["config", "get", "registry"]); return execOutput.toString().replace("\n", ""); } - private getScopeRegistry(scope?: string): string { - const execOutput = ExecUtils.spawnAndGetOutput(npmCmd, ["config", "get", `${scope ?? this.defaultRegistryScope}:registry`]); - if (execOutput.toString().trim() === "undefined") return this.getRegistry(); - return execOutput.toString().replace("\n", ""); - } - - public get location(): string { - return this.customRegistry ?? this.defaultRegistry; - } - /** * NPM login to be able to install from secure registry * @param {string} registry The npm registry to install from. */ - public npmLogin(registry?: string) { + public static npmLogin(registry: string) { ExecUtils.spawnAndGetOutput(npmCmd, [ "login", - "--registry", registry ?? this.customRegistry, + "--registry", registry, "--always-auth", "--auth-type=legacy" ], { @@ -114,19 +97,37 @@ export class NpmRegistryInfo { ); } - public buildRegistryArgs(): Partial { - const registrySpec = this.defaultRegistryScope != null ? `${this.defaultRegistryScope}:registry` : "registry"; - return this.customRegistry != null ? { [registrySpec]: this.location } : {}; - } - - public setPackage(packageInfo: string | IPluginJsonObject) { + public static buildRegistryInfo(packageInfo: string | IPluginJsonObject, userRegistry?: string): INpmRegistryInfo { const packageName = typeof packageInfo === "string" ? packageInfo : packageInfo.package; const packageScope = packageName.startsWith("@") ? packageName.split("/")[0] : undefined; - if (typeof packageInfo === "string" || packageInfo.location == null) { - this.defaultRegistry = packageScope != null ? this.getScopeRegistry(packageScope) : this.getRegistry(); + const isLocationRegistry = npmPackageArg(packageName).registry; + if (userRegistry != null) { + // If --registry was passed on the command line, it takes precedence + return { + location: isLocationRegistry ? userRegistry : packageName, + npmArgs: this.buildRegistryNpmArgs(userRegistry) + }; + } else if (typeof packageInfo === "string" || !packageInfo.location) { + // If installing a plug-in for the first time, get default registry + const defaultRegistry = isLocationRegistry && (packageScope != null ? this.getScopeRegistry(packageScope) : this.getRegistry()); + return { location: isLocationRegistry ? defaultRegistry : packageName, npmArgs: {} }; } else { - this.defaultRegistry = JsUtils.isUrl(packageInfo.location) ? packageInfo.location : undefined; - this.defaultRegistryScope = packageScope; + // If updating a plug-in, fetch registry info from plugins.json + return { + location: packageInfo.location, + npmArgs: JsUtils.isUrl(packageInfo.location) ? this.buildRegistryNpmArgs(packageInfo.location, packageScope) : {} + }; } } + + private static buildRegistryNpmArgs(registryUrl: string, scope?: string): Partial { + const registrySpec = scope != null ? `${scope}:registry` : "registry"; + return { [registrySpec]: registryUrl }; + } + + private static getScopeRegistry(scope: string): string { + const execOutput = ExecUtils.spawnAndGetOutput(npmCmd, ["config", "get", `${scope}:registry`]); + if (execOutput.toString().trim() === "undefined") return this.getRegistry(); + return execOutput.toString().replace("\n", ""); + } } 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 bb7f22cd44..d8a0d95f2e 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 @@ -18,7 +18,7 @@ import { Logger } from "../../../../../logger"; import { IImperativeConfig } from "../../../doc/IImperativeConfig"; import { ImperativeError } from "../../../../../error"; import { IPluginJsonObject } from "../../doc/IPluginJsonObject"; -import { getPackageInfo, installPackages, NpmRegistryInfo } from "../NpmFunctions"; +import { getPackageInfo, installPackages } from "../NpmFunctions"; import { ConfigSchema } from "../../../../../config/src/ConfigSchema"; import { PluginManagementFacility } from "../../PluginManagementFacility"; import { ConfigurationLoader } from "../../../ConfigurationLoader"; @@ -28,6 +28,7 @@ import { IProfileTypeConfiguration } from "../../../../../profiles"; import * as semver from "semver"; import { ConfigUtils } from "../../../../../config"; import { IExtendersJsonOpts } from "../../../../../config/src/doc/IExtenderOpts"; +import { INpmRegistryInfo } from "../../doc/INpmRegistryInfo"; // Helper function to update extenders.json object during plugin install. // Returns true if the object was updated, and false otherwise @@ -74,7 +75,7 @@ export const updateExtendersJson = ( * be converted to an absolute path prior to being passed to the * `npm install` command. * - * @param {NpmRegistryInfo} registryInfo The npm registry to use. + * @param {INpmRegistryInfo} registryInfo The npm registry to use. * * @param {boolean} [installFromFile=false] If installing from a file, the package location is * automatically interpreted as an absolute location. @@ -85,7 +86,7 @@ export const updateExtendersJson = ( * it. * @returns {string} The name of the plugin. */ -export async function install(packageLocation: string, registryInfo: NpmRegistryInfo, installFromFile = false) { +export async function install(packageLocation: string, registryInfo: INpmRegistryInfo, installFromFile = false) { const iConsole = Logger.getImperativeLogger(); let npmPackage = packageLocation; @@ -122,7 +123,7 @@ export async function install(packageLocation: string, registryInfo: NpmRegistry installPackages(npmPackage, { prefix: PMFConstants.instance.PLUGIN_INSTALL_LOCATION, - ...registryInfo.buildRegistryArgs(), + ...registryInfo.npmArgs, }); // We fetch the package name and version of newly installed plugin @@ -169,55 +170,51 @@ export async function install(packageLocation: string, registryInfo: NpmRegistry const pluginImpConfig = ConfigurationLoader.load(null, packageInfo, requirerFunction); iConsole.debug(`Checking for global Zowe client configuration files to update.`); - 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 && Array.isArray(pluginImpConfig.profiles)) { - UpdateImpConfig.addProfiles(pluginImpConfig.profiles); - 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(schemaInfo.resolved)); - } catch (err) { - iConsole.error("Error when adding new profile type for plugin %s: failed to parse schema", newPlugin.package); - } + // Update the Imperative Configuration to add the profiles introduced by the recently installed plugin + const globalLayer = PMFConstants.instance.PLUGIN_CONFIG.layers.find((layer) => layer.global && layer.exists); + if (globalLayer && Array.isArray(pluginImpConfig.profiles)) { + UpdateImpConfig.addProfiles(pluginImpConfig.profiles); + 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(schemaInfo.resolved)); + } 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 = ConfigUtils.readExtendersJson(); + // 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 = ConfigUtils.readExtendersJson(); - // Determine new profile types to add to schema - let shouldUpdate = false; - for (const profile of pluginImpConfig.profiles) { - if (!existingTypes.includes(profile.type)) { - loadedSchema.push(profile); - } else { - const existingType = loadedSchema.find((obj) => obj.type === profile.type); - 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.schema.version = profile.schema.version; - } - } else { + // Determine new profile types to add to schema + let shouldUpdate = false; + for (const profile of pluginImpConfig.profiles) { + if (!existingTypes.includes(profile.type)) { + loadedSchema.push(profile); + } else { + const existingType = loadedSchema.find((obj) => obj.type === profile.type); + 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.schema.version = profile.schema.version; } + } else { + existingType.schema = profile.schema; + existingType.schema.version = profile.schema.version; } - shouldUpdate = updateExtendersJson(extendersJson, packageInfo, profile) || shouldUpdate; } + shouldUpdate = updateExtendersJson(extendersJson, packageInfo, profile) || shouldUpdate; + } - if (shouldUpdate) { - // Update extenders.json (if necessary) after installing the plugin - ConfigUtils.writeExtendersJson(extendersJson); - } - const schema = ConfigSchema.buildSchema(loadedSchema); - ConfigSchema.updateSchema({ layer: "global", schema }); + if (shouldUpdate) { + // Update extenders.json (if necessary) after installing the plugin + ConfigUtils.writeExtendersJson(extendersJson); } + const schema = ConfigSchema.buildSchema(loadedSchema); + ConfigSchema.updateSchema({ layer: "global", schema }); } } } 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 90e001720d..22d6499723 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 @@ -128,29 +128,26 @@ 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 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(schemaInfo.resolved)); - } catch (err) { - iConsole.error("Error when removing profile type for plugin %s: failed to parse schema", npmPackage); - } - // 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 }); - } + // Update the Imperative Configuration to add the profiles introduced by the recently installed plugin + const globalLayer = PMFConstants.instance.PLUGIN_CONFIG.layers.find((layer) => layer.global && layer.exists); + if (globalLayer) { + 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(schemaInfo.resolved)); + } catch (err) { + iConsole.error("Error when removing profile type for plugin %s: failed to parse schema", npmPackage); + } + // 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 }); } } } diff --git a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/update.ts b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/update.ts index af4ae19de3..44df21b6ad 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/update.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/npm-interface/update.ts @@ -11,7 +11,8 @@ import { PMFConstants } from "../PMFConstants"; import { Logger } from "../../../../../logger"; -import { getPackageInfo, installPackages, NpmRegistryInfo } from "../NpmFunctions"; +import { getPackageInfo, installPackages } from "../NpmFunctions"; +import { INpmRegistryInfo } from "../../doc/INpmRegistryInfo"; /** * @TODO - allow multiple packages to be updated? @@ -19,10 +20,10 @@ import { getPackageInfo, installPackages, NpmRegistryInfo } from "../NpmFunction * * @param {string} packageName A package name. This value is a valid npm package name. * - * @param {NpmRegistryInfo} registryInfo The npm registry to use. + * @param {INpmRegistryInfo} registryInfo The npm registry to use. * */ -export async function update(packageName: string, registryInfo: NpmRegistryInfo) { +export async function update(packageName: string, registryInfo: INpmRegistryInfo) { const iConsole = Logger.getImperativeLogger(); const npmPackage = packageName; @@ -33,7 +34,7 @@ export async function update(packageName: string, registryInfo: NpmRegistryInfo) installPackages(npmPackage, { prefix: PMFConstants.instance.PLUGIN_INSTALL_LOCATION, - ...registryInfo.buildRegistryArgs(), + ...registryInfo.npmArgs, }); // We fetch the package version of newly installed plugin @@ -45,4 +46,3 @@ export async function update(packageName: string, registryInfo: NpmRegistryInfo) // return the package version so the plugins.json file can be updated return packageVersion; } - From 9d1c2cebef9adf36202b13290d11b310c83dfb30 Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Thu, 24 Oct 2024 12:57:08 -0400 Subject: [PATCH 05/10] Revert potentially breaking changes and fix failing tests Signed-off-by: Timothy Johnson --- .../plugins/suites/InstallingPlugins.ts | 10 +-- .../cmd/install/install.handler.unit.test.ts | 55 +++++++++++--- .../cmd/update/update.handler.unit.test.ts | 3 +- .../npm-interface/update.unit.test.ts | 6 +- .../plugins/cmd/install/install.handler.ts | 15 ++-- .../src/plugins/utilities/NpmFunctions.ts | 26 ++++--- .../utilities/npm-interface/install.ts | 76 ++++++++++--------- .../utilities/npm-interface/uninstall.ts | 43 ++++++----- 8 files changed, 142 insertions(+), 92 deletions(-) diff --git a/packages/imperative/__tests__/src/packages/imperative/plugins/suites/InstallingPlugins.ts b/packages/imperative/__tests__/src/packages/imperative/plugins/suites/InstallingPlugins.ts index c1e6c22702..4bc6abe53b 100644 --- a/packages/imperative/__tests__/src/packages/imperative/plugins/suites/InstallingPlugins.ts +++ b/packages/imperative/__tests__/src/packages/imperative/plugins/suites/InstallingPlugins.ts @@ -83,7 +83,7 @@ describe("Installing Plugins", () => { name: "override-plugin", usage: "override-plugin" }, - location: { + sample_registry: { location: "imperative-sample-plugin", name: "imperative-sample-plugin", usage: "sample-plugin" @@ -667,7 +667,7 @@ describe("Installing Plugins", () => { const savedPluginJson = readFileSync(pluginJsonLocation); const expectedContent: IPluginJson = fileContent as IPluginJson; - expectedContent[plugins.normal.name].location = envNpmRegistry; + expectedContent[plugins.normal.name].location = plugins.normal.location; expect(savedPluginJson).toEqual(expectedContent); }); @@ -735,7 +735,7 @@ describe("Installing Plugins", () => { const actualJson = readFileSync(pluginJsonLocation); // Add missing registry to expected - expectedJson[plugins.normal.name].location = envNpmRegistry; + expectedJson[plugins.normal.name].location = plugins.normal.location; // Add missing normal2 plugin not present in before each expectedJson[plugins.normal3.name] = { @@ -751,7 +751,7 @@ describe("Installing Plugins", () => { it("should error when a package and --file is specified", function () { expect( T.stripNewLines( - executeCommandString(this, `${pluginGroup} install ${plugins.location.location} --file ${testFile}`).stderr + executeCommandString(this, `${pluginGroup} install ${plugins.sample_registry.location} --file ${testFile}`).stderr ) ).toContain("Option --file can not be specified if positional package... is as well. They are mutually exclusive."); }); @@ -760,7 +760,7 @@ describe("Installing Plugins", () => { expect( T.stripNewLines( executeCommandString(this, - `${pluginGroup} install ${plugins.location.location} --file ${testFile} --registry ${TEST_REGISTRY}`).stderr + `${pluginGroup} install ${plugins.sample_registry.location} --file ${testFile} --registry ${TEST_REGISTRY}`).stderr ) ).toContain("The following options conflict (mutually exclusive)"); }); diff --git a/packages/imperative/src/imperative/__tests__/plugins/cmd/install/install.handler.unit.test.ts b/packages/imperative/src/imperative/__tests__/plugins/cmd/install/install.handler.unit.test.ts index 103cfd866a..2e4bcdad4e 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/cmd/install/install.handler.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/cmd/install/install.handler.unit.test.ts @@ -207,7 +207,10 @@ describe("Plugin Management Facility install handler", () => { wasGetRegistryCalled(); expect(mocks.install).toHaveBeenCalledTimes(2); - wasInstallCallValid(`${fileJson.a.package}@${fileJson.a.version}`, { location: packageRegistry, npmArgs: {} }, true); + wasInstallCallValid(`${fileJson.a.package}@${fileJson.a.version}`, { + location: packageRegistry, + npmArgs: { registry: packageRegistry } + }, true); wasInstallCallValid(fileJson.plugin2.package, { location: packageRegistry2, npmArgs: { registry: packageRegistry2 } @@ -258,7 +261,10 @@ describe("Plugin Management Facility install handler", () => { wasGetRegistryCalled(); // Check that install worked as expected - wasInstallCallValid(params.arguments.plugin[0], { location: packageRegistry, npmArgs: {} }); + wasInstallCallValid(params.arguments.plugin[0], { + location: packageRegistry, + npmArgs: { registry: packageRegistry } + }); // Check that success is output wasInstallSuccessful(params); @@ -319,9 +325,13 @@ describe("Plugin Management Facility install handler", () => { // Validate that install was called with each of these values expect(mocks.install).toHaveBeenCalledTimes(params.arguments.plugin.length); - wasInstallCallValid(params.arguments.plugin[0], { location: packageRegistry, npmArgs: {} }); - wasInstallCallValid(params.arguments.plugin[1], { location: packageRegistry, npmArgs: {} }); - wasInstallCallValid(params.arguments.plugin[2], { location: packageRegistry, npmArgs: {} }); + const registryInfo: INpmRegistryInfo = { + location: packageRegistry, + npmArgs: { registry: packageRegistry } + }; + wasInstallCallValid(params.arguments.plugin[0], registryInfo); + wasInstallCallValid(params.arguments.plugin[1], registryInfo); + wasInstallCallValid(params.arguments.plugin[2], registryInfo); wasInstallSuccessful(params); }); @@ -407,7 +417,10 @@ describe("Plugin Management Facility install handler", () => { expect(e).toBeUndefined(); } - wasInstallCallValid("@public/sample1", { location: "publicRegistryUrl", npmArgs: {} }); + wasInstallCallValid("@public/sample1", { + location: packageRegistry, + npmArgs: { registry: packageRegistry, "@public:registry": "publicRegistryUrl" } + }); }); it("should handle installed plugins via project/directory", async () => { const handler = new InstallHandler(); @@ -422,7 +435,10 @@ describe("Plugin Management Facility install handler", () => { expect(e).toBeUndefined(); } - wasInstallCallValid("path/to/dir", { location: "path/to/dir", npmArgs: {} }); + wasInstallCallValid("path/to/dir", { + location: "path/to/dir", + npmArgs: { registry: packageRegistry } + }); }); it("should handle installed plugins via tarball file", async () => { const handler = new InstallHandler(); @@ -437,7 +453,10 @@ describe("Plugin Management Facility install handler", () => { expect(e).toBeUndefined(); } - wasInstallCallValid("path/to/dir/file.tgz", { location: "path/to/dir/file.tgz", npmArgs: {} }); + wasInstallCallValid("path/to/dir/file.tgz", { + location: "path/to/dir/file.tgz", + npmArgs: { registry: packageRegistry } + }); }); it("should handle multiple installed plugins via tarball, directory, and registry", async () => { const handler = new InstallHandler(); @@ -455,9 +474,21 @@ describe("Plugin Management Facility install handler", () => { } expect(mocks.install).toHaveBeenCalledTimes(params.arguments.plugin.length); - wasInstallCallValid("@public/sample1", { location: "publicRegistryUrl", npmArgs: {} }); - wasInstallCallValid("@private/sample1", { location: "privateRegistryUrl", npmArgs: {} }); - wasInstallCallValid("path/to/dir", { location: "path/to/dir", npmArgs: {} }); - wasInstallCallValid("path/to/dir/file.tgz", { location: "path/to/dir/file.tgz", npmArgs: {} }); + wasInstallCallValid("@public/sample1", { + location: packageRegistry, + npmArgs: { registry: packageRegistry, "@public:registry": "publicRegistryUrl" } + }); + wasInstallCallValid("@private/sample1", { + location: packageRegistry, + npmArgs: { registry: packageRegistry, "@private:registry": "privateRegistryUrl" } + }); + wasInstallCallValid("path/to/dir", { + location: "path/to/dir", + npmArgs: { registry: packageRegistry } + }); + wasInstallCallValid("path/to/dir/file.tgz", { + location: "path/to/dir/file.tgz", + npmArgs: { registry: packageRegistry } + }); }); }); diff --git a/packages/imperative/src/imperative/__tests__/plugins/cmd/update/update.handler.unit.test.ts b/packages/imperative/src/imperative/__tests__/plugins/cmd/update/update.handler.unit.test.ts index 4351cba397..e3c7922ff7 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/cmd/update/update.handler.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/cmd/update/update.handler.unit.test.ts @@ -9,6 +9,7 @@ * */ +/* eslint-disable jest/expect-expect */ jest.mock("child_process"); jest.mock("jsonfile"); jest.mock("../../../../src/plugins/utilities/PMFConstants"); @@ -157,7 +158,7 @@ describe("Plugin Management Facility update handler", () => { wasNpmLoginCallValid(packageRegistry); wasWriteFileSyncValid(PMFConstants.instance.PLUGIN_JSON, fileJson); wasUpdateCallValid(packageName, { - location: resolveVal, + location: packageRegistry, npmArgs: { registry: packageRegistry } }); wasUpdateSuccessful(params); diff --git a/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/update.unit.test.ts b/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/update.unit.test.ts index 2b34db20ff..240fc1eacf 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/update.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/utilities/npm-interface/update.unit.test.ts @@ -26,7 +26,8 @@ describe("PMF: update Interface", () => { const mocks = { installPackages: jest.spyOn(npmFns, "installPackages"), readFileSync: jest.spyOn(jsonfile, "readFileSync"), - getPackageInfo: jest.spyOn(npmFns, "getPackageInfo") + getPackageInfo: jest.spyOn(npmFns, "getPackageInfo"), + getScopeRegistry: jest.spyOn(npmFns.NpmRegistryUtils as any, "getScopeRegistry") }; const packageName = "pretty-format"; @@ -44,6 +45,7 @@ describe("PMF: update Interface", () => { * readFileSync(plugins.json), we must reset readFileSync to return an empty set before each test. */ mocks.readFileSync.mockReturnValue({}); + mocks.getScopeRegistry.mockReturnValue(packageRegistry); }); afterAll(() => { @@ -105,6 +107,6 @@ describe("PMF: update Interface", () => { expect(data).toEqual(packageVersion); // Validate the update - wasNpmInstallCallValid(scopedPackageName, { "@org:registry": packageRegistry }); + wasNpmInstallCallValid(scopedPackageName, { registry: packageRegistry, "@org:registry": packageRegistry }); }); }); diff --git a/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts b/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts index ad08ea59e9..607e2827c4 100644 --- a/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts +++ b/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts @@ -76,7 +76,7 @@ export default class InstallHandler implements ICommandHandler { if (params.arguments.plugin != null && params.arguments.plugin.length > 0 && typeof params.arguments.file !== "undefined") { throw new ImperativeError({ msg: `Option ${chalk.yellow.bold("--file")} can not be specified if positional ${chalk.yellow.bold("package...")} is as well. ` + - `They are mutually exclusive.`, + `They are mutually exclusive.` }); } else { try { @@ -94,8 +94,6 @@ export default class InstallHandler implements ICommandHandler { "Install 3rd party plug-ins at your own risk.\n" ); - params.response.console.log("Location = " + installRegistry); - // This section determines which npm logic needs to take place if (params.arguments.plugin == null || params.arguments.plugin.length === 0) { const configFile = typeof params.arguments.file === "undefined" ? @@ -108,7 +106,8 @@ export default class InstallHandler implements ICommandHandler { const packageJson: IPluginJson = readFileSync(configFile); if (Object.keys(packageJson).length === 0) { - params.response.console.log("No packages were found in " + configFile + ", so no plugins were installed."); + params.response.console.log("No packages were found in " + + configFile + ", so no plugins were installed."); return; } @@ -119,7 +118,9 @@ export default class InstallHandler implements ICommandHandler { // Registry is typed as optional in the doc but the function expects it // to be passed. So we'll always set it if it hasn't been done yet. const registryInfo = NpmRegistryUtils.buildRegistryInfo(packageInfo, params.arguments.registry); - packageInfo.location ??= registryInfo.location; + if (!packageInfo.location) { + packageInfo.location = registryInfo.location; + } this.console.debug(`Installing plugin: ${packageName}`); this.console.debug(`Package: ${packageInfo.package}`); @@ -136,6 +137,7 @@ export default class InstallHandler implements ICommandHandler { params.response.console.log("\n_______________________________________________________________"); const pluginName = await install(packageArgument, registryInfo, true); params.response.console.log("Installed plugin name = '" + pluginName + "'"); + params.response.console.log("Location = " + packageInfo.location); params.response.console.log(runValidatePlugin(pluginName)); } } @@ -147,6 +149,7 @@ export default class InstallHandler implements ICommandHandler { const registryInfo = NpmRegistryUtils.buildRegistryInfo(packageString, params.arguments.registry); const pluginName = await install(packageString, registryInfo); params.response.console.log("Installed plugin name = '" + pluginName + "'"); + params.response.console.log("Location = " + registryInfo.location); params.response.console.log(runValidatePlugin(pluginName)); } } @@ -168,7 +171,7 @@ export default class InstallHandler implements ICommandHandler { throw new ImperativeError({ msg: installResultMsg, causeErrors: e, - additionalDetails: e.message, + additionalDetails: e.message }); } } diff --git a/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts b/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts index 23d8f32b37..840cdfbc60 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts @@ -100,34 +100,40 @@ export class NpmRegistryUtils { public static buildRegistryInfo(packageInfo: string | IPluginJsonObject, userRegistry?: string): INpmRegistryInfo { const packageName = typeof packageInfo === "string" ? packageInfo : packageInfo.package; const packageScope = packageName.startsWith("@") ? packageName.split("/")[0] : undefined; - const isLocationRegistry = npmPackageArg(packageName).registry; if (userRegistry != null) { // If --registry was passed on the command line, it takes precedence return { - location: isLocationRegistry ? userRegistry : packageName, - npmArgs: this.buildRegistryNpmArgs(userRegistry) + location: userRegistry, + npmArgs: this.buildRegistryNpmArgs(userRegistry, packageScope) }; } else if (typeof packageInfo === "string" || !packageInfo.location) { // If installing a plug-in for the first time, get default registry - const defaultRegistry = isLocationRegistry && (packageScope != null ? this.getScopeRegistry(packageScope) : this.getRegistry()); - return { location: isLocationRegistry ? defaultRegistry : packageName, npmArgs: {} }; + const defaultRegistry = this.getRegistry(); + return { + location: npmPackageArg(packageName).registry ? defaultRegistry : packageName, + npmArgs: this.buildRegistryNpmArgs(defaultRegistry, packageScope) + }; } else { // If updating a plug-in, fetch registry info from plugins.json + const cachedRegistry = JsUtils.isUrl(packageInfo.location) ? packageInfo.location : undefined; return { location: packageInfo.location, - npmArgs: JsUtils.isUrl(packageInfo.location) ? this.buildRegistryNpmArgs(packageInfo.location, packageScope) : {} + npmArgs: this.buildRegistryNpmArgs(cachedRegistry ?? this.getRegistry(), packageScope) }; } } private static buildRegistryNpmArgs(registryUrl: string, scope?: string): Partial { - const registrySpec = scope != null ? `${scope}:registry` : "registry"; - return { [registrySpec]: registryUrl }; + const npmArgs: INpmRegistryInfo["npmArgs"] = { registry: registryUrl }; + if (scope != null) { + npmArgs[`${scope}:registry`] = this.getScopeRegistry(scope); + } + return npmArgs; } - private static getScopeRegistry(scope: string): string { + private static getScopeRegistry(scope: string): string | undefined { const execOutput = ExecUtils.spawnAndGetOutput(npmCmd, ["config", "get", `${scope}:registry`]); - if (execOutput.toString().trim() === "undefined") return this.getRegistry(); + if (execOutput.toString().trim() === "undefined") return; return execOutput.toString().replace("\n", ""); } } 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 d8a0d95f2e..70ef2e0f39 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 @@ -170,51 +170,55 @@ export async function install(packageLocation: string, registryInfo: INpmRegistr const pluginImpConfig = ConfigurationLoader.load(null, packageInfo, requirerFunction); iConsole.debug(`Checking for global Zowe client configuration files to update.`); - // Update the Imperative Configuration to add the profiles introduced by the recently installed plugin - const globalLayer = PMFConstants.instance.PLUGIN_CONFIG.layers.find((layer) => layer.global && layer.exists); - if (globalLayer && Array.isArray(pluginImpConfig.profiles)) { - UpdateImpConfig.addProfiles(pluginImpConfig.profiles); - 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(schemaInfo.resolved)); - } catch (err) { - iConsole.error("Error when adding new profile type for plugin %s: failed to parse schema", newPlugin.package); - } + 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 && Array.isArray(pluginImpConfig.profiles)) { + UpdateImpConfig.addProfiles(pluginImpConfig.profiles); + 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(schemaInfo.resolved)); + } 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 = ConfigUtils.readExtendersJson(); + // 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 = ConfigUtils.readExtendersJson(); - // Determine new profile types to add to schema - let shouldUpdate = false; - for (const profile of pluginImpConfig.profiles) { - if (!existingTypes.includes(profile.type)) { - loadedSchema.push(profile); - } else { - const existingType = loadedSchema.find((obj) => obj.type === profile.type); - if (semver.valid(existingType.schema.version)) { - if (semver.valid(profile.schema.version) && semver.gt(profile.schema.version, existingType.schema.version)) { + // Determine new profile types to add to schema + let shouldUpdate = false; + for (const profile of pluginImpConfig.profiles) { + if (!existingTypes.includes(profile.type)) { + loadedSchema.push(profile); + } else { + const existingType = loadedSchema.find((obj) => obj.type === profile.type); + 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.schema.version = profile.schema.version; + } + } else { existingType.schema = profile.schema; existingType.schema.version = profile.schema.version; } - } else { - existingType.schema = profile.schema; - existingType.schema.version = profile.schema.version; } + shouldUpdate = updateExtendersJson(extendersJson, packageInfo, profile) || shouldUpdate; } - shouldUpdate = updateExtendersJson(extendersJson, packageInfo, profile) || shouldUpdate; - } - if (shouldUpdate) { - // Update extenders.json (if necessary) after installing the plugin - ConfigUtils.writeExtendersJson(extendersJson); + if (shouldUpdate) { + // Update extenders.json (if necessary) after installing the plugin + ConfigUtils.writeExtendersJson(extendersJson); + } + const schema = ConfigSchema.buildSchema(loadedSchema); + ConfigSchema.updateSchema({ layer: "global", schema }); } - const schema = ConfigSchema.buildSchema(loadedSchema); - ConfigSchema.updateSchema({ layer: "global", schema }); } } } 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 22d6499723..90e001720d 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 @@ -128,26 +128,29 @@ export function uninstall(packageName: string): void { throw new Error("Failed to uninstall plugin, install folder still exists:\n " + installFolder); } - // Update the Imperative Configuration to add the profiles introduced by the recently installed plugin - const globalLayer = PMFConstants.instance.PLUGIN_CONFIG.layers.find((layer) => layer.global && layer.exists); - if (globalLayer) { - 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(schemaInfo.resolved)); - } catch (err) { - iConsole.error("Error when removing profile type for plugin %s: failed to parse schema", npmPackage); - } - // 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 }); + 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 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(schemaInfo.resolved)); + } catch (err) { + iConsole.error("Error when removing profile type for plugin %s: failed to parse schema", npmPackage); + } + // 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 6a1e1e0c716c9d08d6a6bacf37fb63fb457cd684 Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Thu, 24 Oct 2024 14:18:33 -0400 Subject: [PATCH 06/10] Add doc and clean up tests Signed-off-by: Timothy Johnson --- .../cmd/install/install.handler.unit.test.ts | 80 +++++-------------- .../cmd/update/update.handler.unit.test.ts | 18 ++--- .../src/plugins/doc/INpmRegistryInfo.ts | 7 ++ .../src/plugins/utilities/NpmFunctions.ts | 11 +++ 4 files changed, 43 insertions(+), 73 deletions(-) diff --git a/packages/imperative/src/imperative/__tests__/plugins/cmd/install/install.handler.unit.test.ts b/packages/imperative/src/imperative/__tests__/plugins/cmd/install/install.handler.unit.test.ts index 2e4bcdad4e..dbd84c1ba0 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/cmd/install/install.handler.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/cmd/install/install.handler.unit.test.ts @@ -44,7 +44,6 @@ import { PMFConstants } from "../../../../src/plugins/utilities/PMFConstants"; import { TextUtils } from "../../../../../utilities"; import { NpmRegistryUtils } from "../../../../src/plugins/utilities/NpmFunctions"; import * as spawn from "cross-spawn"; -import { INpmRegistryInfo } from "../../../../src/plugins/doc/INpmRegistryInfo"; describe("Plugin Management Facility install handler", () => { @@ -132,16 +131,17 @@ describe("Plugin Management Facility install handler", () => { */ const wasInstallCallValid = ( packageLocation: string, - registry: INpmRegistryInfo, - installFromFile = false + registry: string, + installFromFile = false, + extraNpmArgs = {} ) => { if (installFromFile) { expect(mocks.install).toHaveBeenCalledWith( - packageLocation, registry, true + packageLocation, { location: registry, npmArgs: { registry, ...extraNpmArgs } }, true ); } else { expect(mocks.install).toHaveBeenCalledWith( - packageLocation, registry + packageLocation, { location: registry, npmArgs: { registry, ...extraNpmArgs } } ); } }; @@ -207,14 +207,8 @@ describe("Plugin Management Facility install handler", () => { wasGetRegistryCalled(); expect(mocks.install).toHaveBeenCalledTimes(2); - wasInstallCallValid(`${fileJson.a.package}@${fileJson.a.version}`, { - location: packageRegistry, - npmArgs: { registry: packageRegistry } - }, true); - wasInstallCallValid(fileJson.plugin2.package, { - location: packageRegistry2, - npmArgs: { registry: packageRegistry2 } - }, true); + wasInstallCallValid(`${fileJson.a.package}@${fileJson.a.version}`, packageRegistry, true); + wasInstallCallValid(fileJson.plugin2.package, packageRegistry2, true); expect(mocks.runValidatePlugin).toHaveBeenCalledTimes(2); expect(mocks.runValidatePlugin).toHaveBeenCalledWith("a"); @@ -261,10 +255,7 @@ describe("Plugin Management Facility install handler", () => { wasGetRegistryCalled(); // Check that install worked as expected - wasInstallCallValid(params.arguments.plugin[0], { - location: packageRegistry, - npmArgs: { registry: packageRegistry } - }); + wasInstallCallValid(params.arguments.plugin[0], packageRegistry); // Check that success is output wasInstallSuccessful(params); @@ -280,10 +271,7 @@ describe("Plugin Management Facility install handler", () => { await handler.process(params as IHandlerParameters); // Validate the call to install with specified plugin and registry - wasInstallCallValid(params.arguments.plugin[0], { - location: params.arguments.registry, - npmArgs: { registry: params.arguments.registry } - }); + wasInstallCallValid(params.arguments.plugin[0], params.arguments.registry); wasInstallSuccessful(params); }); @@ -303,10 +291,7 @@ describe("Plugin Management Facility install handler", () => { wasNpmLoginCallValid(packageRegistry); // Check that install worked as expected - wasInstallCallValid(params.arguments.plugin[0], { - location: params.arguments.registry, - npmArgs: { registry: params.arguments.registry } - }); + wasInstallCallValid(params.arguments.plugin[0], params.arguments.registry); // Check that success is output wasInstallSuccessful(params); @@ -325,13 +310,9 @@ describe("Plugin Management Facility install handler", () => { // Validate that install was called with each of these values expect(mocks.install).toHaveBeenCalledTimes(params.arguments.plugin.length); - const registryInfo: INpmRegistryInfo = { - location: packageRegistry, - npmArgs: { registry: packageRegistry } - }; - wasInstallCallValid(params.arguments.plugin[0], registryInfo); - wasInstallCallValid(params.arguments.plugin[1], registryInfo); - wasInstallCallValid(params.arguments.plugin[2], registryInfo); + wasInstallCallValid(params.arguments.plugin[0], packageRegistry); + wasInstallCallValid(params.arguments.plugin[1], packageRegistry); + wasInstallCallValid(params.arguments.plugin[2], packageRegistry); wasInstallSuccessful(params); }); @@ -417,10 +398,7 @@ describe("Plugin Management Facility install handler", () => { expect(e).toBeUndefined(); } - wasInstallCallValid("@public/sample1", { - location: packageRegistry, - npmArgs: { registry: packageRegistry, "@public:registry": "publicRegistryUrl" } - }); + wasInstallCallValid("@public/sample1", packageRegistry, false, { "@public:registry": "publicRegistryUrl" }); }); it("should handle installed plugins via project/directory", async () => { const handler = new InstallHandler(); @@ -435,10 +413,7 @@ describe("Plugin Management Facility install handler", () => { expect(e).toBeUndefined(); } - wasInstallCallValid("path/to/dir", { - location: "path/to/dir", - npmArgs: { registry: packageRegistry } - }); + wasInstallCallValid("path/to/dir", "path/to/dir", false, { registry: packageRegistry }); }); it("should handle installed plugins via tarball file", async () => { const handler = new InstallHandler(); @@ -453,10 +428,7 @@ describe("Plugin Management Facility install handler", () => { expect(e).toBeUndefined(); } - wasInstallCallValid("path/to/dir/file.tgz", { - location: "path/to/dir/file.tgz", - npmArgs: { registry: packageRegistry } - }); + wasInstallCallValid("path/to/dir/file.tgz", "path/to/dir/file.tgz", false, { registry: packageRegistry }); }); it("should handle multiple installed plugins via tarball, directory, and registry", async () => { const handler = new InstallHandler(); @@ -474,21 +446,9 @@ describe("Plugin Management Facility install handler", () => { } expect(mocks.install).toHaveBeenCalledTimes(params.arguments.plugin.length); - wasInstallCallValid("@public/sample1", { - location: packageRegistry, - npmArgs: { registry: packageRegistry, "@public:registry": "publicRegistryUrl" } - }); - wasInstallCallValid("@private/sample1", { - location: packageRegistry, - npmArgs: { registry: packageRegistry, "@private:registry": "privateRegistryUrl" } - }); - wasInstallCallValid("path/to/dir", { - location: "path/to/dir", - npmArgs: { registry: packageRegistry } - }); - wasInstallCallValid("path/to/dir/file.tgz", { - location: "path/to/dir/file.tgz", - npmArgs: { registry: packageRegistry } - }); + wasInstallCallValid("@public/sample1", packageRegistry, false, { "@public:registry": "publicRegistryUrl" }); + wasInstallCallValid("@private/sample1", packageRegistry, false, { "@private:registry": "privateRegistryUrl" }); + wasInstallCallValid("path/to/dir", "path/to/dir", false, { registry: packageRegistry }); + wasInstallCallValid("path/to/dir/file.tgz", "path/to/dir/file.tgz", false, { registry: packageRegistry }); }); }); diff --git a/packages/imperative/src/imperative/__tests__/plugins/cmd/update/update.handler.unit.test.ts b/packages/imperative/src/imperative/__tests__/plugins/cmd/update/update.handler.unit.test.ts index e3c7922ff7..cd13230b3c 100644 --- a/packages/imperative/src/imperative/__tests__/plugins/cmd/update/update.handler.unit.test.ts +++ b/packages/imperative/src/imperative/__tests__/plugins/cmd/update/update.handler.unit.test.ts @@ -25,7 +25,6 @@ import { NpmRegistryUtils } from "../../../../src/plugins/utilities/NpmFunctions import * as childProcess from "child_process"; import * as jsonfile from "jsonfile"; import * as npmInterface from "../../../../src/plugins/utilities/npm-interface"; -import { INpmRegistryInfo } from "../../../../src/plugins/doc/INpmRegistryInfo"; describe("Plugin Management Facility update handler", () => { @@ -58,7 +57,6 @@ describe("Plugin Management Facility update handler", () => { jest.spyOn(Logger, "getImperativeLogger").mockReturnValue(new Logger(new Console())); mocks.execSyncSpy.mockReturnValue(packageRegistry); mocks.readFileSyncSpy.mockReturnValue({}); - NpmRegistryUtils.npmLogin(packageRegistry); }); /** @@ -88,10 +86,10 @@ describe("Plugin Management Facility update handler", () => { */ const wasUpdateCallValid = ( packageNameParm: string, - registry: INpmRegistryInfo + registry: string ) => { expect(mocks.updateSpy).toHaveBeenCalledWith( - packageNameParm, registry + packageNameParm, { location: registry, npmArgs: { registry } } ); }; @@ -151,16 +149,14 @@ describe("Plugin Management Facility update handler", () => { const params = getIHandlerParametersObject(); params.arguments.plugin = pluginName; params.arguments.registry = packageRegistry; + params.arguments.login = true; await handler.process(params as IHandlerParameters); // Validate the call to login wasNpmLoginCallValid(packageRegistry); wasWriteFileSyncValid(PMFConstants.instance.PLUGIN_JSON, fileJson); - wasUpdateCallValid(packageName, { - location: packageRegistry, - npmArgs: { registry: packageRegistry } - }); + wasUpdateCallValid(packageName, packageRegistry); wasUpdateSuccessful(params); }); @@ -186,12 +182,8 @@ describe("Plugin Management Facility update handler", () => { await handler.process(params as IHandlerParameters); // Validate the call to login - wasNpmLoginCallValid(packageRegistry); wasWriteFileSyncValid(PMFConstants.instance.PLUGIN_JSON, fileJson); - wasUpdateCallValid(resolveVal, { - location: packageRegistry, - npmArgs: { registry: packageRegistry } - }); + wasUpdateCallValid(resolveVal, packageRegistry); wasUpdateSuccessful(params); }); }); diff --git a/packages/imperative/src/imperative/src/plugins/doc/INpmRegistryInfo.ts b/packages/imperative/src/imperative/src/plugins/doc/INpmRegistryInfo.ts index 0e0ff9c3ee..220f675a5d 100644 --- a/packages/imperative/src/imperative/src/plugins/doc/INpmRegistryInfo.ts +++ b/packages/imperative/src/imperative/src/plugins/doc/INpmRegistryInfo.ts @@ -15,6 +15,13 @@ import { INpmInstallArgs } from "./INpmInstallArgs"; * Location info for an npm package. */ export interface INpmRegistryInfo { + /** + * The origin of npm package (registry URL or absolute path) + */ location: string; + + /** + * Defines npm config values to pass to `npm install` command + */ npmArgs: Partial; } diff --git a/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts b/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts index 840cdfbc60..849cd4a6ff 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts @@ -74,6 +74,11 @@ export async function getPackageInfo(pkgSpec: string): Promise<{ name: string, v } export class NpmRegistryUtils { + /** + * Get the registry to install to. + * @param userRegistry Registry override specified on the command line + * @return {string} + */ public static getRegistry(userRegistry?: string): string { if (userRegistry != null) return userRegistry; const execOutput = ExecUtils.spawnAndGetOutput(npmCmd, ["config", "get", "registry"]); @@ -97,6 +102,12 @@ export class NpmRegistryUtils { ); } + /** + * Get package location and npm registry args for installing it. + * @param packageInfo Plugin name or object from plugins.json + * @param userRegistry Registry override specified on the command line + * @returns Location info for npm package to be installed + */ public static buildRegistryInfo(packageInfo: string | IPluginJsonObject, userRegistry?: string): INpmRegistryInfo { const packageName = typeof packageInfo === "string" ? packageInfo : packageInfo.package; const packageScope = packageName.startsWith("@") ? packageName.split("/")[0] : undefined; From 14a02863613ce3eb4aff9c34090f51360738d76b Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Thu, 24 Oct 2024 15:08:03 -0400 Subject: [PATCH 07/10] Fix npm args for scoped registry Signed-off-by: Timothy Johnson --- .../src/imperative/src/plugins/cmd/install/install.handler.ts | 4 ++-- .../src/imperative/src/plugins/utilities/NpmFunctions.ts | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts b/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts index 607e2827c4..36cbcb6964 100644 --- a/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts +++ b/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts @@ -135,9 +135,9 @@ export default class InstallHandler implements ICommandHandler { this.console.debug(`Package: ${packageArgument}`); params.response.console.log("\n_______________________________________________________________"); + params.response.console.log("Location = " + packageInfo.location); const pluginName = await install(packageArgument, registryInfo, true); params.response.console.log("Installed plugin name = '" + pluginName + "'"); - params.response.console.log("Location = " + packageInfo.location); params.response.console.log(runValidatePlugin(pluginName)); } } @@ -147,9 +147,9 @@ export default class InstallHandler implements ICommandHandler { for (const packageString of params.arguments.plugin) { params.response.console.log("\n_______________________________________________________________"); const registryInfo = NpmRegistryUtils.buildRegistryInfo(packageString, params.arguments.registry); + params.response.console.log("Location = " + registryInfo.location); const pluginName = await install(packageString, registryInfo); params.response.console.log("Installed plugin name = '" + pluginName + "'"); - params.response.console.log("Location = " + registryInfo.location); params.response.console.log(runValidatePlugin(pluginName)); } } diff --git a/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts b/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts index 849cd4a6ff..b581b2e900 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts @@ -47,7 +47,8 @@ export function installPackages(npmPackage: string, npmArgs: INpmInstallArgs): s const args = ["install", npmPackage, "-g", "--legacy-peer-deps"]; for (const [k, v] of Object.entries(npmArgs)) { if (v != null) { - args.push(`--${k}`, v); + // If npm arg starts with @ like @zowe:registry, must use = as separator + args.push(...k.startsWith("@") ? [`--${k}=${v}`] : [`--${k}`, v]); } } const execOutput = ExecUtils.spawnAndGetOutput(npmCmd, args, { From 7243824d19ccd3212b9335fee7a22178066535b7 Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Thu, 24 Oct 2024 15:09:13 -0400 Subject: [PATCH 08/10] Update changelog Signed-off-by: Timothy Johnson --- packages/imperative/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/imperative/CHANGELOG.md b/packages/imperative/CHANGELOG.md index d155d3a4ac..7298bb3e18 100644 --- a/packages/imperative/CHANGELOG.md +++ b/packages/imperative/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to the Imperative package will be documented in this file. ## Recent Changes -- BugFix: Fixed an issue where `plugins install` command could fail when installing scoped package because scoped registry was used to fetch all dependencies. [#2317](https://github.com/zowe/zowe-cli/issues/2317) +- BugFix: Fixed an issue where the `plugins install` command could fail when installing a scoped package because scoped registry was used to fetch all dependencies. [#2317](https://github.com/zowe/zowe-cli/issues/2317) ## `8.2.0` From d1aedb29fe7e4683bd40eb310d1bc82fac2ef4fd Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Thu, 24 Oct 2024 16:20:29 -0400 Subject: [PATCH 09/10] Make command output prettier Signed-off-by: Timothy Johnson --- .../src/imperative/src/plugins/cmd/install/install.handler.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts b/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts index 36cbcb6964..e49163d0d4 100644 --- a/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts +++ b/packages/imperative/src/imperative/src/plugins/cmd/install/install.handler.ts @@ -135,7 +135,7 @@ export default class InstallHandler implements ICommandHandler { this.console.debug(`Package: ${packageArgument}`); params.response.console.log("\n_______________________________________________________________"); - params.response.console.log("Location = " + packageInfo.location); + params.response.console.log("Location = " + packageInfo.location + "\n"); const pluginName = await install(packageArgument, registryInfo, true); params.response.console.log("Installed plugin name = '" + pluginName + "'"); params.response.console.log(runValidatePlugin(pluginName)); @@ -147,7 +147,7 @@ export default class InstallHandler implements ICommandHandler { for (const packageString of params.arguments.plugin) { params.response.console.log("\n_______________________________________________________________"); const registryInfo = NpmRegistryUtils.buildRegistryInfo(packageString, params.arguments.registry); - params.response.console.log("Location = " + registryInfo.location); + params.response.console.log("Location = " + registryInfo.location + "\n"); const pluginName = await install(packageString, registryInfo); params.response.console.log("Installed plugin name = '" + pluginName + "'"); params.response.console.log(runValidatePlugin(pluginName)); From 95a11d622ea821121412d307a71770ab0501d967 Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Thu, 24 Oct 2024 17:52:37 -0400 Subject: [PATCH 10/10] Address PR feedback and handle edge case for remote packages Signed-off-by: Timothy Johnson --- .../src/imperative/src/plugins/utilities/NpmFunctions.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts b/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts index b581b2e900..b400464b07 100644 --- a/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts +++ b/packages/imperative/src/imperative/src/plugins/utilities/NpmFunctions.ts @@ -16,7 +16,7 @@ import { StdioOptions } from "child_process"; import { readFileSync } from "jsonfile"; import * as npmPackageArg from "npm-package-arg"; import * as pacote from "pacote"; -import { ExecUtils, JsUtils } from "../../../../utilities"; +import { ExecUtils } from "../../../../utilities"; import { INpmInstallArgs } from "../doc/INpmInstallArgs"; import { IPluginJsonObject } from "../doc/IPluginJsonObject"; import { INpmRegistryInfo } from "../doc/INpmRegistryInfo"; @@ -97,9 +97,8 @@ export class NpmRegistryUtils { "--registry", registry, "--always-auth", "--auth-type=legacy" - ], { - stdio: [0,1,2] - } + ], + { stdio: "inherit" } ); } @@ -127,7 +126,7 @@ export class NpmRegistryUtils { }; } else { // If updating a plug-in, fetch registry info from plugins.json - const cachedRegistry = JsUtils.isUrl(packageInfo.location) ? packageInfo.location : undefined; + const cachedRegistry = npmPackageArg(packageInfo.package).registry ? packageInfo.location : undefined; return { location: packageInfo.location, npmArgs: this.buildRegistryNpmArgs(cachedRegistry ?? this.getRegistry(), packageScope)