Implemented No-overwrite for uploads using cp and mv command

Customizing error message

test cases for no_overwrite for upload operations using cp command

Code cleanup

Test cases for move command

Removed unwanted code changes

code quality improvement

Author: Rakshil Modi

awscli/customizations/s3/results.py

@@ -23,6 +23,7 @@
 from awscli.customizations.s3.subscribers import OnDoneFilteredSubscriber
 from awscli.customizations.s3.utils import WarningResult, human_readable_size
 from awscli.customizations.utils import uni_print
+from awscli.customizations.s3.utils import create_warning
 
 LOGGER = logging.getLogger(__name__)
 
@@ -123,7 +124,12 @@ def _on_failure(self, future, e):
             if isinstance(e, FatalError):
                 error_result_cls = ErrorResult
             self._result_queue.put(error_result_cls(exception=e))
-        else:
+        elif self._is_precondition_failed(e):
+                error_message = f"as it already exists on {self._dest}"
+                warning = create_warning(self._src, error_message, True)
+                self._result_queue.put(warning)
+                return 
+        else :
             self._result_queue.put(
                 FailureResult(
                     transfer_type=self._transfer_type,
@@ -132,6 +138,11 @@ def _on_failure(self, future, e):
                     exception=e,
                 )
             )
+    
+    def _is_precondition_failed(self, exception):
+        """Check if this is a PreconditionFailed error"""
+        return (hasattr(exception, 'response') and 
+            exception.response.get('Error', {}).get('Code') == 'PreconditionFailed')
 
 
 class BaseResultHandler:

awscli/customizations/s3/subcommands.py

@@ -642,6 +642,16 @@
     ),
 }
 
+NO_OVERWRITE = {
+    'name': 'no-overwrite',
+    'action': 'store_true',
+    'help_text': (
+        "This flag prevents overwriting of files at the destination. With this flag, "
+        "only the files not present at the destination will be transferred "
+        "and others will be skipped during the operation."
+    ),
+}
+
 TRANSFER_ARGS = [
     DRYRUN,
     QUIET,
@@ -1057,7 +1067,7 @@ class CpCommand(S3TransferCommand):
             }
         ]
         + TRANSFER_ARGS
-        + [METADATA, COPY_PROPS, METADATA_DIRECTIVE, EXPECTED_SIZE, RECURSIVE]
+        + [METADATA, COPY_PROPS, METADATA_DIRECTIVE, EXPECTED_SIZE, RECURSIVE, NO_OVERWRITE]
     )
 
 
@@ -1081,6 +1091,7 @@ class MvCommand(S3TransferCommand):
             METADATA_DIRECTIVE,
             RECURSIVE,
             VALIDATE_SAME_S3_PATHS,
+            NO_OVERWRITE,
         ]
     )
 

awscli/customizations/s3/utils.py

@@ -489,6 +489,7 @@ def map_put_object_params(cls, request_params, cli_params):
         cls._set_sse_c_request_params(request_params, cli_params)
         cls._set_request_payer_param(request_params, cli_params)
         cls._set_checksum_algorithm_param(request_params, cli_params)
+        cls._set_no_overwrite_param(request_params, cli_params)
 
     @classmethod
     def map_get_object_params(cls, request_params, cli_params):
@@ -557,6 +558,12 @@ def map_delete_object_params(cls, request_params, cli_params):
     @classmethod
     def map_list_objects_v2_params(cls, request_params, cli_params):
         cls._set_request_payer_param(request_params, cli_params)
+    
+    @classmethod
+    def _set_no_overwrite_param(cls, request_params, cli_params):
+        """Mapping No overwrite header with IfNoneMatch"""
+        if cli_params.get('no_overwrite'):
+            request_params['IfNoneMatch'] = "*"
 
     @classmethod
     def _set_request_payer_param(cls, request_params, cli_params):

awscli/s3transfer/manager.py

@@ -195,6 +195,7 @@ class TransferManager:
         + [
             'ChecksumType',
             'MpuObjectSize',
+            'IfNoneMatch',
         ]
         + FULL_OBJECT_CHECKSUM_ARGS
     )

