Allow .json .html .xml style urls and also allow these formats to be specified in a "?format=..." query string.

6 files changed, 146 insertions, 64 deletions

diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py
index 910d06ae..1b3aa241 100644
--- a/djangorestframework/mixins.py
+++ b/djangorestframework/mixins.py
@@ -11,6 +11,7 @@ from django.http.multipartparser import LimitBytes
 
 from djangorestframework import status
 from djangorestframework.parsers import FormParser, MultiPartParser
+from djangorestframework.renderers import BaseRenderer
 from djangorestframework.resources import Resource, FormResource, ModelResource
 from djangorestframework.response import Response, ErrorResponse
 from djangorestframework.utils import as_tuple, MSIE_USER_AGENT_REGEX
@@ -290,7 +291,7 @@ 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), self._default_renderer.media_type)
+            accept_list = ['*/*']
 
         # Check the acceptable media types against each renderer,
         # attempting more specific media types first
@@ -298,12 +299,12 @@ class ResponseMixin(object):
         #     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 accepted_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, media_type
-       
+                for accepted_media_type in accepted_media_type_lst:
+                    if renderer.can_handle_response(accepted_media_type):
+                        return renderer, accepted_media_type
+
         # No acceptable renderers were found
         raise ErrorResponse(status.HTTP_406_NOT_ACCEPTABLE,
                                 {'detail': 'Could not satisfy the client\'s Accept header',
@@ -316,6 +317,13 @@ class ResponseMixin(object):
         Return an list of all the media types that this view can render.
         """
         return [renderer.media_type for renderer in self.renderers]
+    
+    @property
+    def _rendered_formats(self):
+        """
+        Return a list of all the formats that this view can render.
+        """
+        return [renderer.format for renderer in self.renderers]
 
     @property
     def _default_renderer(self):
@@ -486,7 +494,10 @@ class ReadModelMixin(object):
                 instance = model.objects.get(pk=args[-1], **kwargs)
             else:
                 # Otherwise assume the kwargs uniquely identify the model
-                instance = model.objects.get(**kwargs)
+                filtered_keywords = kwargs.copy()
+                if BaseRenderer._FORMAT_QUERY_PARAM in filtered_keywords:
+                    del filtered_keywords[BaseRenderer._FORMAT_QUERY_PARAM]
+                instance = model.objects.get(**filtered_keywords)
         except model.DoesNotExist:
             raise ErrorResponse(status.HTTP_404_NOT_FOUND)
 
diff --git a/djangorestframework/renderers.py b/djangorestframework/renderers.py
index 13cd52f5..e09e2abc 100644
--- a/djangorestframework/renderers.py
+++ b/djangorestframework/renderers.py
@@ -40,8 +40,11 @@ class BaseRenderer(object):
     All renderers must extend this class, set the :attr:`media_type` attribute,
     and override the :meth:`render` method.
     """
+    
+    _FORMAT_QUERY_PARAM = 'format'
 
     media_type = None
+    format = None
 
     def __init__(self, view):
         self.view = view
@@ -58,6 +61,11 @@ class BaseRenderer(object):
         This may be overridden to provide for other behavior, but typically you'll
         instead want to just set the :attr:`media_type` attribute on the class.
         """
+        format = self.view.kwargs.get(self._FORMAT_QUERY_PARAM, None)
+        if format is None:
+            format = self.view.request.GET.get(self._FORMAT_QUERY_PARAM, None)
+        if format is not None:
+            return format == self.format
         return media_type_matches(self.media_type, accept)
 
     def render(self, obj=None, media_type=None):
@@ -84,6 +92,7 @@ class JSONRenderer(BaseRenderer):
     """
 
     media_type = 'application/json'
+    format = 'json'
 
     def render(self, obj=None, media_type=None):
         """
@@ -111,6 +120,7 @@ class XMLRenderer(BaseRenderer):
     """
 
     media_type = 'application/xml'
+    format = 'xml'
 
     def render(self, obj=None, media_type=None):
         """
@@ -289,12 +299,12 @@ class DocumentingTemplateRenderer(BaseRenderer):
             'version': VERSION,
             'markeddown': markeddown,
             'breadcrumblist': breadcrumb_list,
-            'available_media_types': self.view._rendered_media_types,
+            'available_formats': self.view._rendered_formats,
             'put_form': put_form_instance,
             'post_form': post_form_instance,
             'login_url': login_url,
             'logout_url': logout_url,
-            'ACCEPT_PARAM': getattr(self.view, '_ACCEPT_QUERY_PARAM', None),
+            'FORMAT_PARAM': self._FORMAT_QUERY_PARAM,
             'METHOD_PARAM': getattr(self.view, '_METHOD_PARAM', None),
             'ADMIN_MEDIA_PREFIX': settings.ADMIN_MEDIA_PREFIX
         })
@@ -317,6 +327,7 @@ class DocumentingHTMLRenderer(DocumentingTemplateRenderer):
     """
 
     media_type = 'text/html'
+    format = 'html'
     template = 'renderer.html'
 
 
@@ -328,6 +339,7 @@ class DocumentingXHTMLRenderer(DocumentingTemplateRenderer):
     """
 
     media_type = 'application/xhtml+xml'
+    format = 'xhtml'
     template = 'renderer.html'
 
 
@@ -339,6 +351,7 @@ class DocumentingPlainTextRenderer(DocumentingTemplateRenderer):
     """
 
     media_type = 'text/plain'
+    format = 'txt'
     template = 'renderer.txt'
 
 
diff --git a/djangorestframework/runtests/settings.py b/djangorestframework/runtests/settings.py
index 0cc7f4e3..71664f8b 100644
--- a/djangorestframework/runtests/settings.py
+++ b/djangorestframework/runtests/settings.py
@@ -2,6 +2,7 @@
 
 DEBUG = True
 TEMPLATE_DEBUG = DEBUG
+DEBUG_PROPAGATE_EXCEPTIONS = True
 
 ADMINS = (
     # ('Your Name', 'your_email@domain.com'),
diff --git a/djangorestframework/templates/renderer.html b/djangorestframework/templates/renderer.html
index 97d3837a..5b32d1ec 100644
--- a/djangorestframework/templates/renderer.html
+++ b/djangorestframework/templates/renderer.html
@@ -48,9 +48,9 @@
 				<h2>GET {{ name }}</h2>
 				<div class='submit-row' style='margin: 0; border: 0'>
 				<a href='{{ request.get_full_path }}' rel="nofollow" style='float: left'>GET</a>
-				{% for media_type in available_media_types %}
-				  {% with ACCEPT_PARAM|add:"="|add:media_type as param %}
-				    [<a href='{{ request.get_full_path|add_query_param:param }}' rel="nofollow">{{ media_type }}</a>]
+				{% for format in available_formats %}
+				  {% with FORMAT_PARAM|add:"="|add:format as param %}
+				    [<a href='{{ request.get_full_path|add_query_param:param }}' rel="nofollow">{{ format }}</a>]
 				  {% endwith %}
 				{% endfor %}
 				</div>
@@ -122,4 +122,4 @@
 	</div>
 	</div>
   </body>
-</html>
\ No newline at end of file
+</html>
diff --git a/djangorestframework/tests/renderers.py b/djangorestframework/tests/renderers.py
index 569eb640..bf135e55 100644
--- a/djangorestframework/tests/renderers.py
+++ b/djangorestframework/tests/renderers.py
@@ -2,6 +2,7 @@ from django.conf.urls.defaults import patterns, url
 from django import http
 from django.test import TestCase
 
+from djangorestframework import status
 from djangorestframework.compat import View as DjangoView
 from djangorestframework.renderers import BaseRenderer, JSONRenderer
 from djangorestframework.parsers import JSONParser
@@ -11,7 +12,7 @@ from djangorestframework.utils.mediatypes import add_media_type_param
 
 from StringIO import StringIO
 
-DUMMYSTATUS = 200
+DUMMYSTATUS = status.HTTP_200_OK
 DUMMYCONTENT = 'dummycontent'
 
 RENDERER_A_SERIALIZER = lambda x: 'Renderer A: %s' % x
@@ -19,12 +20,14 @@ RENDERER_B_SERIALIZER = lambda x: 'Renderer B: %s' % x
 
 class RendererA(BaseRenderer):
     media_type = 'mock/renderera'
+    format="formata"
 
     def render(self, obj=None, media_type=None):
         return RENDERER_A_SERIALIZER(obj)
 
 class RendererB(BaseRenderer):
     media_type = 'mock/rendererb'
+    format="formatb"
 
     def render(self, obj=None, media_type=None):
         return RENDERER_B_SERIALIZER(obj)
@@ -32,11 +35,13 @@ class RendererB(BaseRenderer):
 class MockView(ResponseMixin, DjangoView):
     renderers = (RendererA, RendererB)
 
-    def get(self, request):
+    def get(self, request, **kwargs):
         response = Response(DUMMYSTATUS, DUMMYCONTENT)
         return self.render(response)
+    
 
 urlpatterns = patterns('',
+    url(r'^.*\.(?P<format>.+)$', MockView.as_view(renderers=[RendererA, RendererB])),
     url(r'^$', MockView.as_view(renderers=[RendererA, RendererB])),
 )
 
@@ -85,10 +90,58 @@ class RendererIntegrationTests(TestCase):
         self.assertEquals(resp.content, RENDERER_B_SERIALIZER(DUMMYCONTENT))
         self.assertEquals(resp.status_code, DUMMYSTATUS)
     
+    def test_specified_renderer_serializes_content_on_accept_query(self):
+        """The '_accept' query string should behave in the same way as the Accept header."""
+        resp = self.client.get('/?_accept=%s' % RendererB.media_type)
+        self.assertEquals(resp['Content-Type'], RendererB.media_type)
+        self.assertEquals(resp.content, RENDERER_B_SERIALIZER(DUMMYCONTENT))
+        self.assertEquals(resp.status_code, DUMMYSTATUS)
+
     def test_unsatisfiable_accept_header_on_request_returns_406_status(self):
         """If the Accept header is unsatisfiable we should return a 406 Not Acceptable response."""
         resp = self.client.get('/', HTTP_ACCEPT='foo/bar')
-        self.assertEquals(resp.status_code, 406)
+        self.assertEquals(resp.status_code, status.HTTP_406_NOT_ACCEPTABLE)
+
+    def test_specified_renderer_serializes_content_on_format_query(self):
+        """If a 'format' query is specified, the renderer with the matching
+        format attribute should serialize the response."""
+        resp = self.client.get('/?format=%s' % RendererB.format)
+        self.assertEquals(resp['Content-Type'], RendererB.media_type)
+        self.assertEquals(resp.content, RENDERER_B_SERIALIZER(DUMMYCONTENT))
+        self.assertEquals(resp.status_code, DUMMYSTATUS)
+
+    def test_specified_renderer_serializes_content_on_format_kwargs(self):
+        """If a 'format' keyword arg is specified, the renderer with the matching
+        format attribute should serialize the response."""
+        resp = self.client.get('/something.formatb')
+        self.assertEquals(resp['Content-Type'], RendererB.media_type)
+        self.assertEquals(resp.content, RENDERER_B_SERIALIZER(DUMMYCONTENT))
+        self.assertEquals(resp.status_code, DUMMYSTATUS)
+
+    def test_specified_renderer_is_used_on_format_query_with_matching_accept(self):
+        """If both a 'format' query and a matching Accept header specified,
+        the renderer with the matching format attribute should serialize the response."""
+        resp = self.client.get('/?format=%s' % RendererB.format,
+                               HTTP_ACCEPT=RendererB.media_type)
+        self.assertEquals(resp['Content-Type'], RendererB.media_type)
+        self.assertEquals(resp.content, RENDERER_B_SERIALIZER(DUMMYCONTENT))
+        self.assertEquals(resp.status_code, DUMMYSTATUS)
+
+    def test_conflicting_format_query_and_accept_ignores_accept(self):
+        """If a 'format' query is specified that does not match the Accept
+        header, we should only honor the 'format' query string."""
+        resp = self.client.get('/?format=%s' % RendererB.format,
+                               HTTP_ACCEPT='dummy')
+        self.assertEquals(resp['Content-Type'], RendererB.media_type)
+        self.assertEquals(resp.content, RENDERER_B_SERIALIZER(DUMMYCONTENT))
+        self.assertEquals(resp.status_code, DUMMYSTATUS)
+
+    def test_bla(self):
+        resp = self.client.get('/?format=formatb',
+            HTTP_ACCEPT='text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8')
+        self.assertEquals(resp['Content-Type'], RendererB.media_type)
+        self.assertEquals(resp.content, RENDERER_B_SERIALIZER(DUMMYCONTENT))
+        self.assertEquals(resp.status_code, DUMMYSTATUS)
 
 _flat_repr = '{"foo": ["bar", "baz"]}'
 
diff --git a/djangorestframework/views.py b/djangorestframework/views.py
index 18d064e1..e419ab51 100644
--- a/djangorestframework/views.py
+++ b/djangorestframework/views.py
@@ -113,57 +113,61 @@ 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
-        self.headers = {}
-
-        # 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.request = request
+            self.args = args
+            self.kwargs = kwargs
+            self.headers = {}
     
-        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'
-        
-        # merge with headers possibly set at some point in the view
-        response.headers.update(self.headers)
+            # 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)
         
-        return self.render(response)    
-
+            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'
+            
+            # merge with headers possibly set at some point in the view
+            response.headers.update(self.headers)
+            
+            return self.render(response)    
+        except:
+            import traceback
+            traceback.print_exc()
+            raise
 
 class ModelView(View):
     """A RESTful view that maps to a model in the database."""