Add no overwrite for sync operation

Added no overwrite for download operation

handled error during download streaming streaming

Improved upload test cases

Updated test cases

Author: Rakshil Modi

awscli/customizations/s3/subcommands.py

@@ -684,6 +684,7 @@
     REQUEST_PAYER,
     CHECKSUM_MODE,
     CHECKSUM_ALGORITHM,
+    NO_OVERWRITE,
 ]
 
 
@@ -1072,7 +1073,6 @@ class CpCommand(S3TransferCommand):
             METADATA_DIRECTIVE,
             EXPECTED_SIZE,
             RECURSIVE,
-            NO_OVERWRITE,
         ]
     )
 
@@ -1097,7 +1097,6 @@ class MvCommand(S3TransferCommand):
             METADATA_DIRECTIVE,
             RECURSIVE,
             VALIDATE_SAME_S3_PATHS,
-            NO_OVERWRITE,
         ]
     )
 
@@ -1401,7 +1400,8 @@ def run(self):
             self._client, self._source_client, self.parameters
         )
 
-        s3_transfer_handler = S3TransferHandlerFactory(self.parameters)(
+        params = self._get_s3_handler_params()
+        s3_transfer_handler = S3TransferHandlerFactory(params)(
             self._transfer_manager, result_queue
         )
 
@@ -1510,6 +1510,17 @@ def _map_sse_c_params(self, request_parameters, paths_type):
                 },
             )
 
+    def _get_s3_handler_params(self):
+        """
+        Removing no-overwrite params from sync since file to
+        be synced are already separated out using sync strategy
+        """
+        if self.cmd == 'sync':
+            return {
+                k: v for k, v in self.parameters.items() if k != 'no_overwrite'
+            }
+        return self.parameters
+
 
 # TODO: This class is fairly quirky in the sense that it is both a builder
 #  and a data object. In the future we should make the following refactorings

tests/functional/s3/test_cp_command.py

@@ -315,13 +315,7 @@ def test_no_overwrite_flag_when_object_exists_on_target(self):
         # Set up the response to simulate a PreconditionFailed error
         self.http_response.status_code = 412
         self.parsed_responses = [
-            {
-                'Error': {
-                    'Code': 'PreconditionFailed',
-                    'Message': 'At least one of the pre-conditions you specified did not hold',
-                    'Condition': 'If-None-Match',
-                }
-            }
+            self.precondition_failed_error_response(),
         ]
         self.run_cmd(cmdline, expected_rc=0)
         # Verify PutObject was attempted with IfNoneMatch
@@ -367,13 +361,7 @@ def test_no_overwrite_flag_multipart_upload_when_object_exists_on_target(
             {'UploadId': 'foo'},  # CreateMultipartUpload response
             {'ETag': '"foo-1"'},  # UploadPart response
             {'ETag': '"foo-2"'},  # UploadPart response
-            {
-                'Error': {
-                    'Code': 'PreconditionFailed',
-                    'Message': 'At least one of the pre-conditions you specified did not hold',
-                    'Condition': 'If-None-Match',
-                }
-            },  # PreconditionFailed error for CompleteMultipart Upload
+            self.precondition_failed_error_response(),  # PreconditionFailed error for CompleteMultipart Upload
             {},  # AbortMultipartUpload response
         ]
         # Checking for success as file is skipped
@@ -1496,6 +1484,15 @@ def test_streaming_download_error(self):
         )
         self.assertIn(error_message, stderr)
 
+    def test_no_overwrite_cannot_be_used_with_streaming_download(self):
+        command = "s3 cp s3://bucket/streaming.txt - --no-overwrite"
+        _, stderr, _ = self.run_cmd(command, expected_rc=252)
+        error_message = (
+            "--no-overwrite parameter is not supported for "
+            "streaming downloads"
+        )
+        self.assertIn(error_message, stderr)
+
 
 class TestCpCommandWithRequesterPayer(BaseCPCommandTest):
     def setUp(self):

tests/functional/s3/test_mv_command.py

@@ -336,13 +336,7 @@ def test_mv_no_overwrite_flag_when_object_exists_on_target(self):
         # Set up the response to simulate a PreconditionFailed error
         self.http_response.status_code = 412
         self.parsed_responses = [
-            {
-                'Error': {
-                    'Code': 'PreconditionFailed',
-                    'Message': 'At least one of the pre-conditions you specified did not hold',
-                    'Condition': 'If-None-Match',
-                }
-            }
+            self.precondition_failed_error_response(),
         ]
         self.run_cmd(cmdline, expected_rc=0)
         # Verify PutObject was attempted with IfNoneMatch
