diff --git a/lib/common b/lib/common index 1383f3e88b..70ba4ca1ac 160000 --- a/lib/common +++ b/lib/common @@ -1 +1 @@ -Subproject commit 1383f3e88b873cefac8a0c6738ad139727e1f0ed +Subproject commit 70ba4ca1ac271b52c2cafbb4fbdbfb66a00744ed diff --git a/lib/definitions/pacote-service.d.ts b/lib/definitions/pacote-service.d.ts index 9f89b0e6df..8c24302a5c 100644 --- a/lib/definitions/pacote-service.d.ts +++ b/lib/definitions/pacote-service.d.ts @@ -18,7 +18,7 @@ declare global { extractPackage(packageName: string, destinationDirectory: string, options?: IPacoteExtractOptions): Promise; } - interface IPacoteBaseOptions { + interface IPacoteBaseOptions extends IProxySettingsBase { /** * The path to npm cache */ diff --git a/lib/services/pacote-service.ts b/lib/services/pacote-service.ts index a69f12481f..94c12fbd0c 100644 --- a/lib/services/pacote-service.ts +++ b/lib/services/pacote-service.ts @@ -4,48 +4,78 @@ import * as path from "path"; export class PacoteService implements IPacoteService { constructor(private $fs: IFileSystem, - private $npm: INodePackageManager) { } + private $npm: INodePackageManager, + private $proxyService: IProxyService, + private $logger: ILogger) { } public async manifest(packageName: string, options?: IPacoteManifestOptions): Promise { - // In case `tns create myapp --template https://github.com/NativeScript/template-hello-world.git` command is executed, pacote module throws an error if cache option is not provided. - const cache = await this.$npm.getCachePath(); - const manifestOptions = { cache }; + this.$logger.trace(`Calling pacoteService.manifest for packageName: '${packageName}' and options: ${options}`); + const manifestOptions: IPacoteBaseOptions = await this.getPacoteBaseOptions(); if (options) { _.extend(manifestOptions, options); } - if (this.$fs.exists(packageName)) { - packageName = path.resolve(packageName); - } - + packageName = this.getRealPackageName(packageName); + this.$logger.trace(`Calling pacote.manifest for packageName: ${packageName} and options: ${JSON.stringify(manifestOptions, null, 2)}`); return pacote.manifest(packageName, manifestOptions); } public async extractPackage(packageName: string, destinationDirectory: string, options?: IPacoteExtractOptions): Promise { // strip: Remove the specified number of leading path elements. Pathnames with fewer elements will be silently skipped. More info: https://github.com/npm/node-tar/blob/e89c4d37519b1c20133a9f49d5f6b85fa34c203b/README.md // C: Create an archive + this.$logger.trace(`Calling pacoteService.extractPackage for packageName: '${packageName}', destinationDir: '${destinationDirectory}' and options: ${options}`); const extractOptions = { strip: 1, C: destinationDirectory }; if (options) { _.extend(extractOptions, options); } - if (this.$fs.exists(packageName)) { - packageName = path.resolve(packageName); - } + packageName = this.getRealPackageName(packageName); + const pacoteOptions = await this.getPacoteBaseOptions(); - const cache = await this.$npm.getCachePath(); return new Promise((resolve, reject) => { - const source = pacote.tarball.stream(packageName, { cache }); + this.$logger.trace(`Calling pacoteService.extractPackage for packageName: '${packageName}', destinationDir: '${destinationDirectory}' and options: ${options}`); + + const source = pacote.tarball.stream(packageName, pacoteOptions); source.on("error", (err: Error) => { + this.$logger.trace(`Error in source while trying to extract stream from ${packageName}. Error is ${err}`); reject(err); }); + this.$logger.trace(`Creating extract tar stream with options: ${JSON.stringify(extractOptions, null, 2)}`); const destination = tar.x(extractOptions); source.pipe(destination); - destination.on("error", (err: Error) => reject(err)); - destination.on("finish", () => resolve()); + destination.on("error", (err: Error) => { + this.$logger.trace(`Error in destination while trying to extract stream from ${packageName}. Error is ${err}`); + reject(err); + }); + + destination.on("finish", () => { + this.$logger.trace(`Successfully extracted '${packageName}' to ${destinationDirectory}`); + resolve(); + }); }); } + + private async getPacoteBaseOptions(): Promise { + // In case `tns create myapp --template https://github.com/NativeScript/template-hello-world.git` command is executed, pacote module throws an error if cache option is not provided. + const cache = await this.$npm.getCachePath(); + const pacoteOptions = { cache }; + const proxySettings = await this.$proxyService.getCache(); + if (proxySettings) { + _.extend(pacoteOptions, proxySettings); + } + + return pacoteOptions; + } + + private getRealPackageName(packageName: string): string { + if (this.$fs.exists(packageName)) { + this.$logger.trace(`Will resolve the full path to package ${packageName}.`); + packageName = path.resolve(packageName); + } + + return packageName; + } } $injector.register("pacoteService", PacoteService); diff --git a/test/project-service.ts b/test/project-service.ts index 441e8ca461..2e887c6eb0 100644 --- a/test/project-service.ts +++ b/test/project-service.ts @@ -152,6 +152,9 @@ class ProjectIntegrationTest { executeAfterHooks: async (commandName: string, hookArguments?: IDictionary): Promise => undefined }); this.testInjector.register("pacoteService", PacoteService); + this.testInjector.register("proxyService", { + getCache: async (): Promise => null + }); } } diff --git a/test/services/pacote-service.ts b/test/services/pacote-service.ts new file mode 100644 index 0000000000..1d00ae4bc8 --- /dev/null +++ b/test/services/pacote-service.ts @@ -0,0 +1,282 @@ +import { Yok } from "../../lib/common/yok"; +import { assert } from "chai"; +import { PacoteService } from '../../lib/services/pacote-service'; +import { LoggerStub } from "../stubs"; +import { sandbox, SinonSandbox, SinonStub } from "sinon"; +import { EventEmitter } from "events"; +const pacote = require("pacote"); +const tar = require("tar"); +const path = require("path"); + +const npmCachePath = "npmCachePath"; +const packageName = "testPackage"; +const fullPath = `/Users/username/${packageName}`; +const destinationDir = "destinationDir"; +const defaultPacoteOpts: IPacoteBaseOptions = { cache: npmCachePath }; +const errorMessage = "error message"; +const proxySettings: IProxySettings = { + hostname: "hostname", + proxy: "proxy", + port: "8888", + rejectUnauthorized: true, + username: null, + password: null +}; + +interface ITestSetup { + isLocalPackage?: boolean; + useProxySettings?: boolean; + npmGetCachePathError?: Error; +} + +interface ITestCase extends ITestSetup { + manifestOptions?: IPacoteManifestOptions; + additionalExtractOpts?: IPacoteExtractOptions; + name: string; + expectedArgs: any[]; +} + +const createTestInjector = (opts?: ITestSetup): IInjector => { + opts = opts || {}; + + const testInjector = new Yok(); + testInjector.register("logger", LoggerStub); + testInjector.register("pacoteService", PacoteService); + testInjector.register("fs", { + exists: (p: string): boolean => opts.isLocalPackage + }); + + testInjector.register("proxyService", { + getCache: async (): Promise => opts.useProxySettings ? proxySettings : null + }); + + testInjector.register("npm", { + getCachePath: async (): Promise => { + if (opts.npmGetCachePathError) { + throw opts.npmGetCachePathError; + } + + return npmCachePath; + } + }); + + return testInjector; +}; + +class MockStream extends EventEmitter { + public pipe(destination: any, options?: { end?: boolean; }): any { + // Nothing to do here, just mock the method. + } +} + +describe("pacoteService", () => { + const manifestResult: any = {}; + const manifestOptions: IPacoteManifestOptions = { fullMetadata: true }; + let sandboxInstance: SinonSandbox = null; + let manifestStub: SinonStub = null; + let tarballStreamStub: SinonStub = null; + let tarXStub: SinonStub = null; + let tarballSourceStream: MockStream = null; + let tarExtractDestinationStream: MockStream = null; + + beforeEach(() => { + sandboxInstance = sandbox.create(); + manifestStub = sandboxInstance.stub(pacote, "manifest").returns(Promise.resolve(manifestResult)); + tarballSourceStream = new MockStream(); + tarballStreamStub = sandboxInstance.stub(pacote.tarball, "stream").returns(tarballSourceStream); + tarExtractDestinationStream = new MockStream(); + tarXStub = sandboxInstance.stub(tar, "x").returns(tarExtractDestinationStream); + }); + + afterEach(() => { + sandboxInstance.restore(); + }); + + const setupTest = (opts?: ITestSetup): IPacoteService => { + opts = opts || {}; + const testInjector = createTestInjector(opts); + if (opts.isLocalPackage) { + sandboxInstance.stub(path, "resolve").withArgs(packageName).returns(fullPath); + } + + return testInjector.resolve("pacoteService"); + }; + + describe("manifest", () => { + describe("calls pacote.manifest", () => { + + const testData: ITestCase[] = [ + { + name: "with 'cache' only when no opts are passed", + expectedArgs: [packageName, defaultPacoteOpts] + }, + { + name: "with 'cache' and passed options", + manifestOptions, + expectedArgs: [packageName, _.extend({}, defaultPacoteOpts, manifestOptions)] + }, + { + name: "with 'cache' and proxy settings", + useProxySettings: true, + expectedArgs: [packageName, _.extend({}, defaultPacoteOpts, proxySettings)] + }, + { + name: "with 'cache', passed options and proxy settings when proxy is configured", + manifestOptions, + useProxySettings: true, + expectedArgs: [packageName, _.extend({}, defaultPacoteOpts, manifestOptions, proxySettings)] + }, + { + name: "with full path to file when it is local one", + isLocalPackage: true, + expectedArgs: [fullPath, defaultPacoteOpts] + }, + { + name: "with full path to file, 'cache' and passed options when local path is passed", + manifestOptions, + isLocalPackage: true, + expectedArgs: [fullPath, _.extend({}, defaultPacoteOpts, manifestOptions)] + }, + { + name: "with full path to file, 'cache' and proxy settings when proxy is configured", + manifestOptions, + isLocalPackage: true, + useProxySettings: true, + expectedArgs: [fullPath, _.extend({}, defaultPacoteOpts, manifestOptions, proxySettings)] + }, + { + name: "with full path to file, 'cache', passed options and proxy settings when proxy is configured and local path is passed", + manifestOptions, + useProxySettings: true, + isLocalPackage: true, + expectedArgs: [fullPath, _.extend({}, defaultPacoteOpts, manifestOptions, proxySettings)] + }, + ]; + + testData.forEach(testCase => { + it(testCase.name, async () => { + const pacoteService = setupTest(testCase); + const result = await pacoteService.manifest(packageName, testCase.manifestOptions); + + assert.equal(result, manifestResult); + assert.deepEqual(manifestStub.firstCall.args, testCase.expectedArgs); + }); + }); + }); + + it("fails with npm error when unable to get npm cache", async () => { + const npmGetCachePathError = new Error("npm error"); + const pacoteService = setupTest({ npmGetCachePathError }); + await assert.isRejected(pacoteService.manifest(packageName, null), npmGetCachePathError.message); + }); + }); + + describe("extractPackage", () => { + it("fails with correct error when pacote.tarball.stream raises error event", async () => { + const pacoteService = setupTest(); + + const pacoteExtractPackagePromise = pacoteService.extractPackage(packageName, destinationDir); + setImmediate(() => { + tarballSourceStream.emit("error", new Error(errorMessage)); + }); + + await assert.isRejected(pacoteExtractPackagePromise, errorMessage); + }); + + it("fails with correct error when the destination stream raises error event", async () => { + const pacoteService = setupTest(); + + const pacoteExtractPackagePromise = pacoteService.extractPackage(packageName, destinationDir); + setImmediate(() => { + tarExtractDestinationStream.emit("error", new Error(errorMessage)); + }); + + await assert.isRejected(pacoteExtractPackagePromise, errorMessage); + }); + + it("resolves when the destination stream emits finish event", async () => { + const pacoteService = setupTest(); + + const pacoteExtractPackagePromise = pacoteService.extractPackage(packageName, destinationDir); + setImmediate(() => { + tarExtractDestinationStream.emit("finish"); + }); + + await assert.isFulfilled(pacoteExtractPackagePromise); + }); + + describe("passes correct options to tar.x", () => { + const defaultExtractOpts = { strip: 1, C: destinationDir }; + const additionalExtractOpts: IPacoteExtractOptions = { + filter: (p: string, stat: any) => true + }; + + const testData: ITestCase[] = [ + { + name: "when only default options should be passed", + expectedArgs: [defaultExtractOpts], + }, + { + name: "when additional options are passed", + expectedArgs: [_.extend({}, defaultExtractOpts, additionalExtractOpts)], + additionalExtractOpts + }, + ]; + + testData.forEach(testCase => { + it(testCase.name, async () => { + const pacoteService = setupTest(); + + const pacoteExtractPackagePromise = pacoteService.extractPackage(packageName, destinationDir, testCase.additionalExtractOpts); + setImmediate(() => { + tarExtractDestinationStream.emit("finish"); + }); + + await assert.isFulfilled(pacoteExtractPackagePromise); + + assert.deepEqual(tarXStub.firstCall.args, testCase.expectedArgs); + }); + }); + }); + + describe("passes correct options to pacote.tarball.stream", () => { + const testData: ITestCase[] = [ + { + name: "when proxy is not set", + expectedArgs: [packageName, defaultPacoteOpts] + }, + { + name: "when proxy is not set and a local path is passed", + isLocalPackage: true, + expectedArgs: [fullPath, defaultPacoteOpts] + }, + { + name: "when proxy is set", + useProxySettings: true, + expectedArgs: [packageName, _.extend({}, defaultPacoteOpts, proxySettings)] + }, + { + name: "when proxy is set and a local path is passed", + useProxySettings: true, + isLocalPackage: true, + expectedArgs: [fullPath, _.extend({}, defaultPacoteOpts, proxySettings)] + }, + + ]; + + testData.forEach(testCase => { + it(testCase.name, async () => { + const pacoteService = setupTest(testCase); + + const pacoteExtractPackagePromise = pacoteService.extractPackage(packageName, destinationDir); + setImmediate(() => { + tarExtractDestinationStream.emit("finish"); + }); + + await assert.isFulfilled(pacoteExtractPackagePromise); + assert.deepEqual(tarballStreamStub.firstCall.args, testCase.expectedArgs); + }); + }); + }); + }); +});