From b19c58ae17ee54a3a8d193608660d96fd52f83f0 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 6 Nov 2012 10:44:19 +0000 Subject: [PATCH] Support for HTML error templates. Fixes #319. --- docs/api-guide/renderers.md | 16 +++++++ rest_framework/renderers.py | 44 ++++++++++++++++--- rest_framework/response.py | 4 +- rest_framework/tests/htmlrenderer.py | 65 ++++++++++++++++++++++++++++ rest_framework/views.py | 10 +++-- 5 files changed, 129 insertions(+), 10 deletions(-) diff --git a/docs/api-guide/renderers.md b/docs/api-guide/renderers.md index c3d12ddbb..374ff0ab2 100644 --- a/docs/api-guide/renderers.md +++ b/docs/api-guide/renderers.md @@ -257,6 +257,21 @@ In [the words of Roy Fielding][quote], "A REST API should spend almost all of it For good examples of custom media types, see GitHub's use of a custom [application/vnd.github+json] media type, and Mike Amundsen's IANA approved [application/vnd.collection+json] JSON-based hypermedia. +## HTML error views + +Typically a renderer will behave the same regardless of if it's dealing with a regular response, or with a response caused by an exception being raised, such as an `Http404` or `PermissionDenied` exception, or a subclass of `APIException`. + +If you're using either the `TemplateHTMLRenderer` or the `StaticHTMLRenderer` and an exception is raised, the behavior is slightly different, and mirrors [Django's default handling of error views][django-error-views]. + +Exceptions raised and handled by an HTML renderer will attempt to render using one of the following methods, by order of precedence. + +* Load and render a template named `{status_code}.html`. +* Load and render a template named `api_exception.html`. +* Render the HTTP status code and text, for example "404 Not Found". + +Templates will render with a `RequestContext` which includes the `status_code` and `details` keys. + + [cite]: https://docs.djangoproject.com/en/dev/ref/template-response/#the-rendering-process [conneg]: content-negotiation.md [browser-accept-headers]: http://www.gethifi.com/blog/browser-rest-http-accept-headers @@ -265,3 +280,4 @@ For good examples of custom media types, see GitHub's use of a custom [applicati [quote]: http://roy.gbiv.com/untangled/2008/rest-apis-must-be-hypertext-driven [application/vnd.github+json]: http://developer.github.com/v3/media/ [application/vnd.collection+json]: http://www.amundsen.com/media-types/collection/ +[django-error-views]: https://docs.djangoproject.com/en/dev/topics/http/views/#customizing-error-views \ No newline at end of file diff --git a/rest_framework/renderers.py b/rest_framework/renderers.py index 0a659bd1f..22fd6e740 100644 --- a/rest_framework/renderers.py +++ b/rest_framework/renderers.py @@ -10,7 +10,7 @@ import copy import string from django import forms from django.http.multipartparser import parse_header -from django.template import RequestContext, loader +from django.template import RequestContext, loader, Template from django.utils import simplejson as json from rest_framework.compat import yaml from rest_framework.exceptions import ConfigurationError @@ -162,6 +162,10 @@ class TemplateHTMLRenderer(BaseRenderer): media_type = 'text/html' format = 'html' template_name = None + exception_template_names = [ + '%(status_code)s.html', + 'api_exception.html' + ] def render(self, data, accepted_media_type=None, renderer_context=None): """ @@ -178,15 +182,21 @@ class TemplateHTMLRenderer(BaseRenderer): request = renderer_context['request'] response = renderer_context['response'] - template_names = self.get_template_names(response, view) - template = self.resolve_template(template_names) - context = self.resolve_context(data, request) + if response.exception: + template = self.get_exception_template(response) + else: + template_names = self.get_template_names(response, view) + template = self.resolve_template(template_names) + + context = self.resolve_context(data, request, response) return template.render(context) def resolve_template(self, template_names): return loader.select_template(template_names) - def resolve_context(self, data, request): + def resolve_context(self, data, request, response): + if response.exception: + data['status_code'] = response.status_code return RequestContext(request, data) def get_template_names(self, response, view): @@ -198,8 +208,21 @@ class TemplateHTMLRenderer(BaseRenderer): return view.get_template_names() raise ConfigurationError('Returned a template response with no template_name') + def get_exception_template(self, response): + template_names = [name % {'status_code': response.status_code} + for name in self.exception_template_names] -class StaticHTMLRenderer(BaseRenderer): + try: + # Try to find an appropriate error template + return self.resolve_template(template_names) + except: + # Fall back to using eg '404 Not Found' + return Template('%d %s' % (response.status_code, + response.status_text.title())) + + +# Note, subclass TemplateHTMLRenderer simply for the exception behavior +class StaticHTMLRenderer(TemplateHTMLRenderer): """ An HTML renderer class that simply returns pre-rendered HTML. @@ -216,6 +239,15 @@ class StaticHTMLRenderer(BaseRenderer): format = 'html' def render(self, data, accepted_media_type=None, renderer_context=None): + renderer_context = renderer_context or {} + response = renderer_context['response'] + + if response and response.exception: + request = renderer_context['request'] + template = self.get_exception_template(response) + context = self.resolve_context(data, request, response) + return template.render(context) + return data diff --git a/rest_framework/response.py b/rest_framework/response.py index 006d7eebb..0de01204d 100644 --- a/rest_framework/response.py +++ b/rest_framework/response.py @@ -9,7 +9,8 @@ class Response(SimpleTemplateResponse): """ def __init__(self, data=None, status=200, - template_name=None, headers=None): + template_name=None, headers=None, + exception=False): """ Alters the init arguments slightly. For example, drop 'template_name', and instead use 'data'. @@ -21,6 +22,7 @@ class Response(SimpleTemplateResponse): self.data = data self.headers = headers and headers[:] or [] self.template_name = template_name + self.exception = exception @property def rendered_content(self): diff --git a/rest_framework/tests/htmlrenderer.py b/rest_framework/tests/htmlrenderer.py index 10d7e31dc..4caed59ee 100644 --- a/rest_framework/tests/htmlrenderer.py +++ b/rest_framework/tests/htmlrenderer.py @@ -1,4 +1,6 @@ +from django.core.exceptions import PermissionDenied from django.conf.urls.defaults import patterns, url +from django.http import Http404 from django.test import TestCase from django.template import TemplateDoesNotExist, Template import django.template.loader @@ -17,8 +19,22 @@ def example(request): return Response(data, template_name='example.html') +@api_view(('GET',)) +@renderer_classes((TemplateHTMLRenderer,)) +def permission_denied(request): + raise PermissionDenied() + + +@api_view(('GET',)) +@renderer_classes((TemplateHTMLRenderer,)) +def not_found(request): + raise Http404() + + urlpatterns = patterns('', url(r'^$', example), + url(r'^permission_denied$', permission_denied), + url(r'^not_found$', not_found), ) @@ -48,3 +64,52 @@ class TemplateHTMLRendererTests(TestCase): response = self.client.get('/') self.assertContains(response, "example: foobar") self.assertEquals(response['Content-Type'], 'text/html') + + def test_not_found_html_view(self): + response = self.client.get('/not_found') + self.assertEquals(response.status_code, 404) + self.assertEquals(response.content, "404 Not Found") + self.assertEquals(response['Content-Type'], 'text/html') + + def test_permission_denied_html_view(self): + response = self.client.get('/permission_denied') + self.assertEquals(response.status_code, 403) + self.assertEquals(response.content, "403 Forbidden") + self.assertEquals(response['Content-Type'], 'text/html') + + +class TemplateHTMLRendererExceptionTests(TestCase): + urls = 'rest_framework.tests.htmlrenderer' + + def setUp(self): + """ + Monkeypatch get_template + """ + self.get_template = django.template.loader.get_template + + def get_template(template_name): + if template_name == '404.html': + return Template("404: {{ detail }}") + if template_name == '403.html': + return Template("403: {{ detail }}") + raise TemplateDoesNotExist(template_name) + + django.template.loader.get_template = get_template + + def tearDown(self): + """ + Revert monkeypatching + """ + django.template.loader.get_template = self.get_template + + def test_not_found_html_view_with_template(self): + response = self.client.get('/not_found') + self.assertEquals(response.status_code, 404) + self.assertEquals(response.content, "404: Not found") + self.assertEquals(response['Content-Type'], 'text/html') + + def test_permission_denied_html_view_with_template(self): + response = self.client.get('/permission_denied') + self.assertEquals(response.status_code, 403) + self.assertEquals(response.content, "403: Permission denied") + self.assertEquals(response['Content-Type'], 'text/html') diff --git a/rest_framework/views.py b/rest_framework/views.py index 71e1fe6c8..1afbd6974 100644 --- a/rest_framework/views.py +++ b/rest_framework/views.py @@ -320,13 +320,17 @@ class APIView(View): self.headers['X-Throttle-Wait-Seconds'] = '%d' % exc.wait if isinstance(exc, exceptions.APIException): - return Response({'detail': exc.detail}, status=exc.status_code) + return Response({'detail': exc.detail}, + status=exc.status_code, + exception=True) elif isinstance(exc, Http404): return Response({'detail': 'Not found'}, - status=status.HTTP_404_NOT_FOUND) + status=status.HTTP_404_NOT_FOUND, + exception=True) elif isinstance(exc, PermissionDenied): return Response({'detail': 'Permission denied'}, - status=status.HTTP_403_FORBIDDEN) + status=status.HTTP_403_FORBIDDEN, + exception=True) raise # Note: session based authentication is explicitly CSRF validated,