feat: nodejs_npm rewrite local deps

aws_lambda_builders/workflows/nodejs_npm/DESIGN.md

@@ -92,8 +92,6 @@ files.
 
 #### Step 2: Rewrite local dependencies
 
-_(out of scope for the current version)_
-
 To optimise disk space and avoid including development dependencies from other
 locally linked packages, inspect the `package.json` manifest looking for dependencies
 referring to local file paths (can be identified as they start with `.` or `file:`),
@@ -102,8 +100,13 @@ then for each dependency recursively execute the packaging process
 Local dependencies may include other local dependencies themselves, this is a very 
 common way of sharing configuration or development utilities such as linting or testing 
 tools. This means that for each packaged local dependency this packager needs to
-recursively apply the packaging process. It also means that the packager needs to 
-track local paths and avoid re-packaging directories it already visited.
+recursively apply the packaging process.
+
+_(out of scope for the current version)_
+
+It also means that the packager needs to track local paths and avoid re-packaging directories it already visited.
+
+_(out of scope for the current version)_
 
 NPM produces a `tar` archive while packaging that can be directly included as a
 dependency.  This will make NPM unpack and install a copy correctly. Once the
@@ -113,7 +116,6 @@ the manifest to point to `tar` files instead of the original location.
 If the project contains a package lock file, this will cause NPM to ignore changes
 to the package.json manifest. In this case, the packager will need to remove 
 `package-lock.json` so that dependency rewrites take effect. 
-_(out of scope for the current version)_
 
 #### Step 3: Install dependencies
 

aws_lambda_builders/workflows/nodejs_npm/actions.py

@@ -6,6 +6,7 @@
 
 from aws_lambda_builders.actions import BaseAction, Purpose, ActionFailedError
 from .npm import NpmExecutionError
+from .utils import DependencyUtils
 
 LOG = logging.getLogger(__name__)
 
@@ -66,9 +67,32 @@ def execute(self):
 
             self.osutils.extract_tarfile(tarfile_path, self.artifacts_dir)
 
+            self._package_local_dependencies()
+
         except NpmExecutionError as ex:
             raise ActionFailedError(str(ex))
 
+    def _package_local_dependencies(self):
+        """
+        Iterates (recursively) through any local dependencies defined in package.json.
+
+        :raises lambda_builders.actions.ActionFailedError: when any file system operations fail
+        """
+
+        try:
+            LOG.debug("NODEJS searching for local dependencies")
+
+            parent_package_path = self.osutils.abspath(self.osutils.dirname(self.manifest_path))
+
+            DependencyUtils.package_local_dependency(
+                parent_package_path, ".", self.artifacts_dir, self.scratch_dir, None, self.osutils, self.subprocess_npm
+            )
+
+            LOG.debug("NODEJS finished processing local dependencies")
+
+        except OSError as ex:
+            raise ActionFailedError(str(ex))
+
 
 class NodejsNpmInstallAction(BaseAction):
 

aws_lambda_builders/workflows/nodejs_npm/utils.py

