fix: use npm update when building in source (#579)

* fix: use npm update when building in source

* add integ tests which should fail now

* update tests to succeed

* format

aws_lambda_builders/actions.py

@@ -57,7 +57,7 @@ class _ActionMetaClass(type):
     def __new__(mcs, name, bases, class_dict):
         cls = type.__new__(mcs, name, bases, class_dict)
 
-        if cls.__name__ == "BaseAction":
+        if cls.__name__ in ["BaseAction", "NodejsNpmInstallOrUpdateBaseAction"]:
             return cls
 
         # Validate class variables

aws_lambda_builders/workflows/nodejs_npm/actions.py

@@ -72,32 +72,35 @@ def execute(self):
             raise ActionFailedError(str(ex))
 
 
-class NodejsNpmInstallAction(BaseAction):
-
+class NodejsNpmInstallOrUpdateBaseAction(BaseAction):
     """
-    A Lambda Builder Action that installs NPM project dependencies
+    A base Lambda Builder Action that is used for installs or updating NPM project dependencies
     """
 
-    NAME = "NpmInstall"
-    DESCRIPTION = "Installing dependencies from NPM"
     PURPOSE = Purpose.RESOLVE_DEPENDENCIES
 
-    def __init__(self, install_dir: str, subprocess_npm: SubprocessNpm, install_links: Optional[bool] = False):
+    def __init__(self, install_dir: str, subprocess_npm: SubprocessNpm):
         """
         Parameters
         ----------
         install_dir : str
             Dependencies will be installed in this directory.
         subprocess_npm : SubprocessNpm
             An instance of the NPM process wrapper
-        install_links : Optional[bool]
-            Uses the --install-links npm option if True, by default False
         """
 
-        super(NodejsNpmInstallAction, self).__init__()
+        super().__init__()
         self.install_dir = install_dir
         self.subprocess_npm = subprocess_npm
-        self.install_links = install_links
+
+
+class NodejsNpmInstallAction(NodejsNpmInstallOrUpdateBaseAction):
+    """
+    A Lambda Builder Action that installs NPM project dependencies
+    """
+
+    NAME = "NpmInstall"
+    DESCRIPTION = "Installing dependencies from NPM"
 
     def execute(self):
         """
@@ -109,9 +112,38 @@ def execute(self):
             LOG.debug("NODEJS installing in: %s", self.install_dir)
 
             command = ["install", "-q", "--no-audit", "--no-save", "--unsafe-perm", "--production"]
-            if self.install_links:
-                command.append("--install-links")
+            self.subprocess_npm.run(command, cwd=self.install_dir)
+
+        except NpmExecutionError as ex:
+            raise ActionFailedError(str(ex))
+
+
+class NodejsNpmUpdateAction(NodejsNpmInstallOrUpdateBaseAction):
+    """
+    A Lambda Builder Action that installs NPM project dependencies
+    """
 
+    NAME = "NpmUpdate"
+    DESCRIPTION = "Updating dependencies from NPM"
+
+    def execute(self):
+        """
+        Runs the action.
+
+        :raises lambda_builders.actions.ActionFailedError: when NPM execution fails
+        """
+        try:
+            LOG.debug("NODEJS updating in: %s", self.install_dir)
+
+            command = [
+                "update",
+                "--no-audit",
+                "--no-save",
+                "--unsafe-perm",
+                "--production",
+                "--no-package-lock",
+                "--install-links",
+            ]
             self.subprocess_npm.run(command, cwd=self.install_dir)
 
         except NpmExecutionError as ex:

aws_lambda_builders/workflows/nodejs_npm/workflow.py

@@ -22,6 +22,7 @@
     NodejsNpmPackAction,
     NodejsNpmrcAndLockfileCopyAction,
     NodejsNpmrcCleanUpAction,
+    NodejsNpmUpdateAction,
 )
 from aws_lambda_builders.workflows.nodejs_npm.npm import SubprocessNpm
 from aws_lambda_builders.workflows.nodejs_npm.utils import OSUtils
@@ -119,7 +120,7 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim
                     subprocess_npm=subprocess_npm,
                     osutils=osutils,
                     build_options=self.options,
-                    install_links=is_building_in_source,
+                    is_building_in_source=is_building_in_source,
                 )
             )
 
@@ -211,7 +212,7 @@ def get_install_action(
         subprocess_npm: SubprocessNpm,
         osutils: OSUtils,
         build_options: Optional[dict],
-        install_links: Optional[bool] = False,
+        is_building_in_source: Optional[bool] = False,
     ):
         """
         Get the install action used to install dependencies.
@@ -228,8 +229,8 @@ def get_install_action(
             An instance of OS Utilities for file manipulation
         build_options : Optional[dict]
             Object containing build options configurations
-        install_links : Optional[bool]
-            Uses the --install-links npm option if True, by default False
+        is_building_in_source : Optional[bool]
+            States whether --build-in-source flag is set or not
 
         Returns
         -------
@@ -245,12 +246,13 @@ def get_install_action(
 
         if (osutils.file_exists(lockfile_path) or osutils.file_exists(shrinkwrap_path)) and npm_ci_option:
             return NodejsNpmCIAction(
-                install_dir=install_dir, subprocess_npm=subprocess_npm, install_links=install_links
+                install_dir=install_dir, subprocess_npm=subprocess_npm, install_links=is_building_in_source
             )
 
-        return NodejsNpmInstallAction(
-            install_dir=install_dir, subprocess_npm=subprocess_npm, install_links=install_links
-        )
+        if is_building_in_source:
+            return NodejsNpmUpdateAction(install_dir=install_dir, subprocess_npm=subprocess_npm)
+
+        return NodejsNpmInstallAction(install_dir=install_dir, subprocess_npm=subprocess_npm)
 
     @staticmethod
     def can_use_install_links(npm_process: SubprocessNpm) -> bool:

aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py

@@ -118,7 +118,7 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim
                     subprocess_npm=self.subprocess_npm,
                     osutils=self.osutils,
                     build_options=self.options,
-                    install_links=is_building_in_source,
+                    is_building_in_source=is_building_in_source,
                 )
             )
 

tests/integration/workflows/nodejs_npm/test_nodejs_npm.py

@@ -315,6 +315,46 @@ def test_build_in_source_with_download_dependencies(self, runtime):
         output_files = set(os.listdir(self.artifacts_dir))
         self.assertEqual(expected_files, output_files)
 
+    @parameterized.expand([("nodejs12.x",), ("nodejs14.x",), ("nodejs16.x",), ("nodejs18.x",), ("nodejs20.x",)])
+    def test_build_in_source_with_removed_dependencies(self, runtime):
+        # run a build with default requirements and confirm dependencies are downloaded
+        source_dir = os.path.join(self.temp_testdata_dir, "npm-deps")
+
+        self.builder.build(
+            source_dir,
+            self.artifacts_dir,
+            self.scratch_dir,
+            os.path.join(source_dir, "package.json"),
+            runtime=runtime,
+            build_in_source=True,
+        )
+
+        # dependencies installed in source folder
+        source_node_modules = os.path.join(source_dir, "node_modules")
+        self.assertTrue(os.path.isdir(source_node_modules))
+        expected_node_modules_contents = {"minimal-request-promise", ".package-lock.json"}
+        self.assertEqual(set(os.listdir(source_node_modules)), expected_node_modules_contents)
+
+        # update package.json with empty one and re-run the build then confirm node_modules are cleared up
+        shutil.copy2(
+            os.path.join(self.temp_testdata_dir, "no-deps", "package.json"),
+            os.path.join(self.temp_testdata_dir, "npm-deps", "package.json"),
+        )
+
+        self.builder.build(
+            source_dir,
+            self.artifacts_dir,
+            self.scratch_dir,
+            os.path.join(source_dir, "package.json"),
+            runtime=runtime,
+            build_in_source=True,
+        )
+        # dependencies installed in source folder
+        source_node_modules = os.path.join(source_dir, "node_modules")
+        self.assertTrue(os.path.isdir(source_node_modules))
+        self.assertIn(".package-lock.json", set(os.listdir(source_node_modules)))
+        self.assertNotIn("minimal-request-promise", set(os.listdir(source_node_modules)))
+
     @parameterized.expand([("nodejs12.x",), ("nodejs14.x",), ("nodejs16.x",), ("nodejs18.x",), ("nodejs20.x",)])
     def test_build_in_source_with_download_dependencies_local_dependency(self, runtime):
         source_dir = os.path.join(self.temp_testdata_dir, "with-local-dependency")

tests/unit/workflows/nodejs_npm/test_workflow.py

@@ -20,6 +20,7 @@
     NodejsNpmrcCleanUpAction,
     NodejsNpmLockFileCleanUpAction,
     NodejsNpmCIAction,
+    NodejsNpmUpdateAction,
 )
 
 
