Remove BucketLister class

Added change logs

Author: Rakshil Modi

.changes/next-release/enhancement-s3-34855.json

@@ -0,0 +1,5 @@
+{
+  "type": "enhancement",
+  "category": "s3",
+  "description": "Add all-versions flag with AWS CLI S3 rm command to delete all versions of objects present in a versioned-enabled bucket. fixes `#4070 <https://github.com/aws/aws-cli/issues/4070>`__"
+}

awscli/customizations/s3/filegenerator.py

@@ -23,7 +23,6 @@
 from awscli.customizations.s3.utils import (
     EPOCH_TIME,
     BucketLister,
-    BucketVersionLister,
     create_warning,
     find_bucket_key,
     find_dest_path_comp_key,
@@ -414,7 +413,7 @@ def _list_single_object(self, s3_path):
 class VersionedFileGenerator:
     """
     This class generates VersionedFileInfo objects for all versions of objects in a bucket.
-    It uses the BucketVersionLister to list all versions and creates appropriate
+    It uses the BucketLister class to list all versions and creates appropriate
     VersionedFileInfo objects for each version.
     """
 
@@ -447,7 +446,7 @@ def __init__(
         self.request_parameters = {}
         if request_parameters is not None:
             self.request_parameters = request_parameters
-        self._version_lister = BucketVersionLister(client)
+        self._version_lister = BucketLister(client)
 
     def call(self, files):
         """
@@ -466,9 +465,6 @@ def call(self, files):
         for src_path, content, version_id in file_iterator:
             dest_path, compare_key = find_dest_path_comp_key(files, src_path)
 
-            # Check if this is a delete marker
-            is_delete_marker = content.get('DeleteMarker', False)
-
             # Create a VersionedFileInfo for this object version
             yield VersionedFileInfo(
                 src=src_path,
@@ -479,10 +475,8 @@ def call(self, files):
                 src_type=src_type,
                 dest_type=dest_type,
                 operation_name=self.operation_name,
-                # client=self._client,
                 associated_response_data=content,
                 version_id=version_id,
-                is_delete_marker=is_delete_marker,
             )
 
     def list_object_versions(self, s3_path, dir_op):

awscli/customizations/s3/fileinfo.py

@@ -120,12 +120,9 @@ def __init__(self, version_id=None, is_delete_marker=False, **kwargs):
 
         :param version_id: The version ID of the S3 object.
         :type version_id: string
-        :param is_delete_marker: Whether this version is a delete marker.
-        :type is_delete_marker: boolean
         """
         super().__init__(**kwargs)
         self.version_id = version_id
-        self.is_delete_marker = is_delete_marker
 
     def is_glacier_compatible(self):
         """

awscli/customizations/s3/s3handler.py

@@ -656,7 +656,7 @@ def create_batches(self, fileinfos):
         for fileinfo in fileinfos:
             bucket, key = split_s3_bucket_key(fileinfo.src)
             obj_entry = {'Key': key}
-            if hasattr(fileinfo, 'version_id') and fileinfo.version_id:
+            if getattr(fileinfo, 'version_id', None):
                 obj_entry['VersionId'] = fileinfo.version_id
 
             objects_with_fileinfo.append(
@@ -836,14 +836,14 @@ def _submit_transfer_request_for_batch(self, batches):
 
                 # Report success
                 for fileinfo in fileinfos_in_batch:
-                    self._report_sucess(fileinfo)
+                    self._report_success(fileinfo)
 
             except Exception as e:
                 for fileinfo in fileinfos_in_batch:
                     self._report_failure(fileinfo, e)
         return total_objects
 
-    def _report_sucess(self, fileinfo):
+    def _report_success(self, fileinfo):
         src, dest = self._format_src_dest(fileinfo)
         self._result_queue.put(
             SuccessResult(transfer_type='delete', src=src, dest=dest)

awscli/customizations/s3/subcommands.py

@@ -1385,7 +1385,7 @@ def run(self):
         self._map_request_payer_params(rgen_request_parameters)
         rgen_kwargs['request_parameters'] = rgen_request_parameters
 
-        if self.parameters.get('all_versions'):
+        if self.cmd == 'rm' and self.parameters.get('all_versions'):
             versioned_file_generator = VersionedFileGenerator(**fgen_kwargs)
             file_generator = None
             rev_generator = None

awscli/customizations/s3/utils.py

@@ -431,23 +431,6 @@ def list_objects(
                 )
                 yield source_path, content
 
-
-class BucketVersionLister:
-    """List object versions in a bucket
-    This class provides functionality to list all versions of objects in a S3 bucket.
-    It can list versions for a specific object (when prefix is provided) or for all
-    objects in the bucket.
-    """
-
-    def __init__(self, client, date_parser=_date_parser):
-        """
-        Initialize a new BucketVersionLister.
-        :param client: The S3 client to use for listing versions.
-        :param date_parser: A function to parse date strings into datetime objects.
-        """
-        self._client = client
-        self._date_parser = date_parser
-
     def list_object_versions(
         self, bucket, prefix=None, page_size=None, extra_args=None
     ):
@@ -479,25 +462,30 @@ def list_object_versions(
         paginator = self._client.get_paginator('list_object_versions')
         pages = paginator.paginate(**kwargs)
         for page in pages:
-            # Process versions
             versions = page.get('Versions', [])
-            for version in versions:
-                source_path = bucket + '/' + version['Key']
-                version['LastModified'] = self._date_parser(
-                    version['LastModified']
-                )
-                version['DeleteMarker'] = False
-                yield source_path, version, version['VersionId']
 
-            # Process delete markers
+            for (
+                source_path,
+                content,
+                version_id,
+            ) in self._process_object_versions(bucket, versions):
+                yield source_path, content, version_id
+
             delete_markers = page.get('DeleteMarkers', [])
-            for marker in delete_markers:
-                source_path = bucket + '/' + marker['Key']
-                marker['LastModified'] = self._date_parser(
-                    marker['LastModified']
-                )
-                marker['DeleteMarker'] = True
-                yield source_path, marker, marker['VersionId']
+            for (
+                source_path,
+                content,
+                version_id,
+            ) in self._process_object_versions(bucket, delete_markers):
+                yield source_path, content, version_id
+
+    def _process_object_versions(self, bucket, object_versions):
+        for version in object_versions:
+            source_path = bucket + '/' + version['Key']
+            version['LastModified'] = self._date_parser(
+                version['LastModified']
+            )
+            yield source_path, version, version['VersionId']
 
 
 class PrintTask(

tests/unit/customizations/s3/test_s3handler.py

@@ -1305,76 +1305,44 @@ def test_enqueue_batch_deletes(self):
         fileinfos = []
         num_transfers = 5
         for i in range(num_transfers):
-            fileinfo = FileInfo(
+            fileinfo = VersionedFileInfo(
                 src=f'bucket/key{i}',
                 dest=None,
                 operation_name='delete',
                 src_type='s3',
+                version_id=f'version{i}',
             )
-            fileinfo.version_id = f'version{i}'
             fileinfos.append(fileinfo)
 
-        mock_submitter = mock.Mock()
-        mock_submitter.can_submit.return_value = True
-        mock_submitter.submit_batch.return_value = num_transfers
-        self.batch_s3_handler._batch_submitters = [mock_submitter]
-
-        self.batch_s3_handler.call(fileinfos)
-        mock_submitter.submit_batch.assert_called_once()
+        self.batch_s3_handler.call(iter(fileinfos))
+        self.transfer_manager.batch_delete.assert_called_once()
 
     def test_notifies_total_submissions(self):
         fileinfos = []
         num_transfers = 5
         for i in range(num_transfers):
-            fileinfo = FileInfo(
+            fileinfo = VersionedFileInfo(
                 src=f'bucket/key{i}',
                 dest=None,
                 operation_name='delete',
                 src_type='s3',
+                version_id=f'version{i}',
             )
-            fileinfo.version_id = f'version{i}'
             fileinfos.append(fileinfo)
 
-        # Mock the batch delete submitter
-        mock_submitter = mock.Mock()
-        mock_submitter.can_submit.return_value = True
-        mock_submitter.submit_batch.return_value = num_transfers
-        self.batch_s3_handler._batch_submitters = [mock_submitter]
-
-        self.batch_s3_handler.call(fileinfos)
+        self.batch_s3_handler.call(iter(fileinfos))
         assert (
             self.result_recorder.final_expected_files_transferred
             == num_transfers
         )
 
-    def test_call_selects_correct_submitter(self):
-        fileinfo = VersionedFileInfo(
-            src='bucket/key',
-            dest=None,
-            operation_name='delete',
-            src_type='s3',
-            version_id='version123',
-        )
-
-        mock_submitter = mock.Mock()
-        mock_submitter.can_submit.return_value = True
-        mock_submitter.submit_batch.return_value = 1
-        self.batch_s3_handler._batch_submitters = [mock_submitter]
-
-        self.batch_s3_handler.call([fileinfo])
-        mock_submitter.can_submit.assert_called_once_with(fileinfo)
-        mock_submitter.submit_batch.assert_called_once()
-
 
 class TestDeleteBatch:
-    @pytest.fixture(autouse=True)
-    def setUp(self):
+    def test_create_batches_with_multiple_objects(self):
         self.result_queue = queue.Queue()
         self.delete_batch = DeleteBatch(self.result_queue)
         self.bucket = 'mybucket'
         self.key = 'mykey'
-
-    def test_create_batches_with_multiple_objects(self):
         fileinfos = []
         num_objects = MAX_BATCH_SIZE + 5
         for i in range(num_objects):

tests/unit/customizations/s3/test_utils.py

@@ -26,7 +26,6 @@
 from awscli.customizations.s3.utils import (
     AppendFilter,
     BucketLister,
-    BucketVersionLister,
     NonSeekableStream,
     RequestParamsMapper,
     S3PathResolver,
@@ -1082,14 +1081,11 @@ def test_list_object_versions(self):
             {'Versions': [versions[1]], 'DeleteMarkers': delete_markers},
         ]
 
-        lister = BucketVersionLister(self.client, self.date_parser)
+        lister = BucketLister(self.client, self.date_parser)
         objects = list(lister.list_object_versions(bucket='foo'))
         expected_version_a = versions[0].copy()
-        expected_version_a['DeleteMarker'] = False
         expected_version_b = versions[1].copy()
-        expected_version_b['DeleteMarker'] = False
         expected_delete_marker_a = delete_markers[0].copy()
-        expected_delete_marker_a['DeleteMarker'] = True
 
         assert objects == [
             ('foo/a', expected_version_a, 'version1'),
@@ -1115,7 +1111,7 @@ def test_list_object_versions_with_extra_args(self):
                 'DeleteMarkers': [],
             }
         ]
-        lister = BucketVersionLister(self.client, self.date_parser)
+        lister = BucketLister(self.client, self.date_parser)
         list(
             lister.list_object_versions(
                 bucket='mybucket', extra_args={'RequestPayer': 'requester'}