@@ -392,13 +386,7 @@ def test_mv_no_overwrite_flag_multipart_upload_when_object_exists_on_target(
             {'UploadId': 'foo'},  # CreateMultipartUpload response
             {'ETag': '"foo-1"'},  # UploadPart response
             {'ETag': '"foo-2"'},  # UploadPart response
-            {
-                'Error': {
-                    'Code': 'PreconditionFailed',
-                    'Message': 'At least one of the pre-conditions you specified did not hold',
-                    'Condition': 'If-None-Match',
-                }
-            },  # CompleteMultipartUpload response
+            self.precondition_failed_error_response(),  # CompleteMultipartUpload response
             {},  # Abort Multipart
         ]
         self.run_cmd(cmdline, expected_rc=0)

tests/functional/s3/test_sync_command.py

@@ -540,6 +540,91 @@ def test_download_with_checksum_mode_crc64nvme(self):
             ('ChecksumMode', 'ENABLED'), self.operations_called[1][1].items()
         )
 
+    def test_sync_upload_with_no_overwrite_when_file_does_not_exist_at_destination(
+        self,
+    ):
+        self.files.create_file("new_file.txt", "mycontent")
+        self.parsed_responses = [
+            self.list_objects_response(['file.txt']),
+            self.get_object_response(),
+        ]
+        cmdline = (
+            f'{self.prefix} {self.files.rootdir} s3://bucket --no-overwrite'
+        )
+        self.run_cmd(cmdline, expected_rc=0)
+        self.assertEqual(len(self.operations_called), 2)
+        self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
+        self.assertEqual(self.operations_called[1][0].name, 'PutObject')
+        self.assertEqual(self.operations_called[1][1]['Key'], 'new_file.txt')
+
+    def test_sync_upload_with_no_overwrite_when_file_exists_at_destination(
+        self,
+    ):
+        self.files.create_file("new_file.txt", "mycontent")
+        self.parsed_responses = [
+            self.list_objects_response(['new_file.txt']),
+        ]
+        cmdline = (
+            f'{self.prefix} {self.files.rootdir} s3://bucket --no-overwrite'
+        )
+        self.run_cmd(cmdline, expected_rc=0)
+        self.assertEqual(len(self.operations_called), 1)
+        self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
+
+    def test_sync_download_with_no_overwrite_file_not_exists_at_destination(
+        self,
+    ):
+        self.parsed_responses = [
+            self.list_objects_response(['new_file.txt']),
+            self.get_object_response(),
+        ]
+        cmdline = (
+            f'{self.prefix} s3://bucket/ {self.files.rootdir} --no-overwrite'
+        )
+        self.run_cmd(cmdline, expected_rc=0)
+        self.assertEqual(len(self.operations_called), 2)
+        self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
+        self.assertEqual(self.operations_called[1][0].name, 'GetObject')
+        local_file_path = os.path.join(self.files.rootdir, 'new_file.txt')
+        self.assertTrue(os.path.exists(local_file_path))
+
+    def test_sync_download_with_no_overwrite_file_exists_at_destination(self):
+        self.files.create_file('file.txt', 'My content')
+        self.parsed_responses = [
+            self.list_objects_response(['file.txt']),
+        ]
+        cmdline = (
+            f'{self.prefix} s3://bucket/ {self.files.rootdir} --no-overwrite'
+        )
+        self.run_cmd(cmdline, expected_rc=0)
+        self.assertEqual(len(self.operations_called), 1)
+        self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
+
+    def test_sync_copy_with_no_overwrite_file_not_exists_at_destination(self):
+        self.parsed_responses = [
+            self.list_objects_response(['new_file.txt']),
+            self.list_objects_response(['file1.txt']),
+            self.copy_object_response(),
+        ]
+        cmdline = f'{self.prefix} s3://bucket/ s3://bucket2/ --no-overwrite'
+        self.run_cmd(cmdline, expected_rc=0)
+        self.assertEqual(len(self.operations_called), 3)
+        self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
+        self.assertEqual(self.operations_called[1][0].name, 'ListObjectsV2')
+        self.assertEqual(self.operations_called[2][0].name, 'CopyObject')
+        self.assertEqual(self.operations_called[2][1]['Key'], 'new_file.txt')
+
+    def test_sync_copy_with_no_overwrite_file_exists_at_destination(self):
+        self.parsed_responses = [
+            self.list_objects_response(['new_file.txt']),
+            self.list_objects_response(['new_file.txt', 'file1.txt']),
+        ]
+        cmdline = f'{self.prefix} s3://bucket/ s3://bucket2/ --no-overwrite'
+        self.run_cmd(cmdline, expected_rc=0)
+        self.assertEqual(len(self.operations_called), 2)
+        self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
+        self.assertEqual(self.operations_called[1][0].name, 'ListObjectsV2')
+
 
 class TestSyncSourceRegion(BaseS3CLIRunnerTest):
     def test_respects_source_region(self):