awscli/s3transfer/upload.py

@@ -515,7 +515,7 @@ class UploadSubmissionTask(SubmissionTask):
 
     PUT_OBJECT_BLOCKLIST = ["ChecksumType", "MpuObjectSize"]
 
-    CREATE_MULTIPART_BLOCKLIST = FULL_OBJECT_CHECKSUM_ARGS + ["MpuObjectSize"]
+    CREATE_MULTIPART_BLOCKLIST = FULL_OBJECT_CHECKSUM_ARGS + ["MpuObjectSize", "IfNoneMatch"]
 
     UPLOAD_PART_ARGS = [
         'ChecksumAlgorithm',
@@ -534,6 +534,7 @@ class UploadSubmissionTask(SubmissionTask):
         'ExpectedBucketOwner',
         'ChecksumType',
         'MpuObjectSize',
+        'IfNoneMatch',
     ] + FULL_OBJECT_CHECKSUM_ARGS
 
     def _get_upload_input_manager_cls(self, transfer_future):

tests/functional/s3/test_cp_command.py

@@ -295,6 +295,68 @@ def test_operations_used_in_recursive_download(self):
             len(self.operations_called), 1, self.operations_called
         )
         self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
+    
+    def test_for_no_overwrite_flag_when_object_not_exists_on_target(self):
+        """Testing when object with different key is successsfully uploaded using no-overwrite"""
+        full_path = self.files.create_file('foo.txt', 'mycontent')
+        cmdline = '%s %s s3://bucket --no-overwrite' % ( self.prefix,full_path)
+        self.parsed_responses = [
+            {'ETag': '"c8afdb36c52cf4727836669019e69222"'}
+        ]
+        self.run_cmd(cmdline, expected_rc=0)
+        #Verify putObject was called
+        self.assertEqual(
+            len(self.operations_called), 1, self.operations_called
+        )
+        self.assertEqual(self.operations_called[0][0].name, 'PutObject')
+        # Verify the IfNoneMatch condition was set in the request
+        self.assertEqual(self.operations_called[0][1]['IfNoneMatch'], '*')
+    
+    def test_for_no_overwrite_flag_when_object_exists_on_target(self):
+        """Testing  when object already exists using no-overwrite"""
+        full_path = self.files.create_file('foo.txt', 'mycontent')
+        cmdline = '%s %s s3://bucket --no-overwrite' % ( self.prefix,full_path)
+        # 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'
+                }
+            }
+        ]
+        stdout, stderr, rc = self.run_cmd(cmdline, expected_rc=2) #Checking for warning
+        # Verify PutObject was attempted with IfNoneMatch
+        self.assertEqual(len(self.operations_called), 1)
+        self.assertEqual(self.operations_called[0][0].name, 'PutObject')
+        self.assertEqual(self.operations_called[0][1]['IfNoneMatch'], '*')
+
+    def test_for_no_overwrite_flag_multipart_upload_when_object_not_exists_on_target(self):
+        """Testing multipart upload with no-overwrite flag when object doesn't exist"""
+        # Create a large file that will trigger multipart upload
+        full_path = self.files.create_file('foo.txt', 'a' * 10 * (1024**2))
+        cmdline = '%s %s s3://bucket --no-overwrite' % (self.prefix, full_path)
+    
+        # Set up responses for multipart upload
+        self.parsed_responses = [
+            {'UploadId': 'foo'},  # CreateMultipartUpload response
+            {'ETag': '"foo-1"'},  # UploadPart response
+            {'ETag': '"foo-2"'},  # UploadPart response
+            {}                    # CompleteMultipartUpload response
+        ]
+    
+        self.run_cmd(cmdline, expected_rc=0)
+    
+        # Verify all multipart operations were called
+        self.assertEqual(len(self.operations_called), 4)
+        self.assertEqual(self.operations_called[0][0].name, 'CreateMultipartUpload')
+        self.assertEqual(self.operations_called[1][0].name, 'UploadPart')
+        self.assertEqual(self.operations_called[2][0].name, 'UploadPart')
+        self.assertEqual(self.operations_called[3][0].name, 'CompleteMultipartUpload')
+    
+        # Verify the IfNoneMatch condition was set in the CompleteMultipartUpload request
+        self.assertEqual(self.operations_called[3][1]['IfNoneMatch'], '*')
 
     def test_dryrun_download(self):
         self.parsed_responses = [self.head_object_response()]

