Cleaner content negotiation. Occurs after permissions. Optional 'force' flag.

2 files changed, 38 insertions, 26 deletions

diff --git a/djangorestframework/contentnegotiation.py b/djangorestframework/contentnegotiation.py
index 223919ef..1c646ca7 100644
--- a/djangorestframework/contentnegotiation.py
+++ b/djangorestframework/contentnegotiation.py
@@ -1,24 +1,40 @@
 from djangorestframework import exceptions
 from djangorestframework.settings import api_settings
 from djangorestframework.utils.mediatypes import order_by_precedence
+from django.http import Http404
 import re
 
 MSIE_USER_AGENT_REGEX = re.compile(r'^Mozilla/[0-9]+\.[0-9]+ \([^)]*; MSIE [0-9]+\.[0-9]+[a-z]?;[^)]*\)(?!.* Opera )')
 
 
 class BaseContentNegotiation(object):
-    def determine_renderer(self, request, renderers):
-        raise NotImplementedError('.determine_renderer() must be implemented')
+    def negotiate(self, request, renderers, format=None, force=False):
+        raise NotImplementedError('.negotiate() must be implemented')
 
 
 class DefaultContentNegotiation(object):
     settings = api_settings
 
-    def negotiate(self, request, renderers):
+    def negotiate(self, request, renderers, format=None, force=False):
         """
         Given a request and a list of renderers, return a two-tuple of:
         (renderer, media type).
+
+        If force is set, then suppress exceptions, and forcibly return a
+        fallback renderer and media_type.
+        """
+        try:
+            return self._negotiate(request, renderers, format)
+        except (Http404, exceptions.NotAcceptable):
+            if force:
+                return (renderers[0], renderers[0].media_type)
+            raise
+
+    def _negotiate(self, request, renderers, format=None):
+        """
+        Actual implementation of negotiate, inside the 'force' wrapper.
         """
+        renderers = self.filter_renderers(renderers, format)
         accepts = self.get_accept_list(request)
 
         # Check the acceptable media types against each renderer,
@@ -33,6 +49,19 @@ class DefaultContentNegotiation(object):
 
         raise exceptions.NotAcceptable(available_renderers=renderers)
 
+    def filter_renderers(self, renderers, format):
+        """
+        If there is a '.json' style format suffix, only use
+        renderers that accept that format.
+        """
+        if not format:
+            return renderers
+
+        renderers = [renderer for renderer in renderers
+                     if renderer.can_handle_format(format)]
+        if not renderers:
+            raise Http404()
+
     def get_accept_list(self, request):
         """
         Given the incoming request, return a tokenised list of
diff --git a/djangorestframework/views.py b/djangorestframework/views.py
index d1330866..32d403ea 100644
--- a/djangorestframework/views.py
+++ b/djangorestframework/views.py
@@ -197,24 +197,13 @@ class APIView(_View):
         """
         return [throttle(self) for throttle in self.throttle_classes]
 
-    def content_negotiation(self, request):
+    def content_negotiation(self, request, force=False):
         """
         Determine which renderer and media type to use render the response.
         """
         renderers = self.get_renderers()
-
-        if self.format:
-            # If there is a '.json' style format suffix, only use
-            # renderers that accept that format.
-            fallback = renderers[0]
-            renderers = [renderer for renderer in renderers
-                         if renderer.can_handle_format(self.format)]
-            if not renderers:
-                self.format404 = True
-                return (fallback, fallback.media_type)
-
         conneg = self.content_negotiation_class()
-        return conneg.negotiate(request, renderers)
+        return conneg.negotiate(request, renderers, self.format, force)
 
     def check_permissions(self, request, obj=None):
         """
@@ -244,19 +233,17 @@ class APIView(_View):
         Runs anything that needs to occur prior to calling the method handlers.
         """
         self.format = self.get_format_suffix(**kwargs)
-        self.renderer, self.media_type = self.content_negotiation(request)
         self.check_permissions(request)
         self.check_throttles(request)
-        # If the request included a non-existant .format URL suffix,
-        # raise 404, but only after first making permission checks.
-        if getattr(self, 'format404', None):
-            raise Http404()
+        self.renderer, self.media_type = self.content_negotiation(request)
 
     def finalize_response(self, request, response, *args, **kwargs):
         """
         Returns the final response object.
         """
         if isinstance(response, Response):
+            if not getattr(self, 'renderer', None):
+                self.renderer, self.media_type = self.content_negotiation(request, force=True)
             response.renderer = self.renderer
             response.media_type = self.media_type
 
@@ -270,11 +257,7 @@ class APIView(_View):
         Handle any exception that occurs, by returning an appropriate response,
         or re-raising the error.
         """
-        if isinstance(exc, exceptions.NotAcceptable):
-            # Fall back to default renderer
-            self.renderer = exc.available_renderers[0]
-            self.media_type = exc.available_renderers[0].media_type
-        elif isinstance(exc, exceptions.Throttled):
+        if isinstance(exc, exceptions.Throttled):
             # Throttle wait header
             self.headers['X-Throttle-Wait-Seconds'] = '%d' % exc.wait