refactor: add / use 'Client._get_resource' method (googleapis#431)

Use an explicit helper client method for `GET` requests, rather than manipulating client's private `_connection.api_request`.  As a benefit, tests get *way* clearer.

Toward googleapis#38

~~Based on top of the branch from googleapis#430.  I will rebase when that PR merges.~~

google/cloud/storage/_helpers.py

@@ -215,14 +215,13 @@ def reload(
             if_metageneration_match=if_metageneration_match,
             if_metageneration_not_match=if_metageneration_not_match,
         )
-        api_response = client._connection.api_request(
-            method="GET",
-            path=self.path,
+        api_response = client._get_resource(
+            self.path,
             query_params=query_params,
             headers=self._encryption_headers(),
-            _target_object=self,
             timeout=timeout,
             retry=retry,
+            _target_object=self,
         )
         self._set_properties(api_response)
 

google/cloud/storage/acl.py

@@ -85,6 +85,7 @@
 """
 
 from google.cloud.storage.constants import _DEFAULT_TIMEOUT
+from google.cloud.storage.retry import DEFAULT_RETRY
 
 
 class _ACLEntity(object):
@@ -206,6 +207,7 @@ class ACL(object):
 
     # Subclasses must override to provide these attributes (typically,
     # as properties).
+    client = None
     reload_path = None
     save_path = None
     user_project = None
@@ -430,7 +432,7 @@ def _require_client(self, client):
             client = self.client
         return client
 
-    def reload(self, client=None, timeout=_DEFAULT_TIMEOUT):
+    def reload(self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY):
         """Reload the ACL data from Cloud Storage.
 
         If :attr:`user_project` is set, bills the API request to that project.
@@ -445,6 +447,15 @@ def reload(self, client=None, timeout=_DEFAULT_TIMEOUT):
 
             Can also be passed as a tuple (connect_timeout, read_timeout).
             See :meth:`requests.Session.request` documentation for details.
+
+        :type retry: :class:`~google.api_core.retry.Retry`
+        :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.
         """
         path = self.reload_path
         client = self._require_client(client)
@@ -455,10 +466,11 @@ def reload(self, client=None, timeout=_DEFAULT_TIMEOUT):
 
         self.entities.clear()
 
-        found = client._connection.api_request(
-            method="GET", path=path, query_params=query_params, timeout=timeout,
+        found = client._get_resource(
+            path, query_params=query_params, timeout=timeout, retry=retry,
         )
         self.loaded = True
+
         for entry in found.get("items", ()):
             self.add_entity(self.entity_from_dict(entry))
 

google/cloud/storage/blob.py

@@ -704,20 +704,19 @@ def exists(
         try:
             # We intentionally pass `_target_object=None` since fields=name
             # would limit the local properties.
-            client._connection.api_request(
-                method="GET",
-                path=self.path,
+            client._get_resource(
+                self.path,
                 query_params=query_params,
-                _target_object=None,
                 timeout=timeout,
                 retry=retry,
+                _target_object=None,
             )
+        except NotFound:
             # NOTE: This will not fail immediately in a batch. However, when
             #       Batch.finish() is called, the resulting `NotFound` will be
             #       raised.
-            return True
-        except NotFound:
             return False
+        return True
 
     def delete(
         self,
@@ -2829,13 +2828,12 @@ def get_iam_policy(
         if requested_policy_version is not None:
             query_params["optionsRequestedPolicyVersion"] = requested_policy_version
 
-        info = client._connection.api_request(
-            method="GET",
-            path="%s/iam" % (self.path,),
+        info = client._get_resource(
+            "%s/iam" % (self.path,),
             query_params=query_params,
-            _target_object=None,
             timeout=timeout,
             retry=retry,
+            _target_object=None,
         )
         return Policy.from_api_repr(info)
 
@@ -2970,12 +2968,12 @@ def test_iam_permissions(
             query_params["userProject"] = self.user_project
 
         path = "%s/iam/testPermissions" % (self.path,)
-        resp = client._connection.api_request(
-            method="GET",
-            path=path,
+        resp = client._get_resource(
+            path,
             query_params=query_params,
             timeout=timeout,
             retry=retry,
+            _target_object=None,
         )
 
         return resp.get("permissions", [])

google/cloud/storage/bucket.py

@@ -786,20 +786,19 @@ def exists(
         try:
             # We intentionally pass `_target_object=None` since fields=name
             # would limit the local properties.
-            client._connection.api_request(
-                method="GET",
-                path=self.path,
+            client._get_resource(
+                self.path,
                 query_params=query_params,
-                _target_object=None,
                 timeout=timeout,
                 retry=retry,
+                _target_object=None,
             )
+        except NotFound:
             # NOTE: This will not fail immediately in a batch. However, when
             #       Batch.finish() is called, the resulting `NotFound` will be
             #       raised.
-            return True
-        except NotFound:
             return False
+        return True
 
     def create(
         self,
@@ -2882,13 +2881,12 @@ def get_iam_policy(
         if requested_policy_version is not None:
             query_params["optionsRequestedPolicyVersion"] = requested_policy_version
 
-        info = client._connection.api_request(
-            method="GET",
-            path="%s/iam" % (self.path,),
+        info = client._get_resource(
+            "%s/iam" % (self.path,),
             query_params=query_params,
-            _target_object=None,
             timeout=timeout,
             retry=retry,
+            _target_object=None,
         )
         return Policy.from_api_repr(info)
 
@@ -3008,12 +3006,12 @@ def test_iam_permissions(
             query_params["userProject"] = self.user_project
 
         path = "%s/iam/testPermissions" % (self.path,)
-        resp = client._connection.api_request(
-            method="GET",
-            path=path,
+        resp = client._get_resource(
+            path,
             query_params=query_params,
             timeout=timeout,
             retry=retry,
+            _target_object=None,
         )
         return resp.get("permissions", [])
 

google/cloud/storage/client.py

@@ -284,10 +284,9 @@ def get_service_account_email(
         """
         if project is None:
             project = self.project
+
         path = "/projects/%s/serviceAccount" % (project,)
-        api_response = self._base_connection.api_request(
-            method="GET", path=path, timeout=timeout, retry=retry,
-        )
+        api_response = self._get_resource(path, timeout=timeout, retry=retry)
         return api_response["email_address"]
 
     def bucket(self, bucket_name, user_project=None):
@@ -321,6 +320,72 @@ def batch(self):
         """
         return Batch(client=self)
 
+    def _get_resource(
+        self,
+        path,
+        query_params=None,
+        headers=None,
+        timeout=_DEFAULT_TIMEOUT,
+        retry=DEFAULT_RETRY,
+        _target_object=None,
+    ):
+        """Helper for bucket / blob methods making API 'GET' calls.
+
+        Args:
+            path str:
+                The path of the resource to fetch.
+
+            query_params Optional[dict]:
+                HTTP query parameters to be passed
+
+            headers Optional[dict]:
+                HTTP headers to be passed
+
+            timeout (Optional[Union[float, Tuple[float, float]]]):
+                The amount of time, in seconds, to wait for the server response.
+
+                Can also be passed as a tuple (connect_timeout, read_timeout).
+                See :meth:`requests.Session.request` documentation for details.
+
+            retry (Optional[Union[google.api_core.retry.Retry, google.cloud.storage.retry.ConditionalRetryPolicy]]):
+                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.
+
+            _target_object (Union[ \
+                :class:`~google.cloud.storage.bucket.Bucket`, \
+                :class:`~google.cloud.storage.bucket.blob`, \
+            ]):
+                Object to which future data is to be applied -- only relevant
+                in the context of a batch.
+
+        Returns:
+            dict
+                The JSON resource fetched
+
+        Raises:
+            google.cloud.exceptions.NotFound
+                If the bucket is not found.
+        """
+        return self._connection.api_request(
+            method="GET",
+            path=path,
+            query_params=query_params,
+            headers=headers,
+            timeout=timeout,
+            retry=retry,
+            _target_object=_target_object,
+        )
+
     def get_bucket(
         self,
         bucket_or_name,

google/cloud/storage/hmac_key.py

@@ -222,12 +222,8 @@ def exists(self, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY):
             if self.user_project is not None:
                 qs_params["userProject"] = self.user_project
 
-            self._client._connection.api_request(
-                method="GET",
-                path=self.path,
-                query_params=qs_params,
-                timeout=timeout,
-                retry=retry,
+            self._client._get_resource(
+                self.path, query_params=qs_params, timeout=timeout, retry=retry,
             )
         except NotFound:
             return False
@@ -266,12 +262,8 @@ def reload(self, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY):
         if self.user_project is not None:
             qs_params["userProject"] = self.user_project
 
-        self._properties = self._client._connection.api_request(
-            method="GET",
-            path=self.path,
-            query_params=qs_params,
-            timeout=timeout,
-            retry=retry,
+        self._properties = self._client._get_resource(
+            self.path, query_params=qs_params, timeout=timeout, retry=retry,
         )
 
     def update(self, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY_IF_ETAG_IN_JSON):

google/cloud/storage/notification.py

@@ -323,12 +323,8 @@ def exists(self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY):
             query_params["userProject"] = self.bucket.user_project
 
         try:
-            client._connection.api_request(
-                method="GET",
-                path=self.path,
-                query_params=query_params,
-                timeout=timeout,
-                retry=retry,
+            client._get_resource(
+                self.path, query_params=query_params, timeout=timeout, retry=retry,
             )
         except NotFound:
             return False
@@ -381,12 +377,8 @@ def reload(self, client=None, timeout=_DEFAULT_TIMEOUT, retry=DEFAULT_RETRY):
         if self.bucket.user_project is not None:
             query_params["userProject"] = self.bucket.user_project
 
-        response = client._connection.api_request(
-            method="GET",
-            path=self.path,
-            query_params=query_params,
-            timeout=timeout,
-            retry=retry,
+        response = client._get_resource(
+            self.path, query_params=query_params, timeout=timeout, retry=retry,
         )
         self._set_properties(response)