@@ -107,7 +108,7 @@ def test_workflow_sets_up_npm_actions_with_download_dependencies_without_depende
         self.assertIsInstance(workflow.actions[3], CopySourceAction)
         self.assertEqual(workflow.actions[3].source_dir, "source")
         self.assertEqual(workflow.actions[3].dest_dir, "artifacts")
-        self.assertIsInstance(workflow.actions[4], NodejsNpmInstallAction)
+        self.assertIsInstance(workflow.actions[4], NodejsNpmUpdateAction)
         self.assertEqual(workflow.actions[4].install_dir, "not_source")
         self.assertIsInstance(workflow.actions[5], LinkSinglePathAction)
         self.assertEqual(workflow.actions[5]._source, os.path.join("not_source", "node_modules"))
@@ -331,7 +332,7 @@ def test_build_in_source_with_download_dependencies(self, can_use_links_mock):
         self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction)
         self.assertIsInstance(workflow.actions[1], NodejsNpmrcAndLockfileCopyAction)
         self.assertIsInstance(workflow.actions[2], CopySourceAction)
-        self.assertIsInstance(workflow.actions[3], NodejsNpmInstallAction)
+        self.assertIsInstance(workflow.actions[3], NodejsNpmUpdateAction)
         self.assertEqual(workflow.actions[3].install_dir, source_dir)
         self.assertIsInstance(workflow.actions[4], LinkSinglePathAction)
         self.assertEqual(workflow.actions[4]._source, os.path.join(source_dir, "node_modules"))
