From 894bf34451d0b3ff73f75e3ea954088e9078bed8 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 24 May 2011 16:31:17 +0100 Subject: [PATCH] tidy up last bits of renderer media type handling --- djangorestframework/mixins.py | 25 +++++---- djangorestframework/response.py | 1 + djangorestframework/views.py | 93 +++++++++++++++------------------ 3 files changed, 59 insertions(+), 60 deletions(-) diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index d99b6f15b..4d1731787 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -243,21 +243,24 @@ class ResponseMixin(object): self.response = response try: - renderer = self._determine_renderer(self.request) + renderer, media_type = self._determine_renderer(self.request) except ErrorResponse, exc: renderer = self._default_renderer(self) + media_type = renderer.media_type response = exc.response + + # Set the media type of the response + # Note that the renderer *could* override it in .render() if required. + response.media_type = renderer.media_type # Serialize the response content - # TODO: renderer.media_type isn't the right thing to do here... if response.has_content_body: - content = renderer.render(response.cleaned_content, renderer.media_type) + content = renderer.render(response.cleaned_content, media_type) else: content = renderer.render() # Build the HTTP Response - # TODO: renderer.media_type isn't the right thing to do here... - resp = HttpResponse(content, mimetype=renderer.media_type, status=response.status) + resp = HttpResponse(content, mimetype=response.media_type, status=response.status) for (key, val) in response.headers.items(): resp[key] = val @@ -266,8 +269,10 @@ class ResponseMixin(object): def _determine_renderer(self, request): """ - Return the appropriate renderer for the output, given the client's 'Accept' header, - and the content types that this mixin knows how to serve. + Determines the appropriate renderer for the output, given the client's 'Accept' header, + and the :attr:`renderers` set on this class. + + Returns a 2-tuple of `(renderer, media_type)` See: RFC 2616, Section 14 - http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html """ @@ -285,19 +290,19 @@ 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) + return (self._default_renderer(self), self._default_renderer.media_type) # Check the acceptable media types against each renderer, # attempting more specific media types first # NB. The inner loop here isn't as bad as it first looks :) - # We're effectivly looping over max len(accept_list) * len(self.renderers) + # 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 renderer in renderers: for media_type in media_type_lst: if renderer.can_handle_response(media_type): - return renderer + return renderer, media_type # No acceptable renderers were found raise ErrorResponse(status.HTTP_406_NOT_ACCEPTABLE, diff --git a/djangorestframework/response.py b/djangorestframework/response.py index f6bbe3be3..d68ececf8 100644 --- a/djangorestframework/response.py +++ b/djangorestframework/response.py @@ -18,6 +18,7 @@ class Response(object): def __init__(self, status=200, content=None, headers={}): self.status = status + self.media_type = None self.has_content_body = content is not None self.raw_content = content # content prior to filtering self.cleaned_content = content # content after filtering diff --git a/djangorestframework/views.py b/djangorestframework/views.py index ade90cac2..cdb2ba33d 100644 --- a/djangorestframework/views.py +++ b/djangorestframework/views.py @@ -99,59 +99,52 @@ 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 + + # 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.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: - 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.initial(request, *args, **kwargs) - 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' - - return self.render(response) - - except: - import traceback - traceback.print_exc() - raise + # 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) + 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' + + return self.render(response) class ModelView(View):