Simplify negotiation. Drop MSIE hacks. Etc.

8 files changed, 90 insertions, 187 deletions

diff --git a/djangorestframework/exceptions.py b/djangorestframework/exceptions.py
index 6930515d..a405a885 100644
--- a/djangorestframework/exceptions.py
+++ b/djangorestframework/exceptions.py
@@ -31,6 +31,14 @@ class PermissionDenied(APIException):
         self.detail = detail or self.default_detail
 
 
+class InvalidFormat(APIException):
+    status_code = status.HTTP_404_NOT_FOUND
+    default_detail = "Format suffix '.%s' not found."
+
+    def __init__(self, format, detail=None):
+        self.detail = (detail or self.default_detail) % format
+
+
 class MethodNotAllowed(APIException):
     status_code = status.HTTP_405_METHOD_NOT_ALLOWED
     default_detail = "Method '%s' not allowed."
diff --git a/djangorestframework/contentnegotiation.py b/djangorestframework/negotiation.py
index 1c646ca7..201d32ad 100644
--- a/djangorestframework/contentnegotiation.py
+++ b/djangorestframework/negotiation.py
@@ -1,10 +1,6 @@
 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):
@@ -24,17 +20,23 @@ class DefaultContentNegotiation(object):
         fallback renderer and media_type.
         """
         try:
-            return self._negotiate(request, renderers, format)
-        except (Http404, exceptions.NotAcceptable):
+            return self.unforced_negotiate(request, renderers, format)
+        except (exceptions.InvalidFormat, exceptions.NotAcceptable):
             if force:
                 return (renderers[0], renderers[0].media_type)
             raise
 
-    def _negotiate(self, request, renderers, format=None):
+    def unforced_negotiate(self, request, renderers, format=None):
         """
-        Actual implementation of negotiate, inside the 'force' wrapper.
+        As `.negotiate()`, but does not take the optional `force` agument,
+        or suppress exceptions.
         """
-        renderers = self.filter_renderers(renderers, format)
+        # Allow URL style format override.  eg. "?format=json
+        format = format or request.GET.get(self.settings.URL_FORMAT_OVERRIDE)
+
+        if format:
+            renderers = self.filter_renderers(renderers, format)
+
         accepts = self.get_accept_list(request)
 
         # Check the acceptable media types against each renderer,
@@ -51,42 +53,22 @@ class DefaultContentNegotiation(object):
 
     def filter_renderers(self, renderers, format):
         """
-        If there is a '.json' style format suffix, only use
-        renderers that accept that format.
+        If there is a '.json' style format suffix, filter the renderers
+        so that we only negotiation against those 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()
+            raise exceptions.InvalidFormat(format)
+        return renderers
 
     def get_accept_list(self, request):
         """
-        Given the incoming request, return a tokenised list of
-        media type strings.
-        """
-        if self.settings.URL_ACCEPT_OVERRIDE:
-            # URL style accept override.  eg.  "?accept=application/json"
-            override = request.GET.get(self.settings.URL_ACCEPT_OVERRIDE)
-            if override:
-                return [override]
-
-        if (self.settings.IGNORE_MSIE_ACCEPT_HEADER and
-            'HTTP_USER_AGENT' in request.META and
-            MSIE_USER_AGENT_REGEX.match(request.META['HTTP_USER_AGENT']) and
-            request.META.get('HTTP_X_REQUESTED_WITH', '').lower() != 'xmlhttprequest'):
-            # Ignore MSIE's broken accept behavior except for AJAX requests
-            # and do something sensible instead
-            return ['text/html', '*/*']
+        Given the incoming request, return a tokenised list of media
+        type strings.
 
