From 9b7249c5b57e41184e9be2b7c4bda3be19953fc9 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Tue, 6 Jun 2017 10:11:59 -0700 Subject: [PATCH] Adding optional switch to capture project ID in from_service_account_json(). (#3436) Fixes #1883. --- bigtable/google/cloud/bigtable/client.py | 1 + core/google/cloud/client.py | 18 +++++-- core/nox.py | 3 +- core/tests/unit/test_client.py | 69 +++++++++++++++++++++--- spanner/google/cloud/spanner/client.py | 1 + 5 files changed, 82 insertions(+), 10 deletions(-) diff --git a/bigtable/google/cloud/bigtable/client.py b/bigtable/google/cloud/bigtable/client.py index 2f552b1c2564e..764a365dacb25 100644 --- a/bigtable/google/cloud/bigtable/client.py +++ b/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): diff --git a/core/google/cloud/client.py b/core/google/cloud/client.py index e3f6f81326ef0..e7e43faf1e452 100644 --- a/core/google/cloud/client.py +++ b/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) diff --git a/core/nox.py b/core/nox.py index 1b9ef352e3a59..d941d60092b81 100644 --- a/core/nox.py +++ b/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', diff --git a/core/tests/unit/test_client.py b/core/tests/unit/test_client.py index 21a8bccc98453..14eac68abee32 100644 --- a/core/tests/unit/test_client.py +++ b/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') diff --git a/spanner/google/cloud/spanner/client.py b/spanner/google/cloud/spanner/client.py index c95e16e2c23c9..875238aed2bc2 100644 --- a/spanner/google/cloud/spanner/client.py +++ b/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):