cleaned Request/Response/mixins to have similar interface

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):
+    request_class = Request
+    """
+    The class to use as a wrapper for the original request object.
+    """
+
+    def get_parsers(self):
         """
-        Returns a subclass of Django's `HttpRequest` with a richer API,
-        as described in :mod:`request`.
+        Instantiates and returns the list of parsers that will be used by the request
+        to parse its content.
         """
-        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
+        if not hasattr(self, '_parsers'):
+            self._parsers = [r(self) for r in self.parser_classes]
+        return self._parsers
 
-    def get_request(self):
+    def prepare_request(self, request):
         """
-        Returns a custom request instance, with data and attributes copied from the
-        original request.
+        Prepares the request for the request cycle. Returns a custom request instance,
+        with data and attributes copied from the original request.
         """
-        request_class = self.get_request_class()
-        return request_class(self.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
@@ -125,6 +128,17 @@ class ResponseMixin(object):
         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):
         """
         Return an list of all the media types that this view can render.
@@ -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 ##########
 

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)},

djangorestframework/request.py

 """
 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<Request.DATA>`
+    - content automatically parsed according to `Content-Type` header, and available as :meth:`.DATA<Request.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)

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'
 

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: