From 77edad7707c463b3f6feb108d1e55feabc61bdb4 Mon Sep 17 00:00:00 2001 From: Bradley Mclain Date: Tue, 21 Aug 2018 10:43:37 +1000 Subject: [PATCH 01/17] started work on #154 --- Makefile | 13 +++++++++++++ tests/issues/test_issue154.py | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 tests/issues/test_issue154.py diff --git a/Makefile b/Makefile index 1c20009..60672eb 100644 --- a/Makefile +++ b/Makefile @@ -2,6 +2,19 @@ test: python setup.py test +.PHONY: tests-prep +tests-prep: + pip install tox tox-pyenv + pyenv local 2.7.15 3.5.6 3.6.6 pypy2.7-6.0.0 + +.PHONY: tests +tests: + tox + +.PHONY: coverage +coverage: + py.test --cov-report term-missing:skip-covered --cov-config .coveragerc --cov=prestans tests + .PHONY: dist dist: python setup.py sdist bdist_wheel diff --git a/tests/issues/test_issue154.py b/tests/issues/test_issue154.py new file mode 100644 index 0000000..fb7f62d --- /dev/null +++ b/tests/issues/test_issue154.py @@ -0,0 +1,35 @@ +from prestans.http import STATUS +from prestans.rest import RequestHandler + +import pytest +import unittest + + +class NoContentHandler(RequestHandler): + + def get(self): + self.response.status = STATUS.NO_CONTENT + + +@pytest.fixture +def test_app(): + from webtest import TestApp + from prestans.rest import RequestRouter + + api = RequestRouter([ + ('/no-content', NoContentHandler) + ], application_name="api", debug=True) + + return TestApp(app=api) + + +class Issue154(unittest.TestCase): + + def test_204_header_omitted(self): + """ + Request should return no content with header omitted + """ + app = test_app() + resp = app.get('/no-content') + self.assertEqual(resp.status_int, STATUS.NO_CONTENT) + self.assertIsNone(resp.content_type) From 6aee8d1c835c160fb9a8bacf6306d805d8fed03b Mon Sep 17 00:00:00 2001 From: Bradley Mclain Date: Tue, 21 Aug 2018 13:46:31 +1000 Subject: [PATCH 02/17] completes #154 --- prestans/rest/response.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/prestans/rest/response.py b/prestans/rest/response.py index f7eed7c..d8a2e20 100644 --- a/prestans/rest/response.py +++ b/prestans/rest/response.py @@ -140,15 +140,17 @@ def _content_type__get(self): def _content_type__set(self, value): - #: Check to see if response can support the requested mime type - if not isinstance(self._app_iter, BinaryResponse) and value not in self.supported_mime_types: - raise exception.UnsupportedVocabularyError(value, self.supported_mime_types_str) + # skip the mime type check if template is None status code is no content + if self.template is not None or self.status_code != STATUS.NO_CONTENT: + # Check to see if response can support the requested mime type + if not isinstance(self._app_iter, BinaryResponse) and value not in self.supported_mime_types: + raise exception.UnsupportedVocabularyError(value, self.supported_mime_types_str) - #: Keep a reference to the selected serializer - if not isinstance(self._app_iter, BinaryResponse): - self._set_serializer_by_mime_type(value) + # Keep a reference to the selected serializer + if not isinstance(self._app_iter, BinaryResponse): + self._set_serializer_by_mime_type(value) - if not value: + if not value or value is None: self._content_type__del() return if ';' not in value: @@ -241,9 +243,11 @@ def __call__(self, environ, start_response): Overridden WSGI application interface """ - #: prestans' equivalent of webob.Response line 1022 + # prestans equivalent of webob.Response line 1022 if self.template is None or self.status_code == STATUS.NO_CONTENT: + self.content_type = None + start_response(self.status, self.headerlist) if self.template is not None: @@ -325,7 +329,7 @@ def __call__(self, environ, start_response): self.content_type = "text/plain" return [] - #: Content type + # set the content type self.content_type = self._app_iter.mime_type #: Add content disposition header From 5b91cbf8b6a22f2f5ad80d482b53dad0ef8948e3 Mon Sep 17 00:00:00 2001 From: Bradley Mclain Date: Tue, 21 Aug 2018 13:48:29 +1000 Subject: [PATCH 03/17] bumps version number --- prestans/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prestans/__init__.py b/prestans/__init__.py index 758884e..0a60055 100644 --- a/prestans/__init__.py +++ b/prestans/__init__.py @@ -33,5 +33,5 @@ __all__ = ['http', 'types', 'rest', 'parser', 'serializer', 'deserializer', 'provider', 'ext', 'exception'] -__version_info__ = (2, 2, 1) +__version_info__ = (2, 2, 2) __version__ = '.'.join(str(v) for v in __version_info__) From 41712362a57cf66a213e9e63ae083b10f7cdc7ee Mon Sep 17 00:00:00 2001 From: Bradley Mclain Date: Tue, 21 Aug 2018 13:59:41 +1000 Subject: [PATCH 04/17] adds webtest tests requirement --- setup.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 0af19b6..b535317 100644 --- a/setup.py +++ b/setup.py @@ -86,7 +86,8 @@ 'pytest-cov', 'mock', 'tox', - 'tox-pyenv' + 'tox-pyenv', + 'webtest' ], setup_requires=['pytest-runner'], extras_require={ From 153cf5fdb0b4dd3bda9d54336c3227fbdae10886 Mon Sep 17 00:00:00 2001 From: Bradley Mclain Date: Tue, 21 Aug 2018 16:59:13 +1000 Subject: [PATCH 05/17] start work on #84 --- prestans/ext/data/adapters/__init__.py | 28 ++++++++--- tests/issues/test_issue84.py | 69 ++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 8 deletions(-) create mode 100644 tests/issues/test_issue84.py diff --git a/prestans/ext/data/adapters/__init__.py b/prestans/ext/data/adapters/__init__.py index 1527030..5994759 100644 --- a/prestans/ext/data/adapters/__init__.py +++ b/prestans/ext/data/adapters/__init__.py @@ -85,26 +85,38 @@ def register_adapter(self, model_adapter): rest_class_signature = self.generate_signature(model_adapter.rest_model_class) persistent_class_signature = self.generate_signature(model_adapter.persistent_model_class) - # store references to how a rest model maps to a persistent model and vice versa - self._persistent_map[persistent_class_signature] = model_adapter + # store references to rest model self._rest_map[rest_class_signature] = model_adapter + + if persistent_class_signature not in self._persistent_map: + self._persistent_map[persistent_class_signature] = dict() + self._persistent_map[persistent_class_signature][rest_class_signature] = model_adapter - def get_adapter_for_persistent_model(self, persistent_model): + def get_adapter_for_persistent_model(self, persistent_model, rest_model=None): """ :param persistent_model: instance of persistent model :return: the matching model adapter :rtype: ModelAdapter """ - class_signature = self.generate_signature(persistent_model) + persistent_signature = self.generate_signature(persistent_model) - if class_signature not in self._persistent_map: - raise TypeError("No registered Data Adapter for class %s" % class_signature) - - return self._persistent_map[class_signature] + if persistent_signature in self._persistent_map: + sub_map = self._persistent_map[persistent_signature] + + # return the first match if REST model was not specified + if rest_model is None: + return next(iter(sub_map)) + else: + rest_signature = self.generate_signature(persistent_model) + if rest_signature in sub_map: + return self._persistent_map[persistent_signature][rest_signature] + + raise TypeError("No registered Data Adapter for class %s" % persistent_signature) def get_adapter_for_rest_model(self, rest_model): """ :param rest_model: instance of REST model + :param persistent_model: optional persistent model :return: the matching model adapter :rtype: ModelAdapter """ diff --git a/tests/issues/test_issue84.py b/tests/issues/test_issue84.py new file mode 100644 index 0000000..6081b03 --- /dev/null +++ b/tests/issues/test_issue84.py @@ -0,0 +1,69 @@ +from prestans.ext.data import adapters +from prestans.ext.data.adapters import sqlalchemy +from prestans.parser.attribute_filter import AttributeFilter +from prestans import types + +import unittest + + +class UserPersistent(object): + first_name = "first" + last_name = "last" + + +class AddressPersistent(object): + street = "street" + + +class UserREST(types.Model): + first_name = types.String() + last_name = types.String() + + +class AuthorREST(types.Model): + first_name = types.String() + last_name = types.String() + + +class AddressREST(types.Model): + street = types.String() + + +adapters.registry.register_adapter(sqlalchemy.ModelAdapter( + rest_model_class=UserREST, + persistent_model_class=UserPersistent +)) + +adapters.registry.register_adapter(sqlalchemy.ModelAdapter( + rest_model_class=AuthorREST, + persistent_model_class=UserPersistent +)) + + +class Issue84(unittest.TestCase): + def test_correct_adaption(self): + + user = UserPersistent() + user.first_name = "James" + user.last_name = "Hetfield" + + attribute_filter = AttributeFilter.from_model(UserREST(), False) + attribute_filter.first_name = True + + adapted_user = sqlalchemy.adapt_persistent_instance(user, UserREST, attribute_filter) + self.assertEquals(adapted_user.first_name, "James") + self.assertIsNone(adapted_user.last_name) + + def test_incorrect_adaption_raises_exception(self): + address = AddressPersistent() + address.street = "123 Fake Street" + + import logging + for key, value in iter(adapters.registry._persistent_map.items()): + logging.error(key+" -> "+value.rest_model_class.__name__) + + for key, value in iter(adapters.registry._rest_map.items()): + logging.error(key+" -> "+value.persistent_model_class.__name__) + + self.assertRaises(TypeError, sqlalchemy.adapt_persistent_instance, address, UserREST) + From b7f4ae072785b3eb67c8537c5bd5f3c65fbb0ded Mon Sep 17 00:00:00 2001 From: Bradley Mclain Date: Wed, 22 Aug 2018 15:29:04 +1000 Subject: [PATCH 06/17] adds #169, #168, #166, finishes fixing #84 --- prestans/ext/data/adapters/__init__.py | 207 ++++++++++++++- prestans/ext/data/adapters/ndb.py | 156 +----------- prestans/ext/data/adapters/sqlalchemy.py | 173 +------------ .../test_adapter_registry_manager.py | 39 +++ .../adapters/sqlalchemy/test_model_adapter.py | 238 ++++++++++++++++-- tests/issues/test_issue166.py | 56 +++++ tests/issues/test_issue84.py | 45 ++-- 7 files changed, 560 insertions(+), 354 deletions(-) create mode 100644 tests/issues/test_issue166.py diff --git a/prestans/ext/data/adapters/__init__.py b/prestans/ext/data/adapters/__init__.py index 5994759..e4e3e41 100644 --- a/prestans/ext/data/adapters/__init__.py +++ b/prestans/ext/data/adapters/__init__.py @@ -31,14 +31,20 @@ # import inspect -from prestans.types import Model +from prestans import exception +from prestans import parser +from prestans import types class ModelAdapter(object): def __init__(self, rest_model_class, persistent_model_class): + """ + :param rest_model_class: + :param persistent_model_class: + """ - if issubclass(rest_model_class, Model): + if issubclass(rest_model_class, types.Model): self._rest_model_class = rest_model_class else: raise TypeError("rest_model_class must be sub class of prestans.types.Model") @@ -52,9 +58,170 @@ def persistent_model_class(self): @property def rest_model_class(self): return self._rest_model_class - - def adapt_persistent_to_rest(self, persistent_object): - raise NotImplementedError("adapt_persistent_to_rest direct use not allowed") + + def adapt_persistent_to_rest(self, persistent_object, attribute_filter=None): + """ + adapts a persistent model to a rest model by inspecting + """ + + rest_model_instance = self.rest_model_class() + + for attribute_key in rest_model_instance.get_attribute_keys(): + + rest_attr = getattr(self.rest_model_class, attribute_key) + + # don't bother processing if the persistent model doesn't have this attribute + if not hasattr(persistent_object, attribute_key): + + if isinstance(rest_attr, types.Model): + #: If the attribute is a Model, then we set it to None otherwise we get a model + #: with default values, which is invalid when constructing responses + try: + setattr(rest_model_instance, attribute_key, None) + # catch any exception thrown from setattr to give a usable error message + except TypeError as exp: + raise TypeError('Attribute %s, %s' % (attribute_key, str(exp))) + + continue + + # attribute is not visible don't bother processing + elif isinstance(attribute_filter, parser.AttributeFilter) and \ + not attribute_filter.is_attribute_visible(attribute_key): + continue + + # handles prestans array population from SQLAlchemy relationships + elif isinstance(rest_attr, types.Array): + + persistent_attr_value = getattr(persistent_object, attribute_key) + rest_model_array_handle = getattr(rest_model_instance, attribute_key) + + # iterator uses the .append method exposed by prestans arrays to validate + # and populate the collection in the instance. + for collection_element in persistent_attr_value: + if isinstance(rest_attr.element_template, types.Boolean) or \ + isinstance(rest_attr.element_template, types.Float) or \ + isinstance(rest_attr.element_template, types.Integer) or \ + isinstance(rest_attr.element_template, types.String): + rest_model_array_handle.append(collection_element) + else: + element_adapter = registry.get_adapter_for_rest_model(rest_attr.element_template) + + # check if there is a sub model filter + sub_attribute_filter = None + if attribute_filter and attribute_key in attribute_filter: + sub_attribute_filter = getattr(attribute_filter, attribute_key) + + adapted_rest_model = element_adapter.adapt_persistent_to_rest( + collection_element, + sub_attribute_filter + ) + rest_model_array_handle.append(adapted_rest_model) + + elif isinstance(rest_attr, types.Model): + + try: + persistent_attr_value = getattr(persistent_object, attribute_key) + + if persistent_attr_value is None: + adapted_rest_model = None + else: + model_adapter = registry.get_adapter_for_rest_model(rest_attr) + + # check if there is a sub model filter + sub_attribute_filter = None + if isinstance(attribute_filter, parser.AttributeFilter) and \ + attribute_key in attribute_filter: + sub_attribute_filter = getattr(attribute_filter, attribute_key) + + adapted_rest_model = model_adapter.adapt_persistent_to_rest( + persistent_attr_value, + sub_attribute_filter + ) + + setattr(rest_model_instance, attribute_key, adapted_rest_model) + + except TypeError as exp: + raise TypeError('Attribute %s, %s' % (attribute_key, str(exp))) + except exception.DataValidationException as exp: + raise exception.InconsistentPersistentDataError(attribute_key, str(exp)) + + else: + + # otherwise copy the value to the rest model + try: + persistent_attr_value = getattr(persistent_object, attribute_key) + setattr(rest_model_instance, attribute_key, persistent_attr_value) + except TypeError as exp: + raise TypeError('Attribute %s, %s' % (attribute_key, str(exp))) + except exception.ValidationError as exp: + raise exception.InconsistentPersistentDataError(attribute_key, str(exp)) + + return rest_model_instance + + +def adapt_persistent_instance(persistent_object, target_rest_class=None, attribute_filter=None): + """ + Adapts a single persistent instance to a REST model; at present this is a + common method for all persistent backends. + + Refer to: https://groups.google.com/forum/#!topic/prestans-discuss/dO1yx8f60as + for discussion on this feature + """ + + # try and get the adapter and the REST class for the persistent object + if target_rest_class is None: + adapter_instance = registry.get_adapter_for_persistent_model(persistent_object) + else: + if inspect.isclass(target_rest_class): + target_rest_class = target_rest_class() + + adapter_instance = registry.get_adapter_for_persistent_model(persistent_object, target_rest_class) + + # would raise an exception if the attribute_filter differs from the target_rest_class + if attribute_filter is not None and isinstance(attribute_filter, parser.AttributeFilter): + parser.AttributeFilter.from_model(target_rest_class).conforms_to_template_filter(attribute_filter) + + return adapter_instance.adapt_persistent_to_rest(persistent_object, attribute_filter) + + +def adapt_persistent_collection(persistent_collection, target_rest_class=None, attribute_filter=None): + # ensure that collection is iterable and has at least one element + persistent_collection_length = 0 + + # attempt to detect the length of the persistent_collection + if persistent_collection and isinstance(persistent_collection, (list, tuple)): + persistent_collection_length = len(persistent_collection) + # SQLAlchemy query + elif persistent_collection and persistent_collection.__module__ == "sqlalchemy.orm.query": + persistent_collection_length = persistent_collection.count() + # Google App Engine NDB + elif persistent_collection and persistent_collection.__module__ == "google.appengine.ext.ndb": + persistent_collection_length = persistent_collection.count() + + # if the persistent_collection is empty then return a blank array + if persistent_collection_length == 0: + return types.Array(element_template=target_rest_class()) + + # try and get the adapter and the REST class for the persistent object + if target_rest_class is None: + adapter_instance = registry.get_adapter_for_persistent_model( + persistent_collection[0] + ) + else: + if inspect.isclass(target_rest_class): + target_rest_class = target_rest_class() + + adapter_instance = registry.get_adapter_for_persistent_model( + persistent_collection[0], + target_rest_class + ) + + adapted_models = types.Array(element_template=adapter_instance.rest_model_class()) + + for persistent_object in persistent_collection: + adapted_models.append(adapter_instance.adapt_persistent_to_rest(persistent_object, attribute_filter)) + + return adapted_models class AdapterRegistryManager(object): @@ -91,10 +258,28 @@ def register_adapter(self, model_adapter): if persistent_class_signature not in self._persistent_map: self._persistent_map[persistent_class_signature] = dict() self._persistent_map[persistent_class_signature][rest_class_signature] = model_adapter - + + def register_persistent_rest_pair(self, persistent_model_class, rest_model_class): + """ + :param persistent_model_class: + :param rest_model_class: + """ + self.register_adapter(ModelAdapter( + rest_model_class=rest_model_class, + persistent_model_class=persistent_model_class + )) + + def clear_registered_adapters(self): + """ + Clears all of the currently registered model adapters + """ + self._persistent_map.clear() + self._rest_map.clear() + def get_adapter_for_persistent_model(self, persistent_model, rest_model=None): """ :param persistent_model: instance of persistent model + :param rest_model: specific REST model :return: the matching model adapter :rtype: ModelAdapter """ @@ -105,18 +290,18 @@ def get_adapter_for_persistent_model(self, persistent_model, rest_model=None): # return the first match if REST model was not specified if rest_model is None: - return next(iter(sub_map)) + rest_sig = next(iter(sub_map)) + return self._persistent_map[persistent_signature][rest_sig] else: - rest_signature = self.generate_signature(persistent_model) - if rest_signature in sub_map: - return self._persistent_map[persistent_signature][rest_signature] + rest_sig = self.generate_signature(rest_model) + if rest_sig in sub_map: + return self._persistent_map[persistent_signature][rest_sig] raise TypeError("No registered Data Adapter for class %s" % persistent_signature) def get_adapter_for_rest_model(self, rest_model): """ :param rest_model: instance of REST model - :param persistent_model: optional persistent model :return: the matching model adapter :rtype: ModelAdapter """ diff --git a/prestans/ext/data/adapters/ndb.py b/prestans/ext/data/adapters/ndb.py index a2e2bf2..17e4e7e 100644 --- a/prestans/ext/data/adapters/ndb.py +++ b/prestans/ext/data/adapters/ndb.py @@ -31,160 +31,30 @@ # __all__ = ['ModelAdapter'] - -import inspect - -import prestans.types -import prestans.parser - from prestans.ext.data import adapters def adapt_persistent_instance(persistent_object, target_rest_class=None, attribute_filter=None): """ - Adapts a single persistent instance to a REST model; at present this is a - common method for all persistent backends. - - This might be moved to backend specific packages if the need arrises - - Refer to: https://groups.google.com/forum/#!topic/prestans-discuss/dO1yx8f60as - for discussion on this feature + Wrapper on adapters.adapt_persistent_instance for Google App Engine NDB """ - - #: Try and get the adapter and the REST class for the persistent object - if target_rest_class is None: - adapter_instance = adapters.registry.get_adapter_for_persistent_model(persistent_object) - else: - if inspect.isclass(target_rest_class): - target_rest_class = target_rest_class() - - adapter_instance = adapters.registry.get_adapter_for_rest_model(target_rest_class) - - #: Would raise an exception if the attribute_filter differs from the target_rest_class - if attribute_filter is not None and isinstance(attribute_filter, prestans.parser.AttributeFilter): - prestans.parser.AttributeFilter.from_model(target_rest_class).conforms_to_template_filter(attribute_filter) - - return adapter_instance.adapt_persistent_to_rest(persistent_object, attribute_filter) + return adapters.adapt_persistent_instance(persistent_object, target_rest_class, attribute_filter) def adapt_persistent_collection(persistent_collection, target_rest_class=None, attribute_filter=None): - - # ensure that collection is iterable and has at least one element - persistent_collection_length = 0 - - # attempt to reliably detect the length of the persistent_collection - if persistent_collection and isinstance(persistent_collection, (list, tuple)): - persistent_collection_length = len(persistent_collection) - elif persistent_collection: - persistent_collection_length = persistent_collection.count() - - # if the persistent_collection is empty then return a blank array - if persistent_collection_length == 0: - return prestans.types.Array(element_template=target_rest_class()) - - # try and get the adapter and the REST class for the persistent object - if target_rest_class is None: - adapter_instance = adapters.registry.get_adapter_for_persistent_model(persistent_collection[0]) - else: - if inspect.isclass(target_rest_class): - target_rest_class = target_rest_class() - - adapter_instance = adapters.registry.get_adapter_for_rest_model(target_rest_class) - - adapted_models = prestans.types.Array(element_template=adapter_instance.rest_model_class()) - - for persistent_object in persistent_collection: - adapted_models.append(adapter_instance.adapt_persistent_to_rest(persistent_object, attribute_filter)) - - return adapted_models - - -class ModelAdapter(adapters.ModelAdapter): """ - Provides a bridge between REST models and Google Datastore objects + Wrapper on adapters.adapt_persistent_collection for Google App Engine NDB """ + return adapters.adapt_persistent_collection(persistent_collection, target_rest_class, attribute_filter) - def adapt_persistent_to_rest(self, persistent_object, attribute_filter=None): - """ - adapts a persistent model to a rest model by inspecting - - :param self: - :param persistent_object: - :param attribute_filter: - :return: - """ - - rest_model_instance = self.rest_model_class() - - for attribute_key in rest_model_instance.get_attribute_keys(): - - rest_attr = getattr(self.rest_model_class, attribute_key) - - if not hasattr(persistent_object, attribute_key): - - #: Don't bother processing if the persistent model doesn't have this attribute - if issubclass(rest_attr.__class__, prestans.types.Model): - #: If the attribute is a Model, then we set it to None otherwise we get a model - #: with default values, which is invalid when constructing responses - setattr(rest_model_instance, attribute_key, None) - - continue - - #: Attribute not visible don't bother processing - elif isinstance(attribute_filter, prestans.parser.AttributeFilter) and\ - not attribute_filter.is_attribute_visible(attribute_key): - continue - elif isinstance(rest_attr, prestans.types.Array): - - persistent_attr_value = getattr(persistent_object, attribute_key) - rest_model_array_handle = getattr(rest_model_instance, attribute_key) - - #: Iterator uses the .append method exposed by prestans arrays to validate - #: and populate the collection in the instance. - for collection_element in persistent_attr_value: - - if isinstance(rest_attr.element_template, prestans.types.String): - rest_model_array_handle.append(collection_element) - elif isinstance(rest_attr.element_template, prestans.types.Integer): - rest_model_array_handle.append(collection_element) - elif isinstance(rest_attr.element_template, prestans.types.Float): - rest_model_array_handle.append(collection_element) - elif isinstance(rest_attr.element_template, prestans.types.Boolean): - rest_model_array_handle.append(collection_element) - else: - element_adapter = adapters.registry.get_adapter_for_rest_model(rest_attr._element_template) - adapted_rest_model = element_adapter.adapt_persistent_to_rest(collection_element) - rest_model_array_handle.append(adapted_rest_model) - - elif isinstance(rest_attr, prestans.types.Model): - - try: - - persistent_attr_value = getattr(persistent_object, attribute_key) - - if persistent_attr_value is None: - adapted_rest_model = None - else: - model_adapter = adapters.registry.get_adapter_for_rest_model(rest_attr) - adapted_rest_model = model_adapter.adapt_persistent_to_rest(persistent_attr_value) - - setattr(rest_model_instance, attribute_key, adapted_rest_model) - - except TypeError as exp: - raise TypeError('Attribute %s, %s' % (attribute_key, str(exp))) - except prestans.exception.DataValidationException as exp: - raise prestans.exception.InconsistentPersistentDataError(attribute_key, str(exp)) +class ModelAdapter(adapters.ModelAdapter): - else: - - # Default operation is to copy the value and set it, validate will ensure its an acceptable value - try: - persistent_attr_value = getattr(persistent_object, attribute_key) - setattr(rest_model_instance, attribute_key, persistent_attr_value) - except TypeError as exp: - raise TypeError('Attribute %s, %s' % (attribute_key, str(exp))) - except prestans.exception.DataValidationException as exp: - raise prestans.exception.InconsistentPersistentDataError(attribute_key, str(exp)) - - return rest_model_instance + def __init__(self, rest_model_class, persistent_model_class): + import logging + logger = logging.getLogger("prestans") + logger.warn("direct use of %s has been deprecated please use %s instead" % ( + self.__module__ + "." + self.__class__.__name__, + adapters.ModelAdapter.__module__ + "." + adapters.ModelAdapter.__name__ + )) + super(ModelAdapter, self).__init__(rest_model_class, persistent_model_class) diff --git a/prestans/ext/data/adapters/sqlalchemy.py b/prestans/ext/data/adapters/sqlalchemy.py index 470b3ac..4ccfc27 100644 --- a/prestans/ext/data/adapters/sqlalchemy.py +++ b/prestans/ext/data/adapters/sqlalchemy.py @@ -31,177 +31,30 @@ # __all__ = ['ModelAdapter'] -import inspect - -from prestans import exception -from prestans import types -from prestans import parser - from prestans.ext.data import adapters def adapt_persistent_instance(persistent_object, target_rest_class=None, attribute_filter=None): """ - Adapts a single persistent instance to a REST model; at present this is a - common method for all persistent backends. - - This might be moved to backend specific packages if the need arises - - Refer to: https://groups.google.com/forum/#!topic/prestans-discuss/dO1yx8f60as - for discussion on this feature + Wrapper on adapters.adapt_persistent_instance for SQLAlchemy """ - - # try and get the adapter and the REST class for the persistent object - if target_rest_class is None: - adapter_instance = adapters.registry.get_adapter_for_persistent_model(persistent_object) - else: - if inspect.isclass(target_rest_class): - target_rest_class = target_rest_class() - - adapter_instance = adapters.registry.get_adapter_for_rest_model(target_rest_class) - - # would raise an exception if the attribute_filter differs from the target_rest_class - if attribute_filter is not None and isinstance(attribute_filter, parser.AttributeFilter): - parser.AttributeFilter.from_model(target_rest_class).conforms_to_template_filter(attribute_filter) - - return adapter_instance.adapt_persistent_to_rest(persistent_object, attribute_filter) + return adapters.adapt_persistent_instance(persistent_object, target_rest_class, attribute_filter) def adapt_persistent_collection(persistent_collection, target_rest_class=None, attribute_filter=None): - - # ensure that collection is iterable and has at least one element - persistent_collection_length = 0 - - # attempt to reliably detect the length of the persistent_collection - if persistent_collection and isinstance(persistent_collection, (list, tuple)): - persistent_collection_length = len(persistent_collection) - elif persistent_collection and persistent_collection.__module__ == "sqlalchemy.orm.query": - persistent_collection_length = persistent_collection.count() - - # if the persistent_collection is empty then return a blank array - if persistent_collection_length == 0: - return types.Array(element_template=target_rest_class()) - - # try and get the adapter and the REST class for the persistent object - if target_rest_class is None: - adapter_instance = adapters.registry.get_adapter_for_persistent_model(persistent_collection[0]) - else: - if inspect.isclass(target_rest_class): - target_rest_class = target_rest_class() - - adapter_instance = adapters.registry.get_adapter_for_rest_model(target_rest_class) - - adapted_models = types.Array(element_template=adapter_instance.rest_model_class()) - - for persistent_object in persistent_collection: - adapted_models.append(adapter_instance.adapt_persistent_to_rest(persistent_object, attribute_filter)) - - return adapted_models - - -class ModelAdapter(adapters.ModelAdapter): """ - Provide a bridge between REST models and SQLAlchemy objects + Wrapper on adapters.adapt_persistent_collection for SQLAlchemy """ + return adapters.adapt_persistent_collection(persistent_collection, target_rest_class, attribute_filter) - def adapt_persistent_to_rest(self, persistent_object, attribute_filter=None): - """ - adapts a persistent model to a rest model by inspecting - """ - - rest_model_instance = self.rest_model_class() - for attribute_key in rest_model_instance.get_attribute_keys(): - - rest_attr = getattr(self.rest_model_class, attribute_key) - - # don't bother processing if the persistent model doesn't have this attribute - if not hasattr(persistent_object, attribute_key): - - if isinstance(rest_attr, types.Model): - #: If the attribute is a Model, then we set it to None otherwise we get a model - #: with default values, which is invalid when constructing responses - try: - setattr(rest_model_instance, attribute_key, None) - # catch any exception thrown from setattr to give a usable error message - except TypeError as exp: - raise TypeError('Attribute %s, %s' % (attribute_key, str(exp))) - - continue - - # attribute is not visible don't bother processing - elif isinstance(attribute_filter, parser.AttributeFilter) and \ - not attribute_filter.is_attribute_visible(attribute_key): - continue - - # handles prestans array population from SQLAlchemy relationships - elif isinstance(rest_attr, types.Array): - - persistent_attr_value = getattr(persistent_object, attribute_key) - rest_model_array_handle = getattr(rest_model_instance, attribute_key) - - # iterator uses the .append method exposed by prestans arrays to validate - # and populate the collection in the instance. - for collection_element in persistent_attr_value: - if isinstance(rest_attr.element_template, types.String): - rest_model_array_handle.append(collection_element) - elif isinstance(rest_attr.element_template, types.Integer): - rest_model_array_handle.append(collection_element) - elif isinstance(rest_attr.element_template, types.Float): - rest_model_array_handle.append(collection_element) - elif isinstance(rest_attr.element_template, types.Boolean): - rest_model_array_handle.append(collection_element) - else: - element_adapter = adapters.registry.get_adapter_for_rest_model(rest_attr.element_template) - - # check if there is a sub model filter - sub_attribute_filter = None - if attribute_filter and attribute_key in attribute_filter: - sub_attribute_filter = getattr(attribute_filter, attribute_key) - - adapted_rest_model = element_adapter.adapt_persistent_to_rest( - collection_element, - sub_attribute_filter - ) - rest_model_array_handle.append(adapted_rest_model) - - elif isinstance(rest_attr, types.Model): - - try: - persistent_attr_value = getattr(persistent_object, attribute_key) - - if persistent_attr_value is None: - adapted_rest_model = None - else: - model_adapter = adapters.registry.get_adapter_for_rest_model(rest_attr) - - # check if there is a sub model filter - sub_attribute_filter = None - if isinstance(attribute_filter, parser.AttributeFilter) and \ - attribute_key in attribute_filter: - sub_attribute_filter = getattr(attribute_filter, attribute_key) - - adapted_rest_model = model_adapter.adapt_persistent_to_rest( - persistent_attr_value, - sub_attribute_filter - ) - - setattr(rest_model_instance, attribute_key, adapted_rest_model) - - except TypeError as exp: - raise TypeError('Attribute %s, %s' % (attribute_key, str(exp))) - except exception.DataValidationException as exp: - raise exception.InconsistentPersistentDataError(attribute_key, str(exp)) - - else: - - # otherwise copy the value to the rest model - try: - persistent_attr_value = getattr(persistent_object, attribute_key) - setattr(rest_model_instance, attribute_key, persistent_attr_value) - except TypeError as exp: - raise TypeError('Attribute %s, %s' % (attribute_key, str(exp))) - except exception.DataValidationException as exp: - raise exception.InconsistentPersistentDataError(attribute_key, str(exp)) +class ModelAdapter(adapters.ModelAdapter): - return rest_model_instance + def __init__(self, rest_model_class, persistent_model_class): + import logging + logger = logging.getLogger("prestans") + logger.warn("direct use of %s has been deprecated please use %s instead" % ( + self.__module__ + "." + self.__class__.__name__, + adapters.ModelAdapter.__module__ + "." + adapters.ModelAdapter.__name__ + )) + super(ModelAdapter, self).__init__(rest_model_class, persistent_model_class) diff --git a/tests/ext/data/adapters/sqlalchemy/test_adapter_registry_manager.py b/tests/ext/data/adapters/sqlalchemy/test_adapter_registry_manager.py index fff1f05..19fec90 100644 --- a/tests/ext/data/adapters/sqlalchemy/test_adapter_registry_manager.py +++ b/tests/ext/data/adapters/sqlalchemy/test_adapter_registry_manager.py @@ -91,4 +91,43 @@ def test_unknown_adapter_raises_exception(self): self.assertRaises(TypeError, registry_manager.get_adapter_for_rest_model, RESTModelB) self.assertRaises(TypeError, registry_manager.get_adapter_for_persistent_model, PersistentModelB) + def test_register_persistent_rest_pair(self): + registry_manager = adapters.AdapterRegistryManager() + registry_manager.register_persistent_rest_pair( + persistent_model_class=PersistentModelA, + rest_model_class=RESTModelA + ) + + # fetch via the REST model + found_adapter = registry_manager.get_adapter_for_rest_model(RESTModelA()) + self.assertEquals(found_adapter.rest_model_class, RESTModelA) + self.assertEquals(found_adapter.persistent_model_class, PersistentModelA) + + # fetch via the persistent model + found_adapter = registry_manager.get_adapter_for_persistent_model(PersistentModelA()) + self.assertEquals(found_adapter.rest_model_class, RESTModelA) + self.assertEquals(found_adapter.persistent_model_class, PersistentModelA) + + def test_clear_registered_adapters(self): + registry_manager = adapters.AdapterRegistryManager() + registry_manager.register_adapter(adapters.ModelAdapter( + rest_model_class=RESTModelA, + persistent_model_class=PersistentModelA + )) + + # fetch via the REST model + found_adapter = registry_manager.get_adapter_for_rest_model(RESTModelA()) + self.assertEquals(found_adapter.rest_model_class, RESTModelA) + self.assertEquals(found_adapter.persistent_model_class, PersistentModelA) + + # fetch via the persistent model + found_adapter = registry_manager.get_adapter_for_persistent_model(PersistentModelA()) + self.assertEquals(found_adapter.rest_model_class, RESTModelA) + self.assertEquals(found_adapter.persistent_model_class, PersistentModelA) + + # clear the registry + registry_manager.clear_registered_adapters() + # check they have been cleared + self.assertRaises(TypeError, registry_manager.get_adapter_for_rest_model, RESTModelA()) + self.assertRaises(TypeError, registry_manager.get_adapter_for_persistent_model, PersistentModelA()) diff --git a/tests/ext/data/adapters/sqlalchemy/test_model_adapter.py b/tests/ext/data/adapters/sqlalchemy/test_model_adapter.py index 40174aa..1bc39ce 100644 --- a/tests/ext/data/adapters/sqlalchemy/test_model_adapter.py +++ b/tests/ext/data/adapters/sqlalchemy/test_model_adapter.py @@ -1,39 +1,229 @@ import unittest -from prestans.ext.data.adapters import ModelAdapter +from prestans import exception +from prestans.ext.data import adapters +from prestans import parser from prestans import types -class ModelAdapterTest(unittest.TestCase): +class Address(object): + def __init__(self): + self.street = "street" - def test_rest_model_class(self): - class PythonObject(object): - pass +class Person(object): - class RESTModel(types.Model): - name = types.String() + def __init__(self): + self.first_name = "first_name" + self.last_name = "last_name" - model_adapter = ModelAdapter(rest_model_class=RESTModel, persistent_model_class=PythonObject) - self.assertEquals(model_adapter.rest_model_class, RESTModel) - self.assertEquals(model_adapter.persistent_model_class, PythonObject) - self.assertRaises(TypeError, ModelAdapter, rest_model_class=PythonObject, persistent_model_class=None) +class PersonWithAddress(Person): - def test_persistent_model_class(self): - class PythonObject(object): - pass + def __init__(self): + super(PersonWithAddress, self).__init__() + self.address = Address() - class RESTModel(types.Model): - name = types.String() - model_adapter = ModelAdapter(rest_model_class=RESTModel, persistent_model_class=PythonObject) - self.assertEquals(model_adapter.rest_model_class, RESTModel) - self.assertEquals(model_adapter.persistent_model_class, PythonObject) +class PersonWithAddresses(Person): - def test_adapt_persistent_to_rest(self): - class MyModel(types.Model): - name = types.String() + def __init__(self): + super(PersonWithAddresses, self).__init__() + self.addresses = [] - model_adapter = ModelAdapter(MyModel, None) - self.assertRaises(NotImplementedError, model_adapter.adapt_persistent_to_rest, None) + +class PersonWithBasicArrays(Person): + + def __init__(self): + super(PersonWithBasicArrays, self).__init__() + self.booleans = [] + self.floats = [] + self.integers = [] + self.strings = [] + + +class AddressREST(types.Model): + street = types.String() + short_string = types.String(required=False, max_length=10) + + +class PersonREST(types.Model): + first_name = types.String() + last_name = types.String() + short_string = types.String(max_length=10) + + address = AddressREST(required=False) + + addresses = types.Array(element_template=AddressREST()) + + booleans = types.Array(element_template=types.Boolean()) + floats = types.Array(element_template=types.Float()) + integers = types.Array(element_template=types.Integer()) + strings = types.Array(element_template=types.String()) + + +class ModelAdapterUnitTest(unittest.TestCase): + + def setUp(self): + adapters.registry.register_persistent_rest_pair(Address, AddressREST) + + def tearDown(self): + adapters.registry.clear_registered_adapters() + + def test_init_and_getters(self): + + model_adapter = adapters.ModelAdapter(rest_model_class=PersonREST, persistent_model_class=Person) + self.assertEquals(model_adapter.rest_model_class, PersonREST) + self.assertEquals(model_adapter.persistent_model_class, Person) + + def test_init_raises_type_error_for_invalid_rest_model(self): + self.assertRaises(TypeError, adapters.ModelAdapter, rest_model_class=Person, persistent_model_class=None) + + def test_adapt_persistent_to_rest_no_filter(self): + model_adapter = adapters.ModelAdapter(rest_model_class=PersonREST, persistent_model_class=Person) + + person = Person() + person.first_name = "John" + person.last_name = "Doe" + + person_rest = model_adapter.adapt_persistent_to_rest(person) + self.assertEquals(person.first_name, "John") + self.assertEquals(person.last_name, "Doe") + self.assertEquals(person.first_name, person_rest.first_name) + self.assertEquals(person.last_name, person_rest.last_name) + + def test_adapt_persistent_to_rest_with_filter(self): + model_adapter = adapters.ModelAdapter(rest_model_class=PersonREST, persistent_model_class=Person) + + person = Person() + person.first_name = "John" + person.last_name = "Doe" + + attribute_filter = parser.AttributeFilter.from_model(PersonREST(), default_value=False) + attribute_filter.last_name = True + + person_rest = model_adapter.adapt_persistent_to_rest(person, attribute_filter) + self.assertEquals(person.first_name, "John") + self.assertEquals(person.last_name, "Doe") + self.assertIsNone(person_rest.first_name) + self.assertEquals(person.last_name, person_rest.last_name) + + def test_adapt_persistent_to_rest_with_model_as_child(self): + model_adapter = adapters.ModelAdapter(rest_model_class=PersonREST, persistent_model_class=Person) + + person = PersonWithAddress() + person.first_name = "John" + person.last_name = "Doe" + person.address.street = "123 Street Address" + + attribute_filter = parser.AttributeFilter.from_model(PersonREST(), default_value=False) + attribute_filter.last_name = True + attribute_filter.address.street = True + + person_rest = model_adapter.adapt_persistent_to_rest(person, attribute_filter) + self.assertEquals(person.first_name, "John") + self.assertEquals(person.last_name, "Doe") + self.assertEquals(person.address.street, "123 Street Address") + self.assertIsNone(person_rest.first_name) + self.assertEquals(person.last_name, person_rest.last_name) + self.assertEquals(person.address.street, person_rest.address.street) + + def test_adapt_persistent_to_rest_with_model_missing(self): + model_adapter = adapters.ModelAdapter(rest_model_class=PersonREST, persistent_model_class=Person) + + person = PersonWithAddress() + person.first_name = "John" + person.last_name = "Doe" + person.address = None + + person_rest = model_adapter.adapt_persistent_to_rest(person) + self.assertEquals(person.first_name, "John") + self.assertEquals(person.last_name, "Doe") + self.assertEquals(person.first_name, person_rest.first_name) + self.assertEquals(person.last_name, person_rest.last_name) + + def test_adapt_persistent_to_rest_with_basic_arrays_as_children(self): + model_adapter = adapters.ModelAdapter(rest_model_class=PersonREST, persistent_model_class=PersonWithBasicArrays) + + person = PersonWithBasicArrays() + person.first_name = "John" + person.last_name = "Doe" + person.booleans.append(True) + person.floats.append(1.1) + person.integers.append(2) + person.strings.append("string") + + person_rest = model_adapter.adapt_persistent_to_rest(person) + self.assertEquals(person.first_name, "John") + self.assertEquals(person.last_name, "Doe") + self.assertEquals(person.booleans, [True]) + self.assertEquals(person.floats, [1.1]) + self.assertEquals(person.integers, [2]) + self.assertEquals(person.strings, ["string"]) + + self.assertEquals(person.first_name, person_rest.first_name) + self.assertEquals(person.last_name, person_rest.last_name) + self.assertEquals(person.booleans, person_rest.booleans.as_serializable()) + self.assertEquals(person.floats, person_rest.floats.as_serializable()) + self.assertEquals(person.integers, person_rest.integers.as_serializable()) + self.assertEquals(person.strings, person_rest.strings.as_serializable()) + + def test_adapt_persistent_to_rest_with_model_array_as_child(self): + model_adapter = adapters.ModelAdapter(rest_model_class=PersonREST, persistent_model_class=PersonWithAddresses) + + address = Address() + address.street = "123 Street Address" + + person = PersonWithAddresses() + person.first_name = "John" + person.last_name = "Doe" + person.addresses.append(address) + + person_rest = model_adapter.adapt_persistent_to_rest(person) + self.assertEquals(person.first_name, "John") + self.assertEquals(person.last_name, "Doe") + self.assertEquals(person.addresses, [address]) + + self.assertEquals(person.first_name, person_rest.first_name) + self.assertEquals(person.last_name, person_rest.last_name) + self.assertEquals([{"short_string": None, "street": "123 Street Address"}], person_rest.addresses.as_serializable()) + + def test_adapt_persistent_to_rest_with_model_array_as_child_filtered(self): + model_adapter = adapters.ModelAdapter(rest_model_class=PersonREST, persistent_model_class=PersonWithAddresses) + + address = Address() + address.street = "123 Street Address" + + person = PersonWithAddresses() + person.first_name = "John" + person.last_name = "Doe" + person.addresses.append(address) + + attribute_filter = parser.AttributeFilter.from_model(PersonREST(), default_value=False) + attribute_filter.last_name = True + attribute_filter.addresses.street = True + + person_rest = model_adapter.adapt_persistent_to_rest(person, attribute_filter) + self.assertEquals(person.first_name, "John") + self.assertEquals(person.last_name, "Doe") + self.assertEquals(person.addresses, [address]) + + self.assertIsNone(person_rest.first_name) + self.assertEquals(person.last_name, person_rest.last_name) + self.assertEquals([{"short_string": None, "street": "123 Street Address"}], person_rest.addresses.as_serializable()) + + def test_adapt_persistent_to_rest_inconsistent_data_exception_raised_model(self): + model_adapter = adapters.ModelAdapter(rest_model_class=PersonREST, persistent_model_class=Person) + + person = PersonWithAddress() + person.address.short_string = "a longer string" + + self.assertRaises(exception.InconsistentPersistentDataError, model_adapter.adapt_persistent_to_rest, person) + + def test_adapt_persistent_to_rest_inconsistent_data_exception_raised_array(self): + model_adapter = adapters.ModelAdapter(rest_model_class=PersonREST, persistent_model_class=Person) + + person = Person() + person.short_string = "a longer string" + + self.assertRaises(exception.InconsistentPersistentDataError, model_adapter.adapt_persistent_to_rest, person) diff --git a/tests/issues/test_issue166.py b/tests/issues/test_issue166.py new file mode 100644 index 0000000..8866af0 --- /dev/null +++ b/tests/issues/test_issue166.py @@ -0,0 +1,56 @@ +from prestans.ext.data import adapters +from prestans.ext.data.adapters import sqlalchemy +from prestans import types + +import unittest + + +class UserPersistent(object): + first_name = "first" + last_name = "last" + + +class UserREST(types.Model): + first_name = types.String() + last_name = types.String() + + +class AuthorREST(types.Model): + first_name = types.String() + last_name = types.String() + + +adapters.registry.register_adapter(sqlalchemy.ModelAdapter( + rest_model_class=UserREST, + persistent_model_class=UserPersistent +)) + +adapters.registry.register_adapter(sqlalchemy.ModelAdapter( + rest_model_class=AuthorREST, + persistent_model_class=UserPersistent +)) + + +class Issue166(unittest.TestCase): + + def setUp(self): + adapters.registry.register_persistent_rest_pair(UserPersistent, UserREST) + adapters.registry.register_persistent_rest_pair(UserPersistent, AuthorREST) + + def tearDown(self): + adapters.registry.clear_registered_adapters() + + def test_default_rest_model_lookup(self): + + model_adapter = adapters.registry.get_adapter_for_persistent_model(UserPersistent()) + self.assertEquals(model_adapter.rest_model_class, UserREST) + self.assertNotEquals(model_adapter.rest_model_class, AuthorREST) + + def test_specific_rest_model_lookup(self): + model_adapter1 = adapters.registry.get_adapter_for_persistent_model(UserPersistent(), UserREST) + self.assertEquals(model_adapter1.rest_model_class, UserREST) + self.assertNotEquals(model_adapter1.rest_model_class, AuthorREST) + + model_adapter2 = adapters.registry.get_adapter_for_persistent_model(UserPersistent(), AuthorREST) + self.assertEquals(model_adapter2.rest_model_class, AuthorREST) + self.assertNotEquals(model_adapter2.rest_model_class, UserREST) diff --git a/tests/issues/test_issue84.py b/tests/issues/test_issue84.py index 6081b03..197c5d0 100644 --- a/tests/issues/test_issue84.py +++ b/tests/issues/test_issue84.py @@ -29,19 +29,31 @@ class AddressREST(types.Model): street = types.String() -adapters.registry.register_adapter(sqlalchemy.ModelAdapter( - rest_model_class=UserREST, - persistent_model_class=UserPersistent -)) +class Issue84(unittest.TestCase): -adapters.registry.register_adapter(sqlalchemy.ModelAdapter( - rest_model_class=AuthorREST, - persistent_model_class=UserPersistent -)) + def setUp(self): + adapters.registry.register_persistent_rest_pair(UserPersistent, UserREST) + adapters.registry.register_persistent_rest_pair(UserPersistent, AuthorREST) + def tearDown(self): + adapters.registry.clear_registered_adapters() -class Issue84(unittest.TestCase): - def test_correct_adaption(self): + def test_correct_adaption_collection(self): + + user = UserPersistent() + user.first_name = "James" + user.last_name = "Hetfield" + + users = [user] + + attribute_filter = AttributeFilter.from_model(UserREST(), False) + attribute_filter.first_name = True + + adapted_users = sqlalchemy.adapt_persistent_collection(users, UserREST, attribute_filter) + self.assertEquals(adapted_users[0].first_name, "James") + self.assertIsNone(adapted_users[0].last_name) + + def test_correct_adaption_instance(self): user = UserPersistent() user.first_name = "James" @@ -54,16 +66,17 @@ def test_correct_adaption(self): self.assertEquals(adapted_user.first_name, "James") self.assertIsNone(adapted_user.last_name) - def test_incorrect_adaption_raises_exception(self): + def test_incorrect_adaption_raises_exception_collection(self): address = AddressPersistent() address.street = "123 Fake Street" - import logging - for key, value in iter(adapters.registry._persistent_map.items()): - logging.error(key+" -> "+value.rest_model_class.__name__) + addresses = [address] - for key, value in iter(adapters.registry._rest_map.items()): - logging.error(key+" -> "+value.persistent_model_class.__name__) + self.assertRaises(TypeError, sqlalchemy.adapt_persistent_collection, addresses, UserREST) + + def test_incorrect_adaption_raises_exception_instance(self): + address = AddressPersistent() + address.street = "123 Fake Street" self.assertRaises(TypeError, sqlalchemy.adapt_persistent_instance, address, UserREST) From 7d5ad944564d58a5345cc796c123322c167043ad Mon Sep 17 00:00:00 2001 From: Bradley Mclain Date: Wed, 22 Aug 2018 15:31:31 +1000 Subject: [PATCH 07/17] bumps version --- prestans/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prestans/__init__.py b/prestans/__init__.py index 0a60055..3903611 100644 --- a/prestans/__init__.py +++ b/prestans/__init__.py @@ -33,5 +33,5 @@ __all__ = ['http', 'types', 'rest', 'parser', 'serializer', 'deserializer', 'provider', 'ext', 'exception'] -__version_info__ = (2, 2, 2) +__version_info__ = (2, 3, 0) __version__ = '.'.join(str(v) for v in __version_info__) From a93bfb6ca0825badbf9e9cc8682c7da17df800f5 Mon Sep 17 00:00:00 2001 From: Bradley Mclain Date: Wed, 22 Aug 2018 15:55:47 +1000 Subject: [PATCH 08/17] changed default REST adapter lookup to keep explicit reference to last added for backwards compatibility #170 --- prestans/ext/data/adapters/__init__.py | 12 ++++++++---- tests/issues/test_issue166.py | 4 ++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/prestans/ext/data/adapters/__init__.py b/prestans/ext/data/adapters/__init__.py index e4e3e41..1fac885 100644 --- a/prestans/ext/data/adapters/__init__.py +++ b/prestans/ext/data/adapters/__init__.py @@ -228,9 +228,10 @@ class AdapterRegistryManager(object): """ AdapterRegistryManager keeps track of rest to persistent model maps - AdapterRegistryManager should not be instantiated by the applications, a singleton - instance supplied by this package. + New AdapterRegistryManager's should not be instantiated by the application, a singleton + instance is supplied by this package. """ + DEFAULT_REST_ADAPTER = "prestans_rest_default_adapter" def __init__(self): self._persistent_map = dict() @@ -257,6 +258,10 @@ def register_adapter(self, model_adapter): if persistent_class_signature not in self._persistent_map: self._persistent_map[persistent_class_signature] = dict() + + # store a reference to the adapter under both REST signature and default key + # the default is always the last registered model (to match behaviour before this was patched) + self._persistent_map[persistent_class_signature][self.DEFAULT_REST_ADAPTER] = model_adapter self._persistent_map[persistent_class_signature][rest_class_signature] = model_adapter def register_persistent_rest_pair(self, persistent_model_class, rest_model_class): @@ -290,8 +295,7 @@ def get_adapter_for_persistent_model(self, persistent_model, rest_model=None): # return the first match if REST model was not specified if rest_model is None: - rest_sig = next(iter(sub_map)) - return self._persistent_map[persistent_signature][rest_sig] + return self._persistent_map[persistent_signature][self.DEFAULT_REST_ADAPTER] else: rest_sig = self.generate_signature(rest_model) if rest_sig in sub_map: diff --git a/tests/issues/test_issue166.py b/tests/issues/test_issue166.py index 8866af0..a923332 100644 --- a/tests/issues/test_issue166.py +++ b/tests/issues/test_issue166.py @@ -43,8 +43,8 @@ def tearDown(self): def test_default_rest_model_lookup(self): model_adapter = adapters.registry.get_adapter_for_persistent_model(UserPersistent()) - self.assertEquals(model_adapter.rest_model_class, UserREST) - self.assertNotEquals(model_adapter.rest_model_class, AuthorREST) + self.assertEquals(model_adapter.rest_model_class, AuthorREST) + self.assertNotEquals(model_adapter.rest_model_class, UserREST) def test_specific_rest_model_lookup(self): model_adapter1 = adapters.registry.get_adapter_for_persistent_model(UserPersistent(), UserREST) From 5b72d0b224ab4e2743f833638e3c439a6d35005f Mon Sep 17 00:00:00 2001 From: Bradley Mclain Date: Thu, 23 Aug 2018 11:09:42 +1000 Subject: [PATCH 09/17] implements #155 --- prestans/rest/request.py | 21 ++++--- setup.py | 3 +- tests/issues/test_issue155.py | 109 ++++++++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+), 9 deletions(-) create mode 100644 tests/issues/test_issue155.py diff --git a/prestans/rest/request.py b/prestans/rest/request.py index 32190c3..98b6e09 100644 --- a/prestans/rest/request.py +++ b/prestans/rest/request.py @@ -47,6 +47,8 @@ def parsed_body(self): raise AttributeError("access to request.parsed_body is not \ allowed when body_template is set to None") + self.parse_body() + return self._parsed_body @property @@ -122,20 +124,23 @@ def body_template(self, value): return if not isinstance(value, DataCollection): - raise AssertionError("body_template must be an instance of \ - prestans.types.DataCollection") + msg = "body_template must be an instance of %s.%s" % (DataCollection.__module__, DataCollection.__name__) + raise AssertionError(msg) self._body_template = value - #: Get a deserializer based on the Content-Type header - #: Do this here so the handler gets a chance to setup extra serializers + # get a deserializer based on the Content-Type header + # do this here so the handler gets a chance to setup extra serializers self.set_deserializer_by_mime_type(self.content_type) - #: Parse the body using the deserializer - unserialized_body = self.selected_deserializer.loads(self.body) + def parse_body(self): + + if self._parsed_body is None and self._body_template is not None: + # parse the body using the deserializer + unserialized_body = self.selected_deserializer.loads(self.body) - #: Parse the body using the template and attribute_filter - self._parsed_body = value.validate(unserialized_body, self.attribute_filter, self.is_minified) + # valiate the body using the template and attribute_filter + self._parsed_body = self._body_template.validate(unserialized_body, self.attribute_filter, self.is_minified) def register_deserializers(self, deserializers): diff --git a/setup.py b/setup.py index 0af19b6..b535317 100644 --- a/setup.py +++ b/setup.py @@ -86,7 +86,8 @@ 'pytest-cov', 'mock', 'tox', - 'tox-pyenv' + 'tox-pyenv', + 'webtest' ], setup_requires=['pytest-runner'], extras_require={ diff --git a/tests/issues/test_issue155.py b/tests/issues/test_issue155.py new file mode 100644 index 0000000..3e8c530 --- /dev/null +++ b/tests/issues/test_issue155.py @@ -0,0 +1,109 @@ +from prestans.http import STATUS +from prestans import parser +from prestans.provider import auth +from prestans import rest +from prestans import types + +import json +from mock import patch, PropertyMock +import pytest +import unittest + + +class PersonREST(types.Model): + first_name = types.String() + last_name = types.String() + + +BODY = { + "first_name": "Jane", + "last_name": "Doe" +} + + +class AuthContextProvider(auth.Base): + + def current_user_has_role(self, role_name): + return False + + def get_current_user(self): + return None + + def is_authorized_user(self, config): + return False + + def is_authenticated_user(self): + return False + + +class AuthHandler(rest.RequestHandler): + + def handler_will_run(self): + self.__provider_config__.authentication = AuthContextProvider() + + __parser_config__ = parser.Config( + POST=parser.VerbConfig( + body_template=PersonREST(), + response_attribute_filter_default_value=True, + response_template=PersonREST() + ) + ) + + @auth.login_required + def post(self): + self.response.status = STATUS.NO_CONTENT + + +class NoAuthHandler(rest.RequestHandler): + + __parser_config__ = parser.Config( + POST=parser.VerbConfig( + body_template=PersonREST(), + response_attribute_filter_default_value=True, + response_template=PersonREST() + ) + ) + + def post(self): + person = self.request.parsed_body + + self.response.status = STATUS.OK + self.response.body = person + + +@pytest.fixture +def test_app(): + from webtest import TestApp + from prestans.rest import RequestRouter + + api = RequestRouter([ + ('/auth', AuthHandler), + ('/no-auth', NoAuthHandler) + ], application_name="api", debug=True) + + return TestApp(app=api) + + +class Issue155(unittest.TestCase): + + def test_no_auth_parses_body(self): + """ + """ + app = test_app() + resp = app.post_json(url="/no-auth", params=BODY, status="*") + resp_body = json.loads(resp.body if isinstance(resp.body, str) else resp.body.decode()) + + self.assertEquals(resp.status_int, STATUS.OK) + self.assertEquals(resp_body["first_name"], BODY["first_name"]) + self.assertEquals(resp_body["last_name"], BODY["last_name"]) + + @patch("prestans.rest.request.Request.parsed_body", new_callable=PropertyMock) + @patch("prestans.rest.request.Request.parse_body") + def test_unauthenticated_ignores_body(self, parsed_body, parse_body): + """ + """ + app = test_app() + resp = app.post_json(url="/auth", params=BODY, status="*") + self.assertEqual(resp.status_int, STATUS.UNAUTHORIZED) + parsed_body.assert_not_called() + parse_body.assert_not_called() From 5a294aef4330eb17311dea6b4226d861889d29b5 Mon Sep 17 00:00:00 2001 From: Bradley Mclain Date: Thu, 23 Aug 2018 13:51:30 +1000 Subject: [PATCH 10/17] implements #102 --- prestans/exception.py | 21 +++++++++++++++ prestans/rest/request_handler.py | 14 ++++++++-- tests/issues/test_issue102.py | 46 ++++++++++++++++++++++++++++++++ tox.ini | 1 + 4 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 tests/issues/test_issue102.py diff --git a/prestans/exception.py b/prestans/exception.py index f2c705b..af9c142 100644 --- a/prestans/exception.py +++ b/prestans/exception.py @@ -495,6 +495,27 @@ def __init__(self, message="Moved Permanently", response_model=None): super(MovedPermanently, self).__init__(_code, message, response_model) +class Redirect(ResponseException): + + def __init__(self, code, url): + self._url = url + super(Redirect, self).__init__(code, "Redirect") + + @property + def url(self): + return self._url + + +class TemporaryRedirect(Redirect): + def __init__(self, url): + super(TemporaryRedirect, self).__init__(STATUS.TEMPORARY_REDIRECT, url) + + +class PermanentRedirect(Redirect): + def __init__(self, url): + super(PermanentRedirect, self).__init__(STATUS.PERMANENT_REDIRECT, url) + + class PaymentRequired(ResponseException): def __init__(self, message="Payment Required", response_model=None): diff --git a/prestans/rest/request_handler.py b/prestans/rest/request_handler.py index cbf5df9..0fbc0e0 100644 --- a/prestans/rest/request_handler.py +++ b/prestans/rest/request_handler.py @@ -242,6 +242,8 @@ def __call__(self, environ, start_response): self.delete(*self._args) elif request_method == VERB.OPTIONS: self.options(*self._args) + except (exception.PermanentRedirect, exception.TemporaryRedirect) as exp: + self._redirect(exp.url, exp.http_status) # re-raise all prestans exceptions except exception.Base as exp: if isinstance(exception, exception.HandlerException): @@ -331,7 +333,15 @@ def options(self, *args): unimplemented_verb_error.request = self.request raise unimplemented_verb_error - def redirect(self, url, status=STATUS.TEMPORARY_REDIRECT): - + def _redirect(self, url, status=STATUS.TEMPORARY_REDIRECT): self._response.status = status self._response.headers.add("Location", url) + + def redirect(self, url, status=STATUS.TEMPORARY_REDIRECT): + + self.logger.warn("direct use of %s.%s has been deprecated please raise an exception instead" % ( + self.__module__, + "redirect" + )) + + self._redirect(url, status) diff --git a/tests/issues/test_issue102.py b/tests/issues/test_issue102.py new file mode 100644 index 0000000..b08fb1e --- /dev/null +++ b/tests/issues/test_issue102.py @@ -0,0 +1,46 @@ +from prestans import exception +from prestans.http import STATUS +from prestans import rest + +import pytest +import unittest + + +class PermanentRedirectHandler(rest.RequestHandler): + + def get(self): + raise exception.PermanentRedirect("/permanent-new") + + +class TemporaryRedirectHandler(rest.RequestHandler): + + def get(self): + raise exception.TemporaryRedirect("/temporary-new") + + +@pytest.fixture +def test_app(): + from webtest import TestApp + from prestans.rest import RequestRouter + + api = RequestRouter([ + ('/permanent', PermanentRedirectHandler), + ('/temporary', TemporaryRedirectHandler) + ], application_name="api", debug=True) + + return TestApp(app=api) + + +class Issue155(unittest.TestCase): + + def test_permanent_redirect(self): + app = test_app() + + resp = app.get("/permanent") + self.assertEquals(resp.status_int, STATUS.PERMANENT_REDIRECT) + + def test_temporary_redirect(self): + app = test_app() + + resp = app.get("/temporary") + self.assertEquals(resp.status_int, STATUS.TEMPORARY_REDIRECT) diff --git a/tox.ini b/tox.ini index 9fe8ad1..67be850 100644 --- a/tox.ini +++ b/tox.ini @@ -8,5 +8,6 @@ envlist = deps = jinja2 mock pytest + webtest commands = py.test tests \ No newline at end of file From 84dfca403ecbcb1771dbd51d6bf22067c73b1e68 Mon Sep 17 00:00:00 2001 From: Bradley Mclain Date: Thu, 23 Aug 2018 15:03:20 +1000 Subject: [PATCH 11/17] fixes #125 --- prestans/ext/data/adapters/__init__.py | 6 +++- tests/issues/test_issue125.py | 42 ++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 tests/issues/test_issue125.py diff --git a/prestans/ext/data/adapters/__init__.py b/prestans/ext/data/adapters/__init__.py index 1fac885..d868f55 100644 --- a/prestans/ext/data/adapters/__init__.py +++ b/prestans/ext/data/adapters/__init__.py @@ -83,7 +83,11 @@ def adapt_persistent_to_rest(self, persistent_object, attribute_filter=None): raise TypeError('Attribute %s, %s' % (attribute_key, str(exp))) continue - + # ignore class methods + elif inspect.ismethod(getattr(persistent_object, attribute_key)): + import logging + logging.error("ignoring method: "+attribute_key) + continue # attribute is not visible don't bother processing elif isinstance(attribute_filter, parser.AttributeFilter) and \ not attribute_filter.is_attribute_visible(attribute_key): diff --git a/tests/issues/test_issue125.py b/tests/issues/test_issue125.py new file mode 100644 index 0000000..b709052 --- /dev/null +++ b/tests/issues/test_issue125.py @@ -0,0 +1,42 @@ +from prestans.ext.data import adapters +from prestans import types + +from mock import patch +import unittest + + +class Person(object): + def __init__(self): + self.first_name = "John" + + @property + def last_name(self): + return "Doe" + + @classmethod + def class_method(cls): + return "class_method" + + +class PersonREST(types.Model): + first_name = types.String() + last_name = types.String() + class_method = types.String() + + +class Issue125(unittest.TestCase): + + def setUp(self): + adapters.registry.register_persistent_rest_pair(Person, PersonREST) + + def tearDown(self): + adapters.registry.clear_registered_adapters() + + def test_class_method_not_called(self): + + person = Person() + + person_rest = adapters.adapt_persistent_instance(person, PersonREST) + self.assertEquals(person_rest.first_name, person.first_name) + self.assertEquals(person_rest.last_name, person.last_name) + self.assertIsNone(person_rest.class_method) From 5ad50178c07794bd9dcaf18a805e91b0d4dff070 Mon Sep 17 00:00:00 2001 From: Bradley Mclain Date: Fri, 24 Aug 2018 15:37:13 +1000 Subject: [PATCH 12/17] fix for #172 to work with other changes in this release --- prestans/rest/response.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/prestans/rest/response.py b/prestans/rest/response.py index d8a2e20..e07228b 100644 --- a/prestans/rest/response.py +++ b/prestans/rest/response.py @@ -141,7 +141,8 @@ def _content_type__get(self): def _content_type__set(self, value): # skip the mime type check if template is None status code is no content - if self.template is not None or self.status_code != STATUS.NO_CONTENT: + if self.template is not None or \ + self.status_code not in [STATUS.NO_CONTENT, STATUS.PERMANENT_REDIRECT, STATUS.TEMPORARY_REDIRECT]: # Check to see if response can support the requested mime type if not isinstance(self._app_iter, BinaryResponse) and value not in self.supported_mime_types: raise exception.UnsupportedVocabularyError(value, self.supported_mime_types_str) From 6de48f2725dd2400d0dbac08ba3d2419686fc150 Mon Sep 17 00:00:00 2001 From: Bradley Mclain Date: Mon, 27 Aug 2018 16:01:05 +1000 Subject: [PATCH 13/17] cleans up content type setting and serializer determination --- prestans/rest/request_handler.py | 29 ++++++------- prestans/rest/response.py | 70 +++++++++++++++---------------- tests/rest/test_request_router.py | 12 +++--- 3 files changed, 52 insertions(+), 59 deletions(-) diff --git a/prestans/rest/request_handler.py b/prestans/rest/request_handler.py index 0fbc0e0..f759a61 100644 --- a/prestans/rest/request_handler.py +++ b/prestans/rest/request_handler.py @@ -110,37 +110,32 @@ def blueprint(self): return handler_blueprint def _setup_serializers(self): + """ + Auto set the return serializer based on Accept headers + http://docs.webob.org/en/latest/reference.html#header-getters - #: - #: Auto set the return serializer based on Accept headers - #: http://docs.webob.org/en/latest/reference.html#header-getters - #: - - #: Intersection of requested types and supported types tells us if we - #: can in fact respond in one of the request formats + Intersection of requested types and supported types tells us if we + can in fact respond in one of the request formats + """ best_accept_match = self.request.accept.best_match( self.response.supported_mime_types, default_match=self.response.default_serializer.content_type() ) - if best_accept_match is None: - self.logger.error("unsupported mime type in request; accept header reads %s" % \ - self.request.accept) - raise exception.UnsupportedVocabularyError( - self.request.accept, - self.response.supported_mime_types_str - ) + self.logger.info("%s determined as best match for accept header: %s" % ( + best_accept_match, + self.request.accept + )) - #: If content_type is not acceptable it will raise UnsupportedVocabulary + # if content_type is not acceptable it will raise UnsupportedVocabulary self.response.content_type = best_accept_match def __call__(self, environ, start_response): self.logger.info("handler %s.%s; callable execution start" % (self.__module__, self.__class__.__name__)) - self.logger.info("setting default response to %s" % self.request.accept) try: - #: Register additional serializers and de-serializers + # register additional serializers and de-serializers self.request.register_deserializers(self.register_deserializers()) self.response.register_serializers(self.register_serializers()) diff --git a/prestans/rest/response.py b/prestans/rest/response.py index e07228b..603bef5 100644 --- a/prestans/rest/response.py +++ b/prestans/rest/response.py @@ -75,23 +75,33 @@ def selected_serializer(self): def default_serializer(self): return self._default_serializer - #: Used by content_type_set to set get a reference to the serializer object def _set_serializer_by_mime_type(self, mime_type): + """ + :param mime_type: + :return: + + used by content_type_set to set get a reference to the appropriate serializer + """ + + # ignore if binary response + if isinstance(self._app_iter, BinaryResponse): + self.logger.info("ignoring setting serializer for binary response") + return for available_serializer in self._serializers: if available_serializer.content_type() == mime_type: self._selected_serializer = available_serializer + self.logger.info("set serializer for mime type: %s" % mime_type) return + self.logger.info("could not find serializer for mime type: %s" % mime_type) raise exception.UnsupportedVocabularyError(mime_type, self.supported_mime_types_str) - #: - #: is an instance of prestans.types.DataType; mostly a subclass of - #: prestans.types.Model - #: - @property def template(self): + """ + is an instance of prestans.types.DataType; mostly a subclass of prestans.types.Model + """ return self._template @template.setter @@ -115,15 +125,11 @@ def attribute_filter(self): def attribute_filter(self, value): if value is not None and not isinstance(value, AttributeFilter): - raise TypeError("attribue_filter in response must be of \ - type prestans.types.AttributeFilter") + msg = "attribue_filter in response must be of type prestans.types.AttributeFilter" + raise TypeError(msg) self._attribute_filter = value - #: - #: content_type; overrides webob.Response line 606 - #: - def _content_type__get(self): """ Get/set the Content-Type header (or None), *without* the @@ -140,30 +146,26 @@ def _content_type__get(self): def _content_type__set(self, value): - # skip the mime type check if template is None status code is no content - if self.template is not None or \ - self.status_code not in [STATUS.NO_CONTENT, STATUS.PERMANENT_REDIRECT, STATUS.TEMPORARY_REDIRECT]: - # Check to see if response can support the requested mime type - if not isinstance(self._app_iter, BinaryResponse) and value not in self.supported_mime_types: - raise exception.UnsupportedVocabularyError(value, self.supported_mime_types_str) + # skip for responses that have no body + if self.status_code in [STATUS.NO_CONTENT, STATUS.PERMANENT_REDIRECT, STATUS.TEMPORARY_REDIRECT]: + self.logger.info("attempt to set Content-Type to %s being ignored due to empty response" % value) + self._content_type__del() + else: + self._set_serializer_by_mime_type(value) - # Keep a reference to the selected serializer - if not isinstance(self._app_iter, BinaryResponse): - self._set_serializer_by_mime_type(value) + if ';' not in value: + header = self.headers.get('Content-Type', '') + if ';' in header: + params = header.split(';', 1)[1] + value += ';' + params + self.headers['Content-Type'] = value - if not value or value is None: - self._content_type__del() - return - if ';' not in value: - header = self.headers.get('Content-Type', '') - if ';' in header: - params = header.split(';', 1)[1] - value += ';' + params - self.headers['Content-Type'] = value + self.logger.info("Content-Type set to: %s" % value) def _content_type__del(self): self.headers.pop('Content-Type', None) + # content_type; overrides webob.Response line 606 content_type = property( _content_type__get, _content_type__set, @@ -171,11 +173,7 @@ def _content_type__del(self): doc=_content_type__get.__doc__ ) - - #: - #: body; overrides webob.Response line 324 - #: - + # body; overrides webob.Response line 324 @property def body(self): """ @@ -184,7 +182,7 @@ def body(self): body getter will return the validated prestans model. - Webob does the heavy lifiting with headers. + webob does the heavy lifting with headers. """ #: If template is null; return an empty iterable diff --git a/tests/rest/test_request_router.py b/tests/rest/test_request_router.py index 90c59e4..40bfeb5 100644 --- a/tests/rest/test_request_router.py +++ b/tests/rest/test_request_router.py @@ -73,7 +73,7 @@ def test_script_alias_match_with_global_match_group_should_not_pass_call(self): expected_value = 123 - self._test_routing_behavour(environ={ + self._test_routing_behaviour(environ={ "REQUEST_METHOD": prestans.http.VERB.GET, "SCRIPT_NAME": "/mountpoint/some/path/{}".format(expected_value), "PATH_INFO": "", @@ -91,7 +91,7 @@ def test_script_alias_match_with_match_group_should_pass_call(self): with router ``TestRequestRouter`` :return: """ - self._test_routing_behavour(environ={ + self._test_routing_behaviour(environ={ "REQUEST_METHOD": prestans.http.VERB.GET, "SCRIPT_NAME": "/mountpoint", "PATH_INFO": "/some/path/{}".format(123), @@ -105,7 +105,7 @@ def test_script_alias_match_with_solidus_should_pass_call(self): WSGIScriptAliasMatch /mountpoint/(.*) script.wsgi/$1 :return: """ - self._test_routing_behavour(environ={ + self._test_routing_behaviour(environ={ "REQUEST_METHOD": prestans.http.VERB.GET, "SCRIPT_NAME": "/mountpoint/", "PATH_INFO": "/some/path/{}".format(123), @@ -122,7 +122,7 @@ def test_router_should_not_crash_with_no_path_info_field(self): """ expected_value = 123 - self._test_routing_behavour(environ={ + self._test_routing_behaviour(environ={ "REQUEST_METHOD": prestans.http.VERB.GET, "SCRIPT_NAME": "/mountpoint/some/path/{}".format(123), "wsgi.url_scheme": "http", @@ -140,14 +140,14 @@ def test_router_should_not_crash_with_no_script_name_field(self): expected_value = 123 - self._test_routing_behavour(environ={ + self._test_routing_behaviour(environ={ "REQUEST_METHOD": prestans.http.VERB.GET, "PATH_INFO": "/some/path/{}".format(123), "wsgi.url_scheme": "http", "SERVER_NAME": "localhost", "SERVER_PORT": "1234"}) - def _test_routing_behavour(self, environ, should_pass=True, expected_value=123, match=r"/some/path/([0-9]+)"): + def _test_routing_behaviour(self, environ, should_pass=True, expected_value=123, match=r"/some/path/([0-9]+)"): test_router = prestans.rest.RequestRouter([ (match, _UserHandler) From 3e91ccb50febcf3db24322cd1d709511cce70dc6 Mon Sep 17 00:00:00 2001 From: Bradley Mclain Date: Tue, 28 Aug 2018 13:57:22 +1000 Subject: [PATCH 14/17] some cleanup to request router tests --- tests/rest/test_request_router.py | 44 ++++++++++++++----------------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/tests/rest/test_request_router.py b/tests/rest/test_request_router.py index 40bfeb5..d8635a3 100644 --- a/tests/rest/test_request_router.py +++ b/tests/rest/test_request_router.py @@ -29,23 +29,21 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS # SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. # - -import logging import unittest -import prestans.rest - -LOGGER_MCLOGFACE = logging.Logger("temp", level='ERROR') -LOGGER_MCLOGFACE.disabled = 50 # silence the logger +from prestans.http import VERB +from prestans import parser +from prestans import rest +from prestans import types -class MyModel(prestans.types.Model): - id = prestans.types.Integer() +class MyModel(types.Model): + id = types.Integer() -class _UserHandler(prestans.rest.RequestHandler): - __parser_config__ = prestans.parser.Config( - GET=prestans.parser.VerbConfig( +class _UserHandler(rest.RequestHandler): + __parser_config__ = parser.Config( + GET=parser.VerbConfig( response_template=MyModel(), response_attribute_filter_default_value=True ) @@ -63,7 +61,7 @@ def __call__(cls, status, response_headers, exc_info=None): pass -class RequestRouterTest(unittest.TestCase): +class RequestRouterTestCase(unittest.TestCase): def test_script_alias_match_with_global_match_group_should_not_pass_call(self): """ WSGIScriptAliasMatch /mountpoint(.*) script.wsgi @@ -74,7 +72,7 @@ def test_script_alias_match_with_global_match_group_should_not_pass_call(self): expected_value = 123 self._test_routing_behaviour(environ={ - "REQUEST_METHOD": prestans.http.VERB.GET, + "REQUEST_METHOD": VERB.GET, "SCRIPT_NAME": "/mountpoint/some/path/{}".format(expected_value), "PATH_INFO": "", "wsgi.version": (1, 0), @@ -92,7 +90,7 @@ def test_script_alias_match_with_match_group_should_pass_call(self): :return: """ self._test_routing_behaviour(environ={ - "REQUEST_METHOD": prestans.http.VERB.GET, + "REQUEST_METHOD": VERB.GET, "SCRIPT_NAME": "/mountpoint", "PATH_INFO": "/some/path/{}".format(123), "wsgi.url_scheme": "http", @@ -106,7 +104,7 @@ def test_script_alias_match_with_solidus_should_pass_call(self): :return: """ self._test_routing_behaviour(environ={ - "REQUEST_METHOD": prestans.http.VERB.GET, + "REQUEST_METHOD": VERB.GET, "SCRIPT_NAME": "/mountpoint/", "PATH_INFO": "/some/path/{}".format(123), "wsgi.url_scheme": "http", @@ -123,7 +121,7 @@ def test_router_should_not_crash_with_no_path_info_field(self): expected_value = 123 self._test_routing_behaviour(environ={ - "REQUEST_METHOD": prestans.http.VERB.GET, + "REQUEST_METHOD": VERB.GET, "SCRIPT_NAME": "/mountpoint/some/path/{}".format(123), "wsgi.url_scheme": "http", "SERVER_NAME": "localhost", @@ -141,7 +139,7 @@ def test_router_should_not_crash_with_no_script_name_field(self): expected_value = 123 self._test_routing_behaviour(environ={ - "REQUEST_METHOD": prestans.http.VERB.GET, + "REQUEST_METHOD": VERB.GET, "PATH_INFO": "/some/path/{}".format(123), "wsgi.url_scheme": "http", "SERVER_NAME": "localhost", @@ -149,12 +147,14 @@ def test_router_should_not_crash_with_no_script_name_field(self): def _test_routing_behaviour(self, environ, should_pass=True, expected_value=123, match=r"/some/path/([0-9]+)"): - test_router = prestans.rest.RequestRouter([ + from prestans.deserializer import JSON + + test_router = rest.RequestRouter([ (match, _UserHandler) - ], application_name="test-router", logger=LOGGER_MCLOGFACE) + ], application_name="test-router") response = test_router(environ=environ, start_response=MockStartResponse.__call__) - response_parsed = prestans.deserializer.JSON().loads(response[0]) + response_parsed = JSON().loads(response[0]) if should_pass: self.assert_should_find_route(environ, expected_value, match, response_parsed) else: @@ -178,7 +178,3 @@ def gen_route_match_failure_message(self, environ, match, assertion="SHOULD"): route=match, assertion=assertion ) - - -if __name__ == '__main__': - unittest.main() From 8bec517450727b2d9e09d173f5896dfa27adee42 Mon Sep 17 00:00:00 2001 From: Bradley Mclain Date: Tue, 28 Aug 2018 16:43:32 +1000 Subject: [PATCH 15/17] implements #173 --- prestans/provider/auth.py | 12 +++---- prestans/rest/request_handler.py | 31 ++++++++--------- prestans/rest/request_router.py | 58 +++++++++++++++++++++++--------- tests/provider/test_auth.py | 10 ++++++ 4 files changed, 75 insertions(+), 36 deletions(-) diff --git a/prestans/provider/auth.py b/prestans/provider/auth.py index 930e04e..8ec40bf 100644 --- a/prestans/provider/auth.py +++ b/prestans/provider/auth.py @@ -99,7 +99,7 @@ def login_required(http_method_handler): """ @wraps(http_method_handler) - def secure_http_method_handler(self, *args): + def secure_http_method_handler(self, *args, **kwargs): if not self.__provider_config__.authentication: _message = "Service available to authenticated users only, no auth context provider set in handler" @@ -112,7 +112,7 @@ def secure_http_method_handler(self, *args): authentication_error.request = self.request raise authentication_error - http_method_handler(self, *args) + http_method_handler(self, *args, **kwargs) return secure_http_method_handler @@ -128,7 +128,7 @@ def role_required(role_name=None): def _role_required(http_method_handler): @wraps(http_method_handler) - def secure_http_method_handler(self, *args): + def secure_http_method_handler(self, *args, **kwargs): # role name must be provided if role_name is None: @@ -150,7 +150,7 @@ def secure_http_method_handler(self, *args): authorization_error.request = self.request raise authorization_error - http_method_handler(self, *args) + http_method_handler(self, *args, **kwargs) return wraps(http_method_handler)(secure_http_method_handler) @@ -164,7 +164,7 @@ def access_required(config=None): def _access_required(http_method_handler): - def secure_http_method_handler(self, *args): + def secure_http_method_handler(self, *args, **kwargs): # authentication context must be set if not self.__provider_config__.authentication: @@ -180,7 +180,7 @@ def secure_http_method_handler(self, *args): authorization_error.request = self.request raise authorization_error - http_method_handler(self, *args) + http_method_handler(self, *args, **kwargs) return wraps(http_method_handler)(secure_http_method_handler) diff --git a/prestans/rest/request_handler.py b/prestans/rest/request_handler.py index f759a61..6a86584 100644 --- a/prestans/rest/request_handler.py +++ b/prestans/rest/request_handler.py @@ -21,7 +21,7 @@ class RequestHandler(object): __provider_config__ = None __parser_config__ = None - def __init__(self, args, request, response, logger, debug): + def __init__(self, args, kwargs, request, response, logger, debug): if self.__provider_config__ is None: self.__provider_config__ = provider.Config() @@ -29,6 +29,7 @@ def __init__(self, args, request, response, logger, debug): self.__parser_config__ = parser.Config() self._args = args + self._kwargs = kwargs self._request = request self._response = response self._logger = logger @@ -224,19 +225,19 @@ def __call__(self, environ, start_response): #: prestans sets a sensible HTTP status code #: if request_method == VERB.GET: - self.get(*self._args) + self.get(*self._args, **self._kwargs) elif request_method == VERB.HEAD: - self.head(*self._args) + self.head(*self._args, **self._kwargs) elif request_method == VERB.POST: - self.post(*self._args) + self.post(*self._args, **self._kwargs) elif request_method == VERB.PUT: - self.put(*self._args) + self.put(*self._args, **self._kwargs) elif request_method == VERB.PATCH: - self.patch(*self._args) + self.patch(*self._args, **self._kwargs) elif request_method == VERB.DELETE: - self.delete(*self._args) + self.delete(*self._args, **self._kwargs) elif request_method == VERB.OPTIONS: - self.options(*self._args) + self.options(*self._args, **self._kwargs) except (exception.PermanentRedirect, exception.TemporaryRedirect) as exp: self._redirect(exp.url, exp.http_status) # re-raise all prestans exceptions @@ -293,37 +294,37 @@ def handler_raised_exception(self, exp): #: if not overridden prestans returns a Not Implemented error #: - def get(self, *args): + def get(self, *args, **kwargs): unimplemented_verb_error = exception.UnimplementedVerbError(VERB.GET) unimplemented_verb_error.request = self.request raise unimplemented_verb_error - def head(self, *args): + def head(self, *args, **kwargs): unimplemented_verb_error = exception.UnimplementedVerbError(VERB.HEAD) unimplemented_verb_error.request = self.request raise unimplemented_verb_error - def post(self, *args): + def post(self, *args, **kwargs): unimplemented_verb_error = exception.UnimplementedVerbError(VERB.POST) unimplemented_verb_error.request = self.request raise unimplemented_verb_error - def put(self, *args): + def put(self, *args, **kwargs): unimplemented_verb_error = exception.UnimplementedVerbError(VERB.PUT) unimplemented_verb_error.request = self.request raise unimplemented_verb_error - def patch(self, *args): + def patch(self, *args, **kwargs): unimplemented_verb_error = exception.UnimplementedVerbError(VERB.PATCH) unimplemented_verb_error.request = self.request raise unimplemented_verb_error - def delete(self, *args): + def delete(self, *args, **kwargs): unimplemented_verb_error = exception.UnimplementedVerbError(VERB.DELETE) unimplemented_verb_error.request = self.request raise unimplemented_verb_error - def options(self, *args): + def options(self, *args, **kwargs): unimplemented_verb_error = exception.UnimplementedVerbError(VERB.OPTIONS) unimplemented_verb_error.request = self.request raise unimplemented_verb_error diff --git a/prestans/rest/request_router.py b/prestans/rest/request_router.py index d9b99fa..b4f1e08 100644 --- a/prestans/rest/request_router.py +++ b/prestans/rest/request_router.py @@ -92,18 +92,21 @@ def application_name(self): def __call__(self, environ, start_response): # say hello - self.logger.info("%s exposes %i end-points; prestans %s; charset %s; debug %s" % \ - (self._application_name, len(self._routes), __version__, \ - self._charset, self._debug)) + self.logger.info("%s exposes %i end-points; prestans %s; charset %s; debug %s" % ( + self._application_name, len(self._routes), __version__, + self._charset, self._debug + )) # validate serializers and deserializers; are subclasses of prestans.serializer.Base _default_outgoing_mime_types = list() for available_serializer in self._serializers: if not isinstance(available_serializer, serializer.Base): - raise TypeError("registered serializer %s.%s does not \ - inherit from prestans.serializer.Serializer" % (available_serializer.__module__, \ - available_serializer.__class__.__name__)) + msg = "registered serializer %s.%s does not inherit from prestans.serializer.Serializer" % ( + available_serializer.__module__, + available_serializer.__class__.__name__ + ) + raise TypeError(msg) _default_outgoing_mime_types.append(available_serializer.content_type()) @@ -111,10 +114,11 @@ def __call__(self, environ, start_response): for available_deserializer in self._deserializers: if not isinstance(available_deserializer, deserializer.Base): - raise TypeError( - "registered deserializer %s.%s does not inherit from \ - prestans.serializer.DeSerializer" \ - % (available_deserializer.__module__, available_deserializer.__class__.__name__)) + msg = "registered deserializer %s.%s does not inherit from prestans.serializer.DeSerializer" % ( + available_deserializer.__module__, + available_deserializer.__class__.__name__ + ) + raise TypeError(msg) _default_incoming_mime_types.append(available_deserializer.content_type()) @@ -148,6 +152,18 @@ def __call__(self, environ, start_response): # if we've found a match; ensure its a handler subclass and return it's callable if match: + # assemble the args and kwargs + args = match.groups() + kwargs = {} + for key, value in iter(regexp.groupindex.items()): + kwargs[key] = args[value - 1] + + if len(kwargs) > 0: + args = () + + self.logger.info(args) + self.logger.info(kwargs) + if issubclass(handler_class, BlueprintHandler): response = DictionaryResponse( @@ -157,7 +173,8 @@ def __call__(self, environ, start_response): ) request_handler = handler_class( - args=match.groups(), + args=args, + kwargs=kwargs, request=request, response=response, logger=self._logger, @@ -174,7 +191,8 @@ def __call__(self, environ, start_response): response.minify = request.is_minified request_handler = handler_class( - args=match.groups(), + args=args, + kwargs=kwargs, request=request, response=response, logger=self._logger, @@ -197,13 +215,15 @@ def _init_route_map(self, routes): parsed_handler_map = [] - for regexp, handler in routes: + for url, handler in routes: try: handler_name = handler.__name__ except AttributeError: pass + regexp = url + #: Patch regular expression if its incomplete if not regexp.startswith('^'): regexp = '^' + regexp @@ -211,6 +231,14 @@ def _init_route_map(self, routes): regexp += '$' compiled_regex = re.compile(regexp) - parsed_handler_map.append((compiled_regex, handler)) - return parsed_handler_map \ No newline at end of file + arg_count = compiled_regex.groups + kwarg_count = len(compiled_regex.groupindex.items()) + + if arg_count != kwarg_count and kwarg_count > 0: + raise ValueError("%s URL is invalid, cannot mix named and un-named groups" % url) + else: + parsed_handler_map.append((compiled_regex, handler)) + + + return parsed_handler_map diff --git a/tests/provider/test_auth.py b/tests/provider/test_auth.py index 5006241..675bd53 100644 --- a/tests/provider/test_auth.py +++ b/tests/provider/test_auth.py @@ -214,6 +214,7 @@ def get(self): def test_login_required_no_provider_raises_exception(self): handler = self.handler_without_provider( args=[], + kwargs={}, request=self.get_request, response=self.response, logger=self.logger, @@ -225,6 +226,7 @@ def test_login_required_no_provider_raises_exception(self): def test_login_required_unauthenticated_raises_exception(self): handler = self.unauthenticated_handler( args=[], + kwargs={}, request=self.get_request, response=self.response, logger=self.logger, @@ -236,6 +238,7 @@ def test_login_required_unauthenticated_raises_exception(self): def test_login_required_authenticated(self): handler = self.authenticated_handler( args=[], + kwargs={}, request=self.get_request, response=self.response, logger=self.logger, @@ -328,6 +331,7 @@ def put(self): def test_role_required_no_provider_raises_exception(self): handler = self.handler_without_provider( args=[], + kwargs={}, request=self.get_request, response=self.response, logger=self.logger, @@ -339,6 +343,7 @@ def test_role_required_no_provider_raises_exception(self): def test_role_required_none_raises_authorization_error(self): handler = self.handler_with_provider( args=[], + kwargs={}, request=self.get_request, response=self.response, logger=self.logger, @@ -350,6 +355,7 @@ def test_role_required_none_raises_authorization_error(self): def test_role_required_incorrect_role_raises_exception(self): handler = self.handler_with_provider( args=[], + kwargs={}, request=self.post_request, response=self.response, logger=self.logger, @@ -361,6 +367,7 @@ def test_role_required_incorrect_role_raises_exception(self): def test_role_required_success(self): handler = self.handler_with_provider( args=[], + kwargs={}, request=self.put_request, response=self.response, logger=self.logger, @@ -439,6 +446,7 @@ def post(self): def test_access_required_no_provider_raises_exception(self): handler = self.handler_without_provider( args=[], + kwargs={}, request=self.get_request, response=self.response, logger=self.logger, @@ -450,6 +458,7 @@ def test_access_required_no_provider_raises_exception(self): def test_access_required_unauthorized_raises_exception(self): handler = self.handler_with_provider( args=[], + kwargs={}, request=self.post_request, response=self.response, logger=self.logger, @@ -461,6 +470,7 @@ def test_access_required_unauthorized_raises_exception(self): def test_access_required_success(self): handler = self.handler_with_provider( args=[], + kwargs={}, request=self.get_request, response=self.response, logger=self.logger, From e19262ea8dbdc602fd366d666aa37c66acaf3953 Mon Sep 17 00:00:00 2001 From: Bradley Mclain Date: Wed, 29 Aug 2018 13:33:41 +1000 Subject: [PATCH 16/17] cleanup of wording for AttributeFilterDiffers #167 --- prestans/exception.py | 2 +- tests/test_exception.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/prestans/exception.py b/prestans/exception.py index af9c142..687377b 100644 --- a/prestans/exception.py +++ b/prestans/exception.py @@ -320,7 +320,7 @@ class AttributeFilterDiffers(RequestException): def __init__(self, attribute_list): _code = STATUS.BAD_REQUEST - _message = "attribute filter does not contain attributes (%s) that are not part of template" % ( + _message = "attribute filter contains attributes (%s) that are not part of template" % ( ', '.join(attribute_list) ) diff --git a/tests/test_exception.py b/tests/test_exception.py index 4c7c817..c00bd0f 100644 --- a/tests/test_exception.py +++ b/tests/test_exception.py @@ -201,7 +201,7 @@ def test_init(self): self.assertEquals(attribute_filter_differs.http_status, STATUS.BAD_REQUEST) self.assertEquals( attribute_filter_differs.message, - "attribute filter does not contain attributes (cat, dog) that are not part of template" + "attribute filter contains attributes (cat, dog) that are not part of template" ) From c3a085db2f0e6b43e78bea45ba3ed335e5d3a749 Mon Sep 17 00:00:00 2001 From: Bradley Mclain Date: Wed, 29 Aug 2018 15:04:08 +1000 Subject: [PATCH 17/17] some cleanup --- prestans/rest/request.py | 19 ++++++++---- prestans/rest/request_handler.py | 6 ++-- prestans/rest/request_router.py | 17 ++++------- prestans/rest/response.py | 51 +++++++++++++++++++------------- 4 files changed, 54 insertions(+), 39 deletions(-) diff --git a/prestans/rest/request.py b/prestans/rest/request.py index 98b6e09..32800f6 100644 --- a/prestans/rest/request.py +++ b/prestans/rest/request.py @@ -44,8 +44,8 @@ def logger(self): def parsed_body(self): if self.body_template is None: - raise AttributeError("access to request.parsed_body is not \ - allowed when body_template is set to None") + msg = "access to request.parsed_body is not allowed when body_template is set to None" + raise AttributeError(msg) self.parse_body() @@ -67,8 +67,13 @@ def selected_deserializer(self): def default_deserializer(self): return self._default_deserializer - #: Used by content_type_set to set get a referencey to the serializer object def set_deserializer_by_mime_type(self, mime_type): + """ + :param mime_type: + :return: + + Used by content_type_set to set get a reference to the serializer object + """ for deserializer in self._deserializers: if deserializer.content_type() == mime_type: @@ -124,7 +129,10 @@ def body_template(self, value): return if not isinstance(value, DataCollection): - msg = "body_template must be an instance of %s.%s" % (DataCollection.__module__, DataCollection.__name__) + msg = "body_template must be an instance of %s.%s" % ( + DataCollection.__module__, + DataCollection.__name__ + ) raise AssertionError(msg) self._body_template = value @@ -152,8 +160,9 @@ def register_deserializers(self, deserializers): for new_deserializer in deserializers: if not isinstance(new_deserializer, deserializer.Base): - msg = "registered deserializer %s does not inherit from prestans.serializer.DeSerializer" % \ + msg = "registered deserializer %s does not inherit from prestans.serializer.DeSerializer" % ( new_deserializer.__class__.__name__ + ) raise TypeError(msg) self._deserializers = self._deserializers + deserializers diff --git a/prestans/rest/request_handler.py b/prestans/rest/request_handler.py index 6a86584..c3e7a45 100644 --- a/prestans/rest/request_handler.py +++ b/prestans/rest/request_handler.py @@ -253,8 +253,10 @@ def __call__(self, environ, start_response): finally: self.handler_did_run() - self.logger.info("handler %s.%s; callable execution ends" % \ - (self.__module__, self.__class__.__name__)) + self.logger.info("handler %s.%s; callable execution ends" % ( + self.__module__, + self.__class__.__name__ + )) return self.response(environ, start_response) diff --git a/prestans/rest/request_router.py b/prestans/rest/request_router.py index b4f1e08..beb0150 100644 --- a/prestans/rest/request_router.py +++ b/prestans/rest/request_router.py @@ -137,8 +137,8 @@ def __call__(self, environ, start_response): default_deserializer=self._default_deserializer ) - # initialise the Route map - route_map = self._init_route_map(self._routes) + # initialise the route map + route_map = self.generate_route_map(self._routes) try: @@ -201,7 +201,7 @@ def __call__(self, environ, start_response): return request_handler(environ, start_response) - #: Request does not have a matched handler + # request does not have a matched handler no_endpoint = exception.NoEndpointError() no_endpoint.request = request raise no_endpoint @@ -211,20 +211,16 @@ def __call__(self, environ, start_response): error_response = ErrorResponse(exp, self._default_serializer) return error_response(environ, start_response) - def _init_route_map(self, routes): + @classmethod + def generate_route_map(cls, routes): parsed_handler_map = [] for url, handler in routes: - try: - handler_name = handler.__name__ - except AttributeError: - pass - regexp = url - #: Patch regular expression if its incomplete + # patch regular expression if it is incomplete if not regexp.startswith('^'): regexp = '^' + regexp if not regexp.endswith('$'): @@ -240,5 +236,4 @@ def _init_route_map(self, routes): else: parsed_handler_map.append((compiled_regex, handler)) - return parsed_handler_map diff --git a/prestans/rest/response.py b/prestans/rest/response.py index 603bef5..d03cf2a 100644 --- a/prestans/rest/response.py +++ b/prestans/rest/response.py @@ -1,7 +1,5 @@ -import sys import webob -from prestans import __version__ from prestans import exception from prestans.http import STATUS from prestans.parser import AttributeFilter @@ -125,7 +123,7 @@ def attribute_filter(self): def attribute_filter(self, value): if value is not None and not isinstance(value, AttributeFilter): - msg = "attribue_filter in response must be of type prestans.types.AttributeFilter" + msg = "attribute_filter in response must be of type prestans.types.AttributeFilter" raise TypeError(msg) self._attribute_filter = value @@ -202,20 +200,27 @@ def body(self, value): #: value should be a subclass prestans.types.DataCollection if not isinstance(value, DataCollection) and \ not isinstance(value, BinaryResponse): - raise TypeError("%s is not a prestans.types.DataCollection \ - or prestans.types.BinaryResponse subclass" % value.__class__.__name__) + msg = "%s is not a prestans.types.DataCollection or prestans.types.BinaryResponse subclass" % ( + value.__class__.__name__ + ) + raise TypeError(msg) #: Ensure that it matches the return type template if not value.__class__ == self.template.__class__: - raise TypeError("body must of be type %s, given %s" % \ - (self.template.__class__.__name__, value.__class__.__name__)) + msg = "body must of be type %s, given %s" % ( + self.template.__class__.__name__, + value.__class__.__name__ + ) + raise TypeError(msg) #: If it's an array then ensure that element_template matches up if isinstance(self.template, Array) and \ - not isinstance(value.element_template, self.template.element_template.__class__): - raise TypeError("array elements must of be \ - type %s, given %s" % (self.template.element_template.__class__.__name__, \ - value.element_template.__class__.__name__)) + not isinstance(value.element_template, self.template.element_template.__class__): + msg = "array elements must of be type %s, given %s" % ( + self.template.element_template.__class__.__name__, + value.element_template.__class__.__name__ + ) + raise TypeError(msg) #: _app_iter assigned to value #: we need to serialize the contents before we know the length @@ -231,9 +236,11 @@ def register_serializers(self, serializers): for new_serializer in serializers: if not isinstance(new_serializer, serializer.Base): - raise TypeError("registered serializer %s.%s does not inherit from \ - prestans.serializer.Serializer" % (new_serializer.__module__, \ - new_serializer.__class__.__name__)) + msg = "registered serializer %s.%s does not inherit from prestans.serializer.Serializer" % ( + new_serializer.__module__, + new_serializer.__class__.__name__ + ) + raise TypeError(msg) self._serializers = self._serializers + serializers @@ -250,21 +257,23 @@ def __call__(self, environ, start_response): start_response(self.status, self.headerlist) if self.template is not None: - self.logger.warn("handler returns No Content but has a \ - response_template; set template to None") + self.logger.warn("handler returns No Content but has a response_template; set template to None") return [] - #: Ensure what we are able to serialize is serializable + # ensure what we are able to serialize is serializable if not isinstance(self._app_iter, DataCollection) and \ - not isinstance(self._app_iter, BinaryResponse): + not isinstance(self._app_iter, BinaryResponse): if isinstance(self._app_iter, list): - type = "list" + app_iter_type = "list" else: - type = self._app_iter.__name__ + app_iter_type = self._app_iter.__name__ - raise TypeError("handler returns content of type %s; not a prestans.types.DataCollection subclass" % type) + msg = "handler returns content of type %s; not a prestans.types.DataCollection subclass" % ( + app_iter_type + ) + raise TypeError(msg) if isinstance(self._app_iter, DataCollection):