-        if 'HTTP_ACCEPT' in request.META:
-            # Standard HTTP Accept negotiation
-            # Accept header specified
-            tokens = request.META['HTTP_ACCEPT'].split(',')
-            return [token.strip() for token in tokens]
-
-        # Standard HTTP Accept negotiation
-        # No accept header specified
-        return ['*/*']
+        Allows URL style accept override.  eg. "?accept=application/json"
+        """
+        header = request.META.get('HTTP_ACCEPT', '*/*')
+        header = request.GET.get(self.settings.URL_ACCEPT_OVERRIDE, header)
+        return [token.strip() for token in header.split(',')]
diff --git a/djangorestframework/response.py b/djangorestframework/response.py
index 205c0503..29034e25 100644
--- a/djangorestframework/response.py
+++ b/djangorestframework/response.py
@@ -25,7 +25,7 @@ class Response(SimpleTemplateResponse):
 
     @property
     def rendered_content(self):
-        self['Content-Type'] = self.media_type
+        self['Content-Type'] = self.renderer.media_type
         if self.data is None:
             return self.renderer.render()
         return self.renderer.render(self.data, self.media_type)
diff --git a/djangorestframework/settings.py b/djangorestframework/settings.py
index 95215712..4dec1a4d 100644
--- a/djangorestframework/settings.py
+++ b/djangorestframework/settings.py
@@ -38,7 +38,7 @@ DEFAULTS = {
     ),
     'DEFAULT_PERMISSIONS': (),
     'DEFAULT_THROTTLES': (),
-    'DEFAULT_CONTENT_NEGOTIATION': 'djangorestframework.contentnegotiation.DefaultContentNegotiation',
+    'DEFAULT_CONTENT_NEGOTIATION': 'djangorestframework.negotiation.DefaultContentNegotiation',
 
     'UNAUTHENTICATED_USER': 'django.contrib.auth.models.AnonymousUser',
     'UNAUTHENTICATED_TOKEN': None,
@@ -46,8 +46,8 @@ DEFAULTS = {
     'FORM_METHOD_OVERRIDE': '_method',
     'FORM_CONTENT_OVERRIDE': '_content',
     'FORM_CONTENTTYPE_OVERRIDE': '_content_type',
-    'URL_ACCEPT_OVERRIDE': 'accept',
-    'IGNORE_MSIE_ACCEPT_HEADER': True,
+    'URL_ACCEPT_OVERRIDE': '_accept',
+    'URL_FORMAT_OVERRIDE': 'format',
 
     'FORMAT_SUFFIX_KWARG': 'format'
 }
diff --git a/djangorestframework/tests/renderers.py b/djangorestframework/tests/renderers.py
index 718c903f..650798de 100644
--- a/djangorestframework/tests/renderers.py
+++ b/djangorestframework/tests/renderers.py
@@ -169,15 +169,6 @@ class RendererEndToEndTests(TestCase):
         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)
-
 
 _flat_repr = '{"foo": ["bar", "baz"]}'
 _indented_repr = '{\n  "foo": [\n    "bar",\n    "baz"\n  ]\n}'
diff --git a/djangorestframework/tests/response.py b/djangorestframework/tests/response.py
index cdddfff4..5083f6d2 100644
--- a/djangorestframework/tests/response.py
+++ b/djangorestframework/tests/response.py
@@ -1,4 +1,3 @@
-import json
 import unittest
 
 from django.conf.urls.defaults import patterns, url, include
@@ -6,13 +5,11 @@ from django.test import TestCase
 
 from djangorestframework.response import Response
 from djangorestframework.views import APIView
-from djangorestframework.compat import RequestFactory
-from djangorestframework import status, exceptions
+from djangorestframework import status
 from djangorestframework.renderers import (
     BaseRenderer,
     JSONRenderer,
-    DocumentingHTMLRenderer,
-    DEFAULT_RENDERERS
+    DocumentingHTMLRenderer
 )
 
 
@@ -24,126 +21,6 @@ class MockJsonRenderer(BaseRenderer):
     media_type = 'application/json'
 
 
-class TestResponseDetermineRenderer(TestCase):
-
-    def get_response(self, url='', accept_list=[], renderer_classes=[]):
-        kwargs = {}
-        if accept_list is not None:
-            kwargs['HTTP_ACCEPT'] = ','.join(accept_list)
-        request = RequestFactory().get(url, **kwargs)
-        return Response(request=request, renderer_classes=renderer_classes)
-
-    def test_determine_accept_list_accept_header(self):
-        """
-        Test that determine_accept_list takes the Accept header.
-        """
-        accept_list = ['application/pickle', 'application/json']
-        response = self.get_response(accept_list=accept_list)
-        self.assertEqual(response._determine_accept_list(), accept_list)
-
-    def test_determine_accept_list_default(self):
-        """
-        Test that determine_accept_list takes the default renderer if Accept is not specified.
-        """
-        response = self.get_response(accept_list=None)
-        self.assertEqual(response._determine_accept_list(), ['*/*'])
-
-    def test_determine_accept_list_overriden_header(self):
-        """
-        Test Accept header overriding.
-        """
-        accept_list = ['application/pickle', 'application/json']
-        response = self.get_response(url='?_accept=application/x-www-form-urlencoded',
-            accept_list=accept_list)
-        self.assertEqual(response._determine_accept_list(), ['application/x-www-form-urlencoded'])
-
-    def test_determine_renderer(self):
-        """
-        Test that right renderer is chosen, in the order of Accept list.
-        """
-        accept_list = ['application/pickle', 'application/json']
-        renderer_classes = (MockPickleRenderer, MockJsonRenderer)
-        response = self.get_response(accept_list=accept_list, renderer_classes=renderer_classes)
-        renderer, media_type = response._determine_renderer()
-        self.assertEqual(media_type, 'application/pickle')
-        self.assertTrue(isinstance(renderer, MockPickleRenderer))
-
-        renderer_classes = (MockJsonRenderer, )
-        response = self.get_response(accept_list=accept_list, renderer_classes=renderer_classes)
-        renderer, media_type = response._determine_renderer()
-        self.assertEqual(media_type, 'application/json')
-        self.assertTrue(isinstance(renderer, MockJsonRenderer))
-
-    def test_determine_renderer_default(self):
-        """
-        Test determine renderer when Accept was not specified.
-        """
-        renderer_classes = (MockPickleRenderer, )
-        response = self.get_response(accept_list=None, renderer_classes=renderer_classes)
-        renderer, media_type = response._determine_renderer()
-        self.assertEqual(media_type, '*/*')
-        self.assertTrue(isinstance(renderer, MockPickleRenderer))
-
-    def test_determine_renderer_no_renderer(self):
-        """
-        Test determine renderer when no renderer can satisfy the Accept list.
-        """
-        accept_list = ['application/json']
-        renderer_classes = (MockPickleRenderer, )
-        response = self.get_response(accept_list=accept_list, renderer_classes=renderer_classes)
-        self.assertRaises(exceptions.NotAcceptable, response._determine_renderer)
-
-
-class TestResponseRenderContent(TestCase):
-    def get_response(self, url='', accept_list=[], content=None, renderer_classes=None):
-        request = RequestFactory().get(url, HTTP_ACCEPT=','.join(accept_list))
-        return Response(request=request, content=content, renderer_classes=renderer_classes or DEFAULT_RENDERERS)
-
-    def test_render(self):
-        """
-        Test rendering simple data to json.
-        """
-        content = {'a': 1, 'b': [1, 2, 3]}
-        content_type = 'application/json'
-        response = self.get_response(accept_list=[content_type], content=content)
-        response = response.render()
-        self.assertEqual(json.loads(response.content), content)
-        self.assertEqual(response['Content-Type'], content_type)
-
-    def test_render_no_renderer(self):
-        """
-        Test rendering response when no renderer can satisfy accept.
-        """
-        content = 'bla'
-        content_type = 'weirdcontenttype'
-        response = self.get_response(accept_list=[content_type], content=content)
-        response = response.render()
-        self.assertEqual(response.status_code, 406)
-        self.assertIsNotNone(response.content)
-
-    # def test_render_renderer_raises_ImmediateResponse(self):
-    #     """
-    #     Test rendering response when renderer raises ImmediateResponse
-    #     """
-    #     class PickyJSONRenderer(BaseRenderer):
-    #         """
-    #         A renderer that doesn't make much sense, just to try
-    #         out raising an ImmediateResponse
-    #         """
-    #         media_type = 'application/json'
-
-    #         def render(self, obj=None, media_type=None):
-    #             raise ImmediateResponse({'error': '!!!'}, status=400)
-
-    #     response = self.get_response(
-    #         accept_list=['application/json'],
-    #         renderers=[PickyJSONRenderer, JSONRenderer]
-    #     )
-    #     response = response.render()
-    #     self.assertEqual(response.status_code, 400)
-    #     self.assertEqual(response.content, json.dumps({'error': '!!!'}))
-
-
 DUMMYSTATUS = status.HTTP_200_OK
 DUMMYCONTENT = 'dummycontent'
 
