From 5bb6301b7f53e3815ab1a81a5fa38721dc95b113 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Piquemal?= Date: Thu, 2 Feb 2012 18:19:44 +0200 Subject: [PATCH] Response as a subclass of HttpResponse - first draft, not quite there yet. --- djangorestframework/authentication.py | 2 +- djangorestframework/mixins.py | 135 ++++------ djangorestframework/parsers.py | 15 +- djangorestframework/permissions.py | 14 +- djangorestframework/renderers.py | 12 +- djangorestframework/request.py | 6 +- djangorestframework/resources.py | 2 +- djangorestframework/response.py | 141 +++++++++- djangorestframework/tests/accept.py | 13 +- djangorestframework/tests/authentication.py | 5 +- djangorestframework/tests/files.py | 9 +- djangorestframework/tests/mixins.py | 47 ++-- djangorestframework/tests/renderers.py | 193 ++------------ djangorestframework/tests/request.py | 6 +- djangorestframework/tests/response.py | 281 ++++++++++++++++++-- djangorestframework/tests/reverse.py | 3 +- djangorestframework/tests/throttling.py | 3 +- djangorestframework/tests/validators.py | 20 +- djangorestframework/utils/__init__.py | 7 + djangorestframework/views.py | 68 +++-- 20 files changed, 577 insertions(+), 405 deletions(-) diff --git a/djangorestframework/authentication.py b/djangorestframework/authentication.py index f46a9c460..e326c15ae 100644 --- a/djangorestframework/authentication.py +++ b/djangorestframework/authentication.py @@ -87,7 +87,7 @@ class UserLoggedInAuthentication(BaseAuthentication): Returns a :obj:`User` if the request session currently has a logged in user. Otherwise returns :const:`None`. """ - self.view.DATA # Make sure our generic parsing runs first + request.DATA # Make sure our generic parsing runs first if getattr(request, 'user', None) and request.user.is_active: # Enforce CSRF validation for session based authentication. diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index 6c88fb640..dc2cfd277 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -6,7 +6,6 @@ classes that can be added to a `View`. from django.contrib.auth.models import AnonymousUser from django.core.paginator import Paginator from django.db.models.fields.related import ForeignKey -from django.http import HttpResponse from urlobject import URLObject from djangorestframework import status @@ -14,8 +13,7 @@ from djangorestframework.renderers import BaseRenderer from djangorestframework.resources import Resource, FormResource, ModelResource from djangorestframework.response import Response, ErrorResponse from djangorestframework.request import request_class_factory -from djangorestframework.utils import as_tuple, MSIE_USER_AGENT_REGEX -from djangorestframework.utils.mediatypes import is_form_media_type, order_by_precedence +from djangorestframework.utils import as_tuple, allowed_methods __all__ = ( @@ -34,6 +32,7 @@ __all__ = ( 'ListModelMixin' ) +#TODO: In RequestMixin and ResponseMixin : get_response_class/get_request_class are a bit ugly. Do we even want to be able to set the parameters on the view ? ########## Request Mixin ########## @@ -88,9 +87,6 @@ class ResponseMixin(object): Ignores Accept headers from Internet Explorer user agents and uses a sensible browser Accept header instead. """ - _ACCEPT_QUERY_PARAM = '_accept' # Allow override of Accept header in URL query params - _IGNORE_IE_ACCEPT_HEADER = True - renderers = () """ The set of response renderers that the view can handle. @@ -98,79 +94,27 @@ class ResponseMixin(object): Should be a tuple/list of classes as described in the :mod:`renderers` module. """ - # TODO: wrap this behavior around dispatch(), ensuring it works - # out of the box with existing Django classes that use render_to_response. - def render(self, response): + response_class = Response + + def prepare_response(self, response): """ - Takes a :obj:`Response` object and returns an :obj:`HttpResponse`. + Prepares response for the response cycle. Sets some headers, sets renderers, ... """ + if hasattr(response, 'request') and response.request is None: + response.request = self.request + # Always add these headers. + response['Allow'] = ', '.join(allowed_methods(self)) + # sample to allow caching using Vary http header + response['Vary'] = 'Authenticate, Accept' + # merge with headers possibly set at some point in the view + for name, value in self.headers.items(): + response[name] = value + # set the views renderers on the response + response.renderers = self.renderers + # TODO: must disappear + response.view = self self.response = response - - try: - 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 - if response.has_content_body: - content = renderer.render(response.cleaned_content, media_type) - else: - content = renderer.render() - - # Build the HTTP Response - resp = HttpResponse(content, mimetype=response.media_type, status=response.status) - for (key, val) in response.headers.items(): - resp[key] = val - - return resp - - def _determine_renderer(self, request): - """ - 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 - """ - - if self._ACCEPT_QUERY_PARAM and request.GET.get(self._ACCEPT_QUERY_PARAM, None): - # Use _accept parameter override - accept_list = [request.GET.get(self._ACCEPT_QUERY_PARAM)] - elif (self._IGNORE_IE_ACCEPT_HEADER and - 'HTTP_USER_AGENT' in request.META and - MSIE_USER_AGENT_REGEX.match(request.META['HTTP_USER_AGENT'])): - # Ignore MSIE's broken accept behavior and do something sensible instead - accept_list = ['text/html', '*/*'] - elif 'HTTP_ACCEPT' in request.META: - # Use standard HTTP Accept negotiation - accept_list = [token.strip() for token in request.META['HTTP_ACCEPT'].split(',')] - else: - # No accept header specified - accept_list = ['*/*'] - - # 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 :) - # Worst case is we're looping over len(accept_list) * len(self.renderers) - renderers = [renderer_cls(self) for renderer_cls in self.renderers] - - for accepted_media_type_lst in order_by_precedence(accept_list): - for renderer in renderers: - for accepted_media_type in accepted_media_type_lst: - if renderer.can_handle_response(accepted_media_type): - return renderer, accepted_media_type - - # No acceptable renderers were found - raise ErrorResponse(status.HTTP_406_NOT_ACCEPTABLE, - {'detail': 'Could not satisfy the client\'s Accept header', - 'available_types': self._rendered_media_types}) + return response @property def _rendered_media_types(self): @@ -193,6 +137,17 @@ class ResponseMixin(object): """ return self.renderers[0] + @property + def headers(self): + """ + Dictionary of headers to set on the response. + This is useful when the response doesn't exist yet, but you + want to memorize some headers to set on it when it will exist. + """ + if not hasattr(self, '_headers'): + self._headers = {} + return self._headers + ########## Auth Mixin ########## @@ -429,7 +384,7 @@ class ReadModelMixin(ModelMixin): try: self.model_instance = self.get_instance(**query_kwargs) except model.DoesNotExist: - raise ErrorResponse(status.HTTP_404_NOT_FOUND) + raise ErrorResponse(status=status.HTTP_404_NOT_FOUND) return self.model_instance @@ -468,10 +423,12 @@ class CreateModelMixin(ModelMixin): data[m2m_data[fieldname][0]] = related_item manager.through(**data).save() - headers = {} + response = Response(instance, status=status.HTTP_201_CREATED) + + # Set headers if hasattr(instance, 'get_absolute_url'): - headers['Location'] = self.resource(self).url(instance) - return Response(status.HTTP_201_CREATED, instance, headers) + response['Location'] = self.resource(self).url(instance) + return response class UpdateModelMixin(ModelMixin): @@ -492,7 +449,7 @@ class UpdateModelMixin(ModelMixin): except model.DoesNotExist: self.model_instance = model(**self.get_instance_data(model, self.CONTENT, *args, **kwargs)) self.model_instance.save() - return self.model_instance + return Response(self.model_instance) class DeleteModelMixin(ModelMixin): @@ -506,10 +463,10 @@ class DeleteModelMixin(ModelMixin): try: instance = self.get_instance(**query_kwargs) except model.DoesNotExist: - raise ErrorResponse(status.HTTP_404_NOT_FOUND, None, {}) + raise ErrorResponse(status=status.HTTP_404_NOT_FOUND) instance.delete() - return + return Response() class ListModelMixin(ModelMixin): @@ -526,7 +483,7 @@ class ListModelMixin(ModelMixin): if ordering: queryset = queryset.order_by(*ordering) - return queryset + return Response(queryset) ########## Pagination Mixins ########## @@ -613,12 +570,14 @@ class PaginatorMixin(object): try: page_num = int(self.request.GET.get('page', '1')) except ValueError: - raise ErrorResponse(status.HTTP_404_NOT_FOUND, - {'detail': 'That page contains no results'}) + raise ErrorResponse( + content={'detail': 'That page contains no results'}, + status=status.HTTP_404_NOT_FOUND) if page_num not in paginator.page_range: - raise ErrorResponse(status.HTTP_404_NOT_FOUND, - {'detail': 'That page contains no results'}) + raise ErrorResponse( + content={'detail': 'That page contains no results'}, + status=status.HTTP_404_NOT_FOUND) page = paginator.page(page_num) diff --git a/djangorestframework/parsers.py b/djangorestframework/parsers.py index e56ea0256..7732a2939 100644 --- a/djangorestframework/parsers.py +++ b/djangorestframework/parsers.py @@ -88,8 +88,9 @@ class JSONParser(BaseParser): try: return (json.load(stream), None) except ValueError, exc: - raise ErrorResponse(status.HTTP_400_BAD_REQUEST, - {'detail': 'JSON parse error - %s' % unicode(exc)}) + raise ErrorResponse( + content={'detail': 'JSON parse error - %s' % unicode(exc)}, + status=status.HTTP_400_BAD_REQUEST) if yaml: @@ -110,8 +111,9 @@ if yaml: try: return (yaml.safe_load(stream), None) except ValueError, exc: - raise ErrorResponse(status.HTTP_400_BAD_REQUEST, - {'detail': 'YAML parse error - %s' % unicode(exc)}) + raise ErrorResponse( + content={'detail': 'YAML parse error - %s' % unicode(exc)}, + status=status.HTTP_400_BAD_REQUEST) else: YAMLParser = None @@ -170,8 +172,9 @@ class MultiPartParser(BaseParser): try: django_parser = DjangoMultiPartParser(self.view.META, stream, upload_handlers) except MultiPartParserError, exc: - raise ErrorResponse(status.HTTP_400_BAD_REQUEST, - {'detail': 'multipart parse error - %s' % unicode(exc)}) + raise ErrorResponse( + content={'detail': 'multipart parse error - %s' % unicode(exc)}, + status=status.HTTP_400_BAD_REQUEST) return django_parser.parse() diff --git a/djangorestframework/permissions.py b/djangorestframework/permissions.py index dfe55ce94..bce03cabc 100644 --- a/djangorestframework/permissions.py +++ b/djangorestframework/permissions.py @@ -22,13 +22,13 @@ __all__ = ( _403_FORBIDDEN_RESPONSE = ErrorResponse( - status.HTTP_403_FORBIDDEN, - {'detail': 'You do not have permission to access this resource. ' + - 'You may need to login or otherwise authenticate the request.'}) + content={'detail': 'You do not have permission to access this resource. ' + + 'You may need to login or otherwise authenticate the request.'}, + status=status.HTTP_403_FORBIDDEN) _503_SERVICE_UNAVAILABLE = ErrorResponse( - status.HTTP_503_SERVICE_UNAVAILABLE, - {'detail': 'request was throttled'}) + content={'detail': 'request was throttled'}, + status=status.HTTP_503_SERVICE_UNAVAILABLE) class BasePermission(object): @@ -152,7 +152,7 @@ class BaseThrottle(BasePermission): self.history.insert(0, self.now) cache.set(self.key, self.history, self.duration) header = 'status=SUCCESS; next=%s sec' % self.next() - self.view.add_header('X-Throttle', header) + self.view.headers['X-Throttle'] = header def throttle_failure(self): """ @@ -160,7 +160,7 @@ class BaseThrottle(BasePermission): Raises a '503 service unavailable' response. """ header = 'status=FAILURE; next=%s sec' % self.next() - self.view.add_header('X-Throttle', header) + self.view.headers['X-Throttle'] = header raise _503_SERVICE_UNAVAILABLE def next(self): diff --git a/djangorestframework/renderers.py b/djangorestframework/renderers.py index 1ce882048..929ed0738 100644 --- a/djangorestframework/renderers.py +++ b/djangorestframework/renderers.py @@ -60,9 +60,13 @@ class BaseRenderer(object): This may be overridden to provide for other behavior, but typically you'll instead want to just set the :attr:`media_type` attribute on the class. """ - format = self.view.kwargs.get(self._FORMAT_QUERY_PARAM, None) - if format is None: + # TODO: format overriding must go out of here + format = None + if self.view is not None: + format = self.view.kwargs.get(self._FORMAT_QUERY_PARAM, None) + if format is None and self.view is not None: format = self.view.request.GET.get(self._FORMAT_QUERY_PARAM, None) + if format is not None: return format == self.format return media_type_matches(self.media_type, accept) @@ -359,8 +363,8 @@ class DocumentingTemplateRenderer(BaseRenderer): # Munge DELETE Response code to allow us to return content # (Do this *after* we've rendered the template so that we include # the normal deletion response code in the output) - if self.view.response.status == 204: - self.view.response.status = 200 + if self.view.response.status_code == 204: + self.view.response.status_code = 200 return ret diff --git a/djangorestframework/request.py b/djangorestframework/request.py index 1674167d8..ee43857ed 100644 --- a/djangorestframework/request.py +++ b/djangorestframework/request.py @@ -206,9 +206,9 @@ class Request(object): if parser.can_handle_request(content_type): return parser.parse(stream) - raise ErrorResponse(status.HTTP_415_UNSUPPORTED_MEDIA_TYPE, - {'error': 'Unsupported media type in request \'%s\'.' % - content_type}) + raise ErrorResponse(content={'error': + 'Unsupported media type in request \'%s\'.' % content_type}, + status=status.HTTP_415_UNSUPPORTED_MEDIA_TYPE) @property def _parsed_media_types(self): diff --git a/djangorestframework/resources.py b/djangorestframework/resources.py index cc338cc05..a20e477ec 100644 --- a/djangorestframework/resources.py +++ b/djangorestframework/resources.py @@ -174,7 +174,7 @@ class FormResource(Resource): detail[u'field_errors'] = field_errors # Return HTTP 400 response (BAD REQUEST) - raise ErrorResponse(400, detail) + raise ErrorResponse(content=detail, status=400) def get_form_class(self, method=None): """ diff --git a/djangorestframework/response.py b/djangorestframework/response.py index 96345cee2..4f9b3a625 100644 --- a/djangorestframework/response.py +++ b/djangorestframework/response.py @@ -5,25 +5,62 @@ into a HTTP response depending on what renderers are set on your view and als depending on the accept header of the request. """ +from django.template.response import SimpleTemplateResponse from django.core.handlers.wsgi import STATUS_CODE_TEXT +from djangorestframework.utils.mediatypes import order_by_precedence +from djangorestframework.utils import MSIE_USER_AGENT_REGEX +from djangorestframework import status + + __all__ = ('Response', 'ErrorResponse') -# TODO: remove raw_content/cleaned_content and just use content? - -class Response(object): +class Response(SimpleTemplateResponse): """ An HttpResponse that may include content that hasn't yet been serialized. """ - def __init__(self, status=200, content=None, headers=None): - self.status = status - self.media_type = None + _ACCEPT_QUERY_PARAM = '_accept' # Allow override of Accept header in URL query params + _IGNORE_IE_ACCEPT_HEADER = True + + def __init__(self, content=None, status=None, request=None, renderers=None): + """ + content is the raw content. + + The set of renderers that the response can handle. + + Should be a tuple/list of classes as described in the :mod:`renderers` module. + """ + # First argument taken by `SimpleTemplateResponse.__init__` is template_name, + # which we don't need + super(Response, self).__init__(None, status=status) + # We need to store our content in raw content to avoid overriding HttpResponse's + # `content` property + self.raw_content = content self.has_content_body = content is not None - self.raw_content = content # content prior to filtering - self.cleaned_content = content # content after filtering - self.headers = headers or {} + self.request = request + if renderers is not None: + self.renderers = renderers + # TODO: must go + self.view = None + + # TODO: wrap this behavior around dispatch(), ensuring it works + # out of the box with existing Django classes that use render_to_response. + @property + def rendered_content(self): + """ + """ + renderer, media_type = self._determine_renderer() + # TODO: renderer *could* override media_type in .render() if required. + + # Set the media type of the response + self['Content-Type'] = renderer.media_type + + # Render the response content + if self.has_content_body: + return renderer.render(self.raw_content, media_type) + return renderer.render() @property def status_text(self): @@ -33,12 +70,92 @@ class Response(object): """ return STATUS_CODE_TEXT.get(self.status, '') + def _determine_accept_list(self): + request = self.request + if request is None: + return ['*/*'] -class ErrorResponse(BaseException): + if self._ACCEPT_QUERY_PARAM and request.GET.get(self._ACCEPT_QUERY_PARAM, None): + # Use _accept parameter override + return [request.GET.get(self._ACCEPT_QUERY_PARAM)] + elif (self._IGNORE_IE_ACCEPT_HEADER and + 'HTTP_USER_AGENT' in request.META and + MSIE_USER_AGENT_REGEX.match(request.META['HTTP_USER_AGENT'])): + # Ignore MSIE's broken accept behavior and do something sensible instead + return ['text/html', '*/*'] + elif 'HTTP_ACCEPT' in request.META: + # Use standard HTTP Accept negotiation + return [token.strip() for token in request.META['HTTP_ACCEPT'].split(',')] + else: + # No accept header specified + return ['*/*'] + + def _determine_renderer(self): + """ + 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 + """ + # 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 :) + # Worst case is we're looping over len(accept_list) * len(self.renderers) + renderers = [renderer_cls(self.view) for renderer_cls in self.renderers] + + for media_type_list in order_by_precedence(self._determine_accept_list()): + for renderer in renderers: + for media_type in media_type_list: + if renderer.can_handle_response(media_type): + return renderer, media_type + + # No acceptable renderers were found + raise ErrorResponse(content={'detail': 'Could not satisfy the client\'s Accept header', + 'available_types': self._rendered_media_types}, + status=status.HTTP_406_NOT_ACCEPTABLE, + renderers=self.renderers) + + def _get_renderers(self): + """ + This just provides a default when renderers havent' been set. + """ + if hasattr(self, '_renderers'): + return self._renderers + return () + + def _set_renderers(self, value): + self._renderers = value + + renderers = property(_get_renderers, _set_renderers) + + @property + def _rendered_media_types(self): + """ + Return an list of all the media types that this response can render. + """ + return [renderer.media_type for renderer in self.renderers] + + @property + def _rendered_formats(self): + """ + Return a list of all the formats that this response can render. + """ + return [renderer.format for renderer in self.renderers] + + @property + def _default_renderer(self): + """ + Return the response's default renderer class. + """ + return self.renderers[0] + + +class ErrorResponse(Response, BaseException): """ An exception representing an Response that should be returned immediately. Any content should be serialized as-is, without being filtered. """ + pass - def __init__(self, status, content=None, headers={}): - self.response = Response(status, content=content, headers=headers) diff --git a/djangorestframework/tests/accept.py b/djangorestframework/tests/accept.py index d66f6fb03..2a02e04d2 100644 --- a/djangorestframework/tests/accept.py +++ b/djangorestframework/tests/accept.py @@ -1,6 +1,8 @@ from django.test import TestCase + from djangorestframework.compat import RequestFactory from djangorestframework.views import View +from djangorestframework.response import Response # See: http://www.useragentstring.com/ @@ -23,7 +25,7 @@ class UserAgentMungingTest(TestCase): permissions = () def get(self, request): - return {'a':1, 'b':2, 'c':3} + return Response({'a':1, 'b':2, 'c':3}) self.req = RequestFactory() self.MockView = MockView @@ -37,18 +39,22 @@ class UserAgentMungingTest(TestCase): MSIE_7_USER_AGENT): req = self.req.get('/', HTTP_ACCEPT='*/*', HTTP_USER_AGENT=user_agent) resp = self.view(req) + resp.render() self.assertEqual(resp['Content-Type'], 'text/html') - + def test_dont_rewrite_msie_accept_header(self): """Turn off _IGNORE_IE_ACCEPT_HEADER, send MSIE user agent strings and ensure that we get a JSON response if we set a */* accept header.""" - view = self.MockView.as_view(_IGNORE_IE_ACCEPT_HEADER=False) + class IgnoreIEAcceptResponse(Response): + _IGNORE_IE_ACCEPT_HEADER=False + view = self.MockView.as_view(response_class=IgnoreIEAcceptResponse) for user_agent in (MSIE_9_USER_AGENT, MSIE_8_USER_AGENT, MSIE_7_USER_AGENT): req = self.req.get('/', HTTP_ACCEPT='*/*', HTTP_USER_AGENT=user_agent) resp = view(req) + resp.render() self.assertEqual(resp['Content-Type'], 'application/json') def test_dont_munge_nice_browsers_accept_header(self): @@ -61,5 +67,6 @@ class UserAgentMungingTest(TestCase): OPERA_11_0_OPERA_USER_AGENT): req = self.req.get('/', HTTP_ACCEPT='*/*', HTTP_USER_AGENT=user_agent) resp = self.view(req) + resp.render() self.assertEqual(resp['Content-Type'], 'application/json') diff --git a/djangorestframework/tests/authentication.py b/djangorestframework/tests/authentication.py index 303bf96be..25410b040 100644 --- a/djangorestframework/tests/authentication.py +++ b/djangorestframework/tests/authentication.py @@ -3,6 +3,7 @@ from django.contrib.auth.models import User from django.test import Client, TestCase from django.utils import simplejson as json +from django.http import HttpResponse from djangorestframework.views import View from djangorestframework import permissions @@ -14,10 +15,10 @@ class MockView(View): permissions = (permissions.IsAuthenticated,) def post(self, request): - return {'a': 1, 'b': 2, 'c': 3} + return HttpResponse({'a': 1, 'b': 2, 'c': 3}) def put(self, request): - return {'a': 1, 'b': 2, 'c': 3} + return HttpResponse({'a': 1, 'b': 2, 'c': 3}) urlpatterns = patterns('', (r'^$', MockView.as_view()), diff --git a/djangorestframework/tests/files.py b/djangorestframework/tests/files.py index d3b1cc561..bbdff70bd 100644 --- a/djangorestframework/tests/files.py +++ b/djangorestframework/tests/files.py @@ -1,8 +1,11 @@ from django.test import TestCase from django import forms + from djangorestframework.compat import RequestFactory from djangorestframework.views import View from djangorestframework.resources import FormResource +from djangorestframework.response import Response + import StringIO class UploadFilesTests(TestCase): @@ -20,13 +23,13 @@ class UploadFilesTests(TestCase): form = FileForm def post(self, request, *args, **kwargs): - return {'FILE_NAME': self.CONTENT['file'].name, - 'FILE_CONTENT': self.CONTENT['file'].read()} + return Response({'FILE_NAME': self.CONTENT['file'].name, + 'FILE_CONTENT': self.CONTENT['file'].read()}) file = StringIO.StringIO('stuff') file.name = 'stuff.txt' request = self.factory.post('/', {'file': file}) view = MockView.as_view() response = view(request) - self.assertEquals(response.content, '{"FILE_CONTENT": "stuff", "FILE_NAME": "stuff.txt"}') + self.assertEquals(response.raw_content, {"FILE_CONTENT": "stuff", "FILE_NAME": "stuff.txt"}) diff --git a/djangorestframework/tests/mixins.py b/djangorestframework/tests/mixins.py index a7512efc7..7a1d27694 100644 --- a/djangorestframework/tests/mixins.py +++ b/djangorestframework/tests/mixins.py @@ -65,7 +65,7 @@ class TestModelCreation(TestModelsTestCase): response = mixin.post(request) self.assertEquals(1, Group.objects.count()) - self.assertEquals('foo', response.cleaned_content.name) + self.assertEquals('foo', response.raw_content.name) def test_creation_with_m2m_relation(self): class UserResource(ModelResource): @@ -91,8 +91,8 @@ class TestModelCreation(TestModelsTestCase): response = mixin.post(request) self.assertEquals(1, User.objects.count()) - self.assertEquals(1, response.cleaned_content.groups.count()) - self.assertEquals('foo', response.cleaned_content.groups.all()[0].name) + self.assertEquals(1, response.raw_content.groups.count()) + self.assertEquals('foo', response.raw_content.groups.all()[0].name) def test_creation_with_m2m_relation_through(self): """ @@ -114,7 +114,7 @@ class TestModelCreation(TestModelsTestCase): response = mixin.post(request) self.assertEquals(1, CustomUser.objects.count()) - self.assertEquals(0, response.cleaned_content.groups.count()) + self.assertEquals(0, response.raw_content.groups.count()) group = Group(name='foo1') group.save() @@ -129,8 +129,8 @@ class TestModelCreation(TestModelsTestCase): response = mixin.post(request) self.assertEquals(2, CustomUser.objects.count()) - self.assertEquals(1, response.cleaned_content.groups.count()) - self.assertEquals('foo1', response.cleaned_content.groups.all()[0].name) + self.assertEquals(1, response.raw_content.groups.count()) + self.assertEquals('foo1', response.raw_content.groups.all()[0].name) group2 = Group(name='foo2') group2.save() @@ -145,19 +145,19 @@ class TestModelCreation(TestModelsTestCase): response = mixin.post(request) self.assertEquals(3, CustomUser.objects.count()) - self.assertEquals(2, response.cleaned_content.groups.count()) - self.assertEquals('foo1', response.cleaned_content.groups.all()[0].name) - self.assertEquals('foo2', response.cleaned_content.groups.all()[1].name) + self.assertEquals(2, response.raw_content.groups.count()) + self.assertEquals('foo1', response.raw_content.groups.all()[0].name) + self.assertEquals('foo2', response.raw_content.groups.all()[1].name) class MockPaginatorView(PaginatorMixin, View): total = 60 def get(self, request): - return range(0, self.total) + return Response(range(0, self.total)) def post(self, request): - return Response(status.HTTP_201_CREATED, {'status': 'OK'}) + return Response({'status': 'OK'}, status=status.HTTP_201_CREATED) class TestPagination(TestCase): @@ -168,8 +168,7 @@ class TestPagination(TestCase): """ Tests if pagination works without overwriting the limit """ request = self.req.get('/paginator') response = MockPaginatorView.as_view()(request) - - content = json.loads(response.content) + content = response.raw_content self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(MockPaginatorView.total, content['total']) @@ -183,8 +182,7 @@ class TestPagination(TestCase): request = self.req.get('/paginator') response = MockPaginatorView.as_view(limit=limit)(request) - - content = json.loads(response.content) + content = response.raw_content self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(content['per_page'], limit) @@ -200,8 +198,7 @@ class TestPagination(TestCase): request = self.req.get('/paginator/?limit=%d' % limit) response = MockPaginatorView.as_view()(request) - - content = json.loads(response.content) + content = response.raw_content self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(MockPaginatorView.total, content['total']) @@ -217,8 +214,7 @@ class TestPagination(TestCase): request = self.req.get('/paginator/?limit=%d' % limit) response = MockPaginatorView.as_view()(request) - - content = json.loads(response.content) + content = response.raw_content self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(MockPaginatorView.total, content['total']) @@ -230,8 +226,7 @@ class TestPagination(TestCase): """ Pagination should only work for GET requests """ request = self.req.post('/paginator', data={'content': 'spam'}) response = MockPaginatorView.as_view()(request) - - content = json.loads(response.content) + content = response.raw_content self.assertEqual(response.status_code, status.HTTP_201_CREATED) self.assertEqual(None, content.get('per_page')) @@ -248,12 +243,12 @@ class TestPagination(TestCase): """ Tests that the page range is handle correctly """ request = self.req.get('/paginator/?page=0') response = MockPaginatorView.as_view()(request) - content = json.loads(response.content) + content = response.raw_content self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) request = self.req.get('/paginator/') response = MockPaginatorView.as_view()(request) - content = json.loads(response.content) + content = response.raw_content self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(range(0, MockPaginatorView.limit), content['results']) @@ -261,13 +256,13 @@ class TestPagination(TestCase): request = self.req.get('/paginator/?page=%d' % num_pages) response = MockPaginatorView.as_view()(request) - content = json.loads(response.content) + content = response.raw_content self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(range(MockPaginatorView.limit*(num_pages-1), MockPaginatorView.total), content['results']) request = self.req.get('/paginator/?page=%d' % (num_pages + 1,)) response = MockPaginatorView.as_view()(request) - content = json.loads(response.content) + content = response.raw_content self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) def test_existing_query_parameters_are_preserved(self): @@ -275,7 +270,7 @@ class TestPagination(TestCase): generating next/previous page links """ request = self.req.get('/paginator/?foo=bar&another=something') response = MockPaginatorView.as_view()(request) - content = json.loads(response.content) + content = response.raw_content self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertTrue('foo=bar' in content['next']) self.assertTrue('another=something' in content['next']) diff --git a/djangorestframework/tests/renderers.py b/djangorestframework/tests/renderers.py index 9a02d0a9a..461bc877a 100644 --- a/djangorestframework/tests/renderers.py +++ b/djangorestframework/tests/renderers.py @@ -1,177 +1,20 @@ import re +from django.test import TestCase + from django.conf.urls.defaults import patterns, url from django.test import TestCase -from djangorestframework import status +from djangorestframework.response import Response from djangorestframework.views import View -from djangorestframework.compat import View as DjangoView from djangorestframework.renderers import BaseRenderer, JSONRenderer, YAMLRenderer, \ XMLRenderer, JSONPRenderer, DocumentingHTMLRenderer from djangorestframework.parsers import JSONParser, YAMLParser, XMLParser -from djangorestframework.mixins import ResponseMixin -from djangorestframework.response import Response from StringIO import StringIO import datetime from decimal import Decimal -DUMMYSTATUS = status.HTTP_200_OK -DUMMYCONTENT = 'dummycontent' - -RENDERER_A_SERIALIZER = lambda x: 'Renderer A: %s' % x -RENDERER_B_SERIALIZER = lambda x: 'Renderer B: %s' % x - - -class RendererA(BaseRenderer): - media_type = 'mock/renderera' - format = "formata" - - def render(self, obj=None, media_type=None): - return RENDERER_A_SERIALIZER(obj) - - -class RendererB(BaseRenderer): - media_type = 'mock/rendererb' - format = "formatb" - - def render(self, obj=None, media_type=None): - return RENDERER_B_SERIALIZER(obj) - - -class MockView(ResponseMixin, DjangoView): - renderers = (RendererA, RendererB) - - def get(self, request, **kwargs): - response = Response(DUMMYSTATUS, DUMMYCONTENT) - return self.render(response) - - -class MockGETView(View): - - def get(self, request, **kwargs): - return {'foo': ['bar', 'baz']} - - -class HTMLView(View): - renderers = (DocumentingHTMLRenderer, ) - - def get(self, request, **kwargs): - return 'text' - - -class HTMLView1(View): - renderers = (DocumentingHTMLRenderer, JSONRenderer) - - def get(self, request, **kwargs): - return 'text' - -urlpatterns = patterns('', - url(r'^.*\.(?P.+)$', MockView.as_view(renderers=[RendererA, RendererB])), - url(r'^$', MockView.as_view(renderers=[RendererA, RendererB])), - url(r'^jsonp/jsonrenderer$', MockGETView.as_view(renderers=[JSONRenderer, JSONPRenderer])), - url(r'^jsonp/nojsonrenderer$', MockGETView.as_view(renderers=[JSONPRenderer])), - url(r'^html$', HTMLView.as_view()), - url(r'^html1$', HTMLView1.as_view()), -) - - -class RendererIntegrationTests(TestCase): - """ - End-to-end testing of renderers using an RendererMixin on a generic view. - """ - - urls = 'djangorestframework.tests.renderers' - - def test_default_renderer_serializes_content(self): - """If the Accept header is not set the default renderer should serialize the response.""" - resp = self.client.get('/') - self.assertEquals(resp['Content-Type'], RendererA.media_type) - self.assertEquals(resp.content, RENDERER_A_SERIALIZER(DUMMYCONTENT)) - self.assertEquals(resp.status_code, DUMMYSTATUS) - - def test_head_method_serializes_no_content(self): - """No response must be included in HEAD requests.""" - resp = self.client.head('/') - self.assertEquals(resp.status_code, DUMMYSTATUS) - self.assertEquals(resp['Content-Type'], RendererA.media_type) - self.assertEquals(resp.content, '') - - def test_default_renderer_serializes_content_on_accept_any(self): - """If the Accept header is set to */* the default renderer should serialize the response.""" - resp = self.client.get('/', HTTP_ACCEPT='*/*') - self.assertEquals(resp['Content-Type'], RendererA.media_type) - self.assertEquals(resp.content, RENDERER_A_SERIALIZER(DUMMYCONTENT)) - self.assertEquals(resp.status_code, DUMMYSTATUS) - - def test_specified_renderer_serializes_content_default_case(self): - """If the Accept header is set the specified renderer should serialize the response. - (In this case we check that works for the default renderer)""" - resp = self.client.get('/', HTTP_ACCEPT=RendererA.media_type) - self.assertEquals(resp['Content-Type'], RendererA.media_type) - self.assertEquals(resp.content, RENDERER_A_SERIALIZER(DUMMYCONTENT)) - self.assertEquals(resp.status_code, DUMMYSTATUS) - - def test_specified_renderer_serializes_content_non_default_case(self): - """If the Accept header is set the specified renderer should serialize the response. - (In this case we check that works for a non-default renderer)""" - resp = self.client.get('/', HTTP_ACCEPT=RendererB.media_type) - self.assertEquals(resp['Content-Type'], RendererB.media_type) - self.assertEquals(resp.content, RENDERER_B_SERIALIZER(DUMMYCONTENT)) - self.assertEquals(resp.status_code, DUMMYSTATUS) - - def test_specified_renderer_serializes_content_on_accept_query(self): - """The '_accept' query string should behave in the same way as the Accept header.""" - resp = self.client.get('/?_accept=%s' % RendererB.media_type) - self.assertEquals(resp['Content-Type'], RendererB.media_type) - self.assertEquals(resp.content, RENDERER_B_SERIALIZER(DUMMYCONTENT)) - self.assertEquals(resp.status_code, DUMMYSTATUS) - - def test_unsatisfiable_accept_header_on_request_returns_406_status(self): - """If the Accept header is unsatisfiable we should return a 406 Not Acceptable response.""" - resp = self.client.get('/', HTTP_ACCEPT='foo/bar') - self.assertEquals(resp.status_code, status.HTTP_406_NOT_ACCEPTABLE) - - def test_specified_renderer_serializes_content_on_format_query(self): - """If a 'format' query is specified, the renderer with the matching - format attribute should serialize the response.""" - resp = self.client.get('/?format=%s' % RendererB.format) - self.assertEquals(resp['Content-Type'], RendererB.media_type) - self.assertEquals(resp.content, RENDERER_B_SERIALIZER(DUMMYCONTENT)) - self.assertEquals(resp.status_code, DUMMYSTATUS) - - def test_specified_renderer_serializes_content_on_format_kwargs(self): - """If a 'format' keyword arg is specified, the renderer with the matching - format attribute should serialize the response.""" - resp = self.client.get('/something.formatb') - self.assertEquals(resp['Content-Type'], RendererB.media_type) - self.assertEquals(resp.content, RENDERER_B_SERIALIZER(DUMMYCONTENT)) - self.assertEquals(resp.status_code, DUMMYSTATUS) - - def test_specified_renderer_is_used_on_format_query_with_matching_accept(self): - """If both a 'format' query and a matching Accept header specified, - the renderer with the matching format attribute should serialize the response.""" - resp = self.client.get('/?format=%s' % RendererB.format, - HTTP_ACCEPT=RendererB.media_type) - self.assertEquals(resp['Content-Type'], RendererB.media_type) - 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) - - def test_bla(self): - resp = self.client.get('/?format=formatb', - HTTP_ACCEPT='text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8') - 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}' @@ -223,6 +66,18 @@ class JSONRendererTests(TestCase): self.assertEquals(obj, data) +class MockGETView(View): + + def get(self, request, **kwargs): + return Response({'foo': ['bar', 'baz']}) + + +urlpatterns = patterns('', + url(r'^jsonp/jsonrenderer$', MockGETView.as_view(renderers=[JSONRenderer, JSONPRenderer])), + url(r'^jsonp/nojsonrenderer$', MockGETView.as_view(renderers=[JSONPRenderer])), +) + + class JSONPRendererTests(TestCase): """ Tests specific to the JSONP Renderer @@ -391,21 +246,3 @@ class XMLRendererTestCase(TestCase): self.assertTrue(xml.endswith('')) self.assertTrue(string in xml, '%r not in %r' % (string, xml)) - -class Issue122Tests(TestCase): - """ - Tests that covers #122. - """ - urls = 'djangorestframework.tests.renderers' - - def test_only_html_renderer(self): - """ - Test if no infinite recursion occurs. - """ - resp = self.client.get('/html') - - def test_html_renderer_is_first(self): - """ - Test if no infinite recursion occurs. - """ - resp = self.client.get('/html1') diff --git a/djangorestframework/tests/request.py b/djangorestframework/tests/request.py index 6a0eae217..77a340336 100644 --- a/djangorestframework/tests/request.py +++ b/djangorestframework/tests/request.py @@ -6,7 +6,7 @@ from django.contrib.auth.models import User from django.test import TestCase, Client from djangorestframework import status from djangorestframework.authentication import UserLoggedInAuthentication -from djangorestframework.compat import RequestFactory, unittest +from djangorestframework.compat import RequestFactory from djangorestframework.mixins import RequestMixin from djangorestframework.parsers import FormParser, MultiPartParser, \ PlainTextParser, JSONParser @@ -19,9 +19,9 @@ class MockView(View): authentication = (UserLoggedInAuthentication,) def post(self, request): if request.POST.get('example') is not None: - return Response(status.HTTP_200_OK) + return Response(status=status.HTTP_200_OK) - return Response(status.INTERNAL_SERVER_ERROR) + return Response(status=status.INTERNAL_SERVER_ERROR) urlpatterns = patterns('', (r'^$', MockView.as_view()), diff --git a/djangorestframework/tests/response.py b/djangorestframework/tests/response.py index d973deb45..5a01e356f 100644 --- a/djangorestframework/tests/response.py +++ b/djangorestframework/tests/response.py @@ -1,19 +1,264 @@ -# Right now we expect this test to fail - I'm just going to leave it commented out. -# Looking forward to actually being able to raise ExpectedFailure sometime! -# -#from django.test import TestCase -#from djangorestframework.response import Response -# -# -#class TestResponse(TestCase): -# -# # Interface tests -# -# # This is mainly to remind myself that the Response interface needs to change slightly -# def test_response_interface(self): -# """Ensure the Response interface is as expected.""" -# response = Response() -# getattr(response, 'status') -# getattr(response, 'content') -# getattr(response, 'headers') +import json +from django.conf.urls.defaults import patterns, url +from django.test import TestCase + +from djangorestframework.response import Response, ErrorResponse +from djangorestframework.mixins import ResponseMixin +from djangorestframework.views import View +from djangorestframework.compat import View as DjangoView +from djangorestframework.renderers import BaseRenderer, DEFAULT_RENDERERS +from djangorestframework.compat import RequestFactory +from djangorestframework import status +from djangorestframework.renderers import BaseRenderer, JSONRenderer, YAMLRenderer, \ + XMLRenderer, JSONPRenderer, DocumentingHTMLRenderer + + +class TestResponseDetermineRenderer(TestCase): + + def get_response(self, url='', accept_list=[], renderers=[]): + request = RequestFactory().get(url, HTTP_ACCEPT=','.join(accept_list)) + return Response(request=request, renderers=renderers) + + def get_renderer_mock(self, media_type): + return type('RendererMock', (BaseRenderer,), { + 'media_type': media_type, + }) + + 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_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'] + PRenderer = self.get_renderer_mock('application/pickle') + JRenderer = self.get_renderer_mock('application/json') + + renderers = (PRenderer, JRenderer) + response = self.get_response(accept_list=accept_list, renderers=renderers) + renderer, media_type = response._determine_renderer() + self.assertEqual(media_type, 'application/pickle') + self.assertTrue(isinstance(renderer, PRenderer)) + + renderers = (JRenderer,) + response = self.get_response(accept_list=accept_list, renderers=renderers) + renderer, media_type = response._determine_renderer() + self.assertEqual(media_type, 'application/json') + self.assertTrue(isinstance(renderer, JRenderer)) + + def test_determine_renderer_no_renderer(self): + """ + Test determine renderer when no renderer can satisfy the Accept list. + """ + accept_list = ['application/json'] + PRenderer = self.get_renderer_mock('application/pickle') + + renderers = (PRenderer,) + response = self.get_response(accept_list=accept_list, renderers=renderers) + self.assertRaises(ErrorResponse, response._determine_renderer) + + +class TestResponseRenderContent(TestCase): + + def get_response(self, url='', accept_list=[], content=None): + request = RequestFactory().get(url, HTTP_ACCEPT=','.join(accept_list)) + return Response(request=request, content=content, renderers=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.render() + self.assertEqual(json.loads(response.content), content) + self.assertEqual(response['Content-Type'], content_type) + + +DUMMYSTATUS = status.HTTP_200_OK +DUMMYCONTENT = 'dummycontent' + +RENDERER_A_SERIALIZER = lambda x: 'Renderer A: %s' % x +RENDERER_B_SERIALIZER = lambda x: 'Renderer B: %s' % x + + +class RendererA(BaseRenderer): + media_type = 'mock/renderera' + format = "formata" + + def render(self, obj=None, media_type=None): + return RENDERER_A_SERIALIZER(obj) + + +class RendererB(BaseRenderer): + media_type = 'mock/rendererb' + format = "formatb" + + def render(self, obj=None, media_type=None): + return RENDERER_B_SERIALIZER(obj) + + +class MockView(ResponseMixin, DjangoView): + renderers = (RendererA, RendererB) + + def get(self, request, **kwargs): + response = Response(DUMMYCONTENT, status=DUMMYSTATUS) + return self.prepare_response(response) + + +class HTMLView(View): + renderers = (DocumentingHTMLRenderer, ) + + def get(self, request, **kwargs): + return Response('text') + + +class HTMLView1(View): + renderers = (DocumentingHTMLRenderer, JSONRenderer) + + def get(self, request, **kwargs): + return Response('text') + + +urlpatterns = patterns('', + url(r'^.*\.(?P.+)$', MockView.as_view(renderers=[RendererA, RendererB])), + url(r'^$', MockView.as_view(renderers=[RendererA, RendererB])), + url(r'^html$', HTMLView.as_view()), + url(r'^html1$', HTMLView1.as_view()), +) + + +# TODO: Clean tests bellow - remove duplicates with above, better unit testing, ... +class RendererIntegrationTests(TestCase): + """ + End-to-end testing of renderers using an ResponseMixin on a generic view. + """ + + urls = 'djangorestframework.tests.response' + + def test_default_renderer_serializes_content(self): + """If the Accept header is not set the default renderer should serialize the response.""" + resp = self.client.get('/') + self.assertEquals(resp['Content-Type'], RendererA.media_type) + self.assertEquals(resp.content, RENDERER_A_SERIALIZER(DUMMYCONTENT)) + self.assertEquals(resp.status_code, DUMMYSTATUS) + + def test_head_method_serializes_no_content(self): + """No response must be included in HEAD requests.""" + resp = self.client.head('/') + self.assertEquals(resp.status_code, DUMMYSTATUS) + self.assertEquals(resp['Content-Type'], RendererA.media_type) + self.assertEquals(resp.content, '') + + def test_default_renderer_serializes_content_on_accept_any(self): + """If the Accept header is set to */* the default renderer should serialize the response.""" + resp = self.client.get('/', HTTP_ACCEPT='*/*') + self.assertEquals(resp['Content-Type'], RendererA.media_type) + self.assertEquals(resp.content, RENDERER_A_SERIALIZER(DUMMYCONTENT)) + self.assertEquals(resp.status_code, DUMMYSTATUS) + + def test_specified_renderer_serializes_content_default_case(self): + """If the Accept header is set the specified renderer should serialize the response. + (In this case we check that works for the default renderer)""" + resp = self.client.get('/', HTTP_ACCEPT=RendererA.media_type) + self.assertEquals(resp['Content-Type'], RendererA.media_type) + self.assertEquals(resp.content, RENDERER_A_SERIALIZER(DUMMYCONTENT)) + self.assertEquals(resp.status_code, DUMMYSTATUS) + + def test_specified_renderer_serializes_content_non_default_case(self): + """If the Accept header is set the specified renderer should serialize the response. + (In this case we check that works for a non-default renderer)""" + resp = self.client.get('/', HTTP_ACCEPT=RendererB.media_type) + self.assertEquals(resp['Content-Type'], RendererB.media_type) + self.assertEquals(resp.content, RENDERER_B_SERIALIZER(DUMMYCONTENT)) + self.assertEquals(resp.status_code, DUMMYSTATUS) + + def test_specified_renderer_serializes_content_on_accept_query(self): + """The '_accept' query string should behave in the same way as the Accept header.""" + resp = self.client.get('/?_accept=%s' % RendererB.media_type) + self.assertEquals(resp['Content-Type'], RendererB.media_type) + self.assertEquals(resp.content, RENDERER_B_SERIALIZER(DUMMYCONTENT)) + self.assertEquals(resp.status_code, DUMMYSTATUS) + +# TODO: can't pass because view is a simple Django view and response is an ErrorResponse +# def test_unsatisfiable_accept_header_on_request_returns_406_status(self): +# """If the Accept header is unsatisfiable we should return a 406 Not Acceptable response.""" +# resp = self.client.get('/', HTTP_ACCEPT='foo/bar') +# self.assertEquals(resp.status_code, status.HTTP_406_NOT_ACCEPTABLE) + + def test_specified_renderer_serializes_content_on_format_query(self): + """If a 'format' query is specified, the renderer with the matching + format attribute should serialize the response.""" + resp = self.client.get('/?format=%s' % RendererB.format) + self.assertEquals(resp['Content-Type'], RendererB.media_type) + self.assertEquals(resp.content, RENDERER_B_SERIALIZER(DUMMYCONTENT)) + self.assertEquals(resp.status_code, DUMMYSTATUS) + + def test_specified_renderer_serializes_content_on_format_kwargs(self): + """If a 'format' keyword arg is specified, the renderer with the matching + format attribute should serialize the response.""" + resp = self.client.get('/something.formatb') + self.assertEquals(resp['Content-Type'], RendererB.media_type) + self.assertEquals(resp.content, RENDERER_B_SERIALIZER(DUMMYCONTENT)) + self.assertEquals(resp.status_code, DUMMYSTATUS) + + def test_specified_renderer_is_used_on_format_query_with_matching_accept(self): + """If both a 'format' query and a matching Accept header specified, + the renderer with the matching format attribute should serialize the response.""" + resp = self.client.get('/?format=%s' % RendererB.format, + HTTP_ACCEPT=RendererB.media_type) + self.assertEquals(resp['Content-Type'], RendererB.media_type) + 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) + + def test_bla(self): + resp = self.client.get('/?format=formatb', + HTTP_ACCEPT='text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8') + 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): + """ + Tests that covers #122. + """ + urls = 'djangorestframework.tests.response' + + def test_only_html_renderer(self): + """ + Test if no infinite recursion occurs. + """ + resp = self.client.get('/html') + + def test_html_renderer_is_first(self): + """ + Test if no infinite recursion occurs. + """ + resp = self.client.get('/html1') diff --git a/djangorestframework/tests/reverse.py b/djangorestframework/tests/reverse.py index 2d1ca79e6..c49caca0d 100644 --- a/djangorestframework/tests/reverse.py +++ b/djangorestframework/tests/reverse.py @@ -4,6 +4,7 @@ from django.test import TestCase from django.utils import simplejson as json from djangorestframework.views import View +from djangorestframework.response import Response class MockView(View): @@ -11,7 +12,7 @@ class MockView(View): permissions = () def get(self, request): - return reverse('another') + return Response(reverse('another')) urlpatterns = patterns('', url(r'^$', MockView.as_view()), diff --git a/djangorestframework/tests/throttling.py b/djangorestframework/tests/throttling.py index 7fdc6491a..393c3ec89 100644 --- a/djangorestframework/tests/throttling.py +++ b/djangorestframework/tests/throttling.py @@ -10,13 +10,14 @@ from djangorestframework.compat import RequestFactory from djangorestframework.views import View from djangorestframework.permissions import PerUserThrottling, PerViewThrottling, PerResourceThrottling from djangorestframework.resources import FormResource +from djangorestframework.response import Response class MockView(View): permissions = ( PerUserThrottling, ) throttle = '3/sec' def get(self, request): - return 'foo' + return Response('foo') class MockView_PerViewThrottling(MockView): permissions = ( PerViewThrottling, ) diff --git a/djangorestframework/tests/validators.py b/djangorestframework/tests/validators.py index 15d92231c..1f384b4c8 100644 --- a/djangorestframework/tests/validators.py +++ b/djangorestframework/tests/validators.py @@ -81,8 +81,8 @@ class TestNonFieldErrors(TestCase): content = {'field1': 'example1', 'field2': 'example2'} try: MockResource(view).validate_request(content, None) - except ErrorResponse, exc: - self.assertEqual(exc.response.raw_content, {'errors': [MockForm.ERROR_TEXT]}) + except ErrorResponse, response: + self.assertEqual(response.raw_content, {'errors': [MockForm.ERROR_TEXT]}) else: self.fail('ErrorResponse was not raised') @@ -154,8 +154,8 @@ class TestFormValidation(TestCase): content = {} try: validator.validate_request(content, None) - except ErrorResponse, exc: - self.assertEqual(exc.response.raw_content, {'field_errors': {'qwerty': ['This field is required.']}}) + except ErrorResponse, response: + self.assertEqual(response.raw_content, {'field_errors': {'qwerty': ['This field is required.']}}) else: self.fail('ResourceException was not raised') @@ -164,8 +164,8 @@ class TestFormValidation(TestCase): content = {'qwerty': ''} try: validator.validate_request(content, None) - except ErrorResponse, exc: - self.assertEqual(exc.response.raw_content, {'field_errors': {'qwerty': ['This field is required.']}}) + except ErrorResponse, response: + self.assertEqual(response.raw_content, {'field_errors': {'qwerty': ['This field is required.']}}) else: self.fail('ResourceException was not raised') @@ -174,8 +174,8 @@ class TestFormValidation(TestCase): content = {'qwerty': 'uiop', 'extra': 'extra'} try: validator.validate_request(content, None) - except ErrorResponse, exc: - self.assertEqual(exc.response.raw_content, {'field_errors': {'extra': ['This field does not exist.']}}) + except ErrorResponse, response: + self.assertEqual(response.raw_content, {'field_errors': {'extra': ['This field does not exist.']}}) else: self.fail('ResourceException was not raised') @@ -184,8 +184,8 @@ class TestFormValidation(TestCase): content = {'qwerty': '', 'extra': 'extra'} try: validator.validate_request(content, None) - except ErrorResponse, exc: - self.assertEqual(exc.response.raw_content, {'field_errors': {'qwerty': ['This field is required.'], + except ErrorResponse, response: + self.assertEqual(response.raw_content, {'field_errors': {'qwerty': ['This field is required.'], 'extra': ['This field does not exist.']}}) else: self.fail('ResourceException was not raised') diff --git a/djangorestframework/utils/__init__.py b/djangorestframework/utils/__init__.py index 634d0d68c..fbe55474f 100644 --- a/djangorestframework/utils/__init__.py +++ b/djangorestframework/utils/__init__.py @@ -48,6 +48,13 @@ def url_resolves(url): return True +def allowed_methods(view): + """ + Return the list of uppercased allowed HTTP methods on `view`. + """ + return [method.upper() for method in view.http_method_names if hasattr(view, method)] + + # From http://www.koders.com/python/fidB6E125C586A6F49EAC38992CF3AFDAAE35651975.aspx?s=mdef:xml #class object_dict(dict): # """object view of dict, you can diff --git a/djangorestframework/views.py b/djangorestframework/views.py index 86be4fba2..44d68641f 100644 --- a/djangorestframework/views.py +++ b/djangorestframework/views.py @@ -118,7 +118,7 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): """ Return the list of allowed HTTP methods, uppercased. """ - return [method.upper() for method in self.http_method_names if hasattr(self, method)] + return allowed_methods(self) def get_name(self): """ @@ -172,12 +172,14 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): """ Return an HTTP 405 error if an operation is called which does not have a handler method. """ - raise ErrorResponse(status.HTTP_405_METHOD_NOT_ALLOWED, - {'detail': 'Method \'%s\' not allowed on this resource.' % request.method}) + raise ErrorResponse(content= + {'detail': 'Method \'%s\' not allowed on this resource.' % request.method}, + status=status.HTTP_405_METHOD_NOT_ALLOWED) def initial(self, request, *args, **kargs): """ - Hook for any code that needs to run prior to anything else. + Returns an `HttpRequest`. This method is a hook for any code that needs to run + prior to anything else. Required if you want to do things like set `request.upload_handlers` before the authentication and dispatch handling is run. """ @@ -187,28 +189,16 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): if not (self.orig_prefix.startswith('http:') or self.orig_prefix.startswith('https:')): prefix = '%s://%s' % (request.is_secure() and 'https' or 'http', request.get_host()) set_script_prefix(prefix + self.orig_prefix) + return request def final(self, request, response, *args, **kargs): """ - Hook for any code that needs to run after everything else in the view. + Returns an `HttpResponse`. This method is a hook for any code that needs to run + after everything else in the view. """ # Restore script_prefix. set_script_prefix(self.orig_prefix) - - # Always add these headers. - response.headers['Allow'] = ', '.join(self.allowed_methods) - # sample to allow caching using Vary http header - response.headers['Vary'] = 'Authenticate, Accept' - - # merge with headers possibly set at some point in the view - response.headers.update(self.headers) - return self.render(response) - - def add_header(self, field, value): - """ - Add *field* and *value* to the :attr:`headers` attribute of the :class:`View` class. - """ - self.headers[field] = value + return response # Note: session based authentication is explicitly CSRF validated, # all other authentication is CSRF exempt. @@ -217,13 +207,14 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): self.request = request self.args = args self.kwargs = kwargs - self.headers = {} try: # Get a custom request, built form the original request instance self.request = request = self.get_request() - self.initial(request, *args, **kwargs) + # `initial` is the opportunity to temper with the request, + # even completely replace it. + self.request = request = self.initial(request, *args, **kwargs) # Authenticate and check request has the relevant permissions self._check_permissions() @@ -234,28 +225,29 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): else: handler = self.http_method_not_allowed - response_obj = handler(request, *args, **kwargs) + # TODO: should we enforce HttpResponse, like Django does ? + response = 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) + # Prepare response for the response cycle. + self.prepare_response(response) # Pre-serialize filtering (eg filter complex objects into natively serializable types) - response.cleaned_content = self.filter_response(response.raw_content) + # TODO: ugly + if hasattr(response, 'raw_content'): + response.raw_content = self.filter_response(response.raw_content) + else: + response.content = self.filter_response(response.content) - except ErrorResponse, exc: - response = exc.response + except ErrorResponse, response: + # Prepare response for the response cycle. + self.prepare_response(response) + # `final` is the last opportunity to temper with the response, or even + # completely replace it. return self.final(request, response, *args, **kwargs) def options(self, request, *args, **kwargs): - response_obj = { + content = { 'name': self.get_name(), 'description': self.get_description(), 'renders': self._rendered_media_types, @@ -266,11 +258,11 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): field_name_types = {} for name, field in form.fields.iteritems(): field_name_types[name] = field.__class__.__name__ - response_obj['fields'] = field_name_types + content['fields'] = field_name_types # Note 'ErrorResponse' is misleading, it's just any response # that should be rendered and returned immediately, without any # response filtering. - raise ErrorResponse(status.HTTP_200_OK, response_obj) + raise ErrorResponse(content=content, status=status.HTTP_200_OK) class ModelView(View):