diff --git a/google/cloud/storage/bucket.py b/google/cloud/storage/bucket.py index 1b31baab7..ccdf8de05 100644 --- a/google/cloud/storage/bucket.py +++ b/google/cloud/storage/bucket.py @@ -1578,6 +1578,9 @@ def delete( ) % (self._MAX_OBJECTS_FOR_ITERATION,) raise ValueError(message) + # Convert to a list of blob names as we want to delete the latest versions of each blob. + # Blob generations propagated from the list op may no longer represent the latest versions. + blobs = [blob.name for blob in blobs] # Ignore 404 errors on delete. self.delete_blobs( blobs, @@ -1787,11 +1790,16 @@ def delete_blobs( for blob in blobs: try: blob_name = blob + generation = None if not isinstance(blob_name, str): blob_name = blob.name + if blob.generation: + generation = blob.generation + self.delete_blob( blob_name, client=client, + generation=generation, if_generation_match=next(if_generation_match, None), if_generation_not_match=next(if_generation_not_match, None), if_metageneration_match=next(if_metageneration_match, None), diff --git a/tests/conformance/test_conformance.py b/tests/conformance/test_conformance.py index 4d16fc36f..119054695 100644 --- a/tests/conformance/test_conformance.py +++ b/tests/conformance/test_conformance.py @@ -345,7 +345,8 @@ def bucket_delete_blob(client, _preconditions, **resources): def bucket_delete_blobs(client, _preconditions, **resources): object = resources.get("object") bucket = client.bucket(resources.get("bucket").name) - sources = [object] + # Generation can be propagated from blob instances + sources = [object.name] source_generations = [object.generation] if _preconditions: bucket.delete_blobs(sources, if_generation_match=source_generations) diff --git a/tests/unit/test_bucket.py b/tests/unit/test_bucket.py index d5206f287..ccb1edea0 100644 --- a/tests/unit/test_bucket.py +++ b/tests/unit/test_bucket.py @@ -1437,7 +1437,11 @@ def test_delete_hit_w_force_delete_blobs(self): client = mock.Mock(spec=["_delete_resource"]) client._delete_resource.return_value = None bucket = self._make_one(client=client, name=name) - blobs = [mock.Mock(spec=[]), mock.Mock(spec=[])] + blob = mock.Mock(spec=["name", "generation"]) + blob.name = "blob-name" + blob.generation = 1512565576797178 + blobs = [blob] + blob_names = [blob.name] bucket.list_blobs = mock.Mock(return_value=iter(blobs)) bucket.delete_blobs = mock.Mock(return_value=None) @@ -1453,7 +1457,44 @@ def test_delete_hit_w_force_delete_blobs(self): ) bucket.delete_blobs.assert_called_once_with( - blobs, + blob_names, + on_error=mock.ANY, + client=client, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY, + ) + + expected_query_params = {} + client._delete_resource.assert_called_once_with( + bucket.path, + query_params=expected_query_params, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY, + _target_object=None, + ) + + def test_delete_hit_w_force_no_blobs(self): + name = "name" + client = mock.Mock(spec=["_delete_resource"]) + client._delete_resource.return_value = None + bucket = self._make_one(client=client, name=name) + no_blobs = [] + bucket.list_blobs = mock.Mock(return_value=iter(no_blobs)) + bucket.delete_blobs = mock.Mock(return_value=None) + + result = bucket.delete(force=True) + + self.assertIsNone(result) + + bucket.list_blobs.assert_called_once_with( + max_results=bucket._MAX_OBJECTS_FOR_ITERATION + 1, + client=client, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY, + ) + + bucket.delete_blobs.assert_called_once_with( + no_blobs, on_error=mock.ANY, client=client, timeout=self._get_default_timeout(), @@ -1474,11 +1515,13 @@ def test_delete_w_force_w_user_project_w_miss_on_blob(self): name = "name" blob_name = "blob-name" + generation = 1512565576797178 client = mock.Mock(spec=["_delete_resource"]) client._delete_resource.return_value = None bucket = self._make_one(client=client, name=name) - blob = mock.Mock(spec=["name"]) + blob = mock.Mock(spec=["name", "generation"]) blob.name = blob_name + blob.generation = generation blobs = [blob] bucket.list_blobs = mock.Mock(return_value=iter(blobs)) bucket.delete_blob = mock.Mock(side_effect=NotFound("testing")) @@ -1490,6 +1533,7 @@ def test_delete_w_force_w_user_project_w_miss_on_blob(self): bucket.delete_blob.assert_called_once_with( blob_name, client=client, + generation=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1649,6 +1693,7 @@ def test_delete_blobs_hit_w_explicit_client_w_timeout(self): bucket.delete_blob.assert_called_once_with( blob_name, client=client, + generation=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1693,6 +1738,7 @@ def test_delete_blobs_w_generation_match_w_retry(self): call_1 = mock.call( blob_name, client=None, + generation=None, if_generation_match=generation_number, if_generation_not_match=None, if_metageneration_match=None, @@ -1703,6 +1749,7 @@ def test_delete_blobs_w_generation_match_w_retry(self): call_2 = mock.call( blob_name2, client=None, + generation=None, if_generation_match=generation_number2, if_generation_not_match=None, if_metageneration_match=None, @@ -1730,6 +1777,7 @@ def test_delete_blobs_w_generation_match_none(self): call_1 = mock.call( blob_name, client=None, + generation=None, if_generation_match=generation_number, if_generation_not_match=None, if_metageneration_match=None, @@ -1740,6 +1788,47 @@ def test_delete_blobs_w_generation_match_none(self): call_2 = mock.call( blob_name2, client=None, + generation=None, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY_IF_GENERATION_SPECIFIED, + ) + bucket.delete_blob.assert_has_calls([call_1, call_2]) + + def test_delete_blobs_w_generation(self): + name = "name" + blob_name = "blob-name" + blob_name2 = "blob-name2" + generation_number = 1512565576797178 + client = mock.Mock(spec=[]) + bucket = self._make_one(client=client, name=name) + bucket.delete_blob = mock.Mock() + blob = mock.Mock(spec=["name", "generation"]) + blob.name = blob_name + blob.generation = generation_number + blob2 = mock.Mock(spec=["name", "generation"]) + blob2.name = blob_name2 + blob2.generation = None + bucket.delete_blobs([blob, blob2]) + + call_1 = mock.call( + blob_name, + client=None, + generation=generation_number, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + timeout=self._get_default_timeout(), + retry=DEFAULT_RETRY_IF_GENERATION_SPECIFIED, + ) + call_2 = mock.call( + blob_name2, + client=None, + generation=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1766,6 +1855,7 @@ def test_delete_blobs_miss_wo_on_error(self): call_1 = mock.call( blob_name, client=None, + generation=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1776,6 +1866,7 @@ def test_delete_blobs_miss_wo_on_error(self): call_2 = mock.call( blob_name2, client=None, + generation=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1804,6 +1895,7 @@ def test_delete_blobs_miss_w_on_error(self): call_1 = mock.call( blob_name, client=None, + generation=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1814,6 +1906,7 @@ def test_delete_blobs_miss_w_on_error(self): call_2 = mock.call( blob_name2, client=None, + generation=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None,