tidy up last bits of renderer media type handling

3 files changed, 59 insertions, 60 deletions

diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py
index d99b6f15..4d173178 100644
--- a/djangorestframework/mixins.py
+++ b/djangorestframework/mixins.py
@@ -243,21 +243,24 @@ class ResponseMixin(object):
         self.response = response
 
         try:
-            renderer = self._determine_renderer(self.request)
+            renderer, media_type = self._determine_renderer(self.request)
         except ErrorResponse, exc:
             renderer = self._default_renderer(self)
+            media_type = renderer.media_type
             response = exc.response
+
+        # Set the media type of the response
+        # Note that the renderer *could* override it in .render() if required.
+        response.media_type = renderer.media_type
         
         # Serialize the response content
-        # TODO: renderer.media_type isn't the right thing to do here...
         if response.has_content_body:
-            content = renderer.render(response.cleaned_content, renderer.media_type)
+            content = renderer.render(response.cleaned_content, media_type)
         else:
             content = renderer.render()
 
         # Build the HTTP Response
-        # TODO: renderer.media_type isn't the right thing to do here...
-        resp = HttpResponse(content, mimetype=renderer.media_type, status=response.status)
+        resp = HttpResponse(content, mimetype=response.media_type, status=response.status)
         for (key, val) in response.headers.items():
             resp[key] = val
 
@@ -266,8 +269,10 @@ class ResponseMixin(object):
 
     def _determine_renderer(self, request):
         """
-        Return the appropriate renderer for the output, given the client's 'Accept' header,
-        and the content types that this mixin knows how to serve.
+        Determines the appropriate renderer for the output, given the client's 'Accept' header,
+        and the :attr:`renderers` set on this class.
+
+        Returns a 2-tuple of `(renderer, media_type)`
 
         See: RFC 2616, Section 14 - http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html
         """
@@ -285,19 +290,19 @@ class ResponseMixin(object):
             accept_list = [token.strip() for token in request.META["HTTP_ACCEPT"].split(',')]
         else:
             # No accept header specified
-            return self._default_renderer(self)
+            return (self._default_renderer(self), self._default_renderer.media_type)
 
         # Check the acceptable media types against each renderer,
         # attempting more specific media types first
         # NB. The inner loop here isn't as bad as it first looks :)
-        #     We're effectivly looping over max len(accept_list) * len(self.renderers)
+        #     Worst case is we're looping over len(accept_list) * len(self.renderers)
         renderers = [renderer_cls(self) for renderer_cls in self.renderers]
 
         for media_type_lst in order_by_precedence(accept_list):
             for renderer in renderers:
                 for media_type in media_type_lst:
                     if renderer.can_handle_response(media_type):
-                        return renderer
+                        return renderer, media_type
        
         # No acceptable renderers were found
         raise ErrorResponse(status.HTTP_406_NOT_ACCEPTABLE,
diff --git a/djangorestframework/response.py b/djangorestframework/response.py
index f6bbe3be..d68ececf 100644
--- a/djangorestframework/response.py
+++ b/djangorestframework/response.py
@@ -18,6 +18,7 @@ class Response(object):
 
     def __init__(self, status=200, content=None, headers={}):
         self.status = status
+        self.media_type = None
         self.has_content_body = content is not None
         self.raw_content = content      # content prior to filtering
         self.cleaned_content = content  # content after filtering
diff --git a/djangorestframework/views.py b/djangorestframework/views.py
index ade90cac..cdb2ba33 100644
--- a/djangorestframework/views.py
+++ b/djangorestframework/views.py
@@ -99,59 +99,52 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView):
     # all other authentication is CSRF exempt.
     @csrf_exempt
     def dispatch(self, request, *args, **kwargs):
+        self.request = request
+        self.args = args
+        self.kwargs = kwargs
+
+        # Calls to 'reverse' will not be fully qualified unless we set the scheme/host/port here.
+        prefix = '%s://%s' % (request.is_secure() and 'https' or 'http', request.get_host())
+        set_script_prefix(prefix)
+
         try:
-            self.request = request
-            self.args = args
-            self.kwargs = kwargs
-    
-            # Calls to 'reverse' will not be fully qualified unless we set the scheme/host/port here.
-            prefix = '%s://%s' % (request.is_secure() and 'https' or 'http', request.get_host())
-            set_script_prefix(prefix)
-    
-            try:
-                self.initial(request, *args, **kwargs)
-    
-                # Authenticate and check request has the relevant permissions
-                self._check_permissions()
-    
-                # Get the appropriate handler method
-                if self.method.lower() in self.http_method_names:
-                    handler = getattr(self, self.method.lower(), self.http_method_not_allowed)
-                else:
-                    handler = self.http_method_not_allowed
-    
-                response_obj = handler(request, *args, **kwargs)
-    
-                # Allow return value to be either HttpResponse, Response, or an object, or None
-                if isinstance(response_obj, HttpResponse):
-                    return response_obj
-                elif isinstance(response_obj, Response):
-                    response = response_obj
-                elif response_obj is not None:
-                    response = Response(status.HTTP_200_OK, response_obj)
-                else:
-                    response = Response(status.HTTP_204_NO_CONTENT)
-    
-                # Pre-serialize filtering (eg filter complex objects into natively serializable types)
-                response.cleaned_content = self.filter_response(response.raw_content)
+            self.initial(request, *args, **kwargs)
         
-            except ErrorResponse, exc:
-                response = exc.response
-    
-            # Always add these headers.
-            #
-            # TODO - this isn't actually the correct way to set the vary header,
-            # also it's currently sub-obtimal for HTTP caching - need to sort that out. 
-            response.headers['Allow'] = ', '.join(self.allowed_methods)
-            response.headers['Vary'] = 'Authenticate, Accept'
-    
-            return self.render(response)
-                    
-        except:
-            import traceback
-            traceback.print_exc()
-            raise
+            # Authenticate and check request has the relevant permissions
+            self._check_permissions()
+
+            # Get the appropriate handler method
+            if self.method.lower() in self.http_method_names:
+                handler = getattr(self, self.method.lower(), self.http_method_not_allowed)
+            else:
+                handler = self.http_method_not_allowed
+
+            response_obj = handler(request, *args, **kwargs)
+
+            # Allow return value to be either HttpResponse, Response, or an object, or None
+            if isinstance(response_obj, HttpResponse):
+                return response_obj
+            elif isinstance(response_obj, Response):
+                response = response_obj
+            elif response_obj is not None:
+                response = Response(status.HTTP_200_OK, response_obj)
+            else:
+                response = Response(status.HTTP_204_NO_CONTENT)
+
+            # Pre-serialize filtering (eg filter complex objects into natively serializable types)
+            response.cleaned_content = self.filter_response(response.raw_content)
     
+        except ErrorResponse, exc:
+            response = exc.response
+        
+        # Always add these headers.
+        #
+        # TODO - this isn't actually the correct way to set the vary header,
+        # also it's currently sub-obtimal for HTTP caching - need to sort that out. 
+        response.headers['Allow'] = ', '.join(self.allowed_methods)
+        response.headers['Vary'] = 'Authenticate, Accept'
+
+        return self.render(response)    
 
 
 class ModelView(View):