From 21292d31e7ad5ec731c9ef3e471f90cb29054686 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Piquemal?= Date: Tue, 7 Feb 2012 15:38:54 +0200 Subject: [PATCH] cleaned Request/Response/mixins to have similar interface --- djangorestframework/mixins.py | 90 +++++++++++----------- djangorestframework/parsers.py | 7 +- djangorestframework/request.py | 95 ++++++++++------------- djangorestframework/tests/request.py | 108 +++++++++++++-------------- djangorestframework/views.py | 12 +-- 5 files changed, 145 insertions(+), 167 deletions(-) diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index c30ef10b7..c1f755b84 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -12,7 +12,7 @@ from djangorestframework import status from djangorestframework.renderers import BaseRenderer from djangorestframework.resources import Resource, FormResource, ModelResource from djangorestframework.response import Response, ImmediateResponse -from djangorestframework.request import request_class_factory +from djangorestframework.request import Request from djangorestframework.utils import as_tuple, allowed_methods @@ -32,7 +32,6 @@ __all__ = ( 'ListModelMixin' ) -#TODO: In RequestMixin and ResponseMixin : get_response_class/get_request_class are a bit ugly. Do we even want to be able to set the parameters on the view ? ########## Request Mixin ########## @@ -41,39 +40,43 @@ class RequestMixin(object): `Mixin` class to enhance API of Django's standard `request`. """ - _USE_FORM_OVERLOADING = True - _METHOD_PARAM = '_method' - _CONTENTTYPE_PARAM = '_content_type' - _CONTENT_PARAM = '_content' - - parsers = () + parser_classes = () """ - The set of parsers that the request can handle. + The set of parsers that the view can handle. Should be a tuple/list of classes as described in the :mod:`parsers` module. """ - def get_request_class(self): - """ - Returns a subclass of Django's `HttpRequest` with a richer API, - as described in :mod:`request`. - """ - if not hasattr(self, '_request_class'): - self._request_class = request_class_factory(self.request) - self._request_class._USE_FORM_OVERLOADING = self._USE_FORM_OVERLOADING - self._request_class._METHOD_PARAM = self._METHOD_PARAM - self._request_class._CONTENTTYPE_PARAM = self._CONTENTTYPE_PARAM - self._request_class._CONTENT_PARAM = self._CONTENT_PARAM - self._request_class.parsers = self.parsers - return self._request_class + request_class = Request + """ + The class to use as a wrapper for the original request object. + """ - def get_request(self): + def get_parsers(self): """ - Returns a custom request instance, with data and attributes copied from the - original request. + Instantiates and returns the list of parsers that will be used by the request + to parse its content. """ - request_class = self.get_request_class() - return request_class(self.request) + if not hasattr(self, '_parsers'): + self._parsers = [r(self) for r in self.parser_classes] + return self._parsers + + def prepare_request(self, request): + """ + Prepares the request for the request cycle. Returns a custom request instance, + with data and attributes copied from the original request. + """ + parsers = self.get_parsers() + request = self.request_class(request, parsers=parsers) + self.request = request + return request + + @property + def _parsed_media_types(self): + """ + Return a list of all the media types that this view can parse. + """ + return [p.media_type for p in self.parser_classes] ########## ResponseMixin ########## @@ -105,8 +108,8 @@ class ResponseMixin(object): def prepare_response(self, response): """ - Prepares the response for the response cycle. This has no effect if the - response is not an instance of :class:`response.Response`. + Prepares the response for the response cycle, and returns the prepared response. + This has no effect if the response is not an instance of :class:`response.Response`. """ if hasattr(response, 'request') and response.request is None: response.request = self.request @@ -124,6 +127,17 @@ class ResponseMixin(object): self.response = response return response + @property + def headers(self): + """ + Dictionary of headers to set on the response. + This is useful when the response doesn't exist yet, but you + want to memorize some headers to set on it when it will exist. + """ + if not hasattr(self, '_headers'): + self._headers = {} + return self._headers + @property def _rendered_media_types(self): """ @@ -138,24 +152,6 @@ class ResponseMixin(object): """ return [renderer.format for renderer in self.get_renderers()] - @property - def _default_renderer(self): - """ - Return the view's default renderer class. - """ - return self.get_renderers()[0] - - @property - def headers(self): - """ - Dictionary of headers to set on the response. - This is useful when the response doesn't exist yet, but you - want to memorize some headers to set on it when it will exist. - """ - if not hasattr(self, '_headers'): - self._headers = {} - return self._headers - ########## Auth Mixin ########## diff --git a/djangorestframework/parsers.py b/djangorestframework/parsers.py index 5fc5c71ee..c041d7ce4 100644 --- a/djangorestframework/parsers.py +++ b/djangorestframework/parsers.py @@ -43,7 +43,7 @@ class BaseParser(object): media_type = None - def __init__(self, view): + def __init__(self, view=None): """ Initialize the parser with the ``View`` instance as state, in case the parser needs to access any metadata on the :obj:`View` object. @@ -167,10 +167,9 @@ class MultiPartParser(BaseParser): `data` will be a :class:`QueryDict` containing all the form parameters. `files` will be a :class:`QueryDict` containing all the form files. """ - # TODO: now self.view is in fact request, but should disappear ... - upload_handlers = self.view._get_upload_handlers() + upload_handlers = self.view.request._get_upload_handlers() try: - django_parser = DjangoMultiPartParser(self.view.META, stream, upload_handlers) + django_parser = DjangoMultiPartParser(self.view.request.META, stream, upload_handlers) except MultiPartParserError, exc: raise ImmediateResponse( content={'detail': 'multipart parse error - %s' % unicode(exc)}, diff --git a/djangorestframework/request.py b/djangorestframework/request.py index 21538aec0..cd6e30974 100644 --- a/djangorestframework/request.py +++ b/djangorestframework/request.py @@ -1,10 +1,10 @@ """ The :mod:`request` module provides a :class:`Request` class that can be used -to enhance the standard `request` object received in all the views. +to wrap the standard `request` object received in all the views, and upgrade its API. -This enhanced request object offers the following : +The wrapped request then offer the following : - - content automatically parsed according to `Content-Type` header, and available as :meth:`request.DATA` + - content automatically parsed according to `Content-Type` header, and available as :meth:`.DATA` - full support of PUT method, including support for file uploads - form overloading of HTTP method, content type and content """ @@ -22,21 +22,9 @@ from StringIO import StringIO __all__ = ('Request',) -def request_class_factory(request): - """ - Builds and returns a request class, to be used as a replacement of Django's built-in. - - In fact :class:`request.Request` needs to be mixed-in with a subclass of `HttpRequest` for use, - and we cannot do that before knowing which subclass of `HttpRequest` is used. So this function - takes a request instance as only argument, and returns a properly mixed-in request class. - """ - request_class = type(request) - return type(request_class.__name__, (Request, request_class), {}) - - class Request(object): """ - A mixin class allowing to enhance Django's standard HttpRequest. + A wrapper allowing to enhance Django's standard HttpRequest. """ _USE_FORM_OVERLOADING = True @@ -44,24 +32,14 @@ class Request(object): _CONTENTTYPE_PARAM = '_content_type' _CONTENT_PARAM = '_content' - parsers = () - """ - The set of parsers that the request can handle. - - Should be a tuple/list of classes as described in the :mod:`parsers` module. - """ - - def __init__(self, request): - # this allows to "copy" a request object into a new instance - # of our custom request class. - - # First, we prepare the attributes to copy. - attrs_dict = request.__dict__.copy() - attrs_dict.pop('method', None) - attrs_dict['_raw_method'] = request.method - - # Then, put them in the instance's own __dict__ - self.__dict__ = attrs_dict + def __init__(self, request=None, parsers=None): + """ + `parsers` is a list/tuple of parser instances and represents the set of psrsers + that the response can handle. + """ + self.request = request + if parsers is not None: + self.parsers = parsers @property def method(self): @@ -111,22 +89,6 @@ class Request(object): self._load_data_and_files() return self._files - def _load_post_and_files(self): - """ - Overrides the parent's `_load_post_and_files` to isolate it - from the form overloading mechanism (see: `_perform_form_overloading`). - """ - # When self.POST or self.FILES are called they need to know the original - # HTTP method, not our overloaded HTTP method. So, we save our overloaded - # HTTP method and restore it after the call to parent. - method_mem = getattr(self, '_method', None) - self._method = self._raw_method - super(Request, self)._load_post_and_files() - if method_mem is None: - del self._method - else: - self._method = method_mem - def _load_data_and_files(self): """ Parses the request content into self.DATA and self.FILES. @@ -145,7 +107,7 @@ class Request(object): self._perform_form_overloading() # if the HTTP method was not overloaded, we take the raw HTTP method if not hasattr(self, '_method'): - self._method = self._raw_method + self._method = self.request.method def _get_stream(self): """ @@ -172,7 +134,8 @@ class Request(object): """ # We only need to use form overloading on form POST requests. - if not self._USE_FORM_OVERLOADING or self._raw_method != 'POST' or not is_form_media_type(self._content_type): + if (not self._USE_FORM_OVERLOADING or self.request.method != 'POST' + or not is_form_media_type(self._content_type)): return # At this point we're committed to parsing the request as form data. @@ -199,10 +162,7 @@ class Request(object): if stream is None or content_type is None: return (None, None) - parsers = as_tuple(self.parsers) - - for parser_cls in parsers: - parser = parser_cls(self) + for parser in as_tuple(self.parsers): if parser.can_handle_request(content_type): return parser.parse(stream) @@ -223,3 +183,26 @@ class Request(object): Return the view's default parser class. """ return self.parsers[0] + + def _get_parsers(self): + """ + This just provides a default when parsers havent' been set. + """ + if hasattr(self, '_parsers'): + return self._parsers + return () + + def _set_parsers(self, value): + self._parsers = value + + parsers = property(_get_parsers, _set_parsers) + + def __getattr__(self, name): + """ + When an attribute is not present on the calling instance, try to get it + from the original request. + """ + if hasattr(self.request, name): + return getattr(self.request, name) + else: + return super(Request, self).__getattribute__(name) diff --git a/djangorestframework/tests/request.py b/djangorestframework/tests/request.py index 77a340336..c92d3f5fa 100644 --- a/djangorestframework/tests/request.py +++ b/djangorestframework/tests/request.py @@ -10,36 +10,19 @@ from djangorestframework.compat import RequestFactory from djangorestframework.mixins import RequestMixin from djangorestframework.parsers import FormParser, MultiPartParser, \ PlainTextParser, JSONParser +from djangorestframework.request import Request from djangorestframework.response import Response from djangorestframework.request import Request from djangorestframework.views import View -from djangorestframework.request import request_class_factory - -class MockView(View): - authentication = (UserLoggedInAuthentication,) - def post(self, request): - if request.POST.get('example') is not None: - return Response(status=status.HTTP_200_OK) - - return Response(status=status.INTERNAL_SERVER_ERROR) - -urlpatterns = patterns('', - (r'^$', MockView.as_view()), -) - -request_class = request_class_factory(RequestFactory().get('/')) class RequestTestCase(TestCase): - def tearDown(self): - request_class.parsers = () - def build_request(self, method, *args, **kwargs): factory = RequestFactory() method = getattr(factory, method) original_request = method(*args, **kwargs) - return request_class(original_request) + return Request(original_request) class TestMethodOverloading(RequestTestCase): @@ -67,14 +50,22 @@ class TestMethodOverloading(RequestTestCase): class TestContentParsing(RequestTestCase): - def tearDown(self): - request_class.parsers = () - def build_request(self, method, *args, **kwargs): factory = RequestFactory() + parsers = kwargs.pop('parsers', None) method = getattr(factory, method) original_request = method(*args, **kwargs) - return request_class(original_request) + rkwargs = {} + if parsers is not None: + rkwargs['parsers'] = parsers + request = Request(original_request, **rkwargs) + # TODO: Just a hack because the parsers need a view. This will be fixed in the future + class Obj(object): pass + obj = Obj() + obj.request = request + for p in request.parsers: + p.view = obj + return request def test_standard_behaviour_determines_no_content_GET(self): """Ensure request.DATA returns None for GET request with no content.""" @@ -89,31 +80,35 @@ class TestContentParsing(RequestTestCase): def test_standard_behaviour_determines_form_content_POST(self): """Ensure request.DATA returns content for POST request with form content.""" form_data = {'qwerty': 'uiop'} - request_class.parsers = (FormParser, MultiPartParser) - request = self.build_request('post', '/', data=form_data) + parsers = (FormParser(), MultiPartParser()) + + request = self.build_request('post', '/', data=form_data, parsers=parsers) self.assertEqual(request.DATA.items(), form_data.items()) def test_standard_behaviour_determines_non_form_content_POST(self): """Ensure request.DATA returns content for POST request with non-form content.""" content = 'qwerty' content_type = 'text/plain' - request_class.parsers = (PlainTextParser,) - request = self.build_request('post', '/', content, content_type=content_type) + parsers = (PlainTextParser(),) + + request = self.build_request('post', '/', content, content_type=content_type, parsers=parsers) self.assertEqual(request.DATA, content) def test_standard_behaviour_determines_form_content_PUT(self): """Ensure request.DATA returns content for PUT request with form content.""" form_data = {'qwerty': 'uiop'} - request_class.parsers = (FormParser, MultiPartParser) - request = self.build_request('put', '/', data=form_data) + parsers = (FormParser(), MultiPartParser()) + + request = self.build_request('put', '/', data=form_data, parsers=parsers) self.assertEqual(request.DATA.items(), form_data.items()) def test_standard_behaviour_determines_non_form_content_PUT(self): """Ensure request.DATA returns content for PUT request with non-form content.""" content = 'qwerty' content_type = 'text/plain' - request_class.parsers = (PlainTextParser,) - request = self.build_request('put', '/', content, content_type=content_type) + parsers = (PlainTextParser(),) + + request = self.build_request('put', '/', content, content_type=content_type, parsers=parsers) self.assertEqual(request.DATA, content) def test_overloaded_behaviour_allows_content_tunnelling(self): @@ -122,16 +117,17 @@ class TestContentParsing(RequestTestCase): content_type = 'text/plain' form_data = {Request._CONTENT_PARAM: content, Request._CONTENTTYPE_PARAM: content_type} - request_class.parsers = (PlainTextParser,) - request = self.build_request('post', '/', form_data) + parsers = (PlainTextParser(),) + + request = self.build_request('post', '/', form_data, parsers=parsers) self.assertEqual(request.DATA, content) def test_accessing_post_after_data_form(self): """Ensures request.POST can be accessed after request.DATA in form request""" form_data = {'qwerty': 'uiop'} - request_class.parsers = (FormParser, MultiPartParser) - request = self.build_request('post', '/', data=form_data) + parsers = (FormParser(), MultiPartParser()) + request = self.build_request('post', '/', data=form_data) self.assertEqual(request.DATA.items(), form_data.items()) self.assertEqual(request.POST.items(), form_data.items()) @@ -142,11 +138,9 @@ class TestContentParsing(RequestTestCase): data = {'qwerty': 'uiop'} content = json.dumps(data) content_type = 'application/json' + parsers = (JSONParser(),) - request_class.parsers = (JSONParser,) - - request = self.build_request('post', '/', content, content_type=content_type) - + request = self.build_request('post', '/', content, content_type=content_type, parsers=parsers) self.assertEqual(request.DATA.items(), data.items()) self.assertEqual(request.POST.items(), []) @@ -157,22 +151,19 @@ class TestContentParsing(RequestTestCase): data = {'qwerty': 'uiop'} content = json.dumps(data) content_type = 'application/json' - - request_class.parsers = (JSONParser,) - + parsers = (JSONParser(),) form_data = {Request._CONTENT_PARAM: content, Request._CONTENTTYPE_PARAM: content_type} - request = self.build_request('post', '/', data=form_data) - + request = self.build_request('post', '/', data=form_data, parsers=parsers) self.assertEqual(request.DATA.items(), data.items()) self.assertEqual(request.POST.items(), form_data.items()) def test_accessing_data_after_post_form(self): """Ensures request.DATA can be accessed after request.POST in form request""" form_data = {'qwerty': 'uiop'} - request_class.parsers = (FormParser, MultiPartParser) - request = self.build_request('post', '/', data=form_data) + parsers = (FormParser, MultiPartParser) + request = self.build_request('post', '/', data=form_data, parsers=parsers) self.assertEqual(request.POST.items(), form_data.items()) self.assertEqual(request.DATA.items(), form_data.items()) @@ -184,11 +175,9 @@ class TestContentParsing(RequestTestCase): data = {'qwerty': 'uiop'} content = json.dumps(data) content_type = 'application/json' + parsers = (JSONParser(),) - request_class.parsers = (JSONParser,) - - request = self.build_request('post', '/', content, content_type=content_type) - + request = self.build_request('post', '/', content, content_type=content_type, parsers=parsers) post_items = request.POST.items() self.assertEqual(len(post_items), 1) @@ -203,17 +192,28 @@ class TestContentParsing(RequestTestCase): data = {'qwerty': 'uiop'} content = json.dumps(data) content_type = 'application/json' - - request_class.parsers = (JSONParser,) - + parsers = (JSONParser(),) form_data = {Request._CONTENT_PARAM: content, Request._CONTENTTYPE_PARAM: content_type} - request = self.build_request('post', '/', data=form_data) + request = self.build_request('post', '/', data=form_data, parsers=parsers) self.assertEqual(request.POST.items(), form_data.items()) self.assertEqual(request.DATA.items(), data.items()) +class MockView(View): + authentication = (UserLoggedInAuthentication,) + def post(self, request): + if request.POST.get('example') is not None: + return Response(status=status.HTTP_200_OK) + + return Response(status=status.INTERNAL_SERVER_ERROR) + +urlpatterns = patterns('', + (r'^$', MockView.as_view()), +) + + class TestContentParsingWithAuthentication(TestCase): urls = 'djangorestframework.tests.request' diff --git a/djangorestframework/views.py b/djangorestframework/views.py index 8ba05e35a..761737c4d 100644 --- a/djangorestframework/views.py +++ b/djangorestframework/views.py @@ -83,12 +83,12 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): renderer_classes = renderers.DEFAULT_RENDERERS """ - List of renderers the resource can serialize the response with, ordered by preference. + List of renderer classes the resource can serialize the response with, ordered by preference. """ - parsers = parsers.DEFAULT_PARSERS + parser_classes = parsers.DEFAULT_PARSERS """ - List of parsers the resource can parse the request with. + List of parser classes the resource can parse the request with. """ authentication = (authentication.UserLoggedInAuthentication, @@ -210,7 +210,7 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): try: # Get a custom request, built form the original request instance - self.request = request = self.get_request() + request = self.prepare_request(request) # `initial` is the opportunity to temper with the request, # even completely replace it. @@ -229,7 +229,7 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): response = handler(request, *args, **kwargs) # Prepare response for the response cycle. - self.prepare_response(response) + response = self.prepare_response(response) # Pre-serialize filtering (eg filter complex objects into natively serializable types) # TODO: ugly hack to handle both HttpResponse and Response. @@ -251,7 +251,7 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): 'name': self.get_name(), 'description': self.get_description(), 'renders': self._rendered_media_types, - 'parses': request._parsed_media_types, + 'parses': self._parsed_media_types, } form = self.get_bound_form() if form is not None: