renderer API work

6 files changed, 222 insertions, 119 deletions

diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py
index 297d3f8d..0a45ef08 100644
--- a/djangorestframework/mixins.py
+++ b/djangorestframework/mixins.py
@@ -17,14 +17,16 @@ import re
 from StringIO import StringIO
 
 
-__all__ = ('RequestMixin',
-           'ResponseMixin',
-           'AuthMixin',
-           'ReadModelMixin',
-           'CreateModelMixin',
-           'UpdateModelMixin',
-           'DeleteModelMixin',
-           'ListModelMixin')
+__all__ = (
+    'RequestMixin',
+    'ResponseMixin',
+    'AuthMixin',
+    'ReadModelMixin',
+    'CreateModelMixin',
+    'UpdateModelMixin',
+    'DeleteModelMixin',
+    'ListModelMixin'
+)
 
 
 ########## Request Mixin ##########
@@ -267,7 +269,7 @@ class ResponseMixin(object):
         
         # Serialize the response content
         if response.has_content_body:
-            content = renderer(self).render(output=response.cleaned_content)
+            content = renderer(self).render(response.cleaned_content, renderer.media_type)
         else:
             content = renderer(self).render()
         
diff --git a/djangorestframework/renderers.py b/djangorestframework/renderers.py
index 78dc26b5..6c3d23e2 100644
--- a/djangorestframework/renderers.py
+++ b/djangorestframework/renderers.py
@@ -1,4 +1,5 @@
-"""Renderers are used to serialize a View's output into specific media types.
+"""
+Renderers are used to serialize a View's output into specific media types.
 django-rest-framework also provides HTML and PlainText renderers that help self-document the API,
 by serializing the output along with documentation regarding the Resource, output status and headers,
 and providing forms and links depending on the allowed methods, renderers and parsers on the Resource. 
@@ -7,64 +8,78 @@ from django import forms
 from django.conf import settings
 from django.template import RequestContext, loader
 from django.utils import simplejson as json
-from django import forms
 
-from djangorestframework.utils import dict2xml, url_resolves
+from djangorestframework import status
 from djangorestframework.compat import apply_markdown
+from djangorestframework.utils import dict2xml, url_resolves
 from djangorestframework.utils.breadcrumbs import get_breadcrumbs
 from djangorestframework.utils.description import get_name, get_description
-from djangorestframework import status
+from djangorestframework.utils.mediatypes import get_media_type_params, add_media_type_param
 
-from urllib import quote_plus
-import string
-import re
 from decimal import Decimal
+import re
+import string
+from urllib import quote_plus
+
+__all__ = (
+    'BaseRenderer',
+    'JSONRenderer',
+    'DocumentingHTMLRenderer',
+    'DocumentingXHTMLRenderer',
+    'DocumentingPlainTextRenderer',
+    'XMLRenderer'
+)
 
-# TODO: Rename verbose to something more appropriate
-# TODO: Maybe None could be handled more cleanly.  It'd be nice if it was handled by default,
-#       and only have an renderer output anything if it explicitly provides support for that.
 
 class BaseRenderer(object):
-    """All renderers must extend this class, set the media_type attribute, and
-    override the render() function."""
+    """
+    All renderers must extend this class, set the media_type attribute, and
+    override the render() function.
+    """
     media_type = None
 
     def __init__(self, view):
         self.view = view
 
-    def render(self, output=None, verbose=False):
-        """By default render simply returns the ouput as-is.
-        Override this method to provide for other behaviour."""
-        if output is None:
+    def render(self, obj=None, media_type=None):
+        """
+        By default render simply returns the ouput as-is.
+        Override this method to provide for other behavior.
+        """
+        if obj is None:
             return ''
         
-        return output
+        return obj
 
 
 class TemplateRenderer(BaseRenderer):
-    """A Base class provided for convenience.
+    """
+    A Base class provided for convenience.
 
-    Render the output simply by using the given template.
+    Render the object simply by using the given template.
     To create a template renderer, subclass this, and set
-    the ``media_type`` and ``template`` attributes"""
+    the ``media_type`` and ``template`` attributes
+    """
     media_type = None
     template = None
 
