From ab0b72a7c1a17c6d85530514caa51ce5bd77b592 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Piquemal?= Date: Sun, 22 Jan 2012 21:28:34 +0200 Subject: [PATCH 01/12] .DATA, .FILES, overloaded HTTP method, content type and content available directly on the request - see #128 --- djangorestframework/authentication.py | 4 +- djangorestframework/mixins.py | 165 +++------------- djangorestframework/parsers.py | 5 +- djangorestframework/renderers.py | 16 +- djangorestframework/request.py | 217 +++++++++++++++++++++ djangorestframework/tests/content.py | 233 ----------------------- djangorestframework/tests/methods.py | 32 ---- djangorestframework/tests/renderers.py | 2 +- djangorestframework/tests/request.py | 250 +++++++++++++++++++++++++ djangorestframework/views.py | 13 +- 10 files changed, 510 insertions(+), 427 deletions(-) create mode 100644 djangorestframework/request.py delete mode 100644 djangorestframework/tests/content.py create mode 100644 djangorestframework/tests/request.py diff --git a/djangorestframework/authentication.py b/djangorestframework/authentication.py index b61af32a2..20a5f34a7 100644 --- a/djangorestframework/authentication.py +++ b/djangorestframework/authentication.py @@ -95,8 +95,8 @@ class UserLoggedInAuthentication(BaseAuthentication): # Temporarily replace request.POST with .DATA, to use our generic parsing. # If DATA is not dict-like, use an empty dict. if request.method.upper() == 'POST': - if hasattr(self.view.DATA, 'get'): - request._post = self.view.DATA + if hasattr(request.DATA, 'get'): + request._post = request.DATA else: request._post = {} diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index 7f0870f85..d016b0f12 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -14,11 +14,10 @@ from djangorestframework import status 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 StringIO import StringIO - __all__ = ( # Base behavior mixins @@ -56,150 +55,28 @@ class RequestMixin(object): Should be a tuple/list of classes as described in the :mod:`parsers` module. """ - @property - def method(self): + def get_request_class(self): """ - Returns the HTTP method. - - This should be used instead of just reading :const:`request.method`, as it allows the `method` - to be overridden by using a hidden `form` field on a form POST request. + Returns a custom subclass of Django's `HttpRequest`, providing new facilities + such as direct access to the parsed request content. """ - if not hasattr(self, '_method'): - self._load_method_and_content_type() - return self._method + if not hasattr(self, '_request_class'): + self._request_class = request_class_factory(self.request) + self._request_class._USE_FORM_OVERLOADING = self._USE_FORM_OVERLOADING + self._request_class._METHOD_PARAM = self._METHOD_PARAM + self._request_class._CONTENTTYPE_PARAM = self._CONTENTTYPE_PARAM + self._request_class._CONTENT_PARAM = self._CONTENT_PARAM + self._request_class.parsers = self.parsers + return self._request_class - @property - def content_type(self): + def get_request(self): """ - Returns the content type header. - - This should be used instead of ``request.META.get('HTTP_CONTENT_TYPE')``, - as it allows the content type to be overridden by using a hidden form - field on a form POST request. + Returns a custom request instance, with data and attributes copied from the + original request. """ - if not hasattr(self, '_content_type'): - self._load_method_and_content_type() - return self._content_type - - @property - def DATA(self): - """ - Parses the request body and returns the data. - - Similar to ``request.POST``, except that it handles arbitrary parsers, - and also works on methods other than POST (eg PUT). - """ - if not hasattr(self, '_data'): - self._load_data_and_files() - return self._data - - @property - def FILES(self): - """ - Parses the request body and returns the files. - Similar to ``request.FILES``, except that it handles arbitrary parsers, - and also works on methods other than POST (eg PUT). - """ - if not hasattr(self, '_files'): - self._load_data_and_files() - return self._files - - def _load_data_and_files(self): - """ - Parse the request content into self.DATA and self.FILES. - """ - if not hasattr(self, '_content_type'): - self._load_method_and_content_type() - - if not hasattr(self, '_data'): - (self._data, self._files) = self._parse(self._get_stream(), self._content_type) - - def _load_method_and_content_type(self): - """ - Set the method and content_type, and then check if they've been overridden. - """ - self._method = self.request.method - self._content_type = self.request.META.get('HTTP_CONTENT_TYPE', self.request.META.get('CONTENT_TYPE', '')) - self._perform_form_overloading() - - def _get_stream(self): - """ - Returns an object that may be used to stream the request content. - """ - request = self.request - - try: - content_length = int(request.META.get('CONTENT_LENGTH', request.META.get('HTTP_CONTENT_LENGTH'))) - except (ValueError, TypeError): - content_length = 0 - - # TODO: Add 1.3's LimitedStream to compat and use that. - # NOTE: Currently only supports parsing request body as a stream with 1.3 - if content_length == 0: - return None - elif hasattr(request, 'read'): - return request - return StringIO(request.raw_post_data) - - def _perform_form_overloading(self): - """ - If this is a form POST request, then we need to check if the method and content/content_type have been - overridden by setting them in hidden form fields or not. - """ - - # We only need to use form overloading on form POST requests. - if not self._USE_FORM_OVERLOADING or self._method != 'POST' or not is_form_media_type(self._content_type): - return - - # At this point we're committed to parsing the request as form data. - self._data = data = self.request.POST.copy() - self._files = self.request.FILES - - # Method overloading - change the method and remove the param from the content. - if self._METHOD_PARAM in data: - # NOTE: unlike `get`, `pop` on a `QueryDict` seems to return a list of values. - self._method = self._data.pop(self._METHOD_PARAM)[0].upper() - - # Content overloading - modify the content type, and re-parse. - if self._CONTENT_PARAM in data and self._CONTENTTYPE_PARAM in data: - self._content_type = self._data.pop(self._CONTENTTYPE_PARAM)[0] - stream = StringIO(self._data.pop(self._CONTENT_PARAM)[0]) - (self._data, self._files) = self._parse(stream, self._content_type) - - def _parse(self, stream, content_type): - """ - Parse the request content. - - May raise a 415 ErrorResponse (Unsupported Media Type), or a 400 ErrorResponse (Bad Request). - """ - if stream is None or content_type is None: - return (None, None) - - parsers = as_tuple(self.parsers) - - for parser_cls in parsers: - parser = parser_cls(self) - 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}) - - @property - def _parsed_media_types(self): - """ - Return a list of all the media types that this view can parse. - """ - return [parser.media_type for parser in self.parsers] - - @property - def _default_parser(self): - """ - Return the view's default parser class. - """ - return self.parsers[0] - + request_class = self.get_request_class() + return request_class(self.request) + ########## ResponseMixin ########## @@ -395,7 +272,7 @@ class ResourceMixin(object): May raise an :class:`response.ErrorResponse` with status code 400 (Bad Request). """ if not hasattr(self, '_content'): - self._content = self.validate_request(self.DATA, self.FILES) + self._content = self.validate_request(self.request.DATA, self.request.FILES) return self._content @property @@ -415,7 +292,7 @@ class ResourceMixin(object): return ModelResource(self) elif getattr(self, 'form', None): return FormResource(self) - elif getattr(self, '%s_form' % self.method.lower(), None): + elif getattr(self, '%s_form' % self.request.method.lower(), None): return FormResource(self) return Resource(self) @@ -752,7 +629,7 @@ class PaginatorMixin(object): """ # We don't want to paginate responses for anything other than GET requests - if self.method.upper() != 'GET': + if self.request.method.upper() != 'GET': return self._resource.filter_response(obj) paginator = Paginator(obj, self.get_limit()) diff --git a/djangorestframework/parsers.py b/djangorestframework/parsers.py index c8a014aef..e56ea0256 100644 --- a/djangorestframework/parsers.py +++ b/djangorestframework/parsers.py @@ -165,9 +165,10 @@ class MultiPartParser(BaseParser): `data` will be a :class:`QueryDict` containing all the form parameters. `files` will be a :class:`QueryDict` containing all the form files. """ - upload_handlers = self.view.request._get_upload_handlers() + # TODO: now self.view is in fact request, but should disappear ... + upload_handlers = self.view._get_upload_handlers() try: - django_parser = DjangoMultiPartParser(self.view.request.META, stream, upload_handlers) + 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)}) diff --git a/djangorestframework/renderers.py b/djangorestframework/renderers.py index bb186b0a9..683024efb 100644 --- a/djangorestframework/renderers.py +++ b/djangorestframework/renderers.py @@ -269,32 +269,32 @@ class DocumentingTemplateRenderer(BaseRenderer): # If we're not using content overloading there's no point in supplying a generic form, # as the view won't treat the form's value as the content of the request. - if not getattr(view, '_USE_FORM_OVERLOADING', False): + if not getattr(view.request, '_USE_FORM_OVERLOADING', False): return None # NB. http://jacobian.org/writing/dynamic-form-generation/ class GenericContentForm(forms.Form): - def __init__(self, view): + def __init__(self, request): """We don't know the names of the fields we want to set until the point the form is instantiated, as they are determined by the Resource the form is being created against. Add the fields dynamically.""" super(GenericContentForm, self).__init__() - contenttype_choices = [(media_type, media_type) for media_type in view._parsed_media_types] - initial_contenttype = view._default_parser.media_type + contenttype_choices = [(media_type, media_type) for media_type in request._parsed_media_types] + initial_contenttype = request._default_parser.media_type - self.fields[view._CONTENTTYPE_PARAM] = forms.ChoiceField(label='Content Type', + self.fields[request._CONTENTTYPE_PARAM] = forms.ChoiceField(label='Content Type', choices=contenttype_choices, initial=initial_contenttype) - self.fields[view._CONTENT_PARAM] = forms.CharField(label='Content', + self.fields[request._CONTENT_PARAM] = forms.CharField(label='Content', widget=forms.Textarea) # If either of these reserved parameters are turned off then content tunneling is not possible - if self.view._CONTENTTYPE_PARAM is None or self.view._CONTENT_PARAM is None: + if self.view.request._CONTENTTYPE_PARAM is None or self.view.request._CONTENT_PARAM is None: return None # Okey doke, let's do it - return GenericContentForm(view) + return GenericContentForm(view.request) def render(self, obj=None, media_type=None): """ diff --git a/djangorestframework/request.py b/djangorestframework/request.py new file mode 100644 index 000000000..c0ae46de3 --- /dev/null +++ b/djangorestframework/request.py @@ -0,0 +1,217 @@ +""" +The :mod:`request` module provides a `Request` class that can be used +to replace the standard Django request passed to the views. +This replacement `Request` provides many facilities, like automatically +parsed request content, form overloading of method/content type/content, +better support for HTTP PUT method. +""" + +from django.http import HttpRequest + +from djangorestframework.utils.mediatypes import is_form_media_type, order_by_precedence +from djangorestframework.utils import as_tuple + +from StringIO import StringIO + + +__all__ = ('Request') + + +def request_class_factory(request): + """ + Builds and returns a request class, to be used as a replacement of Django's built-in. + + In fact :class:`request.Request` needs to be mixed-in with a subclass of `HttpRequest` for use, + and we cannot do that before knowing which subclass of `HttpRequest` is used. So this function + takes a request instance as only argument, and returns a properly mixed-in request class. + """ + request_class = type(request) + return type(request_class.__name__, (Request, request_class), {}) + + +class Request(object): + + _USE_FORM_OVERLOADING = True + _METHOD_PARAM = '_method' + _CONTENTTYPE_PARAM = '_content_type' + _CONTENT_PARAM = '_content' + + parsers = () + """ + The set of parsers that the request can handle. + + Should be a tuple/list of classes as described in the :mod:`parsers` module. + """ + + def __init__(self, request): + # this allows to "copy" a request object into a new instance + # of our custom request class. + + # First, we prepare the attributes to copy. + attrs_dict = request.__dict__.copy() + attrs_dict.pop('method', None) + attrs_dict['_raw_method'] = request.method + + # Then, put them in the instance's own __dict__ + self.__dict__ = attrs_dict + + @property + def method(self): + """ + Returns the HTTP method. + + This allows the `method` to be overridden by using a hidden `form` field + on a form POST request. + """ + if not hasattr(self, '_method'): + self._load_method_and_content_type() + return self._method + + @property + def content_type(self): + """ + Returns the content type header. + + This should be used instead of ``request.META.get('HTTP_CONTENT_TYPE')``, + as it allows the content type to be overridden by using a hidden form + field on a form POST request. + """ + if not hasattr(self, '_content_type'): + self._load_method_and_content_type() + return self._content_type + + @property + def DATA(self): + """ + Parses the request body and returns the data. + + Similar to ``request.POST``, except that it handles arbitrary parsers, + and also works on methods other than POST (eg PUT). + """ + if not hasattr(self, '_data'): + self._load_data_and_files() + return self._data + + @property + def FILES(self): + """ + Parses the request body and returns the files. + Similar to ``request.FILES``, except that it handles arbitrary parsers, + and also works on methods other than POST (eg PUT). + """ + if not hasattr(self, '_files'): + self._load_data_and_files() + return self._files + + def _load_post_and_files(self): + """ + Overrides the parent's `_load_post_and_files` to isolate it + from the form overloading mechanism (see: `_perform_form_overloading`). + """ + # When self.POST or self.FILES are called they need to know the original + # HTTP method, not our overloaded HTTP method. So, we save our overloaded + # HTTP method and restore it after the call to parent. + method_mem = getattr(self, '_method', None) + self._method = self._raw_method + super(Request, self)._load_post_and_files() + if method_mem is None: + del self._method + else: + self._method = method_mem + + def _load_data_and_files(self): + """ + Parses the request content into self.DATA and self.FILES. + """ + if not hasattr(self, '_content_type'): + self._load_method_and_content_type() + + if not hasattr(self, '_data'): + (self._data, self._files) = self._parse(self._get_stream(), self._content_type) + + def _load_method_and_content_type(self): + """ + Sets the method and content_type, and then check if they've been overridden. + """ + self._content_type = self.META.get('HTTP_CONTENT_TYPE', self.META.get('CONTENT_TYPE', '')) + self._perform_form_overloading() + # if the HTTP method was not overloaded, we take the raw HTTP method + if not hasattr(self, '_method'): + self._method = self._raw_method + + def _get_stream(self): + """ + Returns an object that may be used to stream the request content. + """ + + try: + content_length = int(self.META.get('CONTENT_LENGTH', self.META.get('HTTP_CONTENT_LENGTH'))) + except (ValueError, TypeError): + content_length = 0 + + # TODO: Add 1.3's LimitedStream to compat and use that. + # NOTE: Currently only supports parsing request body as a stream with 1.3 + if content_length == 0: + return None + elif hasattr(self, 'read'): + return self + return StringIO(self.raw_post_data) + + def _perform_form_overloading(self): + """ + If this is a form POST request, then we need to check if the method and content/content_type have been + overridden by setting them in hidden form fields or not. + """ + + # We only need to use form overloading on form POST requests. + if not self._USE_FORM_OVERLOADING or self._raw_method != 'POST' or not is_form_media_type(self._content_type): + return + + # At this point we're committed to parsing the request as form data. + self._data = data = self.POST.copy() + self._files = self.FILES + + # Method overloading - change the method and remove the param from the content. + if self._METHOD_PARAM in data: + # NOTE: unlike `get`, `pop` on a `QueryDict` seems to return a list of values. + self._method = self._data.pop(self._METHOD_PARAM)[0].upper() + + # Content overloading - modify the content type, and re-parse. + if self._CONTENT_PARAM in data and self._CONTENTTYPE_PARAM in data: + self._content_type = self._data.pop(self._CONTENTTYPE_PARAM)[0] + stream = StringIO(self._data.pop(self._CONTENT_PARAM)[0]) + (self._data, self._files) = self._parse(stream, self._content_type) + + def _parse(self, stream, content_type): + """ + Parse the request content. + + May raise a 415 ErrorResponse (Unsupported Media Type), or a 400 ErrorResponse (Bad Request). + """ + if stream is None or content_type is None: + return (None, None) + + parsers = as_tuple(self.parsers) + + for parser_cls in parsers: + parser = parser_cls(self) + 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}) + + @property + def _parsed_media_types(self): + """ + Return a list of all the media types that this view can parse. + """ + return [parser.media_type for parser in self.parsers] + + @property + def _default_parser(self): + """ + Return the view's default parser class. + """ + return self.parsers[0] diff --git a/djangorestframework/tests/content.py b/djangorestframework/tests/content.py deleted file mode 100644 index 6bae8feb3..000000000 --- a/djangorestframework/tests/content.py +++ /dev/null @@ -1,233 +0,0 @@ -""" -Tests for content parsing, and form-overloaded content parsing. -""" -from django.conf.urls.defaults import patterns -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.mixins import RequestMixin -from djangorestframework.parsers import FormParser, MultiPartParser, \ - PlainTextParser, JSONParser -from djangorestframework.response import Response -from djangorestframework.views import View - -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.INTERNAL_SERVER_ERROR) - -urlpatterns = patterns('', - (r'^$', MockView.as_view()), -) - -class TestContentParsing(TestCase): - def setUp(self): - self.req = RequestFactory() - - def ensure_determines_no_content_GET(self, view): - """Ensure view.DATA returns None for GET request with no content.""" - view.request = self.req.get('/') - self.assertEqual(view.DATA, None) - - def ensure_determines_no_content_HEAD(self, view): - """Ensure view.DATA returns None for HEAD request.""" - view.request = self.req.head('/') - self.assertEqual(view.DATA, None) - - def ensure_determines_form_content_POST(self, view): - """Ensure view.DATA returns content for POST request with form content.""" - form_data = {'qwerty': 'uiop'} - view.parsers = (FormParser, MultiPartParser) - view.request = self.req.post('/', data=form_data) - self.assertEqual(view.DATA.items(), form_data.items()) - - def ensure_determines_non_form_content_POST(self, view): - """Ensure view.RAW_CONTENT returns content for POST request with non-form content.""" - content = 'qwerty' - content_type = 'text/plain' - view.parsers = (PlainTextParser,) - view.request = self.req.post('/', content, content_type=content_type) - self.assertEqual(view.DATA, content) - - def ensure_determines_form_content_PUT(self, view): - """Ensure view.RAW_CONTENT returns content for PUT request with form content.""" - form_data = {'qwerty': 'uiop'} - view.parsers = (FormParser, MultiPartParser) - view.request = self.req.put('/', data=form_data) - self.assertEqual(view.DATA.items(), form_data.items()) - - def ensure_determines_non_form_content_PUT(self, view): - """Ensure view.RAW_CONTENT returns content for PUT request with non-form content.""" - content = 'qwerty' - content_type = 'text/plain' - view.parsers = (PlainTextParser,) - view.request = self.req.post('/', content, content_type=content_type) - self.assertEqual(view.DATA, content) - - def test_standard_behaviour_determines_no_content_GET(self): - """Ensure view.DATA returns None for GET request with no content.""" - self.ensure_determines_no_content_GET(RequestMixin()) - - def test_standard_behaviour_determines_no_content_HEAD(self): - """Ensure view.DATA returns None for HEAD request.""" - self.ensure_determines_no_content_HEAD(RequestMixin()) - - def test_standard_behaviour_determines_form_content_POST(self): - """Ensure view.DATA returns content for POST request with form content.""" - self.ensure_determines_form_content_POST(RequestMixin()) - - def test_standard_behaviour_determines_non_form_content_POST(self): - """Ensure view.DATA returns content for POST request with non-form content.""" - self.ensure_determines_non_form_content_POST(RequestMixin()) - - def test_standard_behaviour_determines_form_content_PUT(self): - """Ensure view.DATA returns content for PUT request with form content.""" - self.ensure_determines_form_content_PUT(RequestMixin()) - - def test_standard_behaviour_determines_non_form_content_PUT(self): - """Ensure view.DATA returns content for PUT request with non-form content.""" - self.ensure_determines_non_form_content_PUT(RequestMixin()) - - def test_overloaded_behaviour_allows_content_tunnelling(self): - """Ensure request.DATA returns content for overloaded POST request""" - content = 'qwerty' - content_type = 'text/plain' - view = RequestMixin() - form_data = {view._CONTENT_PARAM: content, - view._CONTENTTYPE_PARAM: content_type} - view.request = self.req.post('/', form_data) - view.parsers = (PlainTextParser,) - self.assertEqual(view.DATA, content) - - def test_accessing_post_after_data_form(self): - """Ensures request.POST can be accessed after request.DATA in form request""" - form_data = {'qwerty': 'uiop'} - view = RequestMixin() - view.parsers = (FormParser, MultiPartParser) - view.request = self.req.post('/', data=form_data) - - self.assertEqual(view.DATA.items(), form_data.items()) - self.assertEqual(view.request.POST.items(), form_data.items()) - - @unittest.skip('This test was disabled some time ago for some reason') - def test_accessing_post_after_data_for_json(self): - """Ensures request.POST can be accessed after request.DATA in json request""" - from django.utils import simplejson as json - - data = {'qwerty': 'uiop'} - content = json.dumps(data) - content_type = 'application/json' - - view = RequestMixin() - view.parsers = (JSONParser,) - - view.request = self.req.post('/', content, content_type=content_type) - - self.assertEqual(view.DATA.items(), data.items()) - self.assertEqual(view.request.POST.items(), []) - - def test_accessing_post_after_data_for_overloaded_json(self): - """Ensures request.POST can be accessed after request.DATA in overloaded json request""" - from django.utils import simplejson as json - - data = {'qwerty': 'uiop'} - content = json.dumps(data) - content_type = 'application/json' - - view = RequestMixin() - view.parsers = (JSONParser,) - - form_data = {view._CONTENT_PARAM: content, - view._CONTENTTYPE_PARAM: content_type} - - view.request = self.req.post('/', data=form_data) - - self.assertEqual(view.DATA.items(), data.items()) - self.assertEqual(view.request.POST.items(), form_data.items()) - - def test_accessing_data_after_post_form(self): - """Ensures request.DATA can be accessed after request.POST in form request""" - form_data = {'qwerty': 'uiop'} - view = RequestMixin() - view.parsers = (FormParser, MultiPartParser) - view.request = self.req.post('/', data=form_data) - - self.assertEqual(view.request.POST.items(), form_data.items()) - self.assertEqual(view.DATA.items(), form_data.items()) - - def test_accessing_data_after_post_for_json(self): - """Ensures request.DATA can be accessed after request.POST in json request""" - from django.utils import simplejson as json - - data = {'qwerty': 'uiop'} - content = json.dumps(data) - content_type = 'application/json' - - view = RequestMixin() - view.parsers = (JSONParser,) - - view.request = self.req.post('/', content, content_type=content_type) - - post_items = view.request.POST.items() - - self.assertEqual(len(post_items), 1) - self.assertEqual(len(post_items[0]), 2) - self.assertEqual(post_items[0][0], content) - self.assertEqual(view.DATA.items(), data.items()) - - def test_accessing_data_after_post_for_overloaded_json(self): - """Ensures request.DATA can be accessed after request.POST in overloaded json request""" - from django.utils import simplejson as json - - data = {'qwerty': 'uiop'} - content = json.dumps(data) - content_type = 'application/json' - - view = RequestMixin() - view.parsers = (JSONParser,) - - form_data = {view._CONTENT_PARAM: content, - view._CONTENTTYPE_PARAM: content_type} - - view.request = self.req.post('/', data=form_data) - - self.assertEqual(view.request.POST.items(), form_data.items()) - self.assertEqual(view.DATA.items(), data.items()) - -class TestContentParsingWithAuthentication(TestCase): - urls = 'djangorestframework.tests.content' - - def setUp(self): - self.csrf_client = Client(enforce_csrf_checks=True) - self.username = 'john' - self.email = 'lennon@thebeatles.com' - self.password = 'password' - self.user = User.objects.create_user(self.username, self.email, self.password) - self.req = RequestFactory() - - def test_user_logged_in_authentication_has_post_when_not_logged_in(self): - """Ensures request.POST exists after UserLoggedInAuthentication when user doesn't log in""" - content = {'example': 'example'} - - response = self.client.post('/', content) - self.assertEqual(status.HTTP_200_OK, response.status_code, "POST data is malformed") - - response = self.csrf_client.post('/', content) - self.assertEqual(status.HTTP_200_OK, response.status_code, "POST data is malformed") - - # def test_user_logged_in_authentication_has_post_when_logged_in(self): - # """Ensures request.POST exists after UserLoggedInAuthentication when user does log in""" - # self.client.login(username='john', password='password') - # self.csrf_client.login(username='john', password='password') - # content = {'example': 'example'} - - # response = self.client.post('/', content) - # self.assertEqual(status.OK, response.status_code, "POST data is malformed") - - # response = self.csrf_client.post('/', content) - # self.assertEqual(status.OK, response.status_code, "POST data is malformed") diff --git a/djangorestframework/tests/methods.py b/djangorestframework/tests/methods.py index 4b90a21f1..e69de29bb 100644 --- a/djangorestframework/tests/methods.py +++ b/djangorestframework/tests/methods.py @@ -1,32 +0,0 @@ -from django.test import TestCase -from djangorestframework.compat import RequestFactory -from djangorestframework.mixins import RequestMixin - - -class TestMethodOverloading(TestCase): - def setUp(self): - self.req = RequestFactory() - - def test_standard_behaviour_determines_GET(self): - """GET requests identified""" - view = RequestMixin() - view.request = self.req.get('/') - self.assertEqual(view.method, 'GET') - - def test_standard_behaviour_determines_POST(self): - """POST requests identified""" - view = RequestMixin() - view.request = self.req.post('/') - self.assertEqual(view.method, 'POST') - - def test_overloaded_POST_behaviour_determines_overloaded_method(self): - """POST requests can be overloaded to another method by setting a reserved form field""" - view = RequestMixin() - view.request = self.req.post('/', {view._METHOD_PARAM: 'DELETE'}) - self.assertEqual(view.method, 'DELETE') - - def test_HEAD_is_a_valid_method(self): - """HEAD requests identified""" - view = RequestMixin() - view.request = self.req.head('/') - self.assertEqual(view.method, 'HEAD') diff --git a/djangorestframework/tests/renderers.py b/djangorestframework/tests/renderers.py index 84e4390b8..adb46f7fa 100644 --- a/djangorestframework/tests/renderers.py +++ b/djangorestframework/tests/renderers.py @@ -172,7 +172,7 @@ class RendererIntegrationTests(TestCase): self.assertEquals(resp.status_code, DUMMYSTATUS) _flat_repr = '{"foo": ["bar", "baz"]}' -_indented_repr = '{\n "foo": [\n "bar", \n "baz"\n ]\n}' +_indented_repr = '{\n "foo": [\n "bar",\n "baz"\n ]\n}' class JSONRendererTests(TestCase): diff --git a/djangorestframework/tests/request.py b/djangorestframework/tests/request.py new file mode 100644 index 000000000..7f3a485ef --- /dev/null +++ b/djangorestframework/tests/request.py @@ -0,0 +1,250 @@ +""" +Tests for content parsing, and form-overloaded content parsing. +""" +from django.conf.urls.defaults import patterns +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.mixins import RequestMixin +from djangorestframework.parsers import FormParser, MultiPartParser, \ + PlainTextParser, JSONParser +from djangorestframework.response import Response +from djangorestframework.request import Request +from djangorestframework.views import View +from djangorestframework.request import request_class_factory + +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.INTERNAL_SERVER_ERROR) + +urlpatterns = patterns('', + (r'^$', MockView.as_view()), +) + +request_class = request_class_factory(RequestFactory().get('/')) + + +class RequestTestCase(TestCase): + + def tearDown(self): + request_class.parsers = () + + def build_request(self, method, *args, **kwargs): + factory = RequestFactory() + method = getattr(factory, method) + original_request = method(*args, **kwargs) + return request_class(original_request) + + +class TestMethodOverloading(RequestTestCase): + + def test_standard_behaviour_determines_GET(self): + """GET requests identified""" + request = self.build_request('get', '/') + self.assertEqual(request.method, 'GET') + + def test_standard_behaviour_determines_POST(self): + """POST requests identified""" + request = self.build_request('post', '/') + self.assertEqual(request.method, 'POST') + + def test_overloaded_POST_behaviour_determines_overloaded_method(self): + """POST requests can be overloaded to another method by setting a reserved form field""" + request = self.build_request('post', '/', {Request._METHOD_PARAM: 'DELETE'}) + self.assertEqual(request.method, 'DELETE') + + def test_HEAD_is_a_valid_method(self): + """HEAD requests identified""" + request = request = self.build_request('head', '/') + self.assertEqual(request.method, 'HEAD') + + +class TestContentParsing(RequestTestCase): + #TODO: is there any reason why many test cases documented as testing a PUT, + # in fact use a POST !? + + def tearDown(self): + request_class.parsers = () + + def build_request(self, method, *args, **kwargs): + factory = RequestFactory() + method = getattr(factory, method) + original_request = method(*args, **kwargs) + return request_class(original_request) + + def test_standard_behaviour_determines_no_content_GET(self): + """Ensure request.DATA returns None for GET request with no content.""" + request = self.build_request('get', '/') + self.assertEqual(request.DATA, None) + + def test_standard_behaviour_determines_no_content_HEAD(self): + """Ensure request.DATA returns None for HEAD request.""" + request = self.build_request('head', '/') + self.assertEqual(request.DATA, None) + + def test_standard_behaviour_determines_form_content_POST(self): + """Ensure request.DATA returns content for POST request with form content.""" + form_data = {'qwerty': 'uiop'} + request_class.parsers = (FormParser, MultiPartParser) + request = self.build_request('post', '/', data=form_data) + self.assertEqual(request.DATA.items(), form_data.items()) + + def test_standard_behaviour_determines_non_form_content_POST(self): + """Ensure request.DATA returns content for POST request with non-form content.""" + content = 'qwerty' + content_type = 'text/plain' + request_class.parsers = (PlainTextParser,) + request = self.build_request('post', '/', content, content_type=content_type) + self.assertEqual(request.DATA, content) + + def test_standard_behaviour_determines_form_content_PUT(self): + """Ensure request.DATA returns content for PUT request with form content.""" + form_data = {'qwerty': 'uiop'} + request_class.parsers = (FormParser, MultiPartParser) + request = self.build_request('put', '/', data=form_data) + self.assertEqual(request.DATA.items(), form_data.items()) + + def test_standard_behaviour_determines_non_form_content_PUT(self): + """Ensure request.DATA returns content for PUT request with non-form content.""" + content = 'qwerty' + content_type = 'text/plain' + request_class.parsers = (PlainTextParser,) + request = self.build_request('post', '/', content, content_type=content_type) + self.assertEqual(request.DATA, content) + + def test_overloaded_behaviour_allows_content_tunnelling(self): + """Ensure request.DATA returns content for overloaded POST request""" + content = 'qwerty' + content_type = 'text/plain' + form_data = {Request._CONTENT_PARAM: content, + Request._CONTENTTYPE_PARAM: content_type} + request_class.parsers = (PlainTextParser,) + request = self.build_request('post', '/', form_data) + self.assertEqual(request.DATA, content) + + def test_accessing_post_after_data_form(self): + """Ensures request.POST can be accessed after request.DATA in form request""" + form_data = {'qwerty': 'uiop'} + request_class.parsers = (FormParser, MultiPartParser) + request = self.build_request('post', '/', data=form_data) + + self.assertEqual(request.DATA.items(), form_data.items()) + self.assertEqual(request.POST.items(), form_data.items()) + + def test_accessing_post_after_data_for_json(self): + """Ensures request.POST can be accessed after request.DATA in json request""" + from django.utils import simplejson as json + + data = {'qwerty': 'uiop'} + content = json.dumps(data) + content_type = 'application/json' + + request_class.parsers = (JSONParser,) + + request = self.build_request('post', '/', content, content_type=content_type) + + self.assertEqual(request.DATA.items(), data.items()) + self.assertEqual(request.POST.items(), []) + + def test_accessing_post_after_data_for_overloaded_json(self): + """Ensures request.POST can be accessed after request.DATA in overloaded json request""" + from django.utils import simplejson as json + + data = {'qwerty': 'uiop'} + content = json.dumps(data) + content_type = 'application/json' + + request_class.parsers = (JSONParser,) + + form_data = {Request._CONTENT_PARAM: content, + Request._CONTENTTYPE_PARAM: content_type} + + request = self.build_request('post', '/', data=form_data) + + self.assertEqual(request.DATA.items(), data.items()) + self.assertEqual(request.POST.items(), form_data.items()) + + def test_accessing_data_after_post_form(self): + """Ensures request.DATA can be accessed after request.POST in form request""" + form_data = {'qwerty': 'uiop'} + request_class.parsers = (FormParser, MultiPartParser) + request = self.build_request('post', '/', data=form_data) + + self.assertEqual(request.POST.items(), form_data.items()) + self.assertEqual(request.DATA.items(), form_data.items()) + + def test_accessing_data_after_post_for_json(self): + """Ensures request.DATA can be accessed after request.POST in json request""" + from django.utils import simplejson as json + + data = {'qwerty': 'uiop'} + content = json.dumps(data) + content_type = 'application/json' + + request_class.parsers = (JSONParser,) + + request = self.build_request('post', '/', content, content_type=content_type) + + post_items = request.POST.items() + + self.assertEqual(len(post_items), 1) + self.assertEqual(len(post_items[0]), 2) + self.assertEqual(post_items[0][0], content) + self.assertEqual(request.DATA.items(), data.items()) + + def test_accessing_data_after_post_for_overloaded_json(self): + """Ensures request.DATA can be accessed after request.POST in overloaded json request""" + from django.utils import simplejson as json + + data = {'qwerty': 'uiop'} + content = json.dumps(data) + content_type = 'application/json' + + request_class.parsers = (JSONParser,) + + form_data = {Request._CONTENT_PARAM: content, + Request._CONTENTTYPE_PARAM: content_type} + + request = self.build_request('post', '/', data=form_data) + self.assertEqual(request.POST.items(), form_data.items()) + self.assertEqual(request.DATA.items(), data.items()) + + +class TestContentParsingWithAuthentication(TestCase): + urls = 'djangorestframework.tests.request' + + def setUp(self): + self.csrf_client = Client(enforce_csrf_checks=True) + self.username = 'john' + self.email = 'lennon@thebeatles.com' + self.password = 'password' + self.user = User.objects.create_user(self.username, self.email, self.password) + self.req = RequestFactory() + + def test_user_logged_in_authentication_has_post_when_not_logged_in(self): + """Ensures request.POST exists after UserLoggedInAuthentication when user doesn't log in""" + content = {'example': 'example'} + + response = self.client.post('/', content) + self.assertEqual(status.HTTP_200_OK, response.status_code, "POST data is malformed") + + response = self.csrf_client.post('/', content) + self.assertEqual(status.HTTP_200_OK, response.status_code, "POST data is malformed") + + # def test_user_logged_in_authentication_has_post_when_logged_in(self): + # """Ensures request.POST exists after UserLoggedInAuthentication when user does log in""" + # self.client.login(username='john', password='password') + # self.csrf_client.login(username='john', password='password') + # content = {'example': 'example'} + + # response = self.client.post('/', content) + # self.assertEqual(status.OK, response.status_code, "POST data is malformed") + + # response = self.csrf_client.post('/', content) + # self.assertEqual(status.OK, response.status_code, "POST data is malformed") diff --git a/djangorestframework/views.py b/djangorestframework/views.py index 9f53868b3..37eec89fb 100644 --- a/djangorestframework/views.py +++ b/djangorestframework/views.py @@ -81,7 +81,7 @@ 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.' % self.method}) + {'detail': 'Method \'%s\' not allowed on this resource.' % request.method}) def initial(self, request, *args, **kargs): """ @@ -128,17 +128,20 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): self.headers = {} try: + # Get a custom request, built form the original request instance + self.request = request = self.get_request() + 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) + if request.method.lower() in self.http_method_names: + handler = getattr(self, request.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 @@ -164,7 +167,7 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): 'name': get_name(self), 'description': get_description(self), 'renders': self._rendered_media_types, - 'parses': self._parsed_media_types, + 'parses': request._parsed_media_types, } form = self.get_bound_form() if form is not None: From 8b72b7bf92ac6f0d021fcee5286505e75e075eff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Piquemal?= Date: Tue, 24 Jan 2012 19:16:41 +0200 Subject: [PATCH 02/12] corrected request example --- djangorestframework/tests/request.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/djangorestframework/tests/request.py b/djangorestframework/tests/request.py index 7f3a485ef..6a0eae217 100644 --- a/djangorestframework/tests/request.py +++ b/djangorestframework/tests/request.py @@ -66,8 +66,6 @@ class TestMethodOverloading(RequestTestCase): class TestContentParsing(RequestTestCase): - #TODO: is there any reason why many test cases documented as testing a PUT, - # in fact use a POST !? def tearDown(self): request_class.parsers = () @@ -115,7 +113,7 @@ class TestContentParsing(RequestTestCase): content = 'qwerty' content_type = 'text/plain' request_class.parsers = (PlainTextParser,) - request = self.build_request('post', '/', content, content_type=content_type) + request = self.build_request('put', '/', content, content_type=content_type) self.assertEqual(request.DATA, content) def test_overloaded_behaviour_allows_content_tunnelling(self): From 714a90d7559885c15e5b2c86ef6f457fdf857ee0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Piquemal?= Date: Tue, 24 Jan 2012 21:21:10 +0200 Subject: [PATCH 03/12] documentation for request module --- djangorestframework/mixins.py | 8 ++++---- djangorestframework/request.py | 18 ++++++++++++------ docs/library/request.rst | 5 +++++ 3 files changed, 21 insertions(+), 10 deletions(-) create mode 100644 docs/library/request.rst diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index d016b0f12..e53f8e6a6 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -40,7 +40,7 @@ __all__ = ( class RequestMixin(object): """ - `Mixin` class to provide request parsing behavior. + `Mixin` class to enhance API of Django's standard `request`. """ _USE_FORM_OVERLOADING = True @@ -50,15 +50,15 @@ class RequestMixin(object): parsers = () """ - The set of request parsers that the view can handle. + The set of parsers that the request can handle. Should be a tuple/list of classes as described in the :mod:`parsers` module. """ def get_request_class(self): """ - Returns a custom subclass of Django's `HttpRequest`, providing new facilities - such as direct access to the parsed request content. + Returns a subclass of Django's `HttpRequest` with a richer API, + as described in :mod:`request`. """ if not hasattr(self, '_request_class'): self._request_class = request_class_factory(self.request) diff --git a/djangorestframework/request.py b/djangorestframework/request.py index c0ae46de3..40d92eef0 100644 --- a/djangorestframework/request.py +++ b/djangorestframework/request.py @@ -1,9 +1,12 @@ """ -The :mod:`request` module provides a `Request` class that can be used -to replace the standard Django request passed to the views. -This replacement `Request` provides many facilities, like automatically -parsed request content, form overloading of method/content type/content, -better support for HTTP PUT method. +The :mod:`request` module provides a :class:`Request` class that can be used +to enhance the standard `request` object received in all the views. + +This enhanced request object offers the following : + + - content automatically parsed according to `Content-Type` header, and available as :meth:`request.DATA` + - full support of PUT method, including support for file uploads + - form overloading of HTTP method, content type and content """ from django.http import HttpRequest @@ -14,7 +17,7 @@ from djangorestframework.utils import as_tuple from StringIO import StringIO -__all__ = ('Request') +__all__ = ('Request',) def request_class_factory(request): @@ -30,6 +33,9 @@ def request_class_factory(request): class Request(object): + """ + A mixin class allowing to enhance Django's standard HttpRequest. + """ _USE_FORM_OVERLOADING = True _METHOD_PARAM = '_method' diff --git a/docs/library/request.rst b/docs/library/request.rst new file mode 100644 index 000000000..5e99826ad --- /dev/null +++ b/docs/library/request.rst @@ -0,0 +1,5 @@ +:mod:`request` +===================== + +.. automodule:: request + :members: From 152c385f4de37558fe4e522abad5b97f0cf7ddce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Piquemal?= Date: Wed, 25 Jan 2012 00:11:54 +0200 Subject: [PATCH 04/12] enhanced request how-to + example --- djangorestframework/request.py | 2 + docs/howto/requestmixin.rst | 76 +++++++++++++++++++++++++++++ examples/requestexample/__init__.py | 0 examples/requestexample/models.py | 3 ++ examples/requestexample/tests.py | 0 examples/requestexample/urls.py | 7 +++ examples/requestexample/views.py | 76 +++++++++++++++++++++++++++++ examples/sandbox/views.py | 4 +- examples/settings.py | 1 + examples/urls.py | 1 + 10 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 docs/howto/requestmixin.rst create mode 100644 examples/requestexample/__init__.py create mode 100644 examples/requestexample/models.py create mode 100644 examples/requestexample/tests.py create mode 100644 examples/requestexample/urls.py create mode 100644 examples/requestexample/views.py diff --git a/djangorestframework/request.py b/djangorestframework/request.py index 40d92eef0..1674167d8 100644 --- a/djangorestframework/request.py +++ b/djangorestframework/request.py @@ -11,6 +11,8 @@ This enhanced request object offers the following : from django.http import HttpRequest +from djangorestframework.response import ErrorResponse +from djangorestframework import status from djangorestframework.utils.mediatypes import is_form_media_type, order_by_precedence from djangorestframework.utils import as_tuple diff --git a/docs/howto/requestmixin.rst b/docs/howto/requestmixin.rst new file mode 100644 index 000000000..a00fdad0e --- /dev/null +++ b/docs/howto/requestmixin.rst @@ -0,0 +1,76 @@ +Using the enhanced request in all your views +============================================== + +This example shows how you can use Django REST framework's enhanced `request` in your own views, without having to use the full-blown :class:`views.View` class. + +What can it do for you ? Mostly, it will take care of parsing the request's content, and handling equally all HTTP methods ... + +Before +-------- + +In order to support `JSON` or other serial formats, you might have parsed manually the request's content with something like : :: + + class MyView(View): + + def put(self, request, *args, **kwargs): + content_type = request.META['CONTENT_TYPE'] + if (content_type == 'application/json'): + raw_data = request.read() + parsed_data = json.loads(raw_data) + + # PLUS as many `elif` as formats you wish to support ... + + # then do stuff with your data : + self.do_stuff(parsed_data['bla'], parsed_data['hoho']) + + # and finally respond something + +... and you were unhappy because this looks hackish. + +Also, you might have tried uploading files with a PUT request - *and given up* since that's complicated to achieve even with Django 1.3. + + +After +------ + +All the dirty `Content-type` checking and content reading and parsing is done for you, and you only need to do the following : :: + + class MyView(MyBaseViewUsingEnhancedRequest): + + def put(self, request, *args, **kwargs): + self.do_stuff(request.DATA['bla'], request.DATA['hoho']) + # and finally respond something + +So the parsed content is magically available as `.DATA` on the `request` object. + +Also, if you uploaded files, they are available as `.FILES`, like with a normal POST request. + +.. note:: Note that all the above is also valid for a POST request. + + +How to add it to your custom views ? +-------------------------------------- + +Now that you're convinced you need to use the enhanced request object, here is how you can add it to all your custom views : :: + + from django.views.generic.base import View + + from djangorestframework.mixins import RequestMixin + from djangorestframework import parsers + + + class MyBaseViewUsingEnhancedRequest(RequestMixin, View): + """ + Base view enabling the usage of enhanced requests with user defined views. + """ + + parsers = parsers.DEFAULT_PARSERS + + def dispatch(self, request, *args, **kwargs): + self.request = request + request = self.get_request() + return super(MyBaseViewUsingEnhancedRequest, self).dispatch(request, *args, **kwargs) + +And then, use this class as a base for all your custom views. + +.. note:: you can also check the request example. diff --git a/examples/requestexample/__init__.py b/examples/requestexample/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/examples/requestexample/models.py b/examples/requestexample/models.py new file mode 100644 index 000000000..71a836239 --- /dev/null +++ b/examples/requestexample/models.py @@ -0,0 +1,3 @@ +from django.db import models + +# Create your models here. diff --git a/examples/requestexample/tests.py b/examples/requestexample/tests.py new file mode 100644 index 000000000..e69de29bb diff --git a/examples/requestexample/urls.py b/examples/requestexample/urls.py new file mode 100644 index 000000000..a5e3356a1 --- /dev/null +++ b/examples/requestexample/urls.py @@ -0,0 +1,7 @@ +from django.conf.urls.defaults import patterns, url +from requestexample.views import RequestExampleView, MockView, EchoRequestContentView + +urlpatterns = patterns('', + url(r'^$', RequestExampleView.as_view(), name='request-example'), + url(r'^content$', MockView.as_view(view_class=EchoRequestContentView), name='request-content'), +) diff --git a/examples/requestexample/views.py b/examples/requestexample/views.py new file mode 100644 index 000000000..aa8a734f5 --- /dev/null +++ b/examples/requestexample/views.py @@ -0,0 +1,76 @@ +from djangorestframework.compat import View +from django.http import HttpResponse +from django.core.urlresolvers import reverse + +from djangorestframework.mixins import RequestMixin +from djangorestframework.views import View as DRFView +from djangorestframework import parsers + + +class RequestExampleView(DRFView): + """ + A container view for request examples. + """ + + def get(self, request): + return [{'name': 'request.DATA Example', 'url': reverse('request-content')},] + + +class MyBaseViewUsingEnhancedRequest(RequestMixin, View): + """ + Base view enabling the usage of enhanced requests with user defined views. + """ + + parsers = parsers.DEFAULT_PARSERS + + def dispatch(self, request, *args, **kwargs): + self.request = request + request = self.get_request() + return super(MyBaseViewUsingEnhancedRequest, self).dispatch(request, *args, **kwargs) + + +class EchoRequestContentView(MyBaseViewUsingEnhancedRequest): + """ + A view that just reads the items in `request.DATA` and echoes them back. + """ + + def post(self, request, *args, **kwargs): + return HttpResponse(("Found %s in request.DATA, content : %s" % + (type(request.DATA), request.DATA))) + + def put(self, request, *args, **kwargs): + return HttpResponse(("Found %s in request.DATA, content : %s" % + (type(request.DATA), request.DATA))) + + +class MockView(DRFView): + """ + A view that just acts as a proxy to call non-djangorestframework views, while still + displaying the browsable API interface. + """ + + view_class = None + + def dispatch(self, request, *args, **kwargs): + self.request = request + if self.get_request().method in ['PUT', 'POST']: + self.response = self.view_class.as_view()(request, *args, **kwargs) + return super(MockView, self).dispatch(request, *args, **kwargs) + + def get(self, request, *args, **kwargs): + return + + def put(self, request, *args, **kwargs): + return self.response.content + + def post(self, request, *args, **kwargs): + return self.response.content + + def __getattribute__(self, name): + if name == '__name__': + return self.view_class.__name__ + elif name == '__doc__': + return self.view_class.__doc__ + else: + return super(MockView, self).__getattribute__(name) + diff --git a/examples/sandbox/views.py b/examples/sandbox/views.py index f7a3542d7..998887a7b 100644 --- a/examples/sandbox/views.py +++ b/examples/sandbox/views.py @@ -23,6 +23,7 @@ class Sandbox(View): 5. A code highlighting API. 6. A blog posts and comments API. 7. A basic example using permissions. + 8. A basic example using enhanced request. Please feel free to browse, create, edit and delete the resources in these examples.""" @@ -33,5 +34,6 @@ class Sandbox(View): {'name': 'Object store API', 'url': reverse('object-store-root')}, {'name': 'Code highlighting API', 'url': reverse('pygments-root')}, {'name': 'Blog posts API', 'url': reverse('blog-posts-root')}, - {'name': 'Permissions example', 'url': reverse('permissions-example')} + {'name': 'Permissions example', 'url': reverse('permissions-example')}, + {'name': 'Simple request mixin example', 'url': reverse('request-example')} ] diff --git a/examples/settings.py b/examples/settings.py index e12b7f3fe..3b024ea10 100644 --- a/examples/settings.py +++ b/examples/settings.py @@ -112,6 +112,7 @@ INSTALLED_APPS = ( 'pygments_api', 'blogpost', 'permissionsexample', + 'requestexample', ) import os diff --git a/examples/urls.py b/examples/urls.py index 08d97a14a..b71c0a20d 100644 --- a/examples/urls.py +++ b/examples/urls.py @@ -11,6 +11,7 @@ urlpatterns = patterns('', (r'^pygments/', include('pygments_api.urls')), (r'^blog-post/', include('blogpost.urls')), (r'^permissions-example/', include('permissionsexample.urls')), + (r'^request-example/', include('requestexample.urls')), (r'^', include('djangorestframework.urls')), ) 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 05/12] 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): From ca96b4523b4c09489e4bfe726a894a5c6ada78aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Piquemal?= Date: Tue, 7 Feb 2012 13:15:30 +0200 Subject: [PATCH 06/12] cleaned a bit Response/ResponseMixin code, added some documentation + renamed ErrorResponse to ImmediateResponse --- djangorestframework/mixins.py | 46 +++++++++------- djangorestframework/parsers.py | 8 +-- djangorestframework/permissions.py | 10 ++-- djangorestframework/renderers.py | 5 +- djangorestframework/request.py | 6 +-- djangorestframework/resources.py | 16 +++--- djangorestframework/response.py | 59 ++++++++++++-------- djangorestframework/tests/accept.py | 3 +- djangorestframework/tests/mixins.py | 4 +- djangorestframework/tests/renderers.py | 4 +- djangorestframework/tests/response.py | 71 ++++++++++++++++--------- djangorestframework/tests/validators.py | 22 ++++---- djangorestframework/views.py | 14 ++--- 13 files changed, 155 insertions(+), 113 deletions(-) diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index dc2cfd277..c30ef10b7 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -11,7 +11,7 @@ from urlobject import URLObject from djangorestframework import status from djangorestframework.renderers import BaseRenderer from djangorestframework.resources import Resource, FormResource, ModelResource -from djangorestframework.response import Response, ErrorResponse +from djangorestframework.response import Response, ImmediateResponse from djangorestframework.request import request_class_factory from djangorestframework.utils import as_tuple, allowed_methods @@ -80,28 +80,37 @@ class RequestMixin(object): class ResponseMixin(object): """ - Adds behavior for pluggable `Renderers` to a :class:`views.View` class. + Adds behavior for pluggable `renderers` to a :class:`views.View` class. Default behavior is to use standard HTTP Accept header content negotiation. Also supports overriding the content type by specifying an ``_accept=`` parameter in the URL. Ignores Accept headers from Internet Explorer user agents and uses a sensible browser Accept header instead. """ - renderers = () + renderer_classes = () """ The set of response renderers that the view can handle. Should be a tuple/list of classes as described in the :mod:`renderers` module. """ - response_class = Response + def get_renderers(self): + """ + Instantiates and returns the list of renderers that will be used to render + the response. + """ + if not hasattr(self, '_renderers'): + self._renderers = [r(self) for r in self.renderer_classes] + return self._renderers def prepare_response(self, response): """ - Prepares response for the response cycle. Sets some headers, sets renderers, ... + Prepares the response for the response cycle. This has no effect if the + response is not an instance of :class:`response.Response`. """ 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 @@ -109,10 +118,9 @@ class ResponseMixin(object): # 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 + response.renderers = self.get_renderers() self.response = response return response @@ -121,21 +129,21 @@ class ResponseMixin(object): """ Return an list of all the media types that this view can render. """ - return [renderer.media_type for renderer in self.renderers] + return [renderer.media_type for renderer in self.get_renderers()] @property def _rendered_formats(self): """ Return a list of all the formats that this view can render. """ - return [renderer.format for renderer in self.renderers] + return [renderer.format for renderer in self.get_renderers()] @property def _default_renderer(self): """ Return the view's default renderer class. """ - return self.renderers[0] + return self.get_renderers()[0] @property def headers(self): @@ -195,7 +203,7 @@ class AuthMixin(object): # TODO: wrap this behavior around dispatch() def _check_permissions(self): """ - Check user permissions and either raise an ``ErrorResponse`` or return. + Check user permissions and either raise an ``ImmediateResponse`` or return. """ user = self.user for permission_cls in self.permissions: @@ -223,7 +231,7 @@ class ResourceMixin(object): """ Returns the cleaned, validated request content. - May raise an :class:`response.ErrorResponse` with status code 400 (Bad Request). + May raise an :class:`response.ImmediateResponse` with status code 400 (Bad Request). """ if not hasattr(self, '_content'): self._content = self.validate_request(self.request.DATA, self.request.FILES) @@ -234,7 +242,7 @@ class ResourceMixin(object): """ Returns the cleaned, validated query parameters. - May raise an :class:`response.ErrorResponse` with status code 400 (Bad Request). + May raise an :class:`response.ImmediateResponse` with status code 400 (Bad Request). """ return self.validate_request(self.request.GET) @@ -253,7 +261,7 @@ class ResourceMixin(object): def validate_request(self, data, files=None): """ Given the request *data* and optional *files*, return the cleaned, validated content. - May raise an :class:`response.ErrorResponse` with status code 400 (Bad Request) on failure. + May raise an :class:`response.ImmediateResponse` with status code 400 (Bad Request) on failure. """ return self._resource.validate_request(data, files) @@ -384,7 +392,7 @@ class ReadModelMixin(ModelMixin): try: self.model_instance = self.get_instance(**query_kwargs) except model.DoesNotExist: - raise ErrorResponse(status=status.HTTP_404_NOT_FOUND) + raise ImmediateResponse(status=status.HTTP_404_NOT_FOUND) return self.model_instance @@ -463,7 +471,7 @@ class DeleteModelMixin(ModelMixin): try: instance = self.get_instance(**query_kwargs) except model.DoesNotExist: - raise ErrorResponse(status=status.HTTP_404_NOT_FOUND) + raise ImmediateResponse(status=status.HTTP_404_NOT_FOUND) instance.delete() return Response() @@ -570,12 +578,12 @@ class PaginatorMixin(object): try: page_num = int(self.request.GET.get('page', '1')) except ValueError: - raise ErrorResponse( + raise ImmediateResponse( content={'detail': 'That page contains no results'}, status=status.HTTP_404_NOT_FOUND) if page_num not in paginator.page_range: - raise ErrorResponse( + raise ImmediateResponse( content={'detail': 'That page contains no results'}, status=status.HTTP_404_NOT_FOUND) diff --git a/djangorestframework/parsers.py b/djangorestframework/parsers.py index 7732a2939..5fc5c71ee 100644 --- a/djangorestframework/parsers.py +++ b/djangorestframework/parsers.py @@ -17,7 +17,7 @@ from django.http.multipartparser import MultiPartParserError from django.utils import simplejson as json from djangorestframework import status from djangorestframework.compat import yaml -from djangorestframework.response import ErrorResponse +from djangorestframework.response import ImmediateResponse from djangorestframework.utils.mediatypes import media_type_matches from xml.etree import ElementTree as ET import datetime @@ -88,7 +88,7 @@ class JSONParser(BaseParser): try: return (json.load(stream), None) except ValueError, exc: - raise ErrorResponse( + raise ImmediateResponse( content={'detail': 'JSON parse error - %s' % unicode(exc)}, status=status.HTTP_400_BAD_REQUEST) @@ -111,7 +111,7 @@ if yaml: try: return (yaml.safe_load(stream), None) except ValueError, exc: - raise ErrorResponse( + raise ImmediateResponse( content={'detail': 'YAML parse error - %s' % unicode(exc)}, status=status.HTTP_400_BAD_REQUEST) else: @@ -172,7 +172,7 @@ class MultiPartParser(BaseParser): try: django_parser = DjangoMultiPartParser(self.view.META, stream, upload_handlers) except MultiPartParserError, exc: - raise ErrorResponse( + raise ImmediateResponse( 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 bce03cabc..4ddc35cb9 100644 --- a/djangorestframework/permissions.py +++ b/djangorestframework/permissions.py @@ -6,7 +6,7 @@ class to your view by setting your View's :attr:`permissions` class attribute. from django.core.cache import cache from djangorestframework import status -from djangorestframework.response import ErrorResponse +from djangorestframework.response import ImmediateResponse import time __all__ = ( @@ -21,12 +21,12 @@ __all__ = ( ) -_403_FORBIDDEN_RESPONSE = ErrorResponse( +_403_FORBIDDEN_RESPONSE = ImmediateResponse( 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( +_503_SERVICE_UNAVAILABLE = ImmediateResponse( content={'detail': 'request was throttled'}, status=status.HTTP_503_SERVICE_UNAVAILABLE) @@ -43,7 +43,7 @@ class BasePermission(object): def check_permission(self, auth): """ - Should simply return, or raise an :exc:`response.ErrorResponse`. + Should simply return, or raise an :exc:`response.ImmediateResponse`. """ pass @@ -116,7 +116,7 @@ class BaseThrottle(BasePermission): def check_permission(self, auth): """ Check the throttling. - Return `None` or raise an :exc:`.ErrorResponse`. + Return `None` or raise an :exc:`.ImmediateResponse`. """ num, period = getattr(self.view, self.attr_name, self.default).split('/') self.num_requests = int(num) diff --git a/djangorestframework/renderers.py b/djangorestframework/renderers.py index 929ed0738..4e8158aa7 100644 --- a/djangorestframework/renderers.py +++ b/djangorestframework/renderers.py @@ -45,7 +45,7 @@ class BaseRenderer(object): media_type = None format = None - def __init__(self, view): + def __init__(self, view=None): self.view = view def can_handle_response(self, accept): @@ -218,7 +218,8 @@ class DocumentingTemplateRenderer(BaseRenderer): """ # Find the first valid renderer and render the content. (Don't use another documenting renderer.) - renderers = [renderer for renderer in view.renderers if not issubclass(renderer, DocumentingTemplateRenderer)] + renderers = [renderer for renderer in view.renderer_classes + if not issubclass(renderer, DocumentingTemplateRenderer)] if not renderers: return '[No renderers were found]' diff --git a/djangorestframework/request.py b/djangorestframework/request.py index ee43857ed..21538aec0 100644 --- a/djangorestframework/request.py +++ b/djangorestframework/request.py @@ -11,7 +11,7 @@ This enhanced request object offers the following : from django.http import HttpRequest -from djangorestframework.response import ErrorResponse +from djangorestframework.response import ImmediateResponse from djangorestframework import status from djangorestframework.utils.mediatypes import is_form_media_type, order_by_precedence from djangorestframework.utils import as_tuple @@ -194,7 +194,7 @@ class Request(object): """ Parse the request content. - May raise a 415 ErrorResponse (Unsupported Media Type), or a 400 ErrorResponse (Bad Request). + May raise a 415 ImmediateResponse (Unsupported Media Type), or a 400 ImmediateResponse (Bad Request). """ if stream is None or content_type is None: return (None, None) @@ -206,7 +206,7 @@ class Request(object): if parser.can_handle_request(content_type): return parser.parse(stream) - raise ErrorResponse(content={'error': + raise ImmediateResponse(content={'error': 'Unsupported media type in request \'%s\'.' % content_type}, status=status.HTTP_415_UNSUPPORTED_MEDIA_TYPE) diff --git a/djangorestframework/resources.py b/djangorestframework/resources.py index a20e477ec..f478bd521 100644 --- a/djangorestframework/resources.py +++ b/djangorestframework/resources.py @@ -2,7 +2,7 @@ from django import forms from django.core.urlresolvers import reverse, get_urlconf, get_resolver, NoReverseMatch from django.db import models -from djangorestframework.response import ErrorResponse +from djangorestframework.response import ImmediateResponse from djangorestframework.serializer import Serializer, _SkipField from djangorestframework.utils import as_tuple @@ -22,7 +22,7 @@ class BaseResource(Serializer): def validate_request(self, data, files=None): """ Given the request content return the cleaned, validated content. - Typically raises a :exc:`response.ErrorResponse` with status code 400 (Bad Request) on failure. + Typically raises a :exc:`response.ImmediateResponse` with status code 400 (Bad Request) on failure. """ return data @@ -73,19 +73,19 @@ class FormResource(Resource): """ Flag to check for unknown fields when validating a form. If set to false and we receive request data that is not expected by the form it raises an - :exc:`response.ErrorResponse` with status code 400. If set to true, only + :exc:`response.ImmediateResponse` with status code 400. If set to true, only expected fields are validated. """ def validate_request(self, data, files=None): """ Given some content as input return some cleaned, validated content. - Raises a :exc:`response.ErrorResponse` with status code 400 (Bad Request) on failure. + Raises a :exc:`response.ImmediateResponse` with status code 400 (Bad Request) on failure. Validation is standard form validation, with an additional constraint that *no extra unknown fields* may be supplied if :attr:`self.allow_unknown_form_fields` is ``False``. - On failure the :exc:`response.ErrorResponse` content is a dict which may contain :obj:`'errors'` and :obj:`'field-errors'` keys. + On failure the :exc:`response.ImmediateResponse` content is a dict which may contain :obj:`'errors'` and :obj:`'field-errors'` keys. If the :obj:`'errors'` key exists it is a list of strings of non-field errors. If the :obj:`'field-errors'` key exists it is a dict of ``{'field name as string': ['errors as strings', ...]}``. """ @@ -174,7 +174,7 @@ class FormResource(Resource): detail[u'field_errors'] = field_errors # Return HTTP 400 response (BAD REQUEST) - raise ErrorResponse(content=detail, status=400) + raise ImmediateResponse(content=detail, status=400) def get_form_class(self, method=None): """ @@ -273,14 +273,14 @@ class ModelResource(FormResource): def validate_request(self, data, files=None): """ Given some content as input return some cleaned, validated content. - Raises a :exc:`response.ErrorResponse` with status code 400 (Bad Request) on failure. + Raises a :exc:`response.ImmediateResponse` with status code 400 (Bad Request) on failure. Validation is standard form or model form validation, with an additional constraint that no extra unknown fields may be supplied, and that all fields specified by the fields class attribute must be supplied, even if they are not validated by the form/model form. - On failure the ErrorResponse content is a dict which may contain :obj:`'errors'` and :obj:`'field-errors'` keys. + On failure the ImmediateResponse content is a dict which may contain :obj:`'errors'` and :obj:`'field-errors'` keys. If the :obj:`'errors'` key exists it is a list of strings of non-field errors. If the ''field-errors'` key exists it is a dict of {field name as string: list of errors as strings}. """ diff --git a/djangorestframework/response.py b/djangorestframework/response.py index 4f9b3a625..3b692b24e 100644 --- a/djangorestframework/response.py +++ b/djangorestframework/response.py @@ -1,8 +1,18 @@ """ -The :mod:`response` module provides Response classes you can use in your -views to return a certain HTTP response. Typically a response is *rendered* -into a HTTP response depending on what renderers are set on your view and -als depending on the accept header of the request. +The :mod:`response` module provides :class:`Response` and :class:`ImmediateResponse` classes. + +`Response` is a subclass of `HttpResponse`, and can be similarly instantiated and returned +from any view. It is a bit smarter than Django's `HttpResponse` though, for it knows how +to use :mod:`renderers` to automatically render its content to a serial format. +This is achieved by : + + - determining the accepted types by checking for an overload or an `Accept` header in the request + - looking for a suitable renderer and using it on the content given at instantiation + + +`ImmediateResponse` is an exception that inherits from `Response`. It can be used +to abort the request handling (i.e. ``View.get``, ``View.put``, ...), +and immediately returning a response. """ from django.template.response import SimpleTemplateResponse @@ -13,7 +23,7 @@ from djangorestframework.utils import MSIE_USER_AGENT_REGEX from djangorestframework import status -__all__ = ('Response', 'ErrorResponse') +__all__ = ('Response', 'ImmediateResponse') class Response(SimpleTemplateResponse): @@ -26,15 +36,16 @@ class Response(SimpleTemplateResponse): def __init__(self, content=None, status=None, request=None, renderers=None): """ - content is the raw content. + `content` is the raw content, not yet serialized. This must be simple Python + data that renderers can handle (cf: dict, str, ...) - The set of renderers that the response can handle. - - Should be a tuple/list of classes as described in the :mod:`renderers` module. + `renderers` is a list/tuple of renderer instances and represents the set of renderers + that the response can handle. """ # 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 @@ -42,17 +53,14 @@ class Response(SimpleTemplateResponse): 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): """ + The final rendered content. Accessing this attribute triggers the complete rendering cycle : + selecting suitable renderer, setting response's actual content type, rendering data. """ 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 @@ -65,12 +73,20 @@ class Response(SimpleTemplateResponse): @property def status_text(self): """ - Return reason text corresponding to our HTTP response status code. + Returns reason text corresponding to our HTTP response status code. Provided for convenience. """ return STATUS_CODE_TEXT.get(self.status, '') def _determine_accept_list(self): + """ + Returns a list of accepted media types. This list is determined from : + + 1. overload with `_ACCEPT_QUERY_PARAM` + 2. `Accept` header of the request + + If those are useless, a default value is returned instead. + """ request = self.request if request is None: return ['*/*'] @@ -92,7 +108,7 @@ class Response(SimpleTemplateResponse): def _determine_renderer(self): """ - Determines the appropriate renderer for the output, given the client's 'Accept' header, + Determines the appropriate renderer for the output, given the list of accepted media types, and the :attr:`renderers` set on this class. Returns a 2-tuple of `(renderer, media_type)` @@ -103,16 +119,14 @@ class Response(SimpleTemplateResponse): # 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 renderer in self.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', + raise ImmediateResponse(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) @@ -152,10 +166,9 @@ class Response(SimpleTemplateResponse): return self.renderers[0] -class ErrorResponse(Response, BaseException): +class ImmediateResponse(Response, BaseException): """ - An exception representing an Response that should be returned immediately. - Any content should be serialized as-is, without being filtered. + A subclass of :class:`Response` used to abort the current request handling. """ pass diff --git a/djangorestframework/tests/accept.py b/djangorestframework/tests/accept.py index 2a02e04d2..e7dfc3038 100644 --- a/djangorestframework/tests/accept.py +++ b/djangorestframework/tests/accept.py @@ -23,9 +23,10 @@ class UserAgentMungingTest(TestCase): class MockView(View): permissions = () + response_class = Response def get(self, request): - return Response({'a':1, 'b':2, 'c':3}) + return self.response_class({'a':1, 'b':2, 'c':3}) self.req = RequestFactory() self.MockView = MockView diff --git a/djangorestframework/tests/mixins.py b/djangorestframework/tests/mixins.py index 7a1d27694..187ce7193 100644 --- a/djangorestframework/tests/mixins.py +++ b/djangorestframework/tests/mixins.py @@ -6,7 +6,7 @@ from djangorestframework.compat import RequestFactory from django.contrib.auth.models import Group, User from djangorestframework.mixins import CreateModelMixin, PaginatorMixin, ReadModelMixin from djangorestframework.resources import ModelResource -from djangorestframework.response import Response, ErrorResponse +from djangorestframework.response import Response, ImmediateResponse from djangorestframework.tests.models import CustomUser from djangorestframework.tests.testcases import TestModelsTestCase from djangorestframework.views import View @@ -41,7 +41,7 @@ class TestModelRead(TestModelsTestCase): mixin = ReadModelMixin() mixin.resource = GroupResource - self.assertRaises(ErrorResponse, mixin.get, request, id=12345) + self.assertRaises(ImmediateResponse, mixin.get, request, id=12345) class TestModelCreation(TestModelsTestCase): diff --git a/djangorestframework/tests/renderers.py b/djangorestframework/tests/renderers.py index 461bc877a..cc211dce2 100644 --- a/djangorestframework/tests/renderers.py +++ b/djangorestframework/tests/renderers.py @@ -73,8 +73,8 @@ class MockGETView(View): urlpatterns = patterns('', - url(r'^jsonp/jsonrenderer$', MockGETView.as_view(renderers=[JSONRenderer, JSONPRenderer])), - url(r'^jsonp/nojsonrenderer$', MockGETView.as_view(renderers=[JSONPRenderer])), + url(r'^jsonp/jsonrenderer$', MockGETView.as_view(renderer_classes=[JSONRenderer, JSONPRenderer])), + url(r'^jsonp/nojsonrenderer$', MockGETView.as_view(renderer_classes=[JSONPRenderer])), ) diff --git a/djangorestframework/tests/response.py b/djangorestframework/tests/response.py index 5a01e356f..b8cc5c1b6 100644 --- a/djangorestframework/tests/response.py +++ b/djangorestframework/tests/response.py @@ -1,9 +1,10 @@ import json +import unittest from django.conf.urls.defaults import patterns, url from django.test import TestCase -from djangorestframework.response import Response, ErrorResponse +from djangorestframework.response import Response, ImmediateResponse from djangorestframework.mixins import ResponseMixin from djangorestframework.views import View from djangorestframework.compat import View as DjangoView @@ -17,13 +18,16 @@ from djangorestframework.renderers import BaseRenderer, JSONRenderer, YAMLRender class TestResponseDetermineRenderer(TestCase): def get_response(self, url='', accept_list=[], renderers=[]): - request = RequestFactory().get(url, HTTP_ACCEPT=','.join(accept_list)) + kwargs = {} + if accept_list is not None: + kwargs['HTTP_ACCEPT'] = HTTP_ACCEPT=','.join(accept_list) + request = RequestFactory().get(url, **kwargs) 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): """ @@ -32,6 +36,13 @@ class TestResponseDetermineRenderer(TestCase): 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_default(self): + """ + Test that determine_accept_list takes the default renderer if Accept is not specified. + """ + response = self.get_response(accept_list=None) + self.assertEqual(response._determine_accept_list(), ['*/*']) def test_determine_accept_list_overriden_header(self): """ @@ -47,38 +58,46 @@ class TestResponseDetermineRenderer(TestCase): 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') + 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) + response = self.get_response(accept_list=accept_list, renderers=(prenderer, jrenderer)) renderer, media_type = response._determine_renderer() self.assertEqual(media_type, 'application/pickle') - self.assertTrue(isinstance(renderer, PRenderer)) + self.assertTrue(renderer, prenderer) - renderers = (JRenderer,) - response = self.get_response(accept_list=accept_list, renderers=renderers) + response = self.get_response(accept_list=accept_list, renderers=(jrenderer,)) renderer, media_type = response._determine_renderer() self.assertEqual(media_type, 'application/json') - self.assertTrue(isinstance(renderer, JRenderer)) + self.assertTrue(renderer, jrenderer) + + def test_determine_renderer_default(self): + """ + Test determine renderer when Accept was not specified. + """ + prenderer = self.get_renderer_mock('application/pickle') + + response = self.get_response(accept_list=None, renderers=(prenderer,)) + renderer, media_type = response._determine_renderer() + self.assertEqual(media_type, '*/*') + self.assertTrue(renderer, prenderer) 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') + 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) + response = self.get_response(accept_list=accept_list, renderers=(prenderer,)) + self.assertRaises(ImmediateResponse, 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) + return Response(request=request, content=content, renderers=[r() for r in DEFAULT_RENDERERS]) def test_render(self): """ @@ -116,7 +135,7 @@ class RendererB(BaseRenderer): class MockView(ResponseMixin, DjangoView): - renderers = (RendererA, RendererB) + renderer_classes = (RendererA, RendererB) def get(self, request, **kwargs): response = Response(DUMMYCONTENT, status=DUMMYSTATUS) @@ -124,22 +143,22 @@ class MockView(ResponseMixin, DjangoView): class HTMLView(View): - renderers = (DocumentingHTMLRenderer, ) + renderer_classes = (DocumentingHTMLRenderer, ) def get(self, request, **kwargs): return Response('text') class HTMLView1(View): - renderers = (DocumentingHTMLRenderer, JSONRenderer) + renderer_classes = (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'^.*\.(?P.+)$', MockView.as_view(renderer_classes=[RendererA, RendererB])), + url(r'^$', MockView.as_view(renderer_classes=[RendererA, RendererB])), url(r'^html$', HTMLView.as_view()), url(r'^html1$', HTMLView1.as_view()), ) @@ -197,11 +216,11 @@ class RendererIntegrationTests(TestCase): 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) + @unittest.skip('can\'t pass because view is a simple Django view and response is an ImmediateResponse') + 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 diff --git a/djangorestframework/tests/validators.py b/djangorestframework/tests/validators.py index 1f384b4c8..771b31256 100644 --- a/djangorestframework/tests/validators.py +++ b/djangorestframework/tests/validators.py @@ -2,7 +2,7 @@ from django import forms from django.db import models from django.test import TestCase from djangorestframework.resources import FormResource, ModelResource -from djangorestframework.response import ErrorResponse +from djangorestframework.response import ImmediateResponse from djangorestframework.views import View @@ -81,10 +81,10 @@ class TestNonFieldErrors(TestCase): content = {'field1': 'example1', 'field2': 'example2'} try: MockResource(view).validate_request(content, None) - except ErrorResponse, response: + except ImmediateResponse, response: self.assertEqual(response.raw_content, {'errors': [MockForm.ERROR_TEXT]}) else: - self.fail('ErrorResponse was not raised') + self.fail('ImmediateResponse was not raised') class TestFormValidation(TestCase): @@ -120,14 +120,14 @@ class TestFormValidation(TestCase): def validation_failure_raises_response_exception(self, validator): """If form validation fails a ResourceException 400 (Bad Request) should be raised.""" content = {} - self.assertRaises(ErrorResponse, validator.validate_request, content, None) + self.assertRaises(ImmediateResponse, validator.validate_request, content, None) def validation_does_not_allow_extra_fields_by_default(self, validator): """If some (otherwise valid) content includes fields that are not in the form then validation should fail. It might be okay on normal form submission, but for Web APIs we oughta get strict, as it'll help show up broken clients more easily (eg submitting content with a misnamed field)""" content = {'qwerty': 'uiop', 'extra': 'extra'} - self.assertRaises(ErrorResponse, validator.validate_request, content, None) + self.assertRaises(ImmediateResponse, validator.validate_request, content, None) def validation_allows_extra_fields_if_explicitly_set(self, validator): """If we include an allowed_extra_fields paramater on _validate, then allow fields with those names.""" @@ -154,7 +154,7 @@ class TestFormValidation(TestCase): content = {} try: validator.validate_request(content, None) - except ErrorResponse, response: + except ImmediateResponse, response: self.assertEqual(response.raw_content, {'field_errors': {'qwerty': ['This field is required.']}}) else: self.fail('ResourceException was not raised') @@ -164,7 +164,7 @@ class TestFormValidation(TestCase): content = {'qwerty': ''} try: validator.validate_request(content, None) - except ErrorResponse, response: + except ImmediateResponse, response: self.assertEqual(response.raw_content, {'field_errors': {'qwerty': ['This field is required.']}}) else: self.fail('ResourceException was not raised') @@ -174,7 +174,7 @@ class TestFormValidation(TestCase): content = {'qwerty': 'uiop', 'extra': 'extra'} try: validator.validate_request(content, None) - except ErrorResponse, response: + except ImmediateResponse, response: self.assertEqual(response.raw_content, {'field_errors': {'extra': ['This field does not exist.']}}) else: self.fail('ResourceException was not raised') @@ -184,7 +184,7 @@ class TestFormValidation(TestCase): content = {'qwerty': '', 'extra': 'extra'} try: validator.validate_request(content, None) - except ErrorResponse, response: + except ImmediateResponse, response: self.assertEqual(response.raw_content, {'field_errors': {'qwerty': ['This field is required.'], 'extra': ['This field does not exist.']}}) else: @@ -307,14 +307,14 @@ class TestModelFormValidator(TestCase): It might be okay on normal form submission, but for Web APIs we oughta get strict, as it'll help show up broken clients more easily (eg submitting content with a misnamed field)""" content = {'qwerty': 'example', 'uiop': 'example', 'readonly': 'read only', 'extra': 'extra'} - self.assertRaises(ErrorResponse, self.validator.validate_request, content, None) + self.assertRaises(ImmediateResponse, self.validator.validate_request, content, None) def test_validate_requires_fields_on_model_forms(self): """If some (otherwise valid) content includes fields that are not in the form then validation should fail. It might be okay on normal form submission, but for Web APIs we oughta get strict, as it'll help show up broken clients more easily (eg submitting content with a misnamed field)""" content = {'readonly': 'read only'} - self.assertRaises(ErrorResponse, self.validator.validate_request, content, None) + self.assertRaises(ImmediateResponse, self.validator.validate_request, content, None) def test_validate_does_not_require_blankable_fields_on_model_forms(self): """Test standard ModelForm validation behaviour - fields with blank=True are not required.""" diff --git a/djangorestframework/views.py b/djangorestframework/views.py index 44d68641f..8ba05e35a 100644 --- a/djangorestframework/views.py +++ b/djangorestframework/views.py @@ -13,7 +13,7 @@ from django.utils.safestring import mark_safe from django.views.decorators.csrf import csrf_exempt from djangorestframework.compat import View as DjangoView, apply_markdown -from djangorestframework.response import Response, ErrorResponse +from djangorestframework.response import Response, ImmediateResponse from djangorestframework.mixins import * from djangorestframework import resources, renderers, parsers, authentication, permissions, status @@ -81,7 +81,7 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): or `None` to use default behaviour. """ - renderers = renderers.DEFAULT_RENDERERS + renderer_classes = renderers.DEFAULT_RENDERERS """ List of renderers the resource can serialize the response with, ordered by preference. """ @@ -172,7 +172,7 @@ 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(content= + raise ImmediateResponse(content= {'detail': 'Method \'%s\' not allowed on this resource.' % request.method}, status=status.HTTP_405_METHOD_NOT_ALLOWED) @@ -232,13 +232,13 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): self.prepare_response(response) # Pre-serialize filtering (eg filter complex objects into natively serializable types) - # TODO: ugly + # TODO: ugly hack to handle both HttpResponse and Response. if hasattr(response, 'raw_content'): response.raw_content = self.filter_response(response.raw_content) else: response.content = self.filter_response(response.content) - except ErrorResponse, response: + except ImmediateResponse, response: # Prepare response for the response cycle. self.prepare_response(response) @@ -259,10 +259,10 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): for name, field in form.fields.iteritems(): field_name_types[name] = field.__class__.__name__ content['fields'] = field_name_types - # Note 'ErrorResponse' is misleading, it's just any response + # Note 'ImmediateResponse' is misleading, it's just any response # that should be rendered and returned immediately, without any # response filtering. - raise ErrorResponse(content=content, status=status.HTTP_200_OK) + raise ImmediateResponse(content=content, status=status.HTTP_200_OK) class ModelView(View): From 21292d31e7ad5ec731c9ef3e471f90cb29054686 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Piquemal?= Date: Tue, 7 Feb 2012 15:38:54 +0200 Subject: [PATCH 07/12] cleaned Request/Response/mixins to have similar interface --- djangorestframework/mixins.py | 90 +++++++++++----------- djangorestframework/parsers.py | 7 +- djangorestframework/request.py | 95 ++++++++++------------- djangorestframework/tests/request.py | 108 +++++++++++++-------------- djangorestframework/views.py | 12 +-- 5 files changed, 145 insertions(+), 167 deletions(-) diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index c30ef10b7..c1f755b84 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -12,7 +12,7 @@ from djangorestframework import status from djangorestframework.renderers import BaseRenderer from djangorestframework.resources import Resource, FormResource, ModelResource from djangorestframework.response import Response, ImmediateResponse -from djangorestframework.request import request_class_factory +from djangorestframework.request import Request from djangorestframework.utils import as_tuple, allowed_methods @@ -32,7 +32,6 @@ __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 ########## @@ -41,39 +40,43 @@ class RequestMixin(object): `Mixin` class to enhance API of Django's standard `request`. """ - _USE_FORM_OVERLOADING = True - _METHOD_PARAM = '_method' - _CONTENTTYPE_PARAM = '_content_type' - _CONTENT_PARAM = '_content' - - parsers = () + parser_classes = () """ - The set of parsers that the request can handle. + The set of parsers that the view can handle. Should be a tuple/list of classes as described in the :mod:`parsers` module. """ - def get_request_class(self): - """ - Returns a subclass of Django's `HttpRequest` with a richer API, - as described in :mod:`request`. - """ - if not hasattr(self, '_request_class'): - self._request_class = request_class_factory(self.request) - self._request_class._USE_FORM_OVERLOADING = self._USE_FORM_OVERLOADING - self._request_class._METHOD_PARAM = self._METHOD_PARAM - self._request_class._CONTENTTYPE_PARAM = self._CONTENTTYPE_PARAM - self._request_class._CONTENT_PARAM = self._CONTENT_PARAM - self._request_class.parsers = self.parsers - return self._request_class + request_class = Request + """ + The class to use as a wrapper for the original request object. + """ - def get_request(self): + def get_parsers(self): """ - Returns a custom request instance, with data and attributes copied from the - original request. + Instantiates and returns the list of parsers that will be used by the request + to parse its content. """ - request_class = self.get_request_class() - return request_class(self.request) + if not hasattr(self, '_parsers'): + self._parsers = [r(self) for r in self.parser_classes] + return self._parsers + + def prepare_request(self, request): + """ + Prepares the request for the request cycle. Returns a custom request instance, + with data and attributes copied from the original request. + """ + parsers = self.get_parsers() + request = self.request_class(request, parsers=parsers) + self.request = request + return request + + @property + def _parsed_media_types(self): + """ + Return a list of all the media types that this view can parse. + """ + return [p.media_type for p in self.parser_classes] ########## ResponseMixin ########## @@ -105,8 +108,8 @@ class ResponseMixin(object): def prepare_response(self, response): """ - Prepares the response for the response cycle. This has no effect if the - response is not an instance of :class:`response.Response`. + Prepares the response for the response cycle, and returns the prepared response. + This has no effect if the response is not an instance of :class:`response.Response`. """ if hasattr(response, 'request') and response.request is None: response.request = self.request @@ -124,6 +127,17 @@ class ResponseMixin(object): self.response = response return response + @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 + @property def _rendered_media_types(self): """ @@ -138,24 +152,6 @@ class ResponseMixin(object): """ return [renderer.format for renderer in self.get_renderers()] - @property - def _default_renderer(self): - """ - Return the view's default renderer class. - """ - return self.get_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 ########## diff --git a/djangorestframework/parsers.py b/djangorestframework/parsers.py index 5fc5c71ee..c041d7ce4 100644 --- a/djangorestframework/parsers.py +++ b/djangorestframework/parsers.py @@ -43,7 +43,7 @@ class BaseParser(object): media_type = None - def __init__(self, view): + def __init__(self, view=None): """ Initialize the parser with the ``View`` instance as state, in case the parser needs to access any metadata on the :obj:`View` object. @@ -167,10 +167,9 @@ class MultiPartParser(BaseParser): `data` will be a :class:`QueryDict` containing all the form parameters. `files` will be a :class:`QueryDict` containing all the form files. """ - # TODO: now self.view is in fact request, but should disappear ... - upload_handlers = self.view._get_upload_handlers() + upload_handlers = self.view.request._get_upload_handlers() try: - django_parser = DjangoMultiPartParser(self.view.META, stream, upload_handlers) + django_parser = DjangoMultiPartParser(self.view.request.META, stream, upload_handlers) except MultiPartParserError, exc: raise ImmediateResponse( content={'detail': 'multipart parse error - %s' % unicode(exc)}, diff --git a/djangorestframework/request.py b/djangorestframework/request.py index 21538aec0..cd6e30974 100644 --- a/djangorestframework/request.py +++ b/djangorestframework/request.py @@ -1,10 +1,10 @@ """ The :mod:`request` module provides a :class:`Request` class that can be used -to enhance the standard `request` object received in all the views. +to wrap the standard `request` object received in all the views, and upgrade its API. -This enhanced request object offers the following : +The wrapped request then offer the following : - - content automatically parsed according to `Content-Type` header, and available as :meth:`request.DATA` + - content automatically parsed according to `Content-Type` header, and available as :meth:`.DATA` - full support of PUT method, including support for file uploads - form overloading of HTTP method, content type and content """ @@ -22,21 +22,9 @@ from StringIO import StringIO __all__ = ('Request',) -def request_class_factory(request): - """ - Builds and returns a request class, to be used as a replacement of Django's built-in. - - In fact :class:`request.Request` needs to be mixed-in with a subclass of `HttpRequest` for use, - and we cannot do that before knowing which subclass of `HttpRequest` is used. So this function - takes a request instance as only argument, and returns a properly mixed-in request class. - """ - request_class = type(request) - return type(request_class.__name__, (Request, request_class), {}) - - class Request(object): """ - A mixin class allowing to enhance Django's standard HttpRequest. + A wrapper allowing to enhance Django's standard HttpRequest. """ _USE_FORM_OVERLOADING = True @@ -44,24 +32,14 @@ class Request(object): _CONTENTTYPE_PARAM = '_content_type' _CONTENT_PARAM = '_content' - parsers = () - """ - The set of parsers that the request can handle. - - Should be a tuple/list of classes as described in the :mod:`parsers` module. - """ - - def __init__(self, request): - # this allows to "copy" a request object into a new instance - # of our custom request class. - - # First, we prepare the attributes to copy. - attrs_dict = request.__dict__.copy() - attrs_dict.pop('method', None) - attrs_dict['_raw_method'] = request.method - - # Then, put them in the instance's own __dict__ - self.__dict__ = attrs_dict + def __init__(self, request=None, parsers=None): + """ + `parsers` is a list/tuple of parser instances and represents the set of psrsers + that the response can handle. + """ + self.request = request + if parsers is not None: + self.parsers = parsers @property def method(self): @@ -111,22 +89,6 @@ class Request(object): self._load_data_and_files() return self._files - def _load_post_and_files(self): - """ - Overrides the parent's `_load_post_and_files` to isolate it - from the form overloading mechanism (see: `_perform_form_overloading`). - """ - # When self.POST or self.FILES are called they need to know the original - # HTTP method, not our overloaded HTTP method. So, we save our overloaded - # HTTP method and restore it after the call to parent. - method_mem = getattr(self, '_method', None) - self._method = self._raw_method - super(Request, self)._load_post_and_files() - if method_mem is None: - del self._method - else: - self._method = method_mem - def _load_data_and_files(self): """ Parses the request content into self.DATA and self.FILES. @@ -145,7 +107,7 @@ class Request(object): self._perform_form_overloading() # if the HTTP method was not overloaded, we take the raw HTTP method if not hasattr(self, '_method'): - self._method = self._raw_method + self._method = self.request.method def _get_stream(self): """ @@ -172,7 +134,8 @@ class Request(object): """ # We only need to use form overloading on form POST requests. - if not self._USE_FORM_OVERLOADING or self._raw_method != 'POST' or not is_form_media_type(self._content_type): + if (not self._USE_FORM_OVERLOADING or self.request.method != 'POST' + or not is_form_media_type(self._content_type)): return # At this point we're committed to parsing the request as form data. @@ -199,10 +162,7 @@ class Request(object): if stream is None or content_type is None: return (None, None) - parsers = as_tuple(self.parsers) - - for parser_cls in parsers: - parser = parser_cls(self) + for parser in as_tuple(self.parsers): if parser.can_handle_request(content_type): return parser.parse(stream) @@ -223,3 +183,26 @@ class Request(object): Return the view's default parser class. """ return self.parsers[0] + + def _get_parsers(self): + """ + This just provides a default when parsers havent' been set. + """ + if hasattr(self, '_parsers'): + return self._parsers + return () + + def _set_parsers(self, value): + self._parsers = value + + parsers = property(_get_parsers, _set_parsers) + + def __getattr__(self, name): + """ + When an attribute is not present on the calling instance, try to get it + from the original request. + """ + if hasattr(self.request, name): + return getattr(self.request, name) + else: + return super(Request, self).__getattribute__(name) diff --git a/djangorestframework/tests/request.py b/djangorestframework/tests/request.py index 77a340336..c92d3f5fa 100644 --- a/djangorestframework/tests/request.py +++ b/djangorestframework/tests/request.py @@ -10,36 +10,19 @@ from djangorestframework.compat import RequestFactory from djangorestframework.mixins import RequestMixin from djangorestframework.parsers import FormParser, MultiPartParser, \ PlainTextParser, JSONParser +from djangorestframework.request import Request from djangorestframework.response import Response from djangorestframework.request import Request from djangorestframework.views import View -from djangorestframework.request import request_class_factory - -class MockView(View): - authentication = (UserLoggedInAuthentication,) - def post(self, request): - if request.POST.get('example') is not None: - return Response(status=status.HTTP_200_OK) - - return Response(status=status.INTERNAL_SERVER_ERROR) - -urlpatterns = patterns('', - (r'^$', MockView.as_view()), -) - -request_class = request_class_factory(RequestFactory().get('/')) class RequestTestCase(TestCase): - def tearDown(self): - request_class.parsers = () - def build_request(self, method, *args, **kwargs): factory = RequestFactory() method = getattr(factory, method) original_request = method(*args, **kwargs) - return request_class(original_request) + return Request(original_request) class TestMethodOverloading(RequestTestCase): @@ -67,14 +50,22 @@ class TestMethodOverloading(RequestTestCase): class TestContentParsing(RequestTestCase): - def tearDown(self): - request_class.parsers = () - def build_request(self, method, *args, **kwargs): factory = RequestFactory() + parsers = kwargs.pop('parsers', None) method = getattr(factory, method) original_request = method(*args, **kwargs) - return request_class(original_request) + rkwargs = {} + if parsers is not None: + rkwargs['parsers'] = parsers + request = Request(original_request, **rkwargs) + # TODO: Just a hack because the parsers need a view. This will be fixed in the future + class Obj(object): pass + obj = Obj() + obj.request = request + for p in request.parsers: + p.view = obj + return request def test_standard_behaviour_determines_no_content_GET(self): """Ensure request.DATA returns None for GET request with no content.""" @@ -89,31 +80,35 @@ class TestContentParsing(RequestTestCase): def test_standard_behaviour_determines_form_content_POST(self): """Ensure request.DATA returns content for POST request with form content.""" form_data = {'qwerty': 'uiop'} - request_class.parsers = (FormParser, MultiPartParser) - request = self.build_request('post', '/', data=form_data) + parsers = (FormParser(), MultiPartParser()) + + request = self.build_request('post', '/', data=form_data, parsers=parsers) self.assertEqual(request.DATA.items(), form_data.items()) def test_standard_behaviour_determines_non_form_content_POST(self): """Ensure request.DATA returns content for POST request with non-form content.""" content = 'qwerty' content_type = 'text/plain' - request_class.parsers = (PlainTextParser,) - request = self.build_request('post', '/', content, content_type=content_type) + parsers = (PlainTextParser(),) + + request = self.build_request('post', '/', content, content_type=content_type, parsers=parsers) self.assertEqual(request.DATA, content) def test_standard_behaviour_determines_form_content_PUT(self): """Ensure request.DATA returns content for PUT request with form content.""" form_data = {'qwerty': 'uiop'} - request_class.parsers = (FormParser, MultiPartParser) - request = self.build_request('put', '/', data=form_data) + parsers = (FormParser(), MultiPartParser()) + + request = self.build_request('put', '/', data=form_data, parsers=parsers) self.assertEqual(request.DATA.items(), form_data.items()) def test_standard_behaviour_determines_non_form_content_PUT(self): """Ensure request.DATA returns content for PUT request with non-form content.""" content = 'qwerty' content_type = 'text/plain' - request_class.parsers = (PlainTextParser,) - request = self.build_request('put', '/', content, content_type=content_type) + parsers = (PlainTextParser(),) + + request = self.build_request('put', '/', content, content_type=content_type, parsers=parsers) self.assertEqual(request.DATA, content) def test_overloaded_behaviour_allows_content_tunnelling(self): @@ -122,16 +117,17 @@ class TestContentParsing(RequestTestCase): content_type = 'text/plain' form_data = {Request._CONTENT_PARAM: content, Request._CONTENTTYPE_PARAM: content_type} - request_class.parsers = (PlainTextParser,) - request = self.build_request('post', '/', form_data) + parsers = (PlainTextParser(),) + + request = self.build_request('post', '/', form_data, parsers=parsers) self.assertEqual(request.DATA, content) def test_accessing_post_after_data_form(self): """Ensures request.POST can be accessed after request.DATA in form request""" form_data = {'qwerty': 'uiop'} - request_class.parsers = (FormParser, MultiPartParser) - request = self.build_request('post', '/', data=form_data) + parsers = (FormParser(), MultiPartParser()) + request = self.build_request('post', '/', data=form_data) self.assertEqual(request.DATA.items(), form_data.items()) self.assertEqual(request.POST.items(), form_data.items()) @@ -142,11 +138,9 @@ class TestContentParsing(RequestTestCase): data = {'qwerty': 'uiop'} content = json.dumps(data) content_type = 'application/json' + parsers = (JSONParser(),) - request_class.parsers = (JSONParser,) - - request = self.build_request('post', '/', content, content_type=content_type) - + request = self.build_request('post', '/', content, content_type=content_type, parsers=parsers) self.assertEqual(request.DATA.items(), data.items()) self.assertEqual(request.POST.items(), []) @@ -157,22 +151,19 @@ class TestContentParsing(RequestTestCase): data = {'qwerty': 'uiop'} content = json.dumps(data) content_type = 'application/json' - - request_class.parsers = (JSONParser,) - + parsers = (JSONParser(),) form_data = {Request._CONTENT_PARAM: content, Request._CONTENTTYPE_PARAM: content_type} - request = self.build_request('post', '/', data=form_data) - + request = self.build_request('post', '/', data=form_data, parsers=parsers) self.assertEqual(request.DATA.items(), data.items()) self.assertEqual(request.POST.items(), form_data.items()) def test_accessing_data_after_post_form(self): """Ensures request.DATA can be accessed after request.POST in form request""" form_data = {'qwerty': 'uiop'} - request_class.parsers = (FormParser, MultiPartParser) - request = self.build_request('post', '/', data=form_data) + parsers = (FormParser, MultiPartParser) + request = self.build_request('post', '/', data=form_data, parsers=parsers) self.assertEqual(request.POST.items(), form_data.items()) self.assertEqual(request.DATA.items(), form_data.items()) @@ -184,11 +175,9 @@ class TestContentParsing(RequestTestCase): data = {'qwerty': 'uiop'} content = json.dumps(data) content_type = 'application/json' + parsers = (JSONParser(),) - request_class.parsers = (JSONParser,) - - request = self.build_request('post', '/', content, content_type=content_type) - + request = self.build_request('post', '/', content, content_type=content_type, parsers=parsers) post_items = request.POST.items() self.assertEqual(len(post_items), 1) @@ -203,17 +192,28 @@ class TestContentParsing(RequestTestCase): data = {'qwerty': 'uiop'} content = json.dumps(data) content_type = 'application/json' - - request_class.parsers = (JSONParser,) - + parsers = (JSONParser(),) form_data = {Request._CONTENT_PARAM: content, Request._CONTENTTYPE_PARAM: content_type} - request = self.build_request('post', '/', data=form_data) + request = self.build_request('post', '/', data=form_data, parsers=parsers) self.assertEqual(request.POST.items(), form_data.items()) self.assertEqual(request.DATA.items(), data.items()) +class MockView(View): + authentication = (UserLoggedInAuthentication,) + def post(self, request): + if request.POST.get('example') is not None: + return Response(status=status.HTTP_200_OK) + + return Response(status=status.INTERNAL_SERVER_ERROR) + +urlpatterns = patterns('', + (r'^$', MockView.as_view()), +) + + class TestContentParsingWithAuthentication(TestCase): urls = 'djangorestframework.tests.request' diff --git a/djangorestframework/views.py b/djangorestframework/views.py index 8ba05e35a..761737c4d 100644 --- a/djangorestframework/views.py +++ b/djangorestframework/views.py @@ -83,12 +83,12 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): renderer_classes = renderers.DEFAULT_RENDERERS """ - List of renderers the resource can serialize the response with, ordered by preference. + List of renderer classes the resource can serialize the response with, ordered by preference. """ - parsers = parsers.DEFAULT_PARSERS + parser_classes = parsers.DEFAULT_PARSERS """ - List of parsers the resource can parse the request with. + List of parser classes the resource can parse the request with. """ authentication = (authentication.UserLoggedInAuthentication, @@ -210,7 +210,7 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): try: # Get a custom request, built form the original request instance - self.request = request = self.get_request() + request = self.prepare_request(request) # `initial` is the opportunity to temper with the request, # even completely replace it. @@ -229,7 +229,7 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): response = handler(request, *args, **kwargs) # Prepare response for the response cycle. - self.prepare_response(response) + response = self.prepare_response(response) # Pre-serialize filtering (eg filter complex objects into natively serializable types) # TODO: ugly hack to handle both HttpResponse and Response. @@ -251,7 +251,7 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): 'name': self.get_name(), 'description': self.get_description(), 'renders': self._rendered_media_types, - 'parses': request._parsed_media_types, + 'parses': self._parsed_media_types, } form = self.get_bound_form() if form is not None: From 6963fd3623ee217fe489abb25f0ffa8c0781e4cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Piquemal?= Date: Tue, 7 Feb 2012 16:22:14 +0200 Subject: [PATCH 08/12] some docs for Request/Response/mixins --- djangorestframework/mixins.py | 16 ++++++---------- djangorestframework/request.py | 19 ++++++++----------- djangorestframework/response.py | 26 +++++++++++--------------- docs/howto/requestmixin.rst | 9 ++++----- examples/requestexample/views.py | 5 ++--- 5 files changed, 31 insertions(+), 44 deletions(-) diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index c1f755b84..ef4965a55 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -37,7 +37,7 @@ __all__ = ( class RequestMixin(object): """ - `Mixin` class to enhance API of Django's standard `request`. + `Mixin` class enabling the use of :class:`request.Request` in your views. """ parser_classes = () @@ -63,8 +63,8 @@ class RequestMixin(object): def prepare_request(self, request): """ - Prepares the request for the request cycle. Returns a custom request instance, - with data and attributes copied from the original request. + Prepares the request cycle. Returns an instance of :class:`request.Request`, + wrapping the original request object. """ parsers = self.get_parsers() request = self.request_class(request, parsers=parsers) @@ -74,7 +74,7 @@ class RequestMixin(object): @property def _parsed_media_types(self): """ - Return a list of all the media types that this view can parse. + Returns a list of all the media types that this view can parse. """ return [p.media_type for p in self.parser_classes] @@ -83,11 +83,7 @@ class RequestMixin(object): class ResponseMixin(object): """ - Adds behavior for pluggable `renderers` to a :class:`views.View` class. - - Default behavior is to use standard HTTP Accept header content negotiation. - Also supports overriding the content type by specifying an ``_accept=`` parameter in the URL. - Ignores Accept headers from Internet Explorer user agents and uses a sensible browser Accept header instead. + `Mixin` class enabling the use of :class:`response.Response` in your views. """ renderer_classes = () @@ -108,7 +104,7 @@ class ResponseMixin(object): def prepare_response(self, response): """ - Prepares the response for the response cycle, and returns the prepared response. + Prepares and returns `response`. This has no effect if the response is not an instance of :class:`response.Response`. """ if hasattr(response, 'request') and response.request is None: diff --git a/djangorestframework/request.py b/djangorestframework/request.py index cd6e30974..8cf95f185 100644 --- a/djangorestframework/request.py +++ b/djangorestframework/request.py @@ -1,8 +1,8 @@ """ -The :mod:`request` module provides a :class:`Request` class that can be used -to wrap the standard `request` object received in all the views, and upgrade its API. +The :mod:`request` module provides a :class:`Request` class used to wrap the standard `request` +object received in all the views. -The wrapped request then offer the following : +The wrapped request then offers a richer API, in particular : - content automatically parsed according to `Content-Type` header, and available as :meth:`.DATA` - full support of PUT method, including support for file uploads @@ -24,7 +24,11 @@ __all__ = ('Request',) class Request(object): """ - A wrapper allowing to enhance Django's standard HttpRequest. + Wrapper allowing to enhance a standard `HttpRequest` instance. + + Kwargs: + - request(HttpRequest). The original request instance. + - parsers(list/tuple). The parsers to use for parsing the request content. """ _USE_FORM_OVERLOADING = True @@ -33,10 +37,6 @@ class Request(object): _CONTENT_PARAM = '_content' def __init__(self, request=None, parsers=None): - """ - `parsers` is a list/tuple of parser instances and represents the set of psrsers - that the response can handle. - """ self.request = request if parsers is not None: self.parsers = parsers @@ -185,9 +185,6 @@ class Request(object): return self.parsers[0] def _get_parsers(self): - """ - This just provides a default when parsers havent' been set. - """ if hasattr(self, '_parsers'): return self._parsers return () diff --git a/djangorestframework/response.py b/djangorestframework/response.py index 3b692b24e..29fffed30 100644 --- a/djangorestframework/response.py +++ b/djangorestframework/response.py @@ -2,12 +2,13 @@ The :mod:`response` module provides :class:`Response` and :class:`ImmediateResponse` classes. `Response` is a subclass of `HttpResponse`, and can be similarly instantiated and returned -from any view. It is a bit smarter than Django's `HttpResponse` though, for it knows how -to use :mod:`renderers` to automatically render its content to a serial format. -This is achieved by : +from any view. It is a bit smarter than Django's `HttpResponse`, for it renders automatically +its content to a serial format by using a list of :mod:`renderers`. - - determining the accepted types by checking for an overload or an `Accept` header in the request - - looking for a suitable renderer and using it on the content given at instantiation +To determine the content type to which it must render, default behaviour is to use standard +HTTP Accept header content negotiation. But `Response` also supports overriding the content type +by specifying an ``_accept=`` parameter in the URL. Also, `Response` will ignore `Accept` headers +from Internet Explorer user agents and use a sensible browser `Accept` header instead. `ImmediateResponse` is an exception that inherits from `Response`. It can be used @@ -29,19 +30,17 @@ __all__ = ('Response', 'ImmediateResponse') class Response(SimpleTemplateResponse): """ An HttpResponse that may include content that hasn't yet been serialized. + + Kwargs: + - content(object). The raw content, not yet serialized. This must be simple Python \ + data that renderers can handle (e.g.: `dict`, `str`, ...) + - renderers(list/tuple). The renderers to use for rendering the response content. """ _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, not yet serialized. This must be simple Python - data that renderers can handle (cf: dict, str, ...) - - `renderers` is a list/tuple of renderer instances and represents the set of renderers - that the response can handle. - """ # First argument taken by `SimpleTemplateResponse.__init__` is template_name, # which we don't need super(Response, self).__init__(None, status=status) @@ -132,9 +131,6 @@ class Response(SimpleTemplateResponse): 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 () diff --git a/docs/howto/requestmixin.rst b/docs/howto/requestmixin.rst index a00fdad0e..c0aadb3f7 100644 --- a/docs/howto/requestmixin.rst +++ b/docs/howto/requestmixin.rst @@ -1,7 +1,7 @@ Using the enhanced request in all your views ============================================== -This example shows how you can use Django REST framework's enhanced `request` in your own views, without having to use the full-blown :class:`views.View` class. +This example shows how you can use Django REST framework's enhanced `request` - :class:`request.Request` - in your own views, without having to use the full-blown :class:`views.View` class. What can it do for you ? Mostly, it will take care of parsing the request's content, and handling equally all HTTP methods ... @@ -64,13 +64,12 @@ Now that you're convinced you need to use the enhanced request object, here is h Base view enabling the usage of enhanced requests with user defined views. """ - parsers = parsers.DEFAULT_PARSERS + parser_classes = parsers.DEFAULT_PARSERS def dispatch(self, request, *args, **kwargs): - self.request = request - request = self.get_request() + request = self.prepare_request(request) return super(MyBaseViewUsingEnhancedRequest, self).dispatch(request, *args, **kwargs) And then, use this class as a base for all your custom views. -.. note:: you can also check the request example. +.. note:: you can see this live in the examples. diff --git a/examples/requestexample/views.py b/examples/requestexample/views.py index aa8a734f5..5411a3232 100644 --- a/examples/requestexample/views.py +++ b/examples/requestexample/views.py @@ -21,11 +21,10 @@ class MyBaseViewUsingEnhancedRequest(RequestMixin, View): Base view enabling the usage of enhanced requests with user defined views. """ - parsers = parsers.DEFAULT_PARSERS + parser_classes = parsers.DEFAULT_PARSERS def dispatch(self, request, *args, **kwargs): - self.request = request - request = self.get_request() + request = self.prepare_request(request) return super(MyBaseViewUsingEnhancedRequest, self).dispatch(request, *args, **kwargs) From 2cdff1b01e3aca6c56cef433e786e3ae75362739 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Piquemal?= Date: Tue, 7 Feb 2012 16:52:15 +0200 Subject: [PATCH 09/12] modified examples, somethin' still broken, can't find what --- djangorestframework/mixins.py | 2 +- djangorestframework/views.py | 1 + examples/mixin/urls.py | 8 +++---- examples/objectstore/views.py | 10 ++++---- examples/permissionsexample/views.py | 9 +++---- examples/pygments_api/views.py | 6 ++--- examples/requestexample/urls.py | 4 +++- examples/requestexample/views.py | 35 ++-------------------------- examples/resourceexample/views.py | 8 +++---- examples/sandbox/views.py | 5 ++-- examples/views.py | 34 +++++++++++++++++++++++++++ 11 files changed, 66 insertions(+), 56 deletions(-) create mode 100644 examples/views.py diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index ef4965a55..57b855957 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -58,7 +58,7 @@ class RequestMixin(object): to parse its content. """ if not hasattr(self, '_parsers'): - self._parsers = [r(self) for r in self.parser_classes] + self._parsers = [p(self) for p in self.parser_classes] return self._parsers def prepare_request(self, request): diff --git a/djangorestframework/views.py b/djangorestframework/views.py index 761737c4d..5bba6b4eb 100644 --- a/djangorestframework/views.py +++ b/djangorestframework/views.py @@ -15,6 +15,7 @@ from django.views.decorators.csrf import csrf_exempt from djangorestframework.compat import View as DjangoView, apply_markdown from djangorestframework.response import Response, ImmediateResponse from djangorestframework.mixins import * +from djangorestframework.utils import allowed_methods from djangorestframework import resources, renderers, parsers, authentication, permissions, status diff --git a/examples/mixin/urls.py b/examples/mixin/urls.py index a3da3b2cc..6e9e497e6 100644 --- a/examples/mixin/urls.py +++ b/examples/mixin/urls.py @@ -10,12 +10,12 @@ from django.core.urlresolvers import reverse class ExampleView(ResponseMixin, View): """An example view using Django 1.3's class based views. Uses djangorestframework's RendererMixin to provide support for multiple output formats.""" - renderers = DEFAULT_RENDERERS + renderer_classes = DEFAULT_RENDERERS def get(self, request): - response = Response(200, {'description': 'Some example content', - 'url': reverse('mixin-view')}) - return self.render(response) + response = Response({'description': 'Some example content', + 'url': reverse('mixin-view')}, status=200) + return self.prepare_response(response) urlpatterns = patterns('', diff --git a/examples/objectstore/views.py b/examples/objectstore/views.py index d85ed9f44..47f5147a6 100644 --- a/examples/objectstore/views.py +++ b/examples/objectstore/views.py @@ -41,7 +41,7 @@ class ObjectStoreRoot(View): filepaths = [os.path.join(OBJECT_STORE_DIR, file) for file in os.listdir(OBJECT_STORE_DIR) if not file.startswith('.')] ctime_sorted_basenames = [item[0] for item in sorted([(os.path.basename(path), os.path.getctime(path)) for path in filepaths], key=operator.itemgetter(1), reverse=True)] - return [reverse('stored-object', kwargs={'key':key}) for key in ctime_sorted_basenames] + return Response([reverse('stored-object', kwargs={'key':key}) for key in ctime_sorted_basenames]) def post(self, request): """ @@ -51,7 +51,8 @@ class ObjectStoreRoot(View): pathname = os.path.join(OBJECT_STORE_DIR, key) pickle.dump(self.CONTENT, open(pathname, 'wb')) remove_oldest_files(OBJECT_STORE_DIR, MAX_FILES) - return Response(status.HTTP_201_CREATED, self.CONTENT, {'Location': reverse('stored-object', kwargs={'key':key})}) + self.headers['Location'] = reverse('stored-object', kwargs={'key':key}) + return Response(self.CONTENT, status=status.HTTP_201_CREATED) class StoredObject(View): @@ -67,7 +68,7 @@ class StoredObject(View): pathname = os.path.join(OBJECT_STORE_DIR, key) if not os.path.exists(pathname): return Response(status.HTTP_404_NOT_FOUND) - return pickle.load(open(pathname, 'rb')) + return Response(pickle.load(open(pathname, 'rb'))) def put(self, request, key): """ @@ -75,7 +76,7 @@ class StoredObject(View): """ pathname = os.path.join(OBJECT_STORE_DIR, key) pickle.dump(self.CONTENT, open(pathname, 'wb')) - return self.CONTENT + return Response(self.CONTENT) def delete(self, request, key): """ @@ -85,3 +86,4 @@ class StoredObject(View): if not os.path.exists(pathname): return Response(status.HTTP_404_NOT_FOUND) os.remove(pathname) + return Response() diff --git a/examples/permissionsexample/views.py b/examples/permissionsexample/views.py index 86f458f8e..bcf6619cf 100644 --- a/examples/permissionsexample/views.py +++ b/examples/permissionsexample/views.py @@ -1,4 +1,5 @@ from djangorestframework.views import View +from djangorestframework.response import Response from djangorestframework.permissions import PerUserThrottling, IsAuthenticated from django.core.urlresolvers import reverse @@ -9,7 +10,7 @@ class PermissionsExampleView(View): """ def get(self, request): - return [ + return Response([ { 'name': 'Throttling Example', 'url': reverse('throttled-resource') @@ -18,7 +19,7 @@ class PermissionsExampleView(View): 'name': 'Logged in example', 'url': reverse('loggedin-resource') }, - ] + ]) class ThrottlingExampleView(View): @@ -36,7 +37,7 @@ class ThrottlingExampleView(View): """ Handle GET requests. """ - return "Successful response to GET request because throttle is not yet active." + return Response("Successful response to GET request because throttle is not yet active.") class LoggedInExampleView(View): @@ -49,4 +50,4 @@ class LoggedInExampleView(View): permissions = (IsAuthenticated, ) def get(self, request): - return 'You have permission to view this resource' + return Response('You have permission to view this resource') diff --git a/examples/pygments_api/views.py b/examples/pygments_api/views.py index ffea60ae3..44dd2caa5 100644 --- a/examples/pygments_api/views.py +++ b/examples/pygments_api/views.py @@ -61,7 +61,7 @@ class PygmentsRoot(View): Return a list of all currently existing snippets. """ unique_ids = [os.path.split(f)[1] for f in list_dir_sorted_by_ctime(HIGHLIGHTED_CODE_DIR)] - return [reverse('pygments-instance', args=[unique_id]) for unique_id in unique_ids] + return Response([reverse('pygments-instance', args=[unique_id]) for unique_id in unique_ids]) def post(self, request): """ @@ -98,7 +98,7 @@ class PygmentsInstance(View): pathname = os.path.join(HIGHLIGHTED_CODE_DIR, unique_id) if not os.path.exists(pathname): return Response(status.HTTP_404_NOT_FOUND) - return open(pathname, 'r').read() + return Response(open(pathname, 'r').read()) def delete(self, request, unique_id): """ @@ -107,5 +107,5 @@ class PygmentsInstance(View): pathname = os.path.join(HIGHLIGHTED_CODE_DIR, unique_id) if not os.path.exists(pathname): return Response(status.HTTP_404_NOT_FOUND) - return os.remove(pathname) + return Response(os.remove(pathname)) diff --git a/examples/requestexample/urls.py b/examples/requestexample/urls.py index a5e3356a1..3c31e4a9b 100644 --- a/examples/requestexample/urls.py +++ b/examples/requestexample/urls.py @@ -1,5 +1,7 @@ from django.conf.urls.defaults import patterns, url -from requestexample.views import RequestExampleView, MockView, EchoRequestContentView +from requestexample.views import RequestExampleView, EchoRequestContentView +from examples.views import MockView + urlpatterns = patterns('', url(r'^$', RequestExampleView.as_view(), name='request-example'), diff --git a/examples/requestexample/views.py b/examples/requestexample/views.py index 5411a3232..876db864c 100644 --- a/examples/requestexample/views.py +++ b/examples/requestexample/views.py @@ -5,6 +5,7 @@ from django.core.urlresolvers import reverse from djangorestframework.mixins import RequestMixin from djangorestframework.views import View as DRFView from djangorestframework import parsers +from djangorestframework.response import Response class RequestExampleView(DRFView): @@ -13,7 +14,7 @@ class RequestExampleView(DRFView): """ def get(self, request): - return [{'name': 'request.DATA Example', 'url': reverse('request-content')},] + return Response([{'name': 'request.DATA Example', 'url': reverse('request-content')},]) class MyBaseViewUsingEnhancedRequest(RequestMixin, View): @@ -41,35 +42,3 @@ class EchoRequestContentView(MyBaseViewUsingEnhancedRequest): return HttpResponse(("Found %s in request.DATA, content : %s" % (type(request.DATA), request.DATA))) - -class MockView(DRFView): - """ - A view that just acts as a proxy to call non-djangorestframework views, while still - displaying the browsable API interface. - """ - - view_class = None - - def dispatch(self, request, *args, **kwargs): - self.request = request - if self.get_request().method in ['PUT', 'POST']: - self.response = self.view_class.as_view()(request, *args, **kwargs) - return super(MockView, self).dispatch(request, *args, **kwargs) - - def get(self, request, *args, **kwargs): - return - - def put(self, request, *args, **kwargs): - return self.response.content - - def post(self, request, *args, **kwargs): - return self.response.content - - def __getattribute__(self, name): - if name == '__name__': - return self.view_class.__name__ - elif name == '__doc__': - return self.view_class.__doc__ - else: - return super(MockView, self).__getattribute__(name) - diff --git a/examples/resourceexample/views.py b/examples/resourceexample/views.py index e6b5eeb89..44c4176a1 100644 --- a/examples/resourceexample/views.py +++ b/examples/resourceexample/views.py @@ -16,12 +16,12 @@ class ExampleView(View): """ Handle GET requests, returning a list of URLs pointing to 3 other views. """ - return {"Some other resources": [reverse('another-example', kwargs={'num':num}) for num in range(3)]} + return Response({"Some other resources": [reverse('another-example', kwargs={'num':num}) for num in range(3)]}) class AnotherExampleView(View): """ - A basic view, that can be handle GET and POST requests. + A basic view, that can handle GET and POST requests. Applies some simple form validation on POST requests. """ form = MyForm @@ -33,7 +33,7 @@ class AnotherExampleView(View): """ if int(num) > 2: return Response(status.HTTP_404_NOT_FOUND) - return "GET request to AnotherExampleResource %s" % num + return Response("GET request to AnotherExampleResource %s" % num) def post(self, request, num): """ @@ -42,4 +42,4 @@ class AnotherExampleView(View): """ if int(num) > 2: return Response(status.HTTP_404_NOT_FOUND) - return "POST request to AnotherExampleResource %s, with content: %s" % (num, repr(self.CONTENT)) + return Response("POST request to AnotherExampleResource %s, with content: %s" % (num, repr(self.CONTENT))) diff --git a/examples/sandbox/views.py b/examples/sandbox/views.py index 998887a7b..49b59b40f 100644 --- a/examples/sandbox/views.py +++ b/examples/sandbox/views.py @@ -2,6 +2,7 @@ from django.core.urlresolvers import reverse from djangorestframework.views import View +from djangorestframework.response import Response class Sandbox(View): @@ -28,7 +29,7 @@ class Sandbox(View): Please feel free to browse, create, edit and delete the resources in these examples.""" def get(self, request): - return [{'name': 'Simple Resource example', 'url': reverse('example-resource')}, + return Response([{'name': 'Simple Resource example', 'url': reverse('example-resource')}, {'name': 'Simple ModelResource example', 'url': reverse('model-resource-root')}, {'name': 'Simple Mixin-only example', 'url': reverse('mixin-view')}, {'name': 'Object store API', 'url': reverse('object-store-root')}, @@ -36,4 +37,4 @@ class Sandbox(View): {'name': 'Blog posts API', 'url': reverse('blog-posts-root')}, {'name': 'Permissions example', 'url': reverse('permissions-example')}, {'name': 'Simple request mixin example', 'url': reverse('request-example')} - ] + ]) diff --git a/examples/views.py b/examples/views.py new file mode 100644 index 000000000..606edc3a2 --- /dev/null +++ b/examples/views.py @@ -0,0 +1,34 @@ +from djangorestframework.views import View +from djangorestframework.response import Response + + +class MockView(View): + """ + A view that just acts as a proxy to call non-djangorestframework views, while still + displaying the browsable API interface. + """ + + view_class = None + + def dispatch(self, request, *args, **kwargs): + request = self.prepare_request(request) + if request.method in ['PUT', 'POST']: + self.response = self.view_class.as_view()(request, *args, **kwargs) + return super(MockView, self).dispatch(request, *args, **kwargs) + + def get(self, request, *args, **kwargs): + return Response() + + def put(self, request, *args, **kwargs): + return Response(self.response.content) + + def post(self, request, *args, **kwargs): + return Response(self.response.content) + + def __getattribute__(self, name): + if name == '__name__': + return self.view_class.__name__ + elif name == '__doc__': + return self.view_class.__doc__ + else: + return super(MockView, self).__getattribute__(name) From db0b01037a95946938ccd44eae14d8779bfff1a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Piquemal?= Date: Fri, 10 Feb 2012 10:18:39 +0200 Subject: [PATCH 10/12] made suggested fixes --- djangorestframework/mixins.py | 40 ++++++++++-------------------- djangorestframework/parsers.py | 6 ++--- djangorestframework/permissions.py | 4 +-- djangorestframework/request.py | 6 ++--- djangorestframework/resources.py | 2 +- djangorestframework/response.py | 2 +- djangorestframework/views.py | 19 ++++++++------ 7 files changed, 34 insertions(+), 45 deletions(-) diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index 57b855957..516a0f4b5 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -43,7 +43,6 @@ class RequestMixin(object): parser_classes = () """ The set of parsers that the view can handle. - Should be a tuple/list of classes as described in the :mod:`parsers` module. """ @@ -54,22 +53,18 @@ class RequestMixin(object): def get_parsers(self): """ - Instantiates and returns the list of parsers that will be used by the request - to parse its content. + Instantiates and returns the list of parsers the request will use. """ - if not hasattr(self, '_parsers'): - self._parsers = [p(self) for p in self.parser_classes] - return self._parsers + return [p(self) for p in self.parser_classes] - def prepare_request(self, request): + def create_request(self, request): """ - Prepares the request cycle. Returns an instance of :class:`request.Request`, - wrapping the original request object. + Creates and returns an instance of :class:`request.Request`. + This new instance wraps the `request` passed as a parameter, and use the + parsers set on the view. """ parsers = self.get_parsers() - request = self.request_class(request, parsers=parsers) - self.request = request - return request + return self.request_class(request, parsers=parsers) @property def _parsed_media_types(self): @@ -89,38 +84,29 @@ class ResponseMixin(object): renderer_classes = () """ The set of response renderers that the view can handle. - Should be a tuple/list of classes as described in the :mod:`renderers` module. """ def get_renderers(self): """ - Instantiates and returns the list of renderers that will be used to render - the response. + Instantiates and returns the list of renderers the response will use. """ - if not hasattr(self, '_renderers'): - self._renderers = [r(self) for r in self.renderer_classes] - return self._renderers + return [r(self) for r in self.renderer_classes] def prepare_response(self, response): """ - Prepares and returns `response`. + Prepares and returns `response`. This has no effect if the response is not an instance of :class:`response.Response`. """ 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 + # set all the cached headers for name, value in self.headers.items(): response[name] = value # set the views renderers on the response response.renderers = self.get_renderers() - self.response = response return response @property @@ -571,12 +557,12 @@ class PaginatorMixin(object): page_num = int(self.request.GET.get('page', '1')) except ValueError: raise ImmediateResponse( - content={'detail': 'That page contains no results'}, + {'detail': 'That page contains no results'}, status=status.HTTP_404_NOT_FOUND) if page_num not in paginator.page_range: raise ImmediateResponse( - content={'detail': 'That page contains no results'}, + {'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 c041d7ce4..d41e07e8b 100644 --- a/djangorestframework/parsers.py +++ b/djangorestframework/parsers.py @@ -89,7 +89,7 @@ class JSONParser(BaseParser): return (json.load(stream), None) except ValueError, exc: raise ImmediateResponse( - content={'detail': 'JSON parse error - %s' % unicode(exc)}, + {'detail': 'JSON parse error - %s' % unicode(exc)}, status=status.HTTP_400_BAD_REQUEST) @@ -112,7 +112,7 @@ if yaml: return (yaml.safe_load(stream), None) except ValueError, exc: raise ImmediateResponse( - content={'detail': 'YAML parse error - %s' % unicode(exc)}, + {'detail': 'YAML parse error - %s' % unicode(exc)}, status=status.HTTP_400_BAD_REQUEST) else: YAMLParser = None @@ -172,7 +172,7 @@ class MultiPartParser(BaseParser): django_parser = DjangoMultiPartParser(self.view.request.META, stream, upload_handlers) except MultiPartParserError, exc: raise ImmediateResponse( - content={'detail': 'multipart parse error - %s' % unicode(exc)}, + {'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 4ddc35cb9..aa4cd631d 100644 --- a/djangorestframework/permissions.py +++ b/djangorestframework/permissions.py @@ -22,12 +22,12 @@ __all__ = ( _403_FORBIDDEN_RESPONSE = ImmediateResponse( - content={'detail': 'You do not have permission to access this resource. ' + + {'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 = ImmediateResponse( - content={'detail': 'request was throttled'}, + {'detail': 'request was throttled'}, status=status.HTTP_503_SERVICE_UNAVAILABLE) diff --git a/djangorestframework/request.py b/djangorestframework/request.py index 8cf95f185..d4ea1e010 100644 --- a/djangorestframework/request.py +++ b/djangorestframework/request.py @@ -166,9 +166,9 @@ class Request(object): if parser.can_handle_request(content_type): return parser.parse(stream) - raise ImmediateResponse(content={'error': - 'Unsupported media type in request \'%s\'.' % content_type}, - status=status.HTTP_415_UNSUPPORTED_MEDIA_TYPE) + raise ImmediateResponse({ + '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 f478bd521..15b3579de 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 ImmediateResponse(content=detail, status=400) + raise ImmediateResponse(detail, status=400) def get_form_class(self, method=None): """ diff --git a/djangorestframework/response.py b/djangorestframework/response.py index 29fffed30..c5fdccbcd 100644 --- a/djangorestframework/response.py +++ b/djangorestframework/response.py @@ -125,7 +125,7 @@ class Response(SimpleTemplateResponse): return renderer, media_type # No acceptable renderers were found - raise ImmediateResponse(content={'detail': 'Could not satisfy the client\'s Accept header', + raise ImmediateResponse({'detail': 'Could not satisfy the client\'s Accept header', 'available_types': self._rendered_media_types}, status=status.HTTP_406_NOT_ACCEPTABLE, renderers=self.renderers) diff --git a/djangorestframework/views.py b/djangorestframework/views.py index 5bba6b4eb..93e2d3a3c 100644 --- a/djangorestframework/views.py +++ b/djangorestframework/views.py @@ -173,7 +173,7 @@ 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 ImmediateResponse(content= + raise ImmediateResponse( {'detail': 'Method \'%s\' not allowed on this resource.' % request.method}, status=status.HTTP_405_METHOD_NOT_ALLOWED) @@ -199,6 +199,12 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): """ # Restore script_prefix. set_script_prefix(self.orig_prefix) + + # Always add these headers. + response['Allow'] = ', '.join(allowed_methods(self)) + # sample to allow caching using Vary http header + response['Vary'] = 'Authenticate, Accept' + return response # Note: session based authentication is explicitly CSRF validated, @@ -211,7 +217,7 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): try: # Get a custom request, built form the original request instance - request = self.prepare_request(request) + self.request = request = self.create_request(request) # `initial` is the opportunity to temper with the request, # even completely replace it. @@ -230,7 +236,7 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): response = handler(request, *args, **kwargs) # Prepare response for the response cycle. - response = self.prepare_response(response) + self.response = response = self.prepare_response(response) # Pre-serialize filtering (eg filter complex objects into natively serializable types) # TODO: ugly hack to handle both HttpResponse and Response. @@ -241,7 +247,7 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): except ImmediateResponse, response: # Prepare response for the response cycle. - self.prepare_response(response) + self.response = response = self.prepare_response(response) # `final` is the last opportunity to temper with the response, or even # completely replace it. @@ -260,10 +266,7 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): for name, field in form.fields.iteritems(): field_name_types[name] = field.__class__.__name__ content['fields'] = field_name_types - # Note 'ImmediateResponse' is misleading, it's just any response - # that should be rendered and returned immediately, without any - # response filtering. - raise ImmediateResponse(content=content, status=status.HTTP_200_OK) + raise ImmediateResponse(content, status=status.HTTP_200_OK) class ModelView(View): From b33579a7a18c2cbc6e3789d4a7dc78c82fb0fe80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Piquemal?= Date: Fri, 10 Feb 2012 11:05:20 +0200 Subject: [PATCH 11/12] attempt at fixing the examples --- djangorestframework/mixins.py | 4 ++-- djangorestframework/renderers.py | 2 +- djangorestframework/templates/renderer.html | 4 ++-- djangorestframework/tests/mixins.py | 2 +- djangorestframework/tests/response.py | 3 ++- examples/mixin/urls.py | 3 ++- examples/objectstore/views.py | 4 ++-- examples/pygments_api/views.py | 3 ++- examples/requestexample/urls.py | 4 ++-- examples/requestexample/views.py | 2 +- examples/views.py | 8 ++++---- 11 files changed, 21 insertions(+), 18 deletions(-) diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index 516a0f4b5..43dce870e 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -372,7 +372,7 @@ class ReadModelMixin(ModelMixin): except model.DoesNotExist: raise ImmediateResponse(status=status.HTTP_404_NOT_FOUND) - return self.model_instance + return Response(self.model_instance) class CreateModelMixin(ModelMixin): @@ -428,7 +428,7 @@ class UpdateModelMixin(ModelMixin): # TODO: update on the url of a non-existing resource url doesn't work # correctly at the moment - will end up with a new url try: - self.model_instance = self.get_instance(*query_kwargs) + self.model_instance = self.get_instance(**query_kwargs) for (key, val) in self.CONTENT.items(): setattr(self.model_instance, key, val) diff --git a/djangorestframework/renderers.py b/djangorestframework/renderers.py index 4e8158aa7..08022c7c9 100644 --- a/djangorestframework/renderers.py +++ b/djangorestframework/renderers.py @@ -355,7 +355,7 @@ class DocumentingTemplateRenderer(BaseRenderer): 'login_url': login_url, 'logout_url': logout_url, 'FORMAT_PARAM': self._FORMAT_QUERY_PARAM, - 'METHOD_PARAM': getattr(self.view, '_METHOD_PARAM', None), + 'METHOD_PARAM': getattr(self.view.request, '_METHOD_PARAM', None), 'ADMIN_MEDIA_PREFIX': getattr(settings, 'ADMIN_MEDIA_PREFIX', None), }) diff --git a/djangorestframework/templates/renderer.html b/djangorestframework/templates/renderer.html index e396a58f5..8b5c77c7b 100644 --- a/djangorestframework/templates/renderer.html +++ b/djangorestframework/templates/renderer.html @@ -41,7 +41,7 @@

