refactor: use 'Client._patch_resource' in 'Blob.make_private'

tseaver · tseaver · commit 4cdf0c4cb61f · 2021-05-07T23:31:40.000-04:00
Indirecly, via 'ACL.save'. Also, adds 'timeout' and 'retry' support to 'Blob.make_private'. Toward #38.
diff --git a/google/cloud/storage/blob.py b/google/cloud/storage/blob.py
@@ -3013,16 +3013,40 @@ def make_public(
         self.acl.all().grant_read()
         self.acl.save(client=client, timeout=timeout, retry=retry)
 
-    def make_private(self, client=None):
+    def make_private(
+        self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY,
+    ):
         """Update blob's ACL, revoking read access for anonymous users.
 
         :type client: :class:`~google.cloud.storage.client.Client` or
                       ``NoneType``
         :param client: (Optional) The client to use.  If not passed, falls back
                        to the ``client`` stored on the blob's bucket.
+
+        :type timeout: float or tuple
+        :param timeout: (Optional) The amount of time, in seconds, to wait
+            for the server response. The timeout applies to each underlying
+            request.
+
+            Can also be passed as a tuple (connect_timeout, read_timeout).
+            See :meth:`requests.Session.request` documentation for details.
+
+        :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.
+            A google.api_core.retry.Retry value will enable retries, and the object will
+            define retriable response codes and errors and configure backoff and timeout options.
+
+            A google.cloud.storage.retry.ConditionalRetryPolicy value wraps a Retry object and
+            activates it only if certain conditions are met. This class exists to provide safe defaults
+            for RPC calls that are not technically safe to retry normally (due to potential data
+            duplication or other side-effects) but become safe to retry if a condition such as
+            if_metageneration_match is set.
+
+            See the retry.py source code and docstrings in this package (google.cloud.storage.retry) for
+            information on retry types and how to configure them.
         """
         self.acl.all().revoke_read()
-        self.acl.save(client=client)
+        self.acl.save(client=client, timeout=timeout, retry=retry)
 
     def compose(
         self,
diff --git a/tests/unit/test_blob.py b/tests/unit/test_blob.py
@@ -3472,23 +3472,55 @@ def test_make_public_w_timeout_w_retry(self):
             retry=retry,
         )
 
-    def test_make_private(self):
-        BLOB_NAME = "blob-name"
+    def test_make_private_w_defaults(self):
+        blob_name = "blob-name"
         no_permissions = []
-        after = ({"status": http_client.OK}, {"acl": no_permissions})
-        connection = _Connection(after)
-        client = _Client(connection)
+        api_response = {"acl": no_permissions}
+        client = mock.Mock(spec=["_patch_resource"])
+        client._patch_resource.return_value = api_response
         bucket = _Bucket(client=client)
-        blob = self._make_one(BLOB_NAME, bucket=bucket)
+        blob = self._make_one(blob_name, bucket=bucket)
         blob.acl.loaded = True
+
         blob.make_private()
+
         self.assertEqual(list(blob.acl), no_permissions)
-        kw = connection._requested
-        self.assertEqual(len(kw), 1)
-        self.assertEqual(kw[0]["method"], "PATCH")
-        self.assertEqual(kw[0]["path"], "/b/name/o/%s" % BLOB_NAME)
-        self.assertEqual(kw[0]["data"], {"acl": no_permissions})
-        self.assertEqual(kw[0]["query_params"], {"projection": "full"})
+
+        expected_patch_data = {"acl": no_permissions}
+        expected_query_params = {"projection": "full"}
+        client._patch_resource.assert_called_once_with(
+            blob.path,
+            expected_patch_data,
+            query_params=expected_query_params,
+            timeout=self._get_default_timeout(),
+            retry=DEFAULT_RETRY,
+        )
+
+    def test_make_private_w_timeout_w_retry(self):
+        blob_name = "blob-name"
+        no_permissions = []
+        api_response = {"acl": no_permissions}
+        client = mock.Mock(spec=["_patch_resource"])
+        client._patch_resource.return_value = api_response
+        bucket = _Bucket(client=client)
+        blob = self._make_one(blob_name, bucket=bucket)
+        blob.acl.loaded = True
+        timeout = 42
+        retry = mock.Mock(spec=[])
+
+        blob.make_private(timeout=timeout, retry=retry)
+
+        self.assertEqual(list(blob.acl), no_permissions)
+
+        expected_patch_data = {"acl": no_permissions}
+        expected_query_params = {"projection": "full"}
+        client._patch_resource.assert_called_once_with(
+            blob.path,
+            expected_patch_data,
+            query_params=expected_query_params,
+            timeout=timeout,
+            retry=retry,
+        )
 
     def test_compose_wo_content_type_set(self):
         SOURCE_1 = "source-1"