@@ -358,7 +359,7 @@ def test_build_in_source_with_download_dependencies_and_dependencies_dir(self, c
         self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction)
         self.assertIsInstance(workflow.actions[1], NodejsNpmrcAndLockfileCopyAction)
         self.assertIsInstance(workflow.actions[2], CopySourceAction)
-        self.assertIsInstance(workflow.actions[3], NodejsNpmInstallAction)
+        self.assertIsInstance(workflow.actions[3], NodejsNpmUpdateAction)
         self.assertEqual(workflow.actions[3].install_dir, source_dir)
         self.assertIsInstance(workflow.actions[4], LinkSinglePathAction)
         self.assertEqual(workflow.actions[4]._source, os.path.join(source_dir, "node_modules"))
@@ -442,5 +443,5 @@ def test_workflow_revert_build_in_source(self, install_action_mock, install_link
             subprocess_npm=ANY,
             osutils=ANY,
             build_options=ANY,
-            install_links=False,
+            is_building_in_source=False,
         )

tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py

@@ -12,7 +12,11 @@
     LinkSinglePathAction,
 )
 from aws_lambda_builders.architecture import ARM64
-from aws_lambda_builders.workflows.nodejs_npm.actions import NodejsNpmInstallAction, NodejsNpmCIAction
+from aws_lambda_builders.workflows.nodejs_npm.actions import (
+    NodejsNpmInstallAction,
+    NodejsNpmCIAction,
+    NodejsNpmUpdateAction,
+)
 from aws_lambda_builders.workflows.nodejs_npm_esbuild import NodejsNpmEsbuildWorkflow
 from aws_lambda_builders.workflows.nodejs_npm_esbuild.actions import EsbuildBundleAction
 from aws_lambda_builders.workflows.nodejs_npm_esbuild.esbuild import SubprocessEsbuild
@@ -313,7 +317,7 @@ def test_workflow_uses_production_npm_version(self, get_workflow_mock):
             subprocess_npm=ANY,
             osutils=ANY,
             build_options=None,
-            install_links=False,
+            is_building_in_source=False,
         )
 
     @patch("aws_lambda_builders.workflows.nodejs_npm_esbuild.workflow.NodejsNpmEsbuildWorkflow._get_esbuild_subprocess")
@@ -360,7 +364,7 @@ def test_build_in_source(self, install_links_mock):
 
         self.assertEqual(len(workflow.actions), 2)
 
-        self.assertIsInstance(workflow.actions[0], NodejsNpmInstallAction)
+        self.assertIsInstance(workflow.actions[0], NodejsNpmUpdateAction)
         self.assertEqual(workflow.actions[0].install_dir, source_dir)
         self.assertIsInstance(workflow.actions[1], EsbuildBundleAction)
         self.assertEqual(workflow.actions[1]._working_directory, source_dir)
@@ -405,7 +409,7 @@ def test_workflow_sets_up_npm_actions_with_download_dependencies_without_depende
 
         self.assertEqual(len(workflow.actions), 3)
 
-        self.assertIsInstance(workflow.actions[0], NodejsNpmInstallAction)
+        self.assertIsInstance(workflow.actions[0], NodejsNpmUpdateAction)
         self.assertEqual(workflow.actions[0].install_dir, "not_source")
         self.assertIsInstance(workflow.actions[1], LinkSinglePathAction)
         self.assertEqual(workflow.actions[1]._source, os.path.join("not_source", "node_modules"))
@@ -439,7 +443,7 @@ def test_workflow_revert_build_in_source(self, install_action_mock, install_link
             subprocess_npm=ANY,
             osutils=ANY,
             build_options=ANY,
-            install_links=False,
+            is_building_in_source=False,
         )
 
     @parameterized.expand(