From d07dc77e91c1f99b47915b3cef30b565f2618e82 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Fri, 5 Oct 2012 10:23:47 +0100 Subject: [PATCH] Accepted media type uses most specific of client/renderer media types. --- docs/api-guide/generic-views.md | 29 ++++++++++++--- docs/api-guide/renderers.md | 14 +++---- docs/index.md | 2 +- rest_framework/negotiation.py | 16 ++++++-- rest_framework/renderers.py | 19 +--------- rest_framework/response.py | 21 ++++------- rest_framework/tests/decorators.py | 2 +- rest_framework/tests/negotiation.py | 58 +++++++++++++++++++++++++++++ rest_framework/views.py | 2 +- 9 files changed, 112 insertions(+), 51 deletions(-) create mode 100644 rest_framework/tests/negotiation.py diff --git a/docs/api-guide/generic-views.md b/docs/api-guide/generic-views.md index b2284ae50..571cc66f2 100644 --- a/docs/api-guide/generic-views.md +++ b/docs/api-guide/generic-views.md @@ -18,24 +18,25 @@ If the generic views don't suit the needs of your API, you can drop down to usin Typically when using the generic views, you'll override the view, and set several class attributes. class UserList(generics.ListCreateAPIView): - serializer = UserSerializer model = User - permissions = (IsAdminUser,) + serializer_class = UserSerializer + permission_classes = (IsAdminUser,) paginate_by = 100 For more complex cases you might also want to override various methods on the view class. For example. class UserList(generics.ListCreateAPIView): - serializer = UserSerializer model = User - permissions = (IsAdminUser,) + serializer_class = UserSerializer + permission_classes = (IsAdminUser,) def get_paginate_by(self): """ Use smaller pagination for HTML representations. """ - if self.request.accepted_media_type == 'text/html': - return 10 + page_size_param = self.request.QUERY_PARAMS.get('page_size') + if page_size_param: + return int(page_size_param) return 100 For very simple cases you might want to pass through any class attributes using the `.as_view()` method. For example, your URLconf might include something the following entry. @@ -52,24 +53,32 @@ Used for read-only endpoints to represent a collection of model instances. Provides a `get` method handler. +Extends: [MultipleObjectBaseAPIView], [ListModelMixin] + ## ListCreateAPIView Used for read-write endpoints to represent a collection of model instances. Provides `get` and `post` method handlers. +Extends: [MultipleObjectBaseAPIView], [ListModelMixin], [CreateModelMixin] + ## RetrieveAPIView Used for read-only endpoints to represent a single model instance. Provides a `get` method handler. +Extends: [SingleObjectBaseAPIView], [RetrieveModelMixin] + ## RetrieveUpdateDestroyAPIView Used for read-write endpoints to represent a single model instance. Provides `get`, `put` and `delete` method handlers. +Extends: [SingleObjectBaseAPIView], [RetrieveModelMixin], [UpdateModelMixin], [DestroyModelMixin] + --- # Base views @@ -123,3 +132,11 @@ Provides a `.destroy(request, *args, **kwargs)` method, that implements deletion [SingleObjectMixin]: https://docs.djangoproject.com/en/dev/ref/class-based-views/mixins-single-object/ [multiple-object-mixin-classy]: http://ccbv.co.uk/projects/Django/1.4/django.views.generic.list/MultipleObjectMixin/ [single-object-mixin-classy]: http://ccbv.co.uk/projects/Django/1.4/django.views.generic.detail/SingleObjectMixin/ + +[SingleObjectBaseAPIView]: #singleobjectbaseapiview +[MultipleObjectBaseAPIView]: #multipleobjectbaseapiview +[ListModelMixin]: #listmodelmixin +[CreateModelMixin]: #createmodelmixin +[RetrieveModelMixin]: #retrievemodelmixin +[UpdateModelMixin]: #updatemodelmixin +[DestroyModelMixin]: #destroymodelmixin \ No newline at end of file diff --git a/docs/api-guide/renderers.md b/docs/api-guide/renderers.md index 134c37491..e1c83477b 100644 --- a/docs/api-guide/renderers.md +++ b/docs/api-guide/renderers.md @@ -115,7 +115,6 @@ For example: @api_view(('GET',)) @renderer_classes((TemplateHTMLRenderer, JSONRenderer)) - @template_name('list_users.html') def list_users(request): """ A view that can return JSON or HTML representations @@ -123,15 +122,16 @@ For example: """ queryset = Users.objects.filter(active=True) - if request.accepted_renderer.format == 'html': + if request.accepted_media_type == 'text/html': # TemplateHTMLRenderer takes a context dict, - # and does not require serialization. + # and additionally requiresa 'template_name'. + # It does not require serialization. data = {'users': queryset} - else: - # JSONRenderer requires serialized data as normal. - serializer = UserSerializer(instance=queryset) - data = serializer.data + return Response(data, template='list_users.html') + # JSONRenderer requires serialized data as normal. + serializer = UserSerializer(instance=queryset) + data = serializer.data return Response(data) ## Designing your media types diff --git a/docs/index.md b/docs/index.md index e301f77ad..0b49c5ecf 100644 --- a/docs/index.md +++ b/docs/index.md @@ -58,7 +58,7 @@ Note that the base URL can be whatever you want, but you must include `rest_fram ## Quickstart -**TODO** + ## Tutorial diff --git a/rest_framework/negotiation.py b/rest_framework/negotiation.py index 0d3b368cb..73ae78997 100644 --- a/rest_framework/negotiation.py +++ b/rest_framework/negotiation.py @@ -1,6 +1,6 @@ from rest_framework import exceptions from rest_framework.settings import api_settings -from rest_framework.utils.mediatypes import order_by_precedence +from rest_framework.utils.mediatypes import order_by_precedence, media_type_matches class BaseContentNegotiation(object): @@ -46,8 +46,16 @@ class DefaultContentNegotiation(object): for media_type_set in order_by_precedence(accepts): for renderer in renderers: for media_type in media_type_set: - if renderer.can_handle_media_type(media_type): - return renderer, media_type + if media_type_matches(renderer.media_type, media_type): + # Return the most specific media type as accepted. + if len(renderer.media_type) > len(media_type): + # Eg client requests '*/*' + # Accepted media type is 'application/json' + return renderer, renderer.media_type + else: + # Eg client requests 'application/json; indent=8' + # Accepted media type is 'application/json; indent=8' + return renderer, media_type raise exceptions.NotAcceptable(available_renderers=renderers) @@ -57,7 +65,7 @@ class DefaultContentNegotiation(object): so that we only negotiation against those that accept that format. """ renderers = [renderer for renderer in renderers - if renderer.can_handle_format(format)] + if renderer.format == format] if not renderers: raise exceptions.InvalidFormat(format) return renderers diff --git a/rest_framework/renderers.py b/rest_framework/renderers.py index e33fa30e9..6a95815a1 100644 --- a/rest_framework/renderers.py +++ b/rest_framework/renderers.py @@ -15,7 +15,7 @@ from rest_framework.request import clone_request from rest_framework.utils import dict2xml from rest_framework.utils import encoders from rest_framework.utils.breadcrumbs import get_breadcrumbs -from rest_framework.utils.mediatypes import get_media_type_params, add_media_type_param, media_type_matches +from rest_framework.utils.mediatypes import get_media_type_params, add_media_type_param from rest_framework import VERSION from rest_framework import serializers @@ -32,23 +32,6 @@ class BaseRenderer(object): def __init__(self, view=None): self.view = view - def can_handle_format(self, format): - return format == self.format - - def can_handle_media_type(self, media_type): - """ - Returns `True` if this renderer is able to deal with the given - media type. - - The default implementation for this function is to check the media type - argument against the media_type attribute set on the class to see if - they match. - - This may be overridden to provide for other behavior, but typically - you'll instead want to just set the `media_type` attribute on the class. - """ - return media_type_matches(self.media_type, media_type) - def render(self, obj=None, media_type=None): """ Given an object render it into a string. diff --git a/rest_framework/response.py b/rest_framework/response.py index db6bf3e24..fca631c3a 100644 --- a/rest_framework/response.py +++ b/rest_framework/response.py @@ -20,26 +20,21 @@ class Response(SimpleTemplateResponse): super(Response, self).__init__(None, status=status) self.data = data self.headers = headers and headers[:] or [] - self.renderer = renderer - # Accepted media type is the portion of the request Accept header - # that the renderer satisfied. It could be '*/*', or somthing like - # application/json; indent=4 - # - # This is NOT the value that will be returned in the 'Content-Type' - # header, but we do need to know the value in case there are - # any specific parameters which affect the rendering process. + self.accepted_renderer = renderer self.accepted_media_type = accepted_media_type @property def rendered_content(self): - assert self.renderer, "No renderer set on Response" + renderer = self.accepted_renderer - self['Content-Type'] = self.renderer.media_type + assert renderer, "No renderer set on Response" + + self['content-type'] = self.accepted_media_type if self.data is None: - return self.renderer.render() - render_media_type = self.accepted_media_type or self.renderer.media_type - return self.renderer.render(self.data, render_media_type) + return renderer.render() + + return renderer.render(self.data, self.accepted_media_type) @property def status_text(self): diff --git a/rest_framework/tests/decorators.py b/rest_framework/tests/decorators.py index 4be537864..e943d8fef 100644 --- a/rest_framework/tests/decorators.py +++ b/rest_framework/tests/decorators.py @@ -58,7 +58,7 @@ class DecoratorTestCase(TestCase): request = self.factory.get('/') response = view(request) - self.assertTrue(isinstance(response.renderer, JSONRenderer)) + self.assertTrue(isinstance(response.accepted_renderer, JSONRenderer)) def test_parser_classes(self): diff --git a/rest_framework/tests/negotiation.py b/rest_framework/tests/negotiation.py new file mode 100644 index 000000000..dd9f6a764 --- /dev/null +++ b/rest_framework/tests/negotiation.py @@ -0,0 +1,58 @@ +from django.test import TestCase +from django.test.client import RequestFactory +from rest_framework.decorators import api_view, renderer_classes +from rest_framework.negotiation import DefaultContentNegotiation +from rest_framework.response import Response + +factory = RequestFactory() + + +class MockJSONRenderer(object): + media_type = 'application/json' + + def __init__(self, view): + pass + + +class MockHTMLRenderer(object): + media_type = 'text/html' + + def __init__(self, view): + pass + + +@api_view(('GET',)) +@renderer_classes((MockJSONRenderer, MockHTMLRenderer)) +def example(request): + return Response() + + +class TestAcceptedMediaType(TestCase): + def setUp(self): + self.renderers = [MockJSONRenderer(None), MockHTMLRenderer(None)] + self.negotiator = DefaultContentNegotiation() + + def negotiate(self, request): + return self.negotiator.negotiate(request, self.renderers) + + def test_client_without_accept_use_renderer(self): + request = factory.get('/') + accepted_renderer, accepted_media_type = self.negotiate(request) + self.assertEquals(accepted_media_type, 'application/json') + + def test_client_underspecifies_accept_use_renderer(self): + request = factory.get('/', HTTP_ACCEPT='*/*') + accepted_renderer, accepted_media_type = self.negotiate(request) + self.assertEquals(accepted_media_type, 'application/json') + + def test_client_overspecifies_accept_use_client(self): + request = factory.get('/', HTTP_ACCEPT='application/json; indent=8') + accepted_renderer, accepted_media_type = self.negotiate(request) + self.assertEquals(accepted_media_type, 'application/json; indent=8') + + +class IntegrationTests(TestCase): + def test_accepted_negotiation_set_on_request(self): + request = factory.get('/', HTTP_ACCEPT='*/*') + response = example(request) + self.assertEquals(response.accepted_media_type, 'application/json') diff --git a/rest_framework/views.py b/rest_framework/views.py index 2bbdbe173..4dd0d208a 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -211,7 +211,7 @@ class APIView(View): if isinstance(response, Response): if not getattr(self, 'renderer', None): self.renderer, self.accepted_media_type = self.perform_content_negotiation(request, force=True) - response.renderer = self.renderer + response.accepted_renderer = self.renderer response.accepted_media_type = self.accepted_media_type for key, value in self.headers.items():