diff --git a/.gitignore b/.gitignore index ab4e9c47e..033a02778 100644 --- a/.gitignore +++ b/.gitignore @@ -161,6 +161,7 @@ typings/ # Output of 'npm pack' *.tgz +!tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib-1.0.0.tgz # Except test file !tests/functional/workflows/ruby_bundler/test_data/test.tgz diff --git a/aws_lambda_builders/workflows/nodejs_npm/actions.py b/aws_lambda_builders/workflows/nodejs_npm/actions.py index ad6e7a927..8e8114b08 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/actions.py +++ b/aws_lambda_builders/workflows/nodejs_npm/actions.py @@ -195,3 +195,54 @@ def execute(self): except OSError as ex: raise ActionFailedError(str(ex)) + +class NodejsNpmRewriteLocalDependenciesAction(BaseAction): + + """ + A Lambda Builder Action that rewrites local dependencies + """ + + NAME = 'RewriteLocalDependencies' + DESCRIPTION = "Rewrites local dependencies" + PURPOSE = Purpose.RESOLVE_DEPENDENCIES + + def __init__( + self, + work_dir, + original_package_dir, + scratch_dir, + npm_modules_utils, + osutils + ): + super(NodejsNpmRewriteLocalDependenciesAction, self).__init__() + self.work_dir = work_dir + self.original_package_dir = original_package_dir + self.scratch_dir = scratch_dir + self.npm_modules_utils = npm_modules_utils + self.osutils = osutils + + def __rewrite_local_dependencies(self, work_dir, original_package_dir): + for dependency_key in ['dependencies', 'optionalDependencies']: + for (name, module_path) in self.npm_modules_utils.get_local_dependencies(work_dir, dependency_key).items(): + if module_path.startswith('file:'): + module_path = module_path[5:] + + actual_path = self.osutils.joinpath(original_package_dir, module_path) + if self.osutils.is_dir(actual_path): + if self.npm_modules_utils.has_local_dependencies(actual_path): + module_path = self.npm_modules_utils.clean_copy(actual_path, delete_package_lock=True) + self.__rewrite_local_dependencies(module_path, actual_path) + actual_path = module_path + + new_module_path = self.npm_modules_utils.pack_to_tar(actual_path) + else: + new_module_path = actual_path + + new_module_path = 'file:{}'.format(new_module_path) + self.npm_modules_utils.update_dependency(work_dir, name, new_module_path, dependency_key) + + def execute(self): + try: + self.__rewrite_local_dependencies(self.work_dir, self.original_package_dir) + except OSError as ex: + raise ActionFailedError(str(ex)) diff --git a/aws_lambda_builders/workflows/nodejs_npm/npm.py b/aws_lambda_builders/workflows/nodejs_npm/npm.py index b737ff4d1..fb921a06f 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/npm.py +++ b/aws_lambda_builders/workflows/nodejs_npm/npm.py @@ -3,6 +3,7 @@ """ import logging +import json LOG = logging.getLogger(__name__) @@ -88,3 +89,150 @@ def run(self, args, cwd=None): raise NpmExecutionError(message=err.decode('utf8').strip()) return out.decode('utf8').strip() + + +class NpmModulesUtils(object): + + """ + Utility class that abstracts operations on NPM packages + and manifest files + """ + + def __init__(self, osutils, subprocess_npm, scratch_dir): + """ + :type osutils: aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils + :param osutils: An instance of OS Utilities for file manipulation + + :type subprocess_npm: aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm + :param subprocess_npm: An instance of NPM Subprocess for executing the NPM binary + + :type scratch_dir: str + :param scratch_dir: a writable temporary directory + """ + + self.osutils = osutils + self.subprocess_npm = subprocess_npm + self.scratch_dir = scratch_dir + + def clean_copy(self, package_dir, delete_package_lock=False): + + """ + Produces a clean copy of a NPM package source from a project directory, + so it can be packaged without temporary files, development or test resources + or dependencies. + + :type package_dir: str + :param package_dir: Path to a NPM project directory + + :type delete_package_lock: bool + :param delete_package_lock: If true, package-lock.json will be removed in the copy + """ + + target_dir = self.osutils.tempdir(self.scratch_dir) + + package_path = 'file:{}'.format(self.osutils.abspath(package_dir)) + + LOG.debug('NODEJS packaging %s to %s', package_path, self.scratch_dir) + + tarfile_name = self.subprocess_npm.run(['pack', '-q', package_path], cwd=self.scratch_dir) + + LOG.debug('NODEJS packed to %s', tarfile_name) + + tarfile_path = self.osutils.joinpath(self.scratch_dir, tarfile_name) + + LOG.debug('NODEJS extracting to %s', target_dir) + + self.osutils.extract_tarfile(tarfile_path, target_dir) + + package_lock = self.osutils.joinpath(target_dir, 'package', 'package-lock.json') + if delete_package_lock and self.osutils.file_exists(package_lock): + self.osutils.remove_file(package_lock) + + return self.osutils.joinpath(target_dir, 'package') + + def is_local_dependency(self, module_path): + """ + Calculates if the module path from a dependency reference is + local or remote + + :type module_path: str + :param module_path: Dependency reference value (from package.json) + + """ + + return module_path.startswith('file:') or \ + module_path.startswith('.') or \ + module_path.startswith('/') or \ + module_path.startswith('~/') + + def get_local_dependencies(self, package_dir, dependency_key='dependencies'): + """ + Returns a dictionary with only local dependencies from a package.json manifest + + :type package_dir: str + :param package_dir: path to a NPM project directory (containing package.json) + + :type dependency_key: str + :param dependency_key: dependency type to return, corresponds to a key of package.json. + (for example, 'dependencies' or 'optionalDependencies') + """ + + package_json = json.loads(self.osutils.get_text_contents(self.osutils.joinpath(package_dir, 'package.json'))) + if dependency_key not in package_json.keys(): + return {} + + dependencies = package_json[dependency_key] + + return dict( + [(name, module_path) for (name, module_path) in dependencies.items() + if self.is_local_dependency(module_path)] + ) + + def has_local_dependencies(self, package_dir): + """ + Checks if a NPM project has local dependencies + + :type package_dir: str + :param package_dir: path to a NPM project directory (containing package.json) + """ + return len(self.get_local_dependencies(package_dir, 'dependencies')) > 0 or \ + len(self.get_local_dependencies(package_dir, 'optionalDependencies')) > 0 + + def pack_to_tar(self, package_dir): + """ + Runs npm pack to produce a tar containing project sources, which can be used + as a target for project dependencies + + :type package_dir: str + :param package_dir: path to a NPM project directory (containing package.json) + """ + package_path = "file:{}".format(self.osutils.abspath(package_dir)) + + tarfile_name = self.subprocess_npm.run(['pack', '-q', package_path], cwd=self.scratch_dir) + + return self.osutils.joinpath(self.scratch_dir, tarfile_name) + + def update_dependency(self, package_dir, name, module_path, dependency_key='dependencies'): + """ + Updates package.json by rewriting a dependency to point to a specified module path + + :type package_dir: str + :param package_dir: path to a NPM project directory (containing package.json) + + :type name: str + :param name: the name of the dependency (sub-key in package.json) + + :type module_path: str + :param module_path: new destination for the dependency + + :type dependency_key: str + :param dependency_key: dependency type to return, corresponds to a key of package.json. + (for example, 'dependencies' or 'optionalDependencies') + """ + + package_json_path = self.osutils.joinpath(package_dir, 'package.json') + package_json = json.loads(self.osutils.get_text_contents(package_json_path)) + + package_json[dependency_key][name] = module_path + package_json_contents = json.dumps(package_json, ensure_ascii=False) + self.osutils.write_text_contents(package_json_path, package_json_contents) diff --git a/aws_lambda_builders/workflows/nodejs_npm/utils.py b/aws_lambda_builders/workflows/nodejs_npm/utils.py index b34863788..699147c5d 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/utils.py +++ b/aws_lambda_builders/workflows/nodejs_npm/utils.py @@ -7,7 +7,8 @@ import tarfile import subprocess import shutil - +import io +import tempfile class OSUtils(object): @@ -48,3 +49,17 @@ def abspath(self, path): def is_windows(self): return platform.system().lower() == 'windows' + + def get_text_contents(self, filename, encoding='utf-8'): + with io.open(filename, 'r', encoding=encoding) as f: + return f.read() + + def write_text_contents(self, filename, contents, encoding='utf-8'): + with io.open(filename, 'w', encoding=encoding) as f: + f.write(contents) + + def tempdir(self, parent_dir): + return tempfile.mkdtemp(dir=parent_dir) + + def is_dir(self, path): + return os.path.isdir(path) diff --git a/aws_lambda_builders/workflows/nodejs_npm/workflow.py b/aws_lambda_builders/workflows/nodejs_npm/workflow.py index dc6be8ea4..0ceb9abf8 100644 --- a/aws_lambda_builders/workflows/nodejs_npm/workflow.py +++ b/aws_lambda_builders/workflows/nodejs_npm/workflow.py @@ -4,9 +4,10 @@ from aws_lambda_builders.path_resolver import PathResolver from aws_lambda_builders.workflow import BaseWorkflow, Capability from aws_lambda_builders.actions import CopySourceAction -from .actions import NodejsNpmPackAction, NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction +from .actions import NodejsNpmPackAction, NodejsNpmInstallAction, NodejsNpmrcCopyAction, \ + NodejsNpmrcCleanUpAction, NodejsNpmRewriteLocalDependenciesAction from .utils import OSUtils -from .npm import SubprocessNpm +from .npm import SubprocessNpm, NpmModulesUtils class NodejsNpmWorkflow(BaseWorkflow): @@ -62,6 +63,13 @@ def __init__(self, npm_pack, npm_copy_npmrc, CopySourceAction(tar_package_dir, artifacts_dir, excludes=self.EXCLUDED_FILES), + NodejsNpmRewriteLocalDependenciesAction( + artifacts_dir, + source_dir, + tar_dest_dir, + npm_modules_utils=NpmModulesUtils(osutils, subprocess_npm, scratch_dir), + osutils=osutils + ), npm_install, NodejsNpmrcCleanUpAction(artifacts_dir, osutils=osutils) ] diff --git a/tests/functional/workflows/nodejs_npm/test_utils.py b/tests/functional/workflows/nodejs_npm/test_utils.py index 01fd52f01..5c7c0e55c 100644 --- a/tests/functional/workflows/nodejs_npm/test_utils.py +++ b/tests/functional/workflows/nodejs_npm/test_utils.py @@ -2,6 +2,7 @@ import shutil import sys import tempfile +import io from unittest import TestCase @@ -65,7 +66,7 @@ def test_file_exists_checking_if_file_exists_in_a_dir(self): self.assertTrue(self.osutils.file_exists(existing_file)) self.assertFalse(self.osutils.file_exists(nonexisting_file)) - + def test_extract_tarfile_unpacks_a_tar(self): test_tar = os.path.join(os.path.dirname(__file__), "test_data", "test.tgz") @@ -127,3 +128,36 @@ def test_popen_can_accept_cwd(self): self.assertEqual(p.returncode, 0) self.assertEqual(out.decode('utf8').strip(), os.path.abspath(testdata_dir)) + + def test_reads_file_content(self): + scratch_dir = tempfile.mkdtemp() + filename = os.path.join(scratch_dir, 'test_write.txt') + + with io.open(filename, 'w', encoding='utf-8') as f: + f.write(u'hello') + + content = self.osutils.get_text_contents(filename) + self.assertEqual(content, 'hello') + + def test_writes_text_context(self): + scratch_dir = tempfile.mkdtemp() + filename = os.path.join(scratch_dir, 'test_write.txt') + self.osutils.write_text_contents(filename, u'hello') + with io.open(filename, 'r', encoding='utf-8') as f: + content = f.read() + self.assertEqual(content, u'hello') + + def test_should_create_temporary_dir(self): + scratch_dir = tempfile.mkdtemp() + temp_dir = self.osutils.tempdir(scratch_dir) + os.path.isdir(temp_dir) + + def test_is_dir_returns_true_if_dir_exists(self): + dir_exists = self.osutils.is_dir(os.path.dirname(__file__)) + + self.assertTrue(dir_exists) + + def test_is_dir_returns_false_if_dir_does_not_exist(self): + dir_does_not_exist = self.osutils.is_dir(os.path.join(os.path.dirname(__file__), 'some_random_dirname')) + + self.assertFalse(dir_does_not_exist) diff --git a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py index a8533d5ce..405188ebc 100644 --- a/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py +++ b/tests/integration/workflows/nodejs_npm/test_nodejs_npm.py @@ -93,3 +93,78 @@ def test_fails_if_package_json_is_broken(self): runtime=self.runtime) self.assertIn("Unexpected end of JSON input", str(ctx.exception)) + + def test_builds_project_with_relative_dependencies(self): + source_dir = os.path.join(self.TEST_DATA_FOLDER, "relative-dependencies", "lambda") + + self.builder.build(source_dir, self.artifacts_dir, self.scratch_dir, + os.path.join(source_dir, "package.json"), + runtime=self.runtime) + + expected_files = {"package.json", "main.js", "node_modules"} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEquals(expected_files, output_files) + + expected_modules = {"lib"} + output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules"))) + self.assertEquals(expected_modules, output_modules) + + def test_builds_project_with_several_levels_of_relative_dependencies(self): + source_dir = os.path.join(self.TEST_DATA_FOLDER, 'relative-dependencies', 'parent_lambda') + + self.builder.build(source_dir, self.artifacts_dir, self.scratch_dir, + os.path.join(source_dir, 'package.json'), + runtime=self.runtime) + + expected_files = {'package.json', 'parent_main.js', 'node_modules'} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEquals(expected_files, output_files) + + expected_modules = {'lambda', 'lib'} + output_modules = set(os.listdir(os.path.join(self.artifacts_dir, 'node_modules'))) + self.assertEquals(expected_modules, output_modules) + + def test_builds_project_with_locked_dependencies(self): + source_dir = os.path.join(self.TEST_DATA_FOLDER, 'relative-dependencies', 'lambda_with_lock_file') + + self.builder.build(source_dir, self.artifacts_dir, self.scratch_dir, + os.path.join(source_dir, 'package.json'), + runtime=self.runtime) + + expected_files = {'package.json', 'main_lambda_with_lock_file.js', 'node_modules'} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEquals(expected_files, output_files) + + expected_modules = {'lib', 'lib_with_lock_file'} + output_modules = set(os.listdir(os.path.join(self.artifacts_dir, 'node_modules'))) + self.assertEquals(expected_modules, output_modules) + + def test_builds_project_with_locked_sub_dependencies(self): + source_dir = os.path.join(self.TEST_DATA_FOLDER, 'relative-dependencies', 'lambda_without_lock_file') + + self.builder.build(source_dir, self.artifacts_dir, self.scratch_dir, + os.path.join(source_dir, 'package.json'), + runtime=self.runtime) + + expected_files = {'package.json', 'main_lambda_without_lock_file.js', 'node_modules'} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEquals(expected_files, output_files) + + expected_modules = {'lib', 'lib_with_lock_file'} + output_modules = set(os.listdir(os.path.join(self.artifacts_dir, 'node_modules'))) + self.assertEquals(expected_modules, output_modules) + + def test_builds_project_with_local_file_dependencies(self): + source_dir = os.path.join(self.TEST_DATA_FOLDER, 'relative-dependencies', 'lambda_with_tar_dependency') + + self.builder.build(source_dir, self.artifacts_dir, self.scratch_dir, + os.path.join(source_dir, 'package.json'), + runtime=self.runtime) + + expected_files = {'package.json', 'main_lambda_with_tar_dependency.js', 'node_modules'} + output_files = set(os.listdir(self.artifacts_dir)) + self.assertEquals(expected_files, output_files) + + expected_modules = {'lib'} + output_modules = set(os.listdir(os.path.join(self.artifacts_dir, 'node_modules'))) + self.assertEquals(expected_modules, output_modules) diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda/main.js b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda/main.js new file mode 100644 index 000000000..3a185fe95 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda/main.js @@ -0,0 +1,6 @@ +/*global exports, require */ +const lib = require('lib'); +exports.handler = function (event, context) { + 'use strict'; + context.succeed(lib()); +}; diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda/package.json b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda/package.json new file mode 100644 index 000000000..262bd67d1 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda/package.json @@ -0,0 +1,14 @@ +{ + "name": "lambda", + "version": "1.0.0", + "description": "", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "author": "", + "license": "ISC", + "dependencies": { + "lib": "file:../lib" + } +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_lock_file/main_lambda_with_lock_file.js b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_lock_file/main_lambda_with_lock_file.js new file mode 100644 index 000000000..3a185fe95 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_lock_file/main_lambda_with_lock_file.js @@ -0,0 +1,6 @@ +/*global exports, require */ +const lib = require('lib'); +exports.handler = function (event, context) { + 'use strict'; + context.succeed(lib()); +}; diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_lock_file/package-lock.json b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_lock_file/package-lock.json new file mode 100644 index 000000000..ad99ce1f8 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_lock_file/package-lock.json @@ -0,0 +1,20 @@ +{ + "name": "lambda", + "version": "1.0.0", + "lockfileVersion": 1, + "requires": true, + "dependencies": { + "lib_with_lock_file": { + "version": "file:../INVALID_DIR", + "requires": { + "lib": "file:../lib_INVALID_DIR" + }, + "dependencies": { + "lib": { + "version": "file:../lib_INVALID_DIR", + "bundled": true + } + } + } + } +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_lock_file/package.json b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_lock_file/package.json new file mode 100644 index 000000000..4e514c113 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_lock_file/package.json @@ -0,0 +1,14 @@ +{ + "name": "lambda", + "version": "1.0.0", + "description": "", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "author": "", + "license": "ISC", + "dependencies": { + "lib_with_lock_file": "file:../lib_with_lock_file" + } +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_tar_dependency/main_lambda_with_tar_dependency.js b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_tar_dependency/main_lambda_with_tar_dependency.js new file mode 100644 index 000000000..3a185fe95 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_tar_dependency/main_lambda_with_tar_dependency.js @@ -0,0 +1,6 @@ +/*global exports, require */ +const lib = require('lib'); +exports.handler = function (event, context) { + 'use strict'; + context.succeed(lib()); +}; diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_tar_dependency/package.json b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_tar_dependency/package.json new file mode 100644 index 000000000..58d667219 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_with_tar_dependency/package.json @@ -0,0 +1,14 @@ +{ + "name": "lambda", + "version": "1.0.0", + "description": "", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "author": "", + "license": "ISC", + "dependencies": { + "lib": "file:../lib-1.0.0.tgz" + } +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_without_lock_file/main_lambda_without_lock_file.js b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_without_lock_file/main_lambda_without_lock_file.js new file mode 100644 index 000000000..3a185fe95 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_without_lock_file/main_lambda_without_lock_file.js @@ -0,0 +1,6 @@ +/*global exports, require */ +const lib = require('lib'); +exports.handler = function (event, context) { + 'use strict'; + context.succeed(lib()); +}; diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_without_lock_file/package.json b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_without_lock_file/package.json new file mode 100644 index 000000000..4e514c113 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lambda_without_lock_file/package.json @@ -0,0 +1,14 @@ +{ + "name": "lambda", + "version": "1.0.0", + "description": "", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "author": "", + "license": "ISC", + "dependencies": { + "lib_with_lock_file": "file:../lib_with_lock_file" + } +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib-1.0.0.tgz b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib-1.0.0.tgz new file mode 100644 index 000000000..e1b7eeb5d Binary files /dev/null and b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib-1.0.0.tgz differ diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib/index.js b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib/index.js new file mode 100644 index 000000000..498b8cccc --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib/index.js @@ -0,0 +1,5 @@ +/*global module */ +module.exports = function () { + 'use strict'; + return 'hello relative'; +}; diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib/package.json b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib/package.json new file mode 100644 index 000000000..d702d8735 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib/package.json @@ -0,0 +1,11 @@ +{ + "name": "lib", + "version": "1.0.0", + "description": "", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "author": "", + "license": "ISC" +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib_with_lock_file/index.js b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib_with_lock_file/index.js new file mode 100644 index 000000000..498b8cccc --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib_with_lock_file/index.js @@ -0,0 +1,5 @@ +/*global module */ +module.exports = function () { + 'use strict'; + return 'hello relative'; +}; diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib_with_lock_file/package-lock.json b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib_with_lock_file/package-lock.json new file mode 100644 index 000000000..c47bbdcdd --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib_with_lock_file/package-lock.json @@ -0,0 +1,11 @@ +{ + "name": "lib_with_lock_file", + "version": "1.0.0", + "lockfileVersion": 1, + "requires": true, + "dependencies": { + "lib": { + "version": "file:../lib" + } + } +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib_with_lock_file/package.json b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib_with_lock_file/package.json new file mode 100644 index 000000000..244604e6a --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/lib_with_lock_file/package.json @@ -0,0 +1,14 @@ +{ + "name": "lib_with_lock_file", + "version": "1.0.0", + "description": "", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "author": "", + "license": "ISC", + "dependencies": { + "lib": "file:../lib" + } +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/parent_lambda/package.json b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/parent_lambda/package.json new file mode 100644 index 000000000..99463da80 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/parent_lambda/package.json @@ -0,0 +1,14 @@ +{ + "name": "parent_lambda", + "version": "1.0.0", + "description": "", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "author": "", + "license": "ISC", + "dependencies": { + "lambda": "file:../lambda" + } +} diff --git a/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/parent_lambda/parent_main.js b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/parent_lambda/parent_main.js new file mode 100644 index 000000000..3a185fe95 --- /dev/null +++ b/tests/integration/workflows/nodejs_npm/testdata/relative-dependencies/parent_lambda/parent_main.js @@ -0,0 +1,6 @@ +/*global exports, require */ +const lib = require('lib'); +exports.handler = function (event, context) { + 'use strict'; + context.succeed(lib()); +}; diff --git a/tests/unit/workflows/nodejs_npm/test_actions.py b/tests/unit/workflows/nodejs_npm/test_actions.py index 612ccb189..88f27aa2b 100644 --- a/tests/unit/workflows/nodejs_npm/test_actions.py +++ b/tests/unit/workflows/nodejs_npm/test_actions.py @@ -1,9 +1,10 @@ from unittest import TestCase -from mock import patch +from mock import patch, call from aws_lambda_builders.actions import ActionFailedError from aws_lambda_builders.workflows.nodejs_npm.actions import \ - NodejsNpmPackAction, NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction + NodejsNpmPackAction, NodejsNpmInstallAction, NodejsNpmrcCopyAction, \ + NodejsNpmrcCleanUpAction, NodejsNpmRewriteLocalDependenciesAction from aws_lambda_builders.workflows.nodejs_npm.npm import NpmExecutionError @@ -153,3 +154,99 @@ def test_skips_npmrc_removal_if_npmrc_doesnt_exist(self, OSUtilMock): action.execute() osutils.remove_file.assert_not_called() + + +class TestNodejsNpmRewriteLocalDependenciesAction(TestCase): + + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + @patch("aws_lambda_builders.workflows.nodejs_npm.npm.NpmModulesUtils") + def setUp(self, OSUtilMock, NpmModulesUtils): + self.osutils = OSUtilMock.return_value + self.osutils.joinpath.side_effect = \ + lambda original_package_dir, module_path: original_package_dir + '/' + module_path + + self.npm_modules_utils = NpmModulesUtils.return_value + self.npm_modules_utils.pack_to_tar.side_effect = lambda actual_path: actual_path + '.tar' + self.npm_modules_utils.clean_copy.side_effect = lambda actual_path, delete_package_lock: actual_path + '_copy' + + self.action = NodejsNpmRewriteLocalDependenciesAction( + 'work_dir', + 'original_package_dir', + 'scratch_dir', + self.npm_modules_utils, + self.osutils + ) + + def test_does_not_rewrite_if_no_local_dependencies(self): + self.npm_modules_utils.get_local_dependencies.side_effect = [{}, {}] + + self.action.execute() + + self.npm_modules_utils.update_dependency.assert_not_called() + + def test_rewrites_a_single_production_dependency(self): + def local_deps(work_dir, dependency_key): + if work_dir == "work_dir" and dependency_key == "dependencies": + return {"dep": "../dep"} + else: + return {} + + self.npm_modules_utils.get_local_dependencies.side_effect = local_deps + + self.action.execute() + + self.npm_modules_utils.update_dependency.assert_called_with( + 'work_dir', 'dep', 'file:original_package_dir/../dep_copy.tar', 'dependencies' + ) + + def test_rewrites_a_single_optional_dependency(self): + def local_deps(work_dir, dependency_key): + if work_dir == "work_dir" and dependency_key == "optionalDependencies": + return {"dep": "../opt_dep"} + else: + return {} + + self.npm_modules_utils.get_local_dependencies.side_effect = local_deps + + self.action.execute() + + self.npm_modules_utils.update_dependency.assert_called_with( + 'work_dir', 'dep', 'file:original_package_dir/../opt_dep_copy.tar', 'optionalDependencies' + ) + + def test_strips_file_prefix_when_rewriting_dependencies(self): + def local_deps(work_dir, dependency_key): + if work_dir == "work_dir" and dependency_key == "dependencies": + return {"dep": "file:/dep"} + else: + return {} + + self.npm_modules_utils.get_local_dependencies.side_effect = local_deps + + self.action.execute() + + self.osutils.joinpath.assert_called_with('original_package_dir', '/dep') + + def test_rewrites_production_dependencies_recursively(self): + def local_deps(work_dir, dependency_key): + if work_dir == 'work_dir' and dependency_key == 'dependencies': + return {"dep": "../dep"} + elif work_dir == 'original_package_dir/../dep_copy' and dependency_key == 'dependencies': + return {"subdep": "../subdep"} + else: + return {} + + self.npm_modules_utils.get_local_dependencies.side_effect = local_deps + + self.action.execute() + + calls = [ + call( + 'original_package_dir/../dep_copy', + 'subdep', + 'file:original_package_dir/../dep/../subdep_copy.tar', + 'dependencies' + ), + call('work_dir', 'dep', 'file:original_package_dir/../dep_copy.tar', 'dependencies') + ] + self.npm_modules_utils.update_dependency.assert_has_calls(calls) diff --git a/tests/unit/workflows/nodejs_npm/test_npm.py b/tests/unit/workflows/nodejs_npm/test_npm.py index 8eae867da..253ceca1c 100644 --- a/tests/unit/workflows/nodejs_npm/test_npm.py +++ b/tests/unit/workflows/nodejs_npm/test_npm.py @@ -1,7 +1,8 @@ from unittest import TestCase from mock import patch +from parameterized import parameterized -from aws_lambda_builders.workflows.nodejs_npm.npm import SubprocessNpm, NpmExecutionError +from aws_lambda_builders.workflows.nodejs_npm.npm import SubprocessNpm, NpmExecutionError, NpmModulesUtils class FakePopen: @@ -89,3 +90,214 @@ def test_raises_ValueError_if_args_empty(self): self.under_test.run([]) self.assertEqual(raised.exception.args[0], "requires at least one arg") + + +class TestNpmModulesUtils(TestCase): + + @patch("aws_lambda_builders.workflows.nodejs_npm.utils.OSUtils") + @patch("aws_lambda_builders.workflows.nodejs_npm.npm.SubprocessNpm") + def setUp(self, OSUtilMock, SubprocessNpmMock): + + self.osutils = OSUtilMock.return_value + self.subprocess_npm = SubprocessNpmMock.return_value + self.under_test = NpmModulesUtils(self.osutils, self.subprocess_npm, 'scratch_dir') + self.osutils.joinpath.side_effect = lambda a, b, c='': a + '/' + b + '/' + c if c else a + '/' + b + self.osutils.abspath.side_effect = lambda a: '/absolute/' + a + self.osutils.tempdir.side_effect = lambda a: a + '/tempdir' + + def test_get_local_dependencies_reads_package_json(self): + self.osutils.get_text_contents.side_effect = ['{}'] + self.osutils.joinpath.side_effect = ['joined/path'] + + self.under_test.get_local_dependencies('some/dir') + + self.osutils.get_text_contents.assert_called_with('joined/path') + self.osutils.joinpath.assert_called_with('some/dir', 'package.json') + + def test_get_local_dependencies_returns_empty_dict_when_no_dependencies(self): + self.osutils.get_text_contents.side_effect = ['{}'] + + result = self.under_test.get_local_dependencies('some/dir') + + self.assertEqual(result, {}) + + def test_get_local_dependencies_returns_empty_dict_whitout_local_dependencies(self): + self.osutils.get_text_contents.side_effect = ['{"dependencies":{"claudia":"*"}}'] + + result = self.under_test.get_local_dependencies('some/dir') + + self.assertEqual(result, {}) + + def test_get_local_dependencies_return_dict_with_local_dependencies(self): + self.osutils.get_text_contents.side_effect = ['{"dependencies":{"claudia":"file:/claudia/"}}'] + + result = self.under_test.get_local_dependencies('some/dir') + + self.assertEqual(result, {'claudia': 'file:/claudia/'}) + + def test_get_local_dependencies_filters_only_local_dependencies(self): + self.osutils.get_text_contents.side_effect = [ + '{"dependencies":{"claudia":"*","aws-sdk":"*","foo":"file:/foo","bar":"./bar","baz":"/baz"}}' + ] + + result = self.under_test.get_local_dependencies('some/dir') + + self.assertEqual(result, {'foo': 'file:/foo', 'bar': './bar', 'baz': '/baz'}) + + def test_get_local_dependencies_from_a_defined_key(self): + self.osutils.get_text_contents.side_effect = ['{"optionalDependencies":{"claudia":"file:/claudia/"}}'] + + result = self.under_test.get_local_dependencies('some/dir', 'optionalDependencies') + + self.assertEqual(result, {'claudia': 'file:/claudia/'}) + + @parameterized.expand([ + # NPM Version numbers and ranges from https://docs.npmjs.com/files/package.json + "1.0.0 - 2.9999.9999", + ">=1.0.2 <2.1.2", + "<1.0.2", + "<=1.0.2", + ">1.0.2 <=2.3.4", + "2.0.1", + "<1.0.0 || >=2.3.1 <2.4.5 || >=2.5.2 <3.0.0", + "~1.2", + "~1.2.3", + "2.x", + "3.3.x", + "latest", + "*", + # URLs as Dependencies + "http://asdf.com/asdf.tar.gz", + # Git URLs as Dependencies + "git+ssh://git@github.com:npm/cli.git#v1.0.27" + "git+ssh://git@github.com:npm/cli#semver:^5.0" + "git+https://isaacs@github.com/npm/cli.git" + "git://github.com/npm/cli.git#v1.0.27" + # GitHub URLs + "expressjs/express", + "mochajs/mocha#4727d357ea", + "user/repo#feature\/branch" + ]) + def test_is_local_dependency_returns_false_for_remote_dependencies(self, dependency_ref): + print('using', dependency_ref) + self.assertFalse(self.under_test.is_local_dependency(dependency_ref)) + + @parameterized.expand([ + "../foo/bar", + "~/foo/bar", + "./foo/bar", + "/foo/bar", + "file:../foo/bar" + ]) + def test_is_local_dependency_returns_true_for_local_dependencies(self, dependency_ref): + self.assertTrue(self.under_test.is_local_dependency(dependency_ref)) + + def test_has_local_dependencies_returns_false_if_no_dependencies(self): + self.osutils.get_text_contents.side_effect = lambda path: '{"name":"test-project"}' + + self.assertFalse(self.under_test.has_local_dependencies('project_path')) + self.osutils.get_text_contents.assert_called_with('project_path/package.json') + + @parameterized.expand([ + '{"optionalDependencies":{"claudia":"1.0.0"}}', + '{"dependencies":{"claudia":"1.0.0"}}', + '{"dependencies":{"claudia":"1.0.0"}, "optionalDependencies":{"express":"*"}}', + ]) + def test_has_local_dependencies_returns_false_if_only_remote_dependencies(self, package_manifest): + self.osutils.get_text_contents.side_effect = lambda path: package_manifest + + self.assertFalse(self.under_test.has_local_dependencies('project_path')) + + def test_has_local_dependencies_ignores_development_dependencies(self): + package_manifest = '{"devDependencies":{"claudia":"file:../claudia"}}' + self.osutils.get_text_contents.side_effect = lambda path: package_manifest + + self.assertFalse(self.under_test.has_local_dependencies('project_path')) + + @parameterized.expand([ + '{"optionalDependencies":{"claudia":"file:../local"}}', + '{"optionalDependencies":{"express": "*", "claudia":"file:../local"}}', + '{"dependencies":{"claudia":"file:../local"}}', + '{"dependencies":{"express": "*", "claudia":"file:../local"}}', + '{"dependencies":{"claudia":"file:../local"}, "optionalDependencies":{"express":"*"}}', + '{"dependencies":{"claudia":"*"}, "optionalDependencies":{"express":"file:../local"}}', + ]) + def test_has_local_dependencies_returns_false_if_some_local_dependencies(self, package_manifest): + self.osutils.get_text_contents.side_effect = lambda path: package_manifest + + self.assertTrue(self.under_test.has_local_dependencies('project_path')) + + def test_pack_to_tar_uses_subprocess_npm_to_execute_npm_pack(self): + self.subprocess_npm.run.side_effect = ['archive.tar'] + result = self.under_test.pack_to_tar('package_dir') + self.assertEquals(result, 'scratch_dir/archive.tar') + self.subprocess_npm.run.assert_called_with(['pack', '-q', 'file:/absolute/package_dir'], cwd='scratch_dir') + + def test_update_dependency_rewrites_production_dependencies_without_specific_dependency_key(self): + package_manifest = '{"dependencies":{"claudia":"file:../claudia"}}' + self.osutils.get_text_contents.side_effect = lambda path: package_manifest + self.under_test.update_dependency('package_dir', 'claudia', 'file:/a.tar') + self.osutils.get_text_contents.assert_called_with('package_dir/package.json') + self.osutils.write_text_contents.assert_called_with( + 'package_dir/package.json', + '{"dependencies": {"claudia": "file:/a.tar"}}' + ) + + def test_update_dependency_rewrites_production_dependencies_does_not_modify_other_dependencies(self): + package_manifest = '{"dependencies":{"claudia":"file:../claudia", "express": "*"}}' + self.osutils.get_text_contents.side_effect = lambda path: package_manifest + self.under_test.update_dependency('package_dir', 'claudia', 'file:/a.tar') + self.osutils.get_text_contents.assert_called_with('package_dir/package.json') + self.osutils.write_text_contents.assert_called_with( + 'package_dir/package.json', + '{"dependencies": {"claudia": "file:/a.tar", "express": "*"}}' + ) + + def test_update_dependency_rewrites_production_dependencies_with_specific_dependency_key(self): + package_manifest = '{"dependencies":{"claudia":"file:../claudia"}}' + self.osutils.get_text_contents.side_effect = lambda path: package_manifest + self.under_test.update_dependency('package_dir', 'claudia', 'file:/a.tar', 'dependencies') + self.osutils.get_text_contents.assert_called_with('package_dir/package.json') + self.osutils.write_text_contents.assert_called_with( + 'package_dir/package.json', + '{"dependencies": {"claudia": "file:/a.tar"}}' + ) + + def test_update_dependency_rewrites_optional_dependencies_if_requested(self): + package_manifest = '{"optionalDependencies":{"claudia":"file:../claudia"}}' + self.osutils.get_text_contents.side_effect = lambda path: package_manifest + self.under_test.update_dependency('package_dir', 'claudia', 'file:/a.tar', 'optionalDependencies') + self.osutils.get_text_contents.assert_called_with('package_dir/package.json') + self.osutils.write_text_contents.assert_called_with( + 'package_dir/package.json', + '{"optionalDependencies": {"claudia": "file:/a.tar"}}' + ) + + def test_clean_copy_packs_a_project_to_tar_and_extracts_from_tar_into_temp_dir(self): + self.subprocess_npm.run.side_effect = ['archive.tar'] + + result = self.under_test.clean_copy('project_dir') + + self.assertEquals(result, 'scratch_dir/tempdir/package') + self.subprocess_npm.run.assert_called_with(['pack', '-q', 'file:/absolute/project_dir'], cwd='scratch_dir') + self.osutils.extract_tarfile.assert_called_with('scratch_dir/archive.tar', 'scratch_dir/tempdir') + + def test_clean_copy_removes_package_lock_if_requested_and_exists(self): + self.subprocess_npm.run.side_effect = ['archive.tar'] + self.osutils.file_exists.side_effect = [True] + + result = self.under_test.clean_copy('project_dir', True) + + self.assertEquals(result, 'scratch_dir/tempdir/package') + self.osutils.file_exists.assert_called_with('scratch_dir/tempdir/package/package-lock.json') + self.osutils.remove_file.assert_called_with('scratch_dir/tempdir/package/package-lock.json') + + def test_clean_copy_does_not_removes_package_lock_if_it_does_not_exist_even_if_requested(self): + self.subprocess_npm.run.side_effect = ['archive.tar'] + self.osutils.file_exists.side_effect = [False] + + result = self.under_test.clean_copy('project_dir', True) + + self.assertEquals(result, 'scratch_dir/tempdir/package') + self.osutils.file_exists.assert_called_with('scratch_dir/tempdir/package/package-lock.json') + self.osutils.remove_file.assert_not_called() diff --git a/tests/unit/workflows/nodejs_npm/test_workflow.py b/tests/unit/workflows/nodejs_npm/test_workflow.py index c2fe05be3..5f3d29d71 100644 --- a/tests/unit/workflows/nodejs_npm/test_workflow.py +++ b/tests/unit/workflows/nodejs_npm/test_workflow.py @@ -3,7 +3,8 @@ from aws_lambda_builders.actions import CopySourceAction from aws_lambda_builders.workflows.nodejs_npm.workflow import NodejsNpmWorkflow from aws_lambda_builders.workflows.nodejs_npm.actions import \ - NodejsNpmPackAction, NodejsNpmInstallAction, NodejsNpmrcCopyAction, NodejsNpmrcCleanUpAction + NodejsNpmPackAction, NodejsNpmInstallAction, NodejsNpmrcCopyAction, \ + NodejsNpmrcCleanUpAction, NodejsNpmRewriteLocalDependenciesAction class TestNodejsNpmWorkflow(TestCase): @@ -17,7 +18,7 @@ def test_workflow_sets_up_npm_actions(self): workflow = NodejsNpmWorkflow("source", "artifacts", "scratch_dir", "manifest") - self.assertEqual(len(workflow.actions), 5) + self.assertEqual(len(workflow.actions), 6) self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction) @@ -25,6 +26,8 @@ def test_workflow_sets_up_npm_actions(self): self.assertIsInstance(workflow.actions[2], CopySourceAction) - self.assertIsInstance(workflow.actions[3], NodejsNpmInstallAction) + self.assertIsInstance(workflow.actions[3], NodejsNpmRewriteLocalDependenciesAction) - self.assertIsInstance(workflow.actions[4], NodejsNpmrcCleanUpAction) + self.assertIsInstance(workflow.actions[4], NodejsNpmInstallAction) + + self.assertIsInstance(workflow.actions[5], NodejsNpmrcCleanUpAction)