From ebe959b52a10a88975b15c69275b0ef5c50cb9fa Mon Sep 17 00:00:00 2001 From: Karol Majta Date: Sat, 18 May 2013 16:45:05 +0200 Subject: [PATCH] charset param gets now appended to response's Content-Type. Closes #807 --- rest_framework/negotiation.py | 10 ++++- rest_framework/renderers.py | 2 +- rest_framework/response.py | 10 +++-- rest_framework/settings.py | 2 + rest_framework/tests/negotiation.py | 35 +++++++++++++++--- rest_framework/tests/response.py | 57 ++++++++++++++++++++++++++--- rest_framework/views.py | 16 ++++++-- 7 files changed, 111 insertions(+), 21 deletions(-) diff --git a/rest_framework/negotiation.py b/rest_framework/negotiation.py index 4d205c0e8..668c4e5c4 100644 --- a/rest_framework/negotiation.py +++ b/rest_framework/negotiation.py @@ -58,11 +58,17 @@ class DefaultContentNegotiation(BaseContentNegotiation): _MediaType(media_type).precedence): # Eg client requests '*/*' # Accepted media type is 'application/json' - return renderer, renderer.media_type + renderer_and_media_type = renderer, renderer.media_type else: # Eg client requests 'application/json; indent=8' # Accepted media type is 'application/json; indent=8' - return renderer, media_type + renderer_and_media_type = renderer, media_type + if renderer.charset: + charset = renderer.charset + else: + charset = self.__class__.settings.DEFAULT_CHARSET + retval = renderer_and_media_type + (charset,) + return retval raise exceptions.NotAcceptable(available_renderers=renderers) diff --git a/rest_framework/renderers.py b/rest_framework/renderers.py index 8361cd409..65d8b6864 100644 --- a/rest_framework/renderers.py +++ b/rest_framework/renderers.py @@ -36,11 +36,11 @@ class BaseRenderer(object): media_type = None format = None + charset = None def render(self, data, accepted_media_type=None, renderer_context=None): raise NotImplemented('Renderer class requires .render() to be implemented') - class JSONRenderer(BaseRenderer): """ Renderer which serializes to json. diff --git a/rest_framework/response.py b/rest_framework/response.py index 26e4ab371..40372f221 100644 --- a/rest_framework/response.py +++ b/rest_framework/response.py @@ -39,14 +39,18 @@ class Response(SimpleTemplateResponse): def rendered_content(self): renderer = getattr(self, 'accepted_renderer', None) media_type = getattr(self, 'accepted_media_type', None) + charset = getattr(self, 'charset', None) context = getattr(self, 'renderer_context', None) assert renderer, ".accepted_renderer not set on Response" assert media_type, ".accepted_media_type not set on Response" assert context, ".renderer_context not set on Response" context['response'] = self - - self['Content-Type'] = media_type + if charset is not None: + ct = "{0}; charset={1}".format(media_type, charset) + else: + ct = media_type + self['Content-Type'] = ct return renderer.render(self.data, media_type, context) @property @@ -67,4 +71,4 @@ class Response(SimpleTemplateResponse): for key in ('accepted_renderer', 'renderer_context', 'data'): if key in state: del state[key] - return state + return state \ No newline at end of file diff --git a/rest_framework/settings.py b/rest_framework/settings.py index beb511aca..255a95e21 100644 --- a/rest_framework/settings.py +++ b/rest_framework/settings.py @@ -83,6 +83,8 @@ DEFAULTS = { 'FORMAT_SUFFIX_KWARG': 'format', # Input and output formats + 'DEFAULT_CHARSET': None, + 'DATE_INPUT_FORMATS': ( ISO_8601, ), diff --git a/rest_framework/tests/negotiation.py b/rest_framework/tests/negotiation.py index 43721b842..d7ef6470a 100644 --- a/rest_framework/tests/negotiation.py +++ b/rest_framework/tests/negotiation.py @@ -3,18 +3,24 @@ from django.test import TestCase from django.test.client import RequestFactory from rest_framework.negotiation import DefaultContentNegotiation from rest_framework.request import Request +from rest_framework.renderers import BaseRenderer factory = RequestFactory() -class MockJSONRenderer(object): +class MockJSONRenderer(BaseRenderer): media_type = 'application/json' - -class MockHTMLRenderer(object): +class MockHTMLRenderer(BaseRenderer): media_type = 'text/html' +class NoCharsetSpecifiedRenderer(BaseRenderer): + media_type = 'my/media' + +class CharsetSpecifiedRenderer(BaseRenderer): + media_type = 'my/media' + charset = 'mycharset' class TestAcceptedMediaType(TestCase): def setUp(self): @@ -26,15 +32,32 @@ class TestAcceptedMediaType(TestCase): def test_client_without_accept_use_renderer(self): request = Request(factory.get('/')) - accepted_renderer, accepted_media_type = self.select_renderer(request) + accepted_renderer, accepted_media_type, charset = self.select_renderer(request) self.assertEqual(accepted_media_type, 'application/json') def test_client_underspecifies_accept_use_renderer(self): request = Request(factory.get('/', HTTP_ACCEPT='*/*')) - accepted_renderer, accepted_media_type = self.select_renderer(request) + accepted_renderer, accepted_media_type, charset = self.select_renderer(request) self.assertEqual(accepted_media_type, 'application/json') def test_client_overspecifies_accept_use_client(self): request = Request(factory.get('/', HTTP_ACCEPT='application/json; indent=8')) - accepted_renderer, accepted_media_type = self.select_renderer(request) + accepted_renderer, accepted_media_type, charset = self.select_renderer(request) self.assertEqual(accepted_media_type, 'application/json; indent=8') + +class TestCharset(TestCase): + def setUp(self): + self.renderers = [NoCharsetSpecifiedRenderer()] + self.negotiator = DefaultContentNegotiation() + + def test_returns_none_if_no_charset_set(self): + request = Request(factory.get('/')) + renderers = [NoCharsetSpecifiedRenderer()] + _, _, charset = self.negotiator.select_renderer(request, renderers) + self.assertIsNone(charset) + + def test_returns_attribute_from_renderer_if_charset_is_set(self): + request = Request(factory.get('/')) + renderers = [CharsetSpecifiedRenderer()] + _, _, charset = self.negotiator.select_renderer(request, renderers) + self.assertEquals(CharsetSpecifiedRenderer.charset, charset) \ No newline at end of file diff --git a/rest_framework/tests/response.py b/rest_framework/tests/response.py index aecf83f4e..f2a1c635f 100644 --- a/rest_framework/tests/response.py +++ b/rest_framework/tests/response.py @@ -12,7 +12,6 @@ from rest_framework.renderers import ( from rest_framework.settings import api_settings from rest_framework.compat import six - class MockPickleRenderer(BaseRenderer): media_type = 'application/pickle' @@ -20,6 +19,8 @@ class MockPickleRenderer(BaseRenderer): class MockJsonRenderer(BaseRenderer): media_type = 'application/json' +class MockTextMediaRenderer(BaseRenderer): + media_type = 'text/html' DUMMYSTATUS = status.HTTP_200_OK DUMMYCONTENT = 'dummycontent' @@ -43,14 +44,18 @@ class RendererB(BaseRenderer): def render(self, data, media_type=None, renderer_context=None): return RENDERER_B_SERIALIZER(data) +class RendererC(RendererB): + media_type = 'mock/rendererc' + format = 'formatc' + charset = "rendererc" + class MockView(APIView): - renderer_classes = (RendererA, RendererB) + renderer_classes = (RendererA, RendererB, RendererC) def get(self, request, **kwargs): return Response(DUMMYCONTENT, status=DUMMYSTATUS) - class HTMLView(APIView): renderer_classes = (BrowsableAPIRenderer, ) @@ -64,10 +69,9 @@ class HTMLView1(APIView): def get(self, request, **kwargs): return Response('text') - urlpatterns = patterns('', - url(r'^.*\.(?P.+)$', MockView.as_view(renderer_classes=[RendererA, RendererB])), - url(r'^$', MockView.as_view(renderer_classes=[RendererA, RendererB])), + url(r'^.*\.(?P.+)$', MockView.as_view(renderer_classes=[RendererA, RendererB, RendererC])), + url(r'^$', MockView.as_view(renderer_classes=[RendererA, RendererB, RendererC])), url(r'^html$', HTMLView.as_view()), url(r'^html1$', HTMLView1.as_view()), url(r'^restframework', include('rest_framework.urls', namespace='rest_framework')) @@ -173,3 +177,44 @@ class Issue122Tests(TestCase): Test if no infinite recursion occurs. """ self.client.get('/html1') + +class Issue807Testts(TestCase): + """ + Covers #807 + """ + + urls = 'rest_framework.tests.response' + + def test_does_not_append_charset_by_default(self): + """ + For backwards compatibility `REST_FRAMEWORK['DEFAULT_CHARSET']` defaults + to None, so that all legacy code works as expected. + """ + headers = {"HTTP_ACCEPT": RendererA.media_type} + resp = self.client.get('/', **headers) + self.assertEquals(RendererA.media_type, resp['Content-Type']) + + def test_if_there_is_charset_specified_on_renderer_it_gets_appended(self): + """ + If renderer class has charset attribute declared, it gets appended + to Response's Content-Type + """ + resp = self.client.get('/?format=%s' % RendererC.format) + expected = "{0}; charset={1}".format(RendererC.media_type, RendererC.charset) + self.assertEquals(expected, resp['Content-Type']) + + def test_if_there_is_default_charset_specified_it_gets_appended(self): + """ + If user defines `REST_FRAMEWORK['DEFAULT_CHARSET']` it will get appended + to Content-Type of all responses. + """ + original_default_charset = api_settings.DEFAULT_CHARSET + api_settings.DEFAULT_CHARSET = "utf-8" + headers = {'HTTP_ACCEPT': RendererA.media_type} + resp = self.client.get('/', **headers) + expected = "{0}; charset={1}".format( + RendererA.media_type, + api_settings.DEFAULT_CHARSET + ) + self.assertEquals(expected, resp['Content-Type']) + api_settings.DEFAULT_CHARSET = original_default_charset \ No newline at end of file diff --git a/rest_framework/views.py b/rest_framework/views.py index 555fa2f40..035aa6466 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -183,7 +183,9 @@ class APIView(View): return conneg.select_renderer(request, renderers, self.format_kwarg) except Exception: if force: - return (renderers[0], renderers[0].media_type) + charset = renderers[0].charset + charset = charset if charset is not None else api_settings.DEFAULT_CHARSET + return (renderers[0], renderers[0].media_type, renderers[0].charset) raise def perform_authentication(self, request): @@ -250,7 +252,10 @@ class APIView(View): # Perform content negotiation and store the accepted info on the request neg = self.perform_content_negotiation(request) - request.accepted_renderer, request.accepted_media_type = neg + renderer, media_type, charset = neg + request.accepted_renderer = renderer + request.accepted_media_type = media_type + request.accepted_charset = charset def finalize_response(self, request, response, *args, **kwargs): """ @@ -265,11 +270,16 @@ class APIView(View): if isinstance(response, Response): if not getattr(request, 'accepted_renderer', None): neg = self.perform_content_negotiation(request, force=True) - request.accepted_renderer, request.accepted_media_type = neg + renderer, media_type, charset = neg + request.accepted_renderer = renderer + request.accepted_media_type = media_type response.accepted_renderer = request.accepted_renderer response.accepted_media_type = request.accepted_media_type response.renderer_context = self.get_renderer_context() + charset = request.accepted_renderer.charset + charset = charset if charset else api_settings.DEFAULT_CHARSET + response.charset = charset for key, value in self.headers.items(): response[key] = value