{{ name }}

{{ description }}

- 
{{ response.status }} {{ response.status_text }}{% autoescape off %}
+	    
{{ response.status_code }} {{ response.status_text }}{% autoescape off %}
 {% for key, val in response.headers.items %}{{ key }}: {{ val|urlize_quoted_links }}
 {% endfor %}
 {{ content|urlize_quoted_links }}
{% endautoescape %}
@@ -63,7 +63,7 @@ {% endif %} {# Only display the POST/PUT/DELETE forms if method tunneling via POST forms is enabled and the user has permissions on this view. #} - {% if METHOD_PARAM and response.status != 403 %} + {% if METHOD_PARAM and response.status_code != 403 %} {% if 'POST' in view.allowed_methods %}
diff --git a/djangorestframework/tests/mixins.py b/djangorestframework/tests/mixins.py index 187ce7193..3f5835aa7 100644 --- a/djangorestframework/tests/mixins.py +++ b/djangorestframework/tests/mixins.py @@ -31,7 +31,7 @@ class TestModelRead(TestModelsTestCase): mixin.resource = GroupResource response = mixin.get(request, id=group.id) - self.assertEquals(group.name, response.name) + self.assertEquals(group.name, response.raw_content.name) def test_read_404(self): class GroupResource(ModelResource): diff --git a/djangorestframework/tests/response.py b/djangorestframework/tests/response.py index b8cc5c1b6..956036801 100644 --- a/djangorestframework/tests/response.py +++ b/djangorestframework/tests/response.py @@ -139,7 +139,8 @@ class MockView(ResponseMixin, DjangoView): def get(self, request, **kwargs): response = Response(DUMMYCONTENT, status=DUMMYSTATUS) - return self.prepare_response(response) + self.response = self.prepare_response(response) + return self.response class HTMLView(View): diff --git a/examples/mixin/urls.py b/examples/mixin/urls.py index 6e9e497e6..58cf370c1 100644 --- a/examples/mixin/urls.py +++ b/examples/mixin/urls.py @@ -15,7 +15,8 @@ class ExampleView(ResponseMixin, View): def get(self, request): response = Response({'description': 'Some example content', 'url': reverse('mixin-view')}, status=200) - return self.prepare_response(response) + self.response = self.prepare_response(response) + return self.response urlpatterns = patterns('', diff --git a/examples/objectstore/views.py b/examples/objectstore/views.py index 47f5147a6..ae5453948 100644 --- a/examples/objectstore/views.py +++ b/examples/objectstore/views.py @@ -67,7 +67,7 @@ class StoredObject(View): """ pathname = os.path.join(OBJECT_STORE_DIR, key) if not os.path.exists(pathname): - return Response(status.HTTP_404_NOT_FOUND) + return Response(status=status.HTTP_404_NOT_FOUND) return Response(pickle.load(open(pathname, 'rb'))) def put(self, request, key): @@ -84,6 +84,6 @@ class StoredObject(View): """ pathname = os.path.join(OBJECT_STORE_DIR, key) if not os.path.exists(pathname): - return Response(status.HTTP_404_NOT_FOUND) + return Response(status=status.HTTP_404_NOT_FOUND) os.remove(pathname) return Response() diff --git a/examples/pygments_api/views.py b/examples/pygments_api/views.py index 44dd2caa5..d59a52c00 100644 --- a/examples/pygments_api/views.py +++ b/examples/pygments_api/views.py @@ -81,7 +81,8 @@ class PygmentsRoot(View): remove_oldest_files(HIGHLIGHTED_CODE_DIR, MAX_FILES) - return Response(status.HTTP_201_CREATED, headers={'Location': reverse('pygments-instance', args=[unique_id])}) + self.headers['Location'] = reverse('pygments-instance', args=[unique_id]) + return Response(status.HTTP_201_CREATED) class PygmentsInstance(View): diff --git a/examples/requestexample/urls.py b/examples/requestexample/urls.py index 3c31e4a9b..d644a5992 100644 --- a/examples/requestexample/urls.py +++ b/examples/requestexample/urls.py @@ -1,9 +1,9 @@ from django.conf.urls.defaults import patterns, url from requestexample.views import RequestExampleView, EchoRequestContentView -from examples.views import MockView +from examples.views import ProxyView urlpatterns = patterns('', url(r'^$', RequestExampleView.as_view(), name='request-example'), - url(r'^content$', MockView.as_view(view_class=EchoRequestContentView), name='request-content'), + url(r'^content$', ProxyView.as_view(view_class=EchoRequestContentView), name='request-content'), ) diff --git a/examples/requestexample/views.py b/examples/requestexample/views.py index 876db864c..b5d2c1e73 100644 --- a/examples/requestexample/views.py +++ b/examples/requestexample/views.py @@ -25,7 +25,7 @@ class MyBaseViewUsingEnhancedRequest(RequestMixin, View): parser_classes = parsers.DEFAULT_PARSERS def dispatch(self, request, *args, **kwargs): - request = self.prepare_request(request) + self.request = request = self.create_request(request) return super(MyBaseViewUsingEnhancedRequest, self).dispatch(request, *args, **kwargs) diff --git a/examples/views.py b/examples/views.py index 606edc3a2..e7ef2ec94 100644 --- a/examples/views.py +++ b/examples/views.py @@ -2,7 +2,7 @@ from djangorestframework.views import View from djangorestframework.response import Response -class MockView(View): +class ProxyView(View): """ A view that just acts as a proxy to call non-djangorestframework views, while still displaying the browsable API interface. @@ -11,10 +11,10 @@ class MockView(View): view_class = None def dispatch(self, request, *args, **kwargs): - request = self.prepare_request(request) + self.request = request = self.create_request(request) if request.method in ['PUT', 'POST']: self.response = self.view_class.as_view()(request, *args, **kwargs) - return super(MockView, self).dispatch(request, *args, **kwargs) + return super(ProxyView, self).dispatch(request, *args, **kwargs) def get(self, request, *args, **kwargs): return Response() @@ -31,4 +31,4 @@ class MockView(View): elif name == '__doc__': return self.view_class.__doc__ else: - return super(MockView, self).__getattribute__(name) + return super(ProxyView, self).__getattribute__(name) From 821844bb11e5262fb0dfc2fecf2add8fe18d3210 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Piquemal?= Date: Tue, 14 Feb 2012 10:05:28 +0200 Subject: [PATCH 12/12] fixed examples, corrected small bugs in the process --- djangorestframework/renderers.py | 3 ++- djangorestframework/response.py | 10 ++++++++-- djangorestframework/templates/renderer.html | 12 ++++++------ examples/pygments_api/views.py | 4 ++-- examples/views.py | 12 +++++------- 5 files changed, 23 insertions(+), 18 deletions(-) diff --git a/djangorestframework/renderers.py b/djangorestframework/renderers.py index 08022c7c9..2cc9cc88f 100644 --- a/djangorestframework/renderers.py +++ b/djangorestframework/renderers.py @@ -13,7 +13,7 @@ from django.utils import simplejson as json from djangorestframework.compat import yaml -from djangorestframework.utils import dict2xml, url_resolves +from djangorestframework.utils import dict2xml, url_resolves, allowed_methods from djangorestframework.utils.breadcrumbs import get_breadcrumbs from djangorestframework.utils.mediatypes import get_media_type_params, add_media_type_param, media_type_matches from djangorestframework import VERSION @@ -349,6 +349,7 @@ class DocumentingTemplateRenderer(BaseRenderer): 'name': name, 'version': VERSION, 'breadcrumblist': breadcrumb_list, + 'allowed_methods': allowed_methods(self.view), 'available_formats': self.view._rendered_formats, 'put_form': put_form_instance, 'post_form': post_form_instance, diff --git a/djangorestframework/response.py b/djangorestframework/response.py index c5fdccbcd..6c42c898d 100644 --- a/djangorestframework/response.py +++ b/djangorestframework/response.py @@ -75,7 +75,7 @@ class Response(SimpleTemplateResponse): Returns reason text corresponding to our HTTP response status code. Provided for convenience. """ - return STATUS_CODE_TEXT.get(self.status, '') + return STATUS_CODE_TEXT.get(self.status_code, '') def _determine_accept_list(self): """ @@ -166,5 +166,11 @@ class ImmediateResponse(Response, BaseException): """ A subclass of :class:`Response` used to abort the current request handling. """ - pass + def __str__(self): + """ + Since this class is also an exception it has to provide a sensible + representation for the cases when it is treated as an exception. + """ + return ('%s must be caught in try/except block, ' + 'and returned as a normal HttpResponse' % self.__class__.__name__) diff --git a/djangorestframework/templates/renderer.html b/djangorestframework/templates/renderer.html index 8b5c77c7b..808d96641 100644 --- a/djangorestframework/templates/renderer.html +++ b/djangorestframework/templates/renderer.html @@ -29,7 +29,7 @@
- {% if 'OPTIONS' in view.allowed_methods %} + {% if 'OPTIONS' in allowed_methods %} {% csrf_token %} @@ -42,11 +42,11 @@

{{ description }}

{{ response.status_code }} {{ response.status_text }}{% autoescape off %}
-{% for key, val in response.headers.items %}{{ key }}: {{ val|urlize_quoted_links }}
+{% for key, val in response.items %}{{ key }}: {{ val|urlize_quoted_links }}
 {% endfor %}
 {{ content|urlize_quoted_links }}
{% endautoescape %}
- {% if 'GET' in view.allowed_methods %} + {% if 'GET' in allowed_methods %}

GET {{ name }}

@@ -65,7 +65,7 @@ {# Only display the POST/PUT/DELETE forms if method tunneling via POST forms is enabled and the user has permissions on this view. #} {% if METHOD_PARAM and response.status_code != 403 %} - {% if 'POST' in view.allowed_methods %} + {% if 'POST' in allowed_methods %}

POST {{ name }}

@@ -86,7 +86,7 @@ {% endif %} - {% if 'PUT' in view.allowed_methods %} + {% if 'PUT' in allowed_methods %}

PUT {{ name }}

@@ -108,7 +108,7 @@ {% endif %} - {% if 'DELETE' in view.allowed_methods %} + {% if 'DELETE' in allowed_methods %}

DELETE {{ name }}

diff --git a/examples/pygments_api/views.py b/examples/pygments_api/views.py index d59a52c00..852b67309 100644 --- a/examples/pygments_api/views.py +++ b/examples/pygments_api/views.py @@ -82,7 +82,7 @@ class PygmentsRoot(View): remove_oldest_files(HIGHLIGHTED_CODE_DIR, MAX_FILES) self.headers['Location'] = reverse('pygments-instance', args=[unique_id]) - return Response(status.HTTP_201_CREATED) + return Response(status=status.HTTP_201_CREATED) class PygmentsInstance(View): @@ -90,7 +90,7 @@ class PygmentsInstance(View): Simply return the stored highlighted HTML file with the correct mime type. This Resource only renders HTML and uses a standard HTML renderer rather than the renderers.DocumentingHTMLRenderer class. """ - renderers = (HTMLRenderer,) + renderer_classes = (HTMLRenderer,) def get(self, request, unique_id): """ diff --git a/examples/views.py b/examples/views.py index e7ef2ec94..e0e4c3c40 100644 --- a/examples/views.py +++ b/examples/views.py @@ -25,10 +25,8 @@ class ProxyView(View): def post(self, request, *args, **kwargs): return Response(self.response.content) - def __getattribute__(self, name): - if name == '__name__': - return self.view_class.__name__ - elif name == '__doc__': - return self.view_class.__doc__ - else: - return super(ProxyView, self).__getattribute__(name) + def get_name(self): + return self.view_class.__name__ + + def get_description(self, html): + return self.view_class.__doc__