fix: address incorrect usage of request preconditions (#366)

frankyn · web-flow · commit 321658c3b9cc · 2021-01-28T14:43:34.000-08:00
diff --git a/google/cloud/storage/bucket.py b/google/cloud/storage/bucket.py
@@ -2022,6 +2022,9 @@ def rename_blob(
           This method will first duplicate the data and then delete the
           old blob.  This means that with very large objects renaming
           could be a very (temporarily) costly or a very slow operation.
+          If you need more control over the copy and deletion, instead
+          use `google.cloud.storage.blob.Blob.copy_to` and
+          `google.cloud.storage.blob.Blob.delete` directly.
 
         :type blob: :class:`google.cloud.storage.blob.Blob`
         :param blob: The blob to be renamed.
@@ -2079,25 +2082,29 @@ def rename_blob(
         :param if_source_generation_match: (Optional) Makes the operation
                                            conditional on whether the source
                                            object's generation matches the
-                                           given value.
+                                           given value. Also used in the
+                                           delete request.
 
         :type if_source_generation_not_match: long
         :param if_source_generation_not_match: (Optional) Makes the operation
                                                conditional on whether the source
                                                object's generation does not match
-                                               the given value.
+                                               the given value. Also used in the
+                                               delete request.
 
         :type if_source_metageneration_match: long
         :param if_source_metageneration_match: (Optional) Makes the operation
                                                conditional on whether the source
                                                object's current metageneration
-                                               matches the given value.
+                                               matches the given value.Also used in the
+                                               delete request.
 
         :type if_source_metageneration_not_match: long
         :param if_source_metageneration_not_match: (Optional) Makes the operation
                                                    conditional on whether the source
                                                    object's current metageneration
                                                    does not match the given value.
+                                                   Also used in the delete request.
 
         :type retry: google.api_core.retry.Retry or google.cloud.storage.retry.ConditionalRetryPolicy
         :param retry: (Optional) How to retry the RPC. A None value will disable retries.
@@ -2139,10 +2146,10 @@ def rename_blob(
             blob.delete(
                 client=client,
                 timeout=timeout,
-                if_generation_match=if_generation_match,
-                if_generation_not_match=if_generation_not_match,
-                if_metageneration_match=if_metageneration_match,
-                if_metageneration_not_match=if_metageneration_not_match,
+                if_generation_match=if_source_generation_match,
+                if_generation_not_match=if_source_generation_not_match,
+                if_metageneration_match=if_source_metageneration_match,
+                if_metageneration_not_match=if_source_metageneration_not_match,
                 retry=retry,
             )
         return new_blob
diff --git a/tests/unit/test_bucket.py b/tests/unit/test_bucket.py
@@ -1639,7 +1639,8 @@ def test_rename_blob_with_generation_match(self):
         NEW_BLOB_NAME = "new-blob-name"
         DATA = {"name": NEW_BLOB_NAME}
         GENERATION_NUMBER = 6
-        METAGENERATION_NUMBER = 9
+        SOURCE_GENERATION_NUMBER = 7
+        SOURCE_METAGENERATION_NUMBER = 9
 
         connection = _Connection(DATA)
         client = _Client(connection)
@@ -1652,7 +1653,8 @@ def test_rename_blob_with_generation_match(self):
             client=client,
             timeout=42,
             if_generation_match=GENERATION_NUMBER,
-            if_source_metageneration_not_match=METAGENERATION_NUMBER,
+            if_source_generation_match=SOURCE_GENERATION_NUMBER,
+            if_source_metageneration_not_match=SOURCE_METAGENERATION_NUMBER,
         )
 
         self.assertIs(renamed_blob.bucket, bucket)
@@ -1668,7 +1670,8 @@ def test_rename_blob_with_generation_match(self):
             kw["query_params"],
             {
                 "ifGenerationMatch": GENERATION_NUMBER,
-                "ifSourceMetagenerationNotMatch": METAGENERATION_NUMBER,
+                "ifSourceGenerationMatch": SOURCE_GENERATION_NUMBER,
+                "ifSourceMetagenerationNotMatch": SOURCE_METAGENERATION_NUMBER,
             },
         )
         self.assertEqual(kw["timeout"], 42)
@@ -1677,10 +1680,10 @@ def test_rename_blob_with_generation_match(self):
         blob.delete.assert_called_once_with(
             client=client,
             timeout=42,
-            if_generation_match=GENERATION_NUMBER,
+            if_generation_match=SOURCE_GENERATION_NUMBER,
             if_generation_not_match=None,
             if_metageneration_match=None,
-            if_metageneration_not_match=None,
+            if_metageneration_not_match=SOURCE_METAGENERATION_NUMBER,
             retry=DEFAULT_RETRY_IF_GENERATION_SPECIFIED,
         )
 
