Adding optional switch to capture project ID in from_service_account_json(). (#3436)

Fixes #1883.

Author: dhermes

bigtable/google/cloud/bigtable/client.py

@@ -207,6 +207,7 @@ class Client(_ClientFactoryMixin, _ClientProjectMixin):
     _instance_stub_internal = None
     _operations_stub_internal = None
     _table_stub_internal = None
+    _SET_PROJECT = True  # Used by from_service_account_json()
 
     def __init__(self, project=None, credentials=None,
                  read_only=False, admin=False, user_agent=DEFAULT_USER_AGENT):

core/google/cloud/client.py

@@ -14,6 +14,8 @@
 
 """Base classes for client used to interact with Google Cloud APIs."""
 
+import io
+import json
 from pickle import PicklingError
 
 import google.auth.credentials
@@ -40,6 +42,8 @@ class _ClientFactoryMixin(object):
         This class is virtual.
     """
 
+    _SET_PROJECT = False
+
     @classmethod
     def from_service_account_json(cls, json_credentials_path, *args, **kwargs):
         """Factory to retrieve JSON credentials while creating client.
@@ -58,15 +62,21 @@ def from_service_account_json(cls, json_credentials_path, *args, **kwargs):
         :type kwargs: dict
         :param kwargs: Remaining keyword arguments to pass to constructor.
 
-        :rtype: :class:`google.cloud.pubsub.client.Client`
+        :rtype: :class:`_ClientFactoryMixin`
         :returns: The client created with the retrieved JSON credentials.
         :raises: :class:`TypeError` if there is a conflict with the kwargs
                  and the credentials created by the factory.
         """
         if 'credentials' in kwargs:
             raise TypeError('credentials must not be in keyword arguments')
-        credentials = service_account.Credentials.from_service_account_file(
-            json_credentials_path)
+        with io.open(json_credentials_path, 'r', encoding='utf-8') as json_fi:
+            credentials_info = json.load(json_fi)
+        credentials = service_account.Credentials.from_service_account_info(
+            credentials_info)
+        if cls._SET_PROJECT:
+            if 'project' not in kwargs:
+                kwargs['project'] = credentials_info.get('project_id')
+
         kwargs['credentials'] = credentials
         return cls(*args, **kwargs)
 
@@ -207,6 +217,8 @@ class ClientWithProject(Client, _ClientProjectMixin):
              set in the environment.
     """
 
+    _SET_PROJECT = True  # Used by from_service_account_json()
+
     def __init__(self, project=None, credentials=None, _http=None):
         _ClientProjectMixin.__init__(self, project=project)
         Client.__init__(self, credentials=credentials, _http=_http)

core/nox.py

@@ -33,7 +33,8 @@ def unit_tests(session, python_version):
     session.install('-e', '.')
 
     # Run py.test against the unit tests.
-    session.run('py.test', '--quiet',
+    session.run(
+        'py.test', '--quiet',
         '--cov=google.cloud', '--cov=tests.unit', '--cov-append',
         '--cov-config=.coveragerc', '--cov-report=', '--cov-fail-under=97',
         'tests/unit',

core/tests/unit/test_client.py

@@ -12,6 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import io
+import json
 import unittest
 
 import mock
@@ -90,21 +92,32 @@ def test_ctor_bad_credentials(self):
             self._make_one(credentials=CREDENTIALS)
 
     def test_from_service_account_json(self):
-        KLASS = self._get_target_class()
+        from google.cloud import _helpers
+
+        klass = self._get_target_class()
 
+        # Mock both the file opening and the credentials constructor.
+        info = {'dummy': 'value', 'valid': 'json'}
+        json_fi = io.StringIO(_helpers._bytes_to_unicode(json.dumps(info)))
+        file_open_patch = mock.patch(
+            'io.open', return_value=json_fi)
         constructor_patch = mock.patch(
             'google.oauth2.service_account.Credentials.'
-            'from_service_account_file',
+            'from_service_account_info',
             return_value=_make_credentials())
 
-        with constructor_patch as constructor:
-            client_obj = KLASS.from_service_account_json(
-                mock.sentinel.filename)
+        with file_open_patch as file_open:
+            with constructor_patch as constructor:
+                client_obj = klass.from_service_account_json(
+                    mock.sentinel.filename)
 
         self.assertIs(
             client_obj._credentials, constructor.return_value)
         self.assertIsNone(client_obj._http_internal)
-        constructor.assert_called_once_with(mock.sentinel.filename)
+        # Check that mocks were called as expected.
+        file_open.assert_called_once_with(
+            mock.sentinel.filename, 'r', encoding='utf-8')
+        constructor.assert_called_once_with(info)
 
     def test_from_service_account_json_bad_args(self):
         KLASS = self._get_target_class()
@@ -221,3 +234,47 @@ def test_ctor_explicit_bytes(self):
     def test_ctor_explicit_unicode(self):
         PROJECT = u'PROJECT'
         self._explicit_ctor_helper(PROJECT)
+
+    def _from_service_account_json_helper(self, project=None):
+        from google.cloud import _helpers
+
+        klass = self._get_target_class()
+
+        info = {'dummy': 'value', 'valid': 'json'}
+        if project is None:
+            expected_project = 'eye-d-of-project'
+        else:
+            expected_project = project
+
+        info['project_id'] = expected_project
+        # Mock both the file opening and the credentials constructor.
+        json_fi = io.StringIO(_helpers._bytes_to_unicode(json.dumps(info)))
+        file_open_patch = mock.patch(
+            'io.open', return_value=json_fi)
+        constructor_patch = mock.patch(
+            'google.oauth2.service_account.Credentials.'
+            'from_service_account_info',
+            return_value=_make_credentials())
+
+        with file_open_patch as file_open:
+            with constructor_patch as constructor:
+                kwargs = {}
+                if project is not None:
+                    kwargs['project'] = project
+                client_obj = klass.from_service_account_json(
+                    mock.sentinel.filename, **kwargs)
+
+        self.assertIs(
+            client_obj._credentials, constructor.return_value)
+        self.assertIsNone(client_obj._http_internal)
+        self.assertEqual(client_obj.project, expected_project)
+        # Check that mocks were called as expected.
+        file_open.assert_called_once_with(
+            mock.sentinel.filename, 'r', encoding='utf-8')
+        constructor.assert_called_once_with(info)
+
+    def test_from_service_account_json(self):
+        self._from_service_account_json_helper()
+
+    def test_from_service_account_json_project_set(self):
+        self._from_service_account_json_helper(project='prah-jekt')

spanner/google/cloud/spanner/client.py

@@ -102,6 +102,7 @@ class Client(_ClientFactoryMixin, _ClientProjectMixin):
     """
     _instance_admin_api = None
     _database_admin_api = None
+    _SET_PROJECT = True  # Used by from_service_account_json()
 
     def __init__(self, project=None, credentials=None,
                  user_agent=DEFAULT_USER_AGENT):