-    def render(self, output=None, verbose=False):
-        if output is None:
+    def render(self, obj=None, media_type=None):
+        if obj is None:
             return ''
 
-        context = RequestContext(self.request, output)
+        context = RequestContext(self.request, obj)
         return self.template.render(context)
 
 
 class DocumentingTemplateRenderer(BaseRenderer):
-    """Base class for renderers used to self-document the API.
-    Implementing classes should extend this class and set the template attribute."""
+    """
+    Base class for renderers used to self-document the API.
+    Implementing classes should extend this class and set the template attribute.
+    """
     template = None
 
-    def _get_content(self, resource, request, output):
-        """Get the content as if it had been renderted by a non-documenting renderer.
+    def _get_content(self, resource, request, obj, media_type):
+        """Get the content as if it had been rendered by a non-documenting renderer.
 
         (Typically this will be the content as it would have been if the Resource had been
         requested with an 'Accept: */*' header, although with verbose style formatting if appropriate.)"""
@@ -73,8 +88,9 @@ class DocumentingTemplateRenderer(BaseRenderer):
         renderers = [renderer for renderer in resource.renderers if not isinstance(renderer, DocumentingTemplateRenderer)]
         if not renderers:
             return '[No renderers were found]'
-        
-        content = renderers[0](resource).render(output, verbose=True)
+
+        media_type = add_media_type_param(media_type, 'indent', '4')
+        content = renderers[0](resource).render(obj, media_type)
         if not all(char in string.printable for char in content):
             return '[%d bytes of binary content]'
             
@@ -149,8 +165,8 @@ class DocumentingTemplateRenderer(BaseRenderer):
         return GenericContentForm(resource)
 
 
-    def render(self, output=None):
-        content = self._get_content(self.view, self.view.request, output)
+    def render(self, obj=None, media_type=None):
+        content = self._get_content(self.view, self.view.request, obj, media_type)
         form_instance = self._get_form_instance(self.view)
 
         if url_resolves(settings.LOGIN_URL) and url_resolves(settings.LOGOUT_URL):
@@ -194,46 +210,63 @@ class DocumentingTemplateRenderer(BaseRenderer):
 
 
 class JSONRenderer(BaseRenderer):
-    """Renderer which serializes to JSON"""
+    """
+    Renderer which serializes to JSON
+    """
     media_type = 'application/json'
 
-    def render(self, output=None, verbose=False):
-        if output is None:
+    def render(self, obj=None, media_type=None):
+        if obj is None:
             return ''
-        if verbose:
-            return json.dumps(output, indent=4, sort_keys=True)
-        return json.dumps(output)
+
+        indent = get_media_type_params(media_type).get('indent', None)
+        if indent is not None:
+            try:
+                indent = int(indent)
+            except ValueError:
+                indent = None
+
+        sort_keys = indent and True or False
+        return json.dumps(obj, indent=indent, sort_keys=sort_keys)
 
 
 class XMLRenderer(BaseRenderer):
-    """Renderer which serializes to XML."""
+    """
+    Renderer which serializes to XML.
+    """
     media_type = 'application/xml'
 
-    def render(self, output=None, verbose=False):
-        if output is None:
+    def render(self, obj=None, media_type=None):
+        if obj is None:
             return ''
-        return dict2xml(output)
+        return dict2xml(obj)
 
 
 class DocumentingHTMLRenderer(DocumentingTemplateRenderer):
-    """Renderer which provides a browsable HTML interface for an API.
-    See the examples listed in the django-rest-framework documentation to see this in actions."""
+    """
+    Renderer which provides a browsable HTML interface for an API.
+    See the examples at http://api.django-rest-framework.org to see this in action.
+    """
     media_type = 'text/html'
     template = 'renderer.html'
 
 
 class DocumentingXHTMLRenderer(DocumentingTemplateRenderer):
-    """Identical to DocumentingHTMLRenderer, except with an xhtml media type.
+    """
+    Identical to DocumentingHTMLRenderer, except with an xhtml media type.
     We need this to be listed in preference to xml in order to return HTML to WebKit based browsers,
-    given their Accept headers."""
+    given their Accept headers.
+    """
     media_type = 'application/xhtml+xml'
     template = 'renderer.html'
 
 
 class DocumentingPlainTextRenderer(DocumentingTemplateRenderer):
-    """Renderer that serializes the output with the default renderer, but also provides plain-text
-    doumentation of the returned status and headers, and of the resource's name and description.
-    Useful for browsing an API with command line tools."""
+    """
+    Renderer that serializes the object with the default renderer, but also provides plain-text
+    documentation of the returned status and headers, and of the resource's name and description.
+    Useful for browsing an API with command line tools.
+    """
     media_type = 'text/plain'
     template = 'renderer.txt'
 
diff --git a/djangorestframework/tests/renderers.py b/djangorestframework/tests/renderers.py
index df0d9c8d..fcc405a1 100644
--- a/djangorestframework/tests/renderers.py
+++ b/djangorestframework/tests/renderers.py
@@ -2,9 +2,10 @@ from django.conf.urls.defaults import patterns, url
 from django import http
 from django.test import TestCase
 from djangorestframework.compat import View
-from djangorestframework.renderers import BaseRenderer
+from djangorestframework.renderers import BaseRenderer, JSONRenderer
 from djangorestframework.mixins import ResponseMixin
 from djangorestframework.response import Response
+from djangorestframework.utils.mediatypes import add_media_type_param
 
 DUMMYSTATUS = 200
 DUMMYCONTENT = 'dummycontent'
@@ -20,14 +21,14 @@ class MockView(ResponseMixin, View):
 class RendererA(BaseRenderer):
     media_type = 'mock/renderera'
 
-    def render(self, output, verbose=False):
-        return RENDERER_A_SERIALIZER(output)
+    def render(self, obj=None, content_type=None):
+        return RENDERER_A_SERIALIZER(obj)
 
 class RendererB(BaseRenderer):
     media_type = 'mock/rendererb'
 
-    def render(self, output, verbose=False):
-        return RENDERER_B_SERIALIZER(output)
+    def render(self, obj=None, content_type=None):
+        return RENDERER_B_SERIALIZER(obj)
 
 
 urlpatterns = patterns('',
@@ -36,7 +37,9 @@ urlpatterns = patterns('',
 
 
 class RendererIntegrationTests(TestCase):
-    """End-to-end testing of renderers using an RendererMixin on a generic view."""
+    """
+    End-to-end testing of renderers using an RendererMixin on a generic view.
+    """
 
     urls = 'djangorestframework.tests.renderers'
 
@@ -73,4 +76,32 @@ class RendererIntegrationTests(TestCase):
     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)
\ No newline at end of file
+        self.assertEquals(resp.status_code, 406)
+
+
+
+_flat_repr = '{"foo": ["bar", "baz"]}'
+
+_indented_repr = """{
+  "foo": [
+    "bar", 
+    "baz"
+  ]
+}"""
+
+
+class JSONRendererTests(TestCase):
+    """
+    Tests specific to the JSON Renderer
+    """
+    def test_without_content_type_args(self):
+        obj = {'foo':['bar','baz']}
+        renderer = JSONRenderer(None)
+        content = renderer.render(obj, 'application/json')
+        self.assertEquals(content, _flat_repr)
+
+    def test_with_content_type_args(self):
+        obj = {'foo':['bar','baz']}
+        renderer = JSONRenderer(None)
+        content = renderer.render(obj, 'application/json; indent=2')
+        self.assertEquals(content, _indented_repr)
diff --git a/djangorestframework/utils/__init__.py b/djangorestframework/utils/__init__.py
index 9dc769be..67870001 100644
--- a/djangorestframework/utils/__init__.py
+++ b/djangorestframework/utils/__init__.py
@@ -16,7 +16,15 @@ import xml.etree.ElementTree as ET
 MSIE_USER_AGENT_REGEX = re.compile(r'^Mozilla/[0-9]+\.[0-9]+ \([^)]*; MSIE [0-9]+\.[0-9]+[a-z]?;[^)]*\)(?!.* Opera )')
 
 def as_tuple(obj):
-    """Given obj return a tuple"""
+    """
+    Given an object which may be a list/tuple, another object, or None,
+    return that object in list form.
+
+    IE:
+    If the object is already a list/tuple just return it.
+    If the object is not None, return it in a list with a single element.
+    If the object is None return an empty list.
+    """
     if obj is None:
         return ()
     elif isinstance(obj, list):
@@ -27,7 +35,9 @@ def as_tuple(obj):
 
   
 def url_resolves(url):
-    """Return True if the given URL is mapped to a view in the urlconf, False otherwise."""
+    """
+    Return True if the given URL is mapped to a view in the urlconf, False otherwise.
+    """
     try:
         resolve(url)
     except:
diff --git a/djangorestframework/utils/mediatypes.py b/djangorestframework/utils/mediatypes.py
index 3bf914e4..62a5e6f3 100644
--- a/djangorestframework/utils/mediatypes.py
+++ b/djangorestframework/utils/mediatypes.py
@@ -15,7 +15,7 @@ def media_type_matches(lhs, rhs):
 
     Valid media type strings include:
 
-    'application/json indent=4'
+    'application/json; indent=4'
     'application/json'
     'text/*'
     '*/*'
@@ -33,10 +33,28 @@ def is_form_media_type(media_type):
     media_type = _MediaType(media_type)
     return media_type.full_type == 'application/x-www-form-urlencoded' or \
            media_type.full_type == 'multipart/form-data'
-  
-               
+
+
+def add_media_type_param(media_type, key, val):
+    """
+    Add a key, value parameter to a media type string, and return the new media type string.
+    """
+    media_type = _MediaType(media_type)
+    media_type.params[key] = val
+    return str(media_type)
+
+def get_media_type_params(media_type):
+    """
+    Return a dictionary of the parameters on the given media type.
+    """
+    return _MediaType(media_type).params
+
+
+
 class _MediaType(object):
     def __init__(self, media_type_str):
+        if media_type_str is None:
+            media_type_str = ''
         self.orig = media_type_str
         self.full_type, self.params = parse_header(media_type_str)
         self.main_type, sep, self.sub_type = self.full_type.partition('/')
@@ -94,5 +112,8 @@ class _MediaType(object):
         return unicode(self).encode('utf-8')
 
     def __unicode__(self):
-        return self.orig
+        ret = "%s/%s" % (self.main_type, self.sub_type)
+        for key, val in self.params.items():
+            ret += "; %s=%s" % (key, val)
+        return ret
 
diff --git a/djangorestframework/views.py b/djangorestframework/views.py
index 02251885..3ce4e1d6 100644
--- a/djangorestframework/views.py
+++ b/djangorestframework/views.py
@@ -7,11 +7,13 @@ from djangorestframework.mixins import *
 from djangorestframework import resource, renderers, parsers, authentication, permissions, validators, status
 
 
-__all__ = ('BaseView',
-           'ModelView',
-           'InstanceModelView',
-           'ListOrModelView',
-           'ListOrCreateModelView')
+__all__ = (
+    'BaseView',
+    'ModelView',
+    'InstanceModelView',
+    'ListOrModelView',
+    'ListOrCreateModelView'
+)
 
 
 
@@ -78,55 +80,59 @@ class BaseView(RequestMixin, ResponseMixin, AuthMixin, View):
     # 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:
-            # If using a form POST with '_method'/'_content'/'_content_type' overrides, then alter
-            # self.method, self.content_type, self.RAW_CONTENT & self.CONTENT appropriately.
-            self.perform_form_overloading()
-
-            # Authenticate and check request is 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 Response, or an object, or None
-            if 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.resource.object_to_serializable(response.raw_content)
+            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:
+                # If using a form POST with '_method'/'_content'/'_content_type' overrides, then alter
+                # self.method, self.content_type, self.RAW_CONTENT & self.CONTENT appropriately.
+                self.perform_form_overloading()
+    
+                # Authenticate and check request is 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)
     
-        except ErrorResponse, exc:
-            response = exc.response
+                # Allow return value to be either Response, or an object, or None
+                if 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.resource.object_to_serializable(response.raw_content)
+        
+            except ErrorResponse, exc:
+                response = exc.response
+            except:
+                import traceback
+                traceback.print_exc()
+    
+            # 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()
-
-        # 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)