tests/functional/s3/test_mv_command.py

@@ -315,7 +315,69 @@ def test_download_with_checksum_mode_crc32(self):
         self.assertEqual(
             self.operations_called[1][1]['ChecksumMode'], 'ENABLED'
         )
-
+    
+    def test_mv_no_overwrite_flag_when_object_not_exists_on_target(self):
+        """Testing mv when object doesnt exist using no-overwrite"""
+        full_path = self.files.create_file('foo.txt', 'contents')
+        cmdline = f'{self.prefix} {full_path} s3://bucket --no-overwrite'
+        self.run_cmd(cmdline, expected_rc=0)
+        # Verify putObject was called
+        self.assertEqual(len(self.operations_called),1)
+        self.assertEqual(self.operations_called[0][0].name, 'PutObject')
+        # Verify the IfNoneMatch condition was set in the request
+        self.assertEqual(self.operations_called[0][1]['IfNoneMatch'], '*')
+        # Verify source file was deleted (move operation)
+        self.assertFalse(os.path.exists(full_path))
+    
+    def test_mv_no_overwrite_flag_when_object_exists_on_target(self):
+        """Testing mv when object already exists using no-overwrite"""
+        full_path = self.files.create_file('foo.txt', 'mycontent')
+        cmdline = '%s %s s3://bucket/foo.txt --no-overwrite' % (self.prefix, full_path)
+        # 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'
+                }
+            }
+        ]
+        stdout, stderr, rc = self.run_cmd(cmdline, expected_rc=2)
+        # Verify PutObject was attempted with IfNoneMatch
+        self.assertEqual(len(self.operations_called), 1)
+        self.assertEqual(self.operations_called[0][0].name, 'PutObject')
+        self.assertEqual(self.operations_called[0][1]['IfNoneMatch'], '*')
+        # Verify source file was not deleted
+        self.assertTrue(os.path.exists(full_path))
+
+    def test_mv_no_overwrite_flag_multipart_upload_when_object_not_exists_on_target(self):
+        """Testing mv multipart upload with no-overwrite flag when object doesn't exist"""
+        # Create a large file that will trigger multipart upload
+        full_path = self.files.create_file('foo.txt', 'a' * 10 * (1024**2))
+        cmdline = '%s %s s3://bucket/foo.txt --no-overwrite' % (self.prefix, full_path)
+        
+        # Set up responses for multipart upload
+        self.parsed_responses = [
+            {'UploadId': 'foo'},  # CreateMultipartUpload response
+            {'ETag': '"foo-1"'},  # UploadPart response
+            {'ETag': '"foo-2"'},  # UploadPart response
+            {}                    # CompleteMultipartUpload response
+        ]
+        
+        self.run_cmd(cmdline, expected_rc=0)
+        
+        # Verify all multipart operations were called
+        self.assertEqual(len(self.operations_called), 4)
+        self.assertEqual(self.operations_called[0][0].name, 'CreateMultipartUpload')
+        self.assertEqual(self.operations_called[1][0].name, 'UploadPart')
+        self.assertEqual(self.operations_called[2][0].name, 'UploadPart')
+        self.assertEqual(self.operations_called[3][0].name, 'CompleteMultipartUpload')
+        
+        # Verify the IfNoneMatch condition was set in the CompleteMultipartUpload request
+        self.assertEqual(self.operations_called[3][1]['IfNoneMatch'], '*')
+        # Verify source file was deleted (successful move operation)
+        self.assertFalse(os.path.exists(full_path))
 
 class TestMvWithCRTClient(BaseCRTTransferClientTest):
     def test_upload_move_using_crt_client(self):