diff --git a/samcli/lib/sync/flows/function_sync_flow.py b/samcli/lib/sync/flows/function_sync_flow.py index 43cbbdf5af..47e38396ac 100644 --- a/samcli/lib/sync/flows/function_sync_flow.py +++ b/samcli/lib/sync/flows/function_sync_flow.py @@ -60,7 +60,7 @@ def __init__( ) self._function_identifier = function_identifier self._function_provider = self._build_context.function_provider - self._function = cast(Function, self._function_provider.functions.get(self._function_identifier)) + self._function = cast(Function, self._function_provider.get(self._function_identifier)) self._lambda_client = None self._lambda_waiter = None self._lambda_waiter_config = {"Delay": 1, "MaxAttempts": 60} diff --git a/samcli/lib/sync/flows/layer_sync_flow.py b/samcli/lib/sync/flows/layer_sync_flow.py index a58e0cba90..be59543de6 100644 --- a/samcli/lib/sync/flows/layer_sync_flow.py +++ b/samcli/lib/sync/flows/layer_sync_flow.py @@ -12,7 +12,7 @@ from samcli.lib.build.app_builder import ApplicationBuilder from samcli.lib.package.utils import make_zip -from samcli.lib.providers.provider import ResourceIdentifier, Stack, get_resource_by_id, Function +from samcli.lib.providers.provider import ResourceIdentifier, Stack, get_resource_by_id, Function, LayerVersion from samcli.lib.providers.sam_function_provider import SamFunctionProvider from samcli.lib.sync.exceptions import MissingPhysicalResourceError, NoLayerVersionsFoundError from samcli.lib.sync.sync_flow import SyncFlow, ResourceAPICall, ApiCallTypes @@ -20,6 +20,7 @@ from samcli.lib.utils.colors import Colored from samcli.lib.utils.hash import file_checksum from samcli.lib.sync.flows.function_sync_flow import wait_for_function_update_complete +from samcli.lib.utils.osutils import rmtree_if_exists if TYPE_CHECKING: # pragma: no cover from samcli.commands.build.build_context import BuildContext @@ -175,6 +176,18 @@ class LayerSyncFlow(AbstractLayerSyncFlow): """SyncFlow for Lambda Layers""" _new_layer_version: Optional[int] + _layer: LayerVersion + + def __init__( + self, + layer_identifier: str, + build_context: "BuildContext", + deploy_context: "DeployContext", + physical_id_mapping: Dict[str, str], + stacks: List[Stack], + ): + super().__init__(layer_identifier, build_context, deploy_context, physical_id_mapping, stacks) + self._layer = cast(LayerVersion, build_context.layer_provider.get(self._layer_identifier)) def set_up(self) -> None: super().set_up() @@ -205,6 +218,8 @@ def set_up(self) -> None: def gather_resources(self) -> None: """Build layer and ZIP it into a temp file in self._zip_file""" with self._get_lock_chain(): + + rmtree_if_exists(self._layer.get_build_dir(self._build_context.build_dir)) builder = ApplicationBuilder( self._build_context.collect_build_resources(self._layer_identifier), self._build_context.build_dir, diff --git a/samcli/lib/sync/flows/zip_function_sync_flow.py b/samcli/lib/sync/flows/zip_function_sync_flow.py index 81f032fcdb..8987a23e56 100644 --- a/samcli/lib/sync/flows/zip_function_sync_flow.py +++ b/samcli/lib/sync/flows/zip_function_sync_flow.py @@ -20,6 +20,7 @@ from samcli.lib.build.app_builder import ApplicationBuilder from samcli.lib.sync.sync_flow import ResourceAPICall, ApiCallTypes +from samcli.lib.utils.osutils import rmtree_if_exists if TYPE_CHECKING: # pragma: no cover from samcli.commands.deploy.deploy_context import DeployContext @@ -79,6 +80,7 @@ def gather_resources(self) -> None: if self.has_locks(): exit_stack.enter_context(self._get_lock_chain()) + rmtree_if_exists(self._function.get_build_dir(self._build_context.build_dir)) builder = ApplicationBuilder( self._build_context.collect_build_resources(self._function_identifier), self._build_context.build_dir, diff --git a/samcli/lib/utils/osutils.py b/samcli/lib/utils/osutils.py index 174b6ffb50..0813c98b58 100644 --- a/samcli/lib/utils/osutils.py +++ b/samcli/lib/utils/osutils.py @@ -8,7 +8,8 @@ import sys import tempfile from contextlib import contextmanager -from typing import List, Optional +from pathlib import Path +from typing import List, Optional, Union LOG = logging.getLogger(__name__) @@ -69,6 +70,14 @@ def rmtree_callback(function, path, excinfo): LOG.debug("rmtree failed in %s for %s, details: %s", function, path, excinfo) +def rmtree_if_exists(path: Union[str, Path]): + """Removes given path if the path exists""" + path_obj = Path(str(path)) + if path_obj.exists(): + LOG.debug("Cleaning up path %s", str(path)) + shutil.rmtree(path_obj) + + def stdout(): """ Returns the stdout as a byte stream in a Py2/PY3 compatible manner diff --git a/tests/unit/lib/sync/flows/test_layer_sync_flow.py b/tests/unit/lib/sync/flows/test_layer_sync_flow.py index afcab1cdf8..f709f5e682 100644 --- a/tests/unit/lib/sync/flows/test_layer_sync_flow.py +++ b/tests/unit/lib/sync/flows/test_layer_sync_flow.py @@ -60,8 +60,15 @@ def test_setup_with_unknown_layer(self): @patch("samcli.lib.sync.flows.layer_sync_flow.make_zip") @patch("samcli.lib.sync.flows.layer_sync_flow.file_checksum") @patch("samcli.lib.sync.flows.layer_sync_flow.os") + @patch("samcli.lib.sync.flows.layer_sync_flow.rmtree_if_exists") def test_setup_gather_resources( - self, patched_os, patched_file_checksum, patched_make_zip, patched_tempfile, patched_app_builder + self, + patched_rmtree_if_exists, + patched_os, + patched_file_checksum, + patched_make_zip, + patched_tempfile, + patched_app_builder, ): given_collect_build_resources = Mock() self.build_context_mock.collect_build_resources.return_value = given_collect_build_resources @@ -81,6 +88,8 @@ def test_setup_gather_resources( self.layer_sync_flow.gather_resources() + layer_object = self.build_context_mock.layer_provider.get(self.layer_identifier) + patched_rmtree_if_exists.assert_called_with(layer_object.get_build_dir(self.build_context_mock.build_dir)) self.build_context_mock.collect_build_resources.assert_called_with(self.layer_identifier) patched_app_builder.assert_called_with( diff --git a/tests/unit/lib/sync/flows/test_zip_function_sync_flow.py b/tests/unit/lib/sync/flows/test_zip_function_sync_flow.py index 07e1b4d134..7750088a96 100644 --- a/tests/unit/lib/sync/flows/test_zip_function_sync_flow.py +++ b/tests/unit/lib/sync/flows/test_zip_function_sync_flow.py @@ -10,9 +10,11 @@ class TestZipFunctionSyncFlow(TestCase): def create_function_sync_flow(self): + self.build_context_mock = MagicMock() + self.function_identifier = "Function1" sync_flow = ZipFunctionSyncFlow( - "Function1", - build_context=MagicMock(), + self.function_identifier, + build_context=self.build_context_mock, deploy_context=MagicMock(), physical_id_mapping={}, stacks=[MagicMock()], @@ -34,9 +36,18 @@ def test_set_up(self, session_mock, client_provider_mock): @patch("samcli.lib.sync.flows.zip_function_sync_flow.make_zip") @patch("samcli.lib.sync.flows.zip_function_sync_flow.tempfile.gettempdir") @patch("samcli.lib.sync.flows.zip_function_sync_flow.ApplicationBuilder") + @patch("samcli.lib.sync.flows.zip_function_sync_flow.rmtree_if_exists") @patch("samcli.lib.sync.sync_flow.Session") def test_gather_resources( - self, session_mock, builder_mock, gettempdir_mock, make_zip_mock, file_checksum_mock, uuid4_mock, sha256_mock + self, + session_mock, + rmtree_if_exists_mock, + builder_mock, + gettempdir_mock, + make_zip_mock, + file_checksum_mock, + uuid4_mock, + sha256_mock, ): get_mock = MagicMock() get_mock.return_value = "ArtifactFolder1" @@ -53,6 +64,8 @@ def test_gather_resources( sync_flow.set_up() sync_flow.gather_resources() + function_object = self.build_context_mock.function_provider.get(self.function_identifier) + rmtree_if_exists_mock.assert_called_once_with(function_object.get_build_dir(self.build_context_mock.build_dir)) get_mock.assert_called_once_with("Function1") self.assertEqual(sync_flow._artifact_folder, "ArtifactFolder1") make_zip_mock.assert_called_once_with("temp_folder" + os.sep + "data-uuid_value", "ArtifactFolder1") @@ -178,7 +191,7 @@ def test_get_resource_api_calls(self, resource_api_call_mock): function_mock = MagicMock() function_mock.layers = [layer1, layer2] function_mock.codeuri = "CodeUri/" - build_context.function_provider.functions.get.return_value = function_mock + build_context.function_provider.get.return_value = function_mock sync_flow = ZipFunctionSyncFlow( "Function1", build_context=build_context, diff --git a/tests/unit/lib/utils/test_osutils.py b/tests/unit/lib/utils/test_osutils.py index d65dac1436..e09e1b47ee 100644 --- a/tests/unit/lib/utils/test_osutils.py +++ b/tests/unit/lib/utils/test_osutils.py @@ -6,8 +6,9 @@ import sys from unittest import TestCase -from unittest.mock import patch +from unittest.mock import patch, Mock from samcli.lib.utils import osutils +from samcli.lib.utils.osutils import rmtree_if_exists class Test_mkdir_temp(TestCase): @@ -77,3 +78,25 @@ def test_must_return_sys_stdout(self, patched_open, os_walk): patched_open.assert_any_call(os.path.join("b", target_file), "rb") patched_open.assert_any_call(os.path.join("a", target_file), "wb") patched_open.assert_any_call(os.path.join("b", target_file), "wb") + + +class Test_rmtree_if_exists(TestCase): + @patch("samcli.lib.utils.osutils.Path") + @patch("samcli.lib.utils.osutils.shutil.rmtree") + def test_must_skip_if_path_doesnt_exist(self, patched_rmtree, patched_path): + mock_path_obj = Mock() + mock_path_obj.exists.return_value = False + patched_path.return_value = mock_path_obj + + rmtree_if_exists(Mock()) + patched_rmtree.assert_not_called() + + @patch("samcli.lib.utils.osutils.Path") + @patch("samcli.lib.utils.osutils.shutil.rmtree") + def test_must_delete_if_path_exist(self, patched_rmtree, patched_path): + mock_path_obj = Mock() + mock_path_obj.exists.return_value = True + patched_path.return_value = mock_path_obj + + rmtree_if_exists(Mock()) + patched_rmtree.assert_called_with(mock_path_obj)