From e5fb542ac644950f0837fdb38d8192f125125225 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 22 Feb 2016 22:05:58 -0800 Subject: [PATCH] Move conditionally defined ndb helpers into their own module. Towards #433. --- oauth2client/contrib/_appengine_ndb.py | 163 +++++++++++++++++++++++ oauth2client/contrib/appengine.py | 160 +++-------------------- tests/contrib/test__appengine_ndb.py | 172 +++++++++++++++++++++++++ tests/contrib/test_appengine.py | 33 ----- tox.ini | 15 ++- 5 files changed, 366 insertions(+), 177 deletions(-) create mode 100644 oauth2client/contrib/_appengine_ndb.py create mode 100644 tests/contrib/test__appengine_ndb.py diff --git a/oauth2client/contrib/_appengine_ndb.py b/oauth2client/contrib/_appengine_ndb.py new file mode 100644 index 000000000..44c0daca8 --- /dev/null +++ b/oauth2client/contrib/_appengine_ndb.py @@ -0,0 +1,163 @@ +# Copyright 2016 Google Inc. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Google App Engine utilities helper. + +Classes that directly require App Engine's ndb library. Provided +as a separate module in case of failure to import ndb while +other App Engine libraries are present. +""" + +import logging + +from google.appengine.ext import ndb + +from oauth2client import client + + +NDB_KEY = ndb.Key +"""Key constant used by :mod:`oauth2client.contrib.appengine`.""" + +NDB_MODEL = ndb.Model +"""Model constant used by :mod:`oauth2client.contrib.appengine`.""" + +_LOGGER = logging.getLogger(__name__) + + +class SiteXsrfSecretKeyNDB(ndb.Model): + """NDB Model for storage for the sites XSRF secret key. + + Since this model uses the same kind as SiteXsrfSecretKey, it can be + used interchangeably. This simply provides an NDB model for interacting + with the same data the DB model interacts with. + + There should only be one instance stored of this model, the one used + for the site. + """ + secret = ndb.StringProperty() + + @classmethod + def _get_kind(cls): + """Return the kind name for this class.""" + return 'SiteXsrfSecretKey' + + +class FlowNDBProperty(ndb.PickleProperty): + """App Engine NDB datastore Property for Flow. + + Serves the same purpose as the DB FlowProperty, but for NDB models. + Since PickleProperty inherits from BlobProperty, the underlying + representation of the data in the datastore will be the same as in the + DB case. + + Utility property that allows easy storage and retrieval of an + oauth2client.Flow + """ + + def _validate(self, value): + """Validates a value as a proper Flow object. + + Args: + value: A value to be set on the property. + + Raises: + TypeError if the value is not an instance of Flow. + """ + _LOGGER.info('validate: Got type %s', type(value)) + if value is not None and not isinstance(value, client.Flow): + raise TypeError('Property %s must be convertible to a flow ' + 'instance; received: %s.' % (self._name, + value)) + + +class CredentialsNDBProperty(ndb.BlobProperty): + """App Engine NDB datastore Property for Credentials. + + Serves the same purpose as the DB CredentialsProperty, but for NDB + models. Since CredentialsProperty stores data as a blob and this + inherits from BlobProperty, the data in the datastore will be the same + as in the DB case. + + Utility property that allows easy storage and retrieval of Credentials + and subclasses. + """ + + def _validate(self, value): + """Validates a value as a proper credentials object. + + Args: + value: A value to be set on the property. + + Raises: + TypeError if the value is not an instance of Credentials. + """ + _LOGGER.info('validate: Got type %s', type(value)) + if value is not None and not isinstance(value, client.Credentials): + raise TypeError('Property %s must be convertible to a ' + 'credentials instance; received: %s.' % + (self._name, value)) + + def _to_base_type(self, value): + """Converts our validated value to a JSON serialized string. + + Args: + value: A value to be set in the datastore. + + Returns: + A JSON serialized version of the credential, else '' if value + is None. + """ + if value is None: + return '' + else: + return value.to_json() + + def _from_base_type(self, value): + """Converts our stored JSON string back to the desired type. + + Args: + value: A value from the datastore to be converted to the + desired type. + + Returns: + A deserialized Credentials (or subclass) object, else None if + the value can't be parsed. + """ + if not value: + return None + try: + # Uses the from_json method of the implied class of value + credentials = client.Credentials.new_from_json(value) + except ValueError: + credentials = None + return credentials + + +class CredentialsNDBModel(ndb.Model): + """NDB Model for storage of OAuth 2.0 Credentials + + Since this model uses the same kind as CredentialsModel and has a + property which can serialize and deserialize Credentials correctly, it + can be used interchangeably with a CredentialsModel to access, insert + and delete the same entities. This simply provides an NDB model for + interacting with the same data the DB model interacts with. + + Storage of the model is keyed by the user.user_id(). + """ + credentials = CredentialsNDBProperty() + + @classmethod + def _get_kind(cls): + """Return the kind name for this class.""" + return 'CredentialsModel' diff --git a/oauth2client/contrib/appengine.py b/oauth2client/contrib/appengine.py index 2f05254f1..a8b71633b 100644 --- a/oauth2client/contrib/appengine.py +++ b/oauth2client/contrib/appengine.py @@ -46,12 +46,11 @@ from oauth2client.client import Storage from oauth2client.contrib import xsrfutil -# TODO(dhermes): Resolve import issue. # This is a temporary fix for a Google internal issue. try: - from google.appengine.ext import ndb -except ImportError: - ndb = None + from oauth2client.contrib import _appengine_ndb +except ImportError: # pragma: NO COVER + _appengine_ndb = None __author__ = 'jcgregorio@google.com (Joe Gregorio)' @@ -62,6 +61,21 @@ XSRF_MEMCACHE_ID = 'xsrf_secret_key' +if _appengine_ndb is None: + CredentialsNDBModel = None + CredentialsNDBProperty = None + FlowNDBProperty = None + _NDB_KEY = None + _NDB_MODEL = None + SiteXsrfSecretKeyNDB = None +else: + CredentialsNDBModel = _appengine_ndb.CredentialsNDBModel + CredentialsNDBProperty = _appengine_ndb.CredentialsNDBProperty + FlowNDBProperty = _appengine_ndb.FlowNDBProperty + _NDB_KEY = _appengine_ndb.NDB_KEY + _NDB_MODEL = _appengine_ndb.NDB_MODEL + SiteXsrfSecretKeyNDB = _appengine_ndb.SiteXsrfSecretKeyNDB + def _safe_html(s): """Escape text to make it safe to display. @@ -91,24 +105,6 @@ class SiteXsrfSecretKey(db.Model): """ secret = db.StringProperty() -if ndb is not None: - class SiteXsrfSecretKeyNDB(ndb.Model): - """NDB Model for storage for the sites XSRF secret key. - - Since this model uses the same kind as SiteXsrfSecretKey, it can be - used interchangeably. This simply provides an NDB model for interacting - with the same data the DB model interacts with. - - There should only be one instance stored of this model, the one used - for the site. - """ - secret = ndb.StringProperty() - - @classmethod - def _get_kind(cls): - """Return the kind name for this class.""" - return 'SiteXsrfSecretKey' - def _generate_new_xsrf_secret_key(): """Returns a random XSRF secret key.""" @@ -273,35 +269,6 @@ def empty(self, value): return not value -if ndb is not None: - class FlowNDBProperty(ndb.PickleProperty): - """App Engine NDB datastore Property for Flow. - - Serves the same purpose as the DB FlowProperty, but for NDB models. - Since PickleProperty inherits from BlobProperty, the underlying - representation of the data in the datastore will be the same as in the - DB case. - - Utility property that allows easy storage and retrieval of an - oauth2client.Flow - """ - - def _validate(self, value): - """Validates a value as a proper Flow object. - - Args: - value: A value to be set on the property. - - Raises: - TypeError if the value is not an instance of Flow. - """ - logger.info('validate: Got type %s', type(value)) - if value is not None and not isinstance(value, Flow): - raise TypeError('Property %s must be convertible to a flow ' - 'instance; received: %s.' % (self._name, - value)) - - class CredentialsProperty(db.Property): """App Engine datastore Property for Credentials. @@ -346,73 +313,6 @@ def validate(self, value): return value -if ndb is not None: - # TODO(dhermes): Turn this into a JsonProperty and overhaul the Credentials - # and subclass mechanics to use new_from_dict, to_dict, - # from_dict, etc. - class CredentialsNDBProperty(ndb.BlobProperty): - """App Engine NDB datastore Property for Credentials. - - Serves the same purpose as the DB CredentialsProperty, but for NDB - models. Since CredentialsProperty stores data as a blob and this - inherits from BlobProperty, the data in the datastore will be the same - as in the DB case. - - Utility property that allows easy storage and retrieval of Credentials - and subclasses. - """ - - def _validate(self, value): - """Validates a value as a proper credentials object. - - Args: - value: A value to be set on the property. - - Raises: - TypeError if the value is not an instance of Credentials. - """ - logger.info('validate: Got type %s', type(value)) - if value is not None and not isinstance(value, Credentials): - raise TypeError('Property %s must be convertible to a ' - 'credentials instance; received: %s.' % - (self._name, value)) - - def _to_base_type(self, value): - """Converts our validated value to a JSON serialized string. - - Args: - value: A value to be set in the datastore. - - Returns: - A JSON serialized version of the credential, else '' if value - is None. - """ - if value is None: - return '' - else: - return value.to_json() - - def _from_base_type(self, value): - """Converts our stored JSON string back to the desired type. - - Args: - value: A value from the datastore to be converted to the - desired type. - - Returns: - A deserialized Credentials (or subclass) object, else None if - the value can't be parsed. - """ - if not value: - return None - try: - # Uses the from_json method of the implied class of value - credentials = Credentials.new_from_json(value) - except ValueError: - credentials = None - return credentials - - class StorageByKeyName(Storage): """Store and retrieve a credential to and from the App Engine datastore. @@ -460,7 +360,7 @@ def _is_ndb(self): # need worry about new-style classes since ndb and db models are # new-style if isinstance(self._model, type): - if ndb is not None and issubclass(self._model, ndb.Model): + if _NDB_MODEL is not None and issubclass(self._model, _NDB_MODEL): return True elif issubclass(self._model, db.Model): return False @@ -489,7 +389,7 @@ def _delete_entity(self): not the given key is in the datastore. """ if self._is_ndb(): - ndb.Key(self._model, self._key_name).delete() + _NDB_KEY(self._model, self._key_name).delete() else: entity_key = db.Key.from_path(self._model.kind(), self._key_name) db.delete(entity_key) @@ -548,26 +448,6 @@ class CredentialsModel(db.Model): credentials = CredentialsProperty() -if ndb is not None: - class CredentialsNDBModel(ndb.Model): - """NDB Model for storage of OAuth 2.0 Credentials - - Since this model uses the same kind as CredentialsModel and has a - property which can serialize and deserialize Credentials correctly, it - can be used interchangeably with a CredentialsModel to access, insert - and delete the same entities. This simply provides an NDB model for - interacting with the same data the DB model interacts with. - - Storage of the model is keyed by the user.user_id(). - """ - credentials = CredentialsNDBProperty() - - @classmethod - def _get_kind(cls): - """Return the kind name for this class.""" - return 'CredentialsModel' - - def _build_state_value(request_handler, user): """Composes the value for the 'state' parameter. diff --git a/tests/contrib/test__appengine_ndb.py b/tests/contrib/test__appengine_ndb.py new file mode 100644 index 000000000..30b4dc641 --- /dev/null +++ b/tests/contrib/test__appengine_ndb.py @@ -0,0 +1,172 @@ +# Copyright 2016 Google Inc. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import json +import os + +from google.appengine.ext import ndb +from google.appengine.ext import testbed +import mock +import unittest2 + +from oauth2client.client import Credentials +from oauth2client.client import flow_from_clientsecrets +from oauth2client.contrib.appengine import CredentialsNDBProperty +from oauth2client.contrib.appengine import FlowNDBProperty + + +DATA_DIR = os.path.join(os.path.dirname(__file__), '..', 'data') + + +def datafile(filename): + return os.path.join(DATA_DIR, filename) + + +class TestNDBModel(ndb.Model): + flow = FlowNDBProperty() + creds = CredentialsNDBProperty() + + +class TestFlowNDBProperty(unittest2.TestCase): + + def setUp(self): + self.testbed = testbed.Testbed() + self.testbed.activate() + self.testbed.init_datastore_v3_stub() + self.testbed.init_memcache_stub() + + def tearDown(self): + self.testbed.deactivate() + + def test_flow_get_put(self): + instance = TestNDBModel( + flow=flow_from_clientsecrets(datafile('client_secrets.json'), + 'foo', redirect_uri='oob'), + id='foo' + ) + instance.put() + retrieved = TestNDBModel.get_by_id('foo') + + self.assertEqual('foo_client_id', retrieved.flow.client_id) + + @mock.patch('oauth2client.contrib._appengine_ndb._LOGGER') + def test_validate_success(self, mock_logger): + flow_prop = TestNDBModel.flow + flow_val = flow_from_clientsecrets(datafile('client_secrets.json'), + 'foo', redirect_uri='oob') + flow_prop._validate(flow_val) + mock_logger.info.assert_called_once_with('validate: Got type %s', + type(flow_val)) + + @mock.patch('oauth2client.contrib._appengine_ndb._LOGGER') + def test_validate_none(self, mock_logger): + flow_prop = TestNDBModel.flow + flow_val = None + flow_prop._validate(flow_val) + mock_logger.info.assert_called_once_with('validate: Got type %s', + type(flow_val)) + + @mock.patch('oauth2client.contrib._appengine_ndb._LOGGER') + def test_validate_bad_type(self, mock_logger): + flow_prop = TestNDBModel.flow + flow_val = object() + with self.assertRaises(TypeError): + flow_prop._validate(flow_val) + mock_logger.info.assert_called_once_with('validate: Got type %s', + type(flow_val)) + + +class TestCredentialsNDBProperty(unittest2.TestCase): + + def setUp(self): + self.testbed = testbed.Testbed() + self.testbed.activate() + self.testbed.init_datastore_v3_stub() + self.testbed.init_memcache_stub() + + def tearDown(self): + self.testbed.deactivate() + + def test_valid_creds_get_put(self): + creds = Credentials() + instance = TestNDBModel(creds=creds, id='bar') + instance.put() + retrieved = TestNDBModel.get_by_id('bar') + self.assertIsInstance(retrieved.creds, Credentials) + + @mock.patch('oauth2client.contrib._appengine_ndb._LOGGER') + def test_validate_success(self, mock_logger): + creds_prop = TestNDBModel.creds + creds_val = Credentials() + creds_prop._validate(creds_val) + mock_logger.info.assert_called_once_with('validate: Got type %s', + type(creds_val)) + + @mock.patch('oauth2client.contrib._appengine_ndb._LOGGER') + def test_validate_none(self, mock_logger): + creds_prop = TestNDBModel.creds + creds_val = None + creds_prop._validate(creds_val) + mock_logger.info.assert_called_once_with('validate: Got type %s', + type(creds_val)) + + @mock.patch('oauth2client.contrib._appengine_ndb._LOGGER') + def test_validate_bad_type(self, mock_logger): + creds_prop = TestNDBModel.creds + creds_val = object() + with self.assertRaises(TypeError): + creds_prop._validate(creds_val) + mock_logger.info.assert_called_once_with('validate: Got type %s', + type(creds_val)) + + def test__to_base_type_valid_creds(self): + creds_prop = TestNDBModel.creds + creds = Credentials() + creds_json = json.loads(creds_prop._to_base_type(creds)) + self.assertDictEqual(creds_json, { + '_class': 'Credentials', + '_module': 'oauth2client.client', + 'token_expiry': None, + }) + + def test__to_base_type_null_creds(self): + creds_prop = TestNDBModel.creds + self.assertEqual(creds_prop._to_base_type(None), '') + + def test__from_base_type_valid_creds(self): + creds_prop = TestNDBModel.creds + creds_json = json.dumps({ + '_class': 'Credentials', + '_module': 'oauth2client.client', + 'token_expiry': None, + }) + creds = creds_prop._from_base_type(creds_json) + self.assertIsInstance(creds, Credentials) + + def test__from_base_type_false_value(self): + creds_prop = TestNDBModel.creds + self.assertIsNone(creds_prop._from_base_type('')) + self.assertIsNone(creds_prop._from_base_type(False)) + self.assertIsNone(creds_prop._from_base_type(None)) + self.assertIsNone(creds_prop._from_base_type([])) + self.assertIsNone(creds_prop._from_base_type({})) + + def test__from_base_type_bad_json(self): + creds_prop = TestNDBModel.creds + creds_json = '{JK-I-AM-NOT-JSON' + self.assertIsNone(creds_prop._from_base_type(creds_json)) + + +if __name__ == '__main__': # pragma: NO COVER + unittest2.main() diff --git a/tests/contrib/test_appengine.py b/tests/contrib/test_appengine.py index 438548ba8..e9c66ad53 100644 --- a/tests/contrib/test_appengine.py +++ b/tests/contrib/test_appengine.py @@ -12,11 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Discovery document tests - -Unit tests for objects created from discovery documents. -""" - import datetime import httplib2 import json @@ -49,7 +44,6 @@ from oauth2client.contrib.appengine import AppAssertionCredentials from oauth2client.contrib.appengine import CredentialsModel from oauth2client.contrib.appengine import CredentialsNDBModel -from oauth2client.contrib.appengine import FlowNDBProperty from oauth2client.contrib.appengine import FlowProperty from oauth2client.contrib.appengine import OAuth2Decorator from oauth2client.contrib.appengine import OAuth2DecoratorFromClientSecrets @@ -315,33 +309,6 @@ def test_flow_get_put(self): self.assertEqual('foo_client_id', retrieved.flow.client_id) -class TestFlowNDBModel(ndb.Model): - flow = FlowNDBProperty() - - -class FlowNDBPropertyTest(unittest.TestCase): - - def setUp(self): - self.testbed = testbed.Testbed() - self.testbed.activate() - self.testbed.init_datastore_v3_stub() - self.testbed.init_memcache_stub() - - def tearDown(self): - self.testbed.deactivate() - - def test_flow_get_put(self): - instance = TestFlowNDBModel( - flow=flow_from_clientsecrets(datafile('client_secrets.json'), - 'foo', redirect_uri='oob'), - id='foo' - ) - instance.put() - retrieved = TestFlowNDBModel.get_by_id('foo') - - self.assertEqual('foo_client_id', retrieved.flow.client_id) - - def _http_request(*args, **kwargs): resp = httplib2.Response({'status': '200'}) content = json.dumps({'access_token': 'bar'}) diff --git a/tox.ini b/tox.ini index fbff800d7..f05051907 100644 --- a/tox.ini +++ b/tox.ini @@ -16,7 +16,7 @@ deps = {[testenv]basedeps} setenv = pypy: with_gmp=no DJANGO_SETTINGS_MODULE=tests.contrib.test_django_settings -commands = nosetests --ignore-files=test_appengine\.py {posargs} +commands = nosetests --ignore-files=test_appengine\.py --ignore-files=test__appengine_ndb\.py {posargs} [coverbase] basepython = python2.7 @@ -28,18 +28,22 @@ commands = --cover-erase \ --cover-tests \ --cover-branches \ - --ignore-files=test_appengine\.py + --ignore-files=test_appengine\.py \ + --ignore-files=test__appengine_ndb\.py nosetests \ --with-coverage \ --cover-package=oauth2client.contrib.appengine \ + --cover-package=oauth2client.contrib._appengine_ndb \ --cover-package=tests.contrib.test_appengine \ + --cover-package=tests.contrib.test__appengine_ndb \ --with-gae \ --cover-tests \ --cover-branches \ --gae-application=tests/data \ --gae-lib-root={env:GAE_PYTHONPATH:google_appengine} \ --logging-level=INFO \ - tests/contrib/test_appengine.py + tests/contrib/test_appengine.py \ + tests/contrib/test__appengine_ndb.py deps = {[testenv]deps} coverage nosegae @@ -50,6 +54,7 @@ basepython = commands = nosetests \ --ignore-files=test_appengine\.py \ + --ignore-files=test__appengine_ndb\.py \ --ignore-files=test_django_orm\.py \ --ignore-files=test_django_settings\.py \ --ignore-files=test_django_util\.py \ @@ -65,6 +70,7 @@ basepython = commands = nosetests \ --ignore-files=test_appengine\.py \ + --ignore-files=test__appengine_ndb\.py \ --ignore-files=test_django_orm\.py \ --ignore-files=test_django_settings\.py \ --ignore-files=test_django_util\.py \ @@ -114,7 +120,8 @@ commands = --gae-lib-root={env:GAE_PYTHONPATH:google_appengine} \ --gae-application=tests/data \ --logging-level=INFO \ - tests/contrib/test_appengine.py + tests/contrib/test_appengine.py \ + tests/contrib/test__appengine_ndb.py [testenv:system-tests] basepython =