@@ -280,15 +157,6 @@ class RendererIntegrationTests(TestCase):
         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)
-
 
 class Issue122Tests(TestCase):
     """
diff --git a/docs/api-guide/content-negotiation.md b/docs/api-guide/content-negotiation.md
index 01895a4b..ad98de3b 100644
--- a/docs/api-guide/content-negotiation.md
+++ b/docs/api-guide/content-negotiation.md
@@ -1,3 +1,5 @@
+<a class="github" href="negotiation.py"></a>
+
 # Content negotiation
 
 > HTTP has provisions for several mechanisms for "content negotiation" - the process of selecting the best representation for a given response when there are multiple representations available.
diff --git a/docs/topics/rest-hypermedia-hateoas.md b/docs/topics/rest-hypermedia-hateoas.md
new file mode 100644
index 00000000..2bca2ab8
--- /dev/null
+++ b/docs/topics/rest-hypermedia-hateoas.md
@@ -0,0 +1,52 @@
+> You keep using that word "REST". I do not think it means what you think it means.
+>
+> &mdash; Mike Amundsen, [talking at REST fest 2012][cite].
+
+# REST, Hypermedia & HATEOAS
+
+First off, the disclaimer.  The name "Django REST framework" was choosen with a view to making sure the project would be easily found by developers.  Throughout the documentation we try to use the more simple and technically correct terminology of "Web APIs".
+
+If you are serious about designing a Hypermedia APIs, you should look to resources outside of this documentation to help inform your design choices.
+
+The following fall into the "required reading" category.
+
+* Fielding's dissertation - [Architectural Styles and
+the Design of Network-based Software Architectures][dissertation].
+* Fielding's "[REST APIs must be hypertext-driven][hypertext-driven]" blog post.
+* Leonard Richardson & Sam Ruby's [RESTful Web Services][restful-web-services].
+* Mike Amundsen's [Building Hypermedia APIs with HTML5 and Node][building-hypermedia-apis].
+* Steve Klabnik's [Designing Hypermedia APIs][designing-hypermedia-apis].
+* The [Richardson Maturity Model][maturitymodel].
+
+For a more thorough background, check out Klabnik's [Hypermedia API reading list][readinglist].
+
+# Building Hypermedia APIs with REST framework
+
+REST framework is an agnositic Web API toolkit.  It does help guide you towards building well-connected APIs, and makes it easy to design appropriate media types, but it does not strictly enforce any particular design style.
+
+### What REST framework *does* provide.
+
+It is self evident that REST framework makes it possible to build Hypermedia APIs.  The browseable API that it offers is built on HTML - the hypermedia language of the web.
+
+REST framework also includes [serialization] and [parser]/[renderer] components that make it easy to build appropriate media types, [hyperlinked relations][fields] for building well-connected systems, and great support for [content negotiation][conneg].
+
+### What REST framework *doesn't* provide.
+
+What REST framework doesn't do is give you is machine readable hypermedia formats such as [Collection+JSON][collection] by default, or the ability to auto-magically create HATEOAS style APIs.  Doing so would involve making opinionated choices about API design that should really remain outside of the framework's scope.
+
+[cite]: http://vimeo.com/channels/restfest/page:2
+[dissertation]: http://www.ics.uci.edu/~fielding/pubs/dissertation/top.htm
+[hypertext-driven]: http://roy.gbiv.com/untangled/2008/rest-apis-must-be-hypertext-driven
+[restful-web-services]: 
+[building-hypermedia-apis]: …
+[designing-hypermedia-apis]: http://designinghypermediaapis.com/
+[restisover]: http://blog.steveklabnik.com/posts/2012-02-23-rest-is-over
+[readinglist]: http://blog.steveklabnik.com/posts/2012-02-27-hypermedia-api-reading-list
+[maturitymodel]: http://martinfowler.com/articles/richardsonMaturityModel.html
+
+[collection]: http://www.amundsen.com/media-types/collection/
+[serialization]: ../api-guide/serializers.md
+[parser]: ../api-guide/parsers.md
+[renderer]: ../api-guide/renderers.md
+[fields]: ../api-guide/fields.md
+[conneg]: ../api-guide/content-negotiation.md
\ No newline at end of file