@@ -2,12 +2,16 @@
 Commonly used utilities
 """
 
+import json
+import logging
 import os
 import platform
 import tarfile
 import subprocess
 import shutil
 
+LOG = logging.getLogger(__name__)
+
 
 class OSUtils(object):
 
@@ -19,16 +23,28 @@ class OSUtils(object):
     def copy_file(self, file_path, destination_path):
         return shutil.copy2(file_path, destination_path)
 
+    def dir_exists(self, directory):
+        return os.path.isdir(directory)
+
     def extract_tarfile(self, tarfile_path, unpack_dir):
         with tarfile.open(tarfile_path, "r:*") as tar:
             tar.extractall(unpack_dir)
 
     def file_exists(self, filename):
         return os.path.isfile(filename)
 
+    def filename(self, filename):
+        return os.path.basename(filename)
+
     def joinpath(self, *args):
         return os.path.join(*args)
 
+    def mkdir(self, path):
+        return os.mkdir(path)
+
+    def open_file(self, filename, mode="r"):
+        return open(filename, mode)
+
     def popen(self, command, stdout=None, stderr=None, env=None, cwd=None):
         p = subprocess.Popen(command, stdout=stdout, stderr=stderr, env=env, cwd=cwd)
         return p
@@ -48,3 +64,124 @@ def abspath(self, path):
 
     def is_windows(self):
         return platform.system().lower() == "windows"
+
+
+class DependencyUtils(object):
+
+    """
+    Collection of helper functions for managing local NPM dependencies
+    """
+
+    @staticmethod
+    def get_local_dependencies(manifest_path, osutils):
+        """
+        Helper function to extract all local dependencies in a package.json manifest
+        """
+
+        with osutils.open_file(manifest_path) as manifest_file:
+            manifest = json.loads(manifest_file.read())
+
+            if "dependencies" in manifest:
+                return dict(
+                    (k, v) for (k, v) in manifest["dependencies"].items() if DependencyUtils.is_local_dependency(v)
+                )
+            else:
+                return {}
+
+    @staticmethod
+    def is_local_dependency(path):
+        """
+        Helper function to check if package dependency is a local package
+        """
+
+        try:
+            return path.startswith("file:") or path.startswith(".")
+        except AttributeError:
+            return False
+
+    @staticmethod
+    def package_local_dependency(
+        parent_package_path, rel_package_path, artifacts_dir, scratch_dir, output_dir, osutils, subprocess_npm
+    ):
+        """
+        Helper function to recurse local dependencies and package them to a common directory
+        """
+
+        if rel_package_path.startswith("file:"):
+            rel_package_path = rel_package_path[5:].strip()
+
+        package_path = osutils.abspath(osutils.joinpath(parent_package_path, rel_package_path))
+
+        if not osutils.dir_exists(scratch_dir):
+            osutils.mkdir(scratch_dir)
+
+        if output_dir is None:
+            # TODO: get a higher level output_dir to keep process locals between jobs
+            output_dir = osutils.joinpath(artifacts_dir, "@aws_lambda_builders_local_dep")
+            if not osutils.dir_exists(output_dir):
+                osutils.mkdir(output_dir)
+            top_level = True
+        else:
+            tarfile_name = subprocess_npm.run(["pack", "-q", package_path], cwd=scratch_dir).splitlines()[-1]
+            tarfile_path = osutils.joinpath(scratch_dir, tarfile_name)
+
+            LOG.debug("NODEJS extracting child dependency for recursive dependency check")
+
+            osutils.extract_tarfile(tarfile_path, artifacts_dir)
+
+            top_level = False
+
+        local_manifest_path = osutils.joinpath(artifacts_dir, "package", "package.json")
+        local_dependencies = DependencyUtils.get_local_dependencies(local_manifest_path, osutils)
+        for (dep_name, dep_path) in local_dependencies.items():
+            dep_scratch_dir = osutils.joinpath(scratch_dir, str(abs(hash(dep_name))))
+
+            # TODO: if dep_scratch_dir exists (anywhere up path), it means we've already processed it this round, skip
+
+            dep_artifacts_dir = osutils.joinpath(dep_scratch_dir, "unpacked")
+
+            LOG.debug("NODEJS packaging dependency, %s, from %s to %s", dep_name, parent_package_path, output_dir)
+
+            dependency_tarfile_path = DependencyUtils.package_local_dependency(
+                package_path, dep_path, dep_artifacts_dir, dep_scratch_dir, output_dir, osutils, subprocess_npm
+            )
+
+            packaged_dependency_tarfile_path = osutils.joinpath(output_dir, osutils.filename(dependency_tarfile_path))
+            osutils.copy_file(dependency_tarfile_path, output_dir)
+
+            LOG.debug("NODEJS packed localized child dependency to %s", packaged_dependency_tarfile_path)
+
+            LOG.debug("NODEJS updating package.json %s", local_manifest_path)
+
+            DependencyUtils.update_manifest(local_manifest_path, dep_name, packaged_dependency_tarfile_path, osutils)
+
+        if not top_level:
+            localized_package_dir = osutils.joinpath(artifacts_dir, "package")
+
+            LOG.debug("NODEJS repackaging child dependency")
+
+            tarfile_name = subprocess_npm.run(
+                ["pack", "-q", localized_package_dir], cwd=localized_package_dir
+            ).splitlines()[-1]
+
+            return osutils.joinpath(localized_package_dir, tarfile_name)
+
+    @staticmethod
+    def update_manifest(manifest_path, dep_name, dependency_tarfile_path, osutils):
+        """
+        Helper function to update dependency path to localized tar
+        """
+
+        manifest_backup = "{}.bak".format(manifest_path)
+        osutils.copy_file(manifest_path, manifest_backup)
+
+        with osutils.open_file(manifest_backup, "r") as manifest_backup_file:
+            manifest = json.loads(manifest_backup_file.read())
+
+            if "dependencies" in manifest and dep_name in manifest["dependencies"]:
+                manifest["dependencies"][dep_name] = "file:{}".format(dependency_tarfile_path)
+
+                with osutils.open_file(manifest_path, "w") as manifest_write_file:
+                    manifest_write_file.write(json.dumps(manifest, indent=4))
+
+        osutils.remove_file(manifest_backup)

aws_lambda_builders/workflows/nodejs_npm/workflow.py

@@ -4,7 +4,12 @@
 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,
+)
 from .utils import OSUtils
 from .npm import SubprocessNpm
 

tests/functional/workflows/nodejs_npm/test_utils.py

@@ -123,3 +123,66 @@ 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_dir_exists(self):
+        self.assertFalse(self.osutils.dir_exists("20201210_some_directory_that_should_not_exist"))
+
+        temp_dir = tempfile.mkdtemp()
+
+        self.assertTrue(self.osutils.dir_exists(temp_dir))
+
+        shutil.rmtree(temp_dir)
+
+    def test_mkdir_makes_directory(self):
+        dir_to_create = os.path.join(tempfile.gettempdir(), "20201210_some_directory_that_should_not_exist")
+        self.assertFalse(os.path.isdir(dir_to_create))
+
+        self.osutils.mkdir(dir_to_create)
+
+        self.assertTrue(os.path.isdir(dir_to_create))
+
+        shutil.rmtree(dir_to_create)
+
+    def test_open_file_opens_file_for_reading(self):
+        temp_dir = tempfile.mkdtemp()
+
+        file_to_open = os.path.join(temp_dir, "test_open.txt")
+
+        with open(file_to_open, "w") as fid:
+            fid.write("this is text")
+
+        with self.osutils.open_file(file_to_open) as fid:
+            content = fid.read()
+
+        self.assertEqual("this is text", content)
+
+        shutil.rmtree(temp_dir)
+
+    def test_open_file_opens_file_for_writing(self):
+        temp_dir = tempfile.mkdtemp()
+
+        file_to_open = os.path.join(temp_dir, "test_open.txt")
+
+        with self.osutils.open_file(file_to_open, "w") as fid:
+            fid.write("this is some other text")
+
+        with self.osutils.open_file(file_to_open) as fid:
+            content = fid.read()
+
+        self.assertEqual("this is some other text", content)
+
+        shutil.rmtree(temp_dir)
+
+
+class TestDependencyUtils(TestCase):
+    def test_is_local_dependency_file_prefix(self):
+        self.assertTrue(utils.DependencyUtils.is_local_dependency("file:./local/dep"))
+
+    def test_is_local_dependency_dot_prefix(self):
+        self.assertTrue(utils.DependencyUtils.is_local_dependency("./local/dep"))
+
+    def test_is_local_dependency_package_name(self):
+        self.assertFalse(utils.DependencyUtils.is_local_dependency("typescript"))
+
+    def test_is_local_dependency_invalid(self):
+        self.assertFalse(utils.DependencyUtils.is_local_dependency(None))

tests/integration/workflows/nodejs_npm/test_nodejs_npm.py

@@ -77,6 +77,29 @@ def test_builds_project_with_remote_dependencies(self):
         output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules")))
         self.assertEqual(expected_modules, output_modules)
 
+    def test_builds_project_with_local_dependencies(self):
+        source_dir = os.path.join(self.TEST_DATA_FOLDER, "local-deps")
+
+        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", "included.js", "node_modules"}
+        output_files = set(os.listdir(self.artifacts_dir))
+        self.assertEqual(expected_files, output_files)
+
+        expected_modules = {"@mockcompany"}
+        output_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules")))
+        self.assertEqual(expected_modules, output_modules)
+
+        expected_sub_modules = {"module-a", "module-b", "module-c"}
+        output_sub_modules = set(os.listdir(os.path.join(self.artifacts_dir, "node_modules", "@mockcompany")))
+        self.assertEqual(expected_sub_modules, output_sub_modules)
+
     def test_builds_project_with_npmrc(self):
         source_dir = os.path.join(self.TEST_DATA_FOLDER, "npmrc")
 

tests/integration/workflows/nodejs_npm/testdata/local-deps/excluded.js

@@ -0,0 +1,2 @@
+//excluded
+const x = 1;

tests/integration/workflows/nodejs_npm/testdata/local-deps/included.js

@@ -0,0 +1,6 @@
+const module_a = require('@mockcompany/module-a')
+
+//included
+const x = 1;
+
+const greetings = module_a.sayHello();

tests/integration/workflows/nodejs_npm/testdata/local-deps/package.json

@@ -0,0 +1,12 @@
+{
+  "name": "localdeps",
+  "version": "1.0.0",
+  "description": "",
+  "files": ["included.js"],
+  "keywords": [],
+  "author": "",
+  "license": "APACHE2.0",
+  "dependencies": {
+    "@mockcompany/module-a": "file:../modules/module_a" 
+  }
+}

tests/integration/workflows/nodejs_npm/testdata/modules/module_a/index.js

@@ -0,0 +1,5 @@
+const module_b = require('@mockcompany/module-b');
+
+exports.sayHello = function() {
+  return 'hello from module a! module b says: ' + module_b.sayHello();
+}

tests/integration/workflows/nodejs_npm/testdata/modules/module_a/package.json

@@ -0,0 +1,12 @@
+{
+  "name": "@mockcompany/module-a",
+  "version": "1.0.0",
+  "description": "",
+  "files": ["index.js"],
+  "keywords": [],
+  "author": "",
+  "license": "APACHE2.0",
+  "dependencies": {
+    "@mockcompany/module-b": "file:../module_b" 
+  }
+}