From 4d126796752cc3c79a24fd9caed49da6c525096f Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 10 May 2011 16:01:58 +0100 Subject: [PATCH] More tests, getting new serialization into resource --- djangorestframework/authentication.py | 4 +- djangorestframework/mixins.py | 23 ++++-- djangorestframework/renderers.py | 25 ++++--- djangorestframework/resource.py | 81 ++++++++++++++++++++- djangorestframework/response.py | 19 +++-- djangorestframework/status.py | 6 +- djangorestframework/templates/renderer.html | 2 +- djangorestframework/tests/resource.py | 31 ++++++++ djangorestframework/validators.py | 18 +++-- 9 files changed, 173 insertions(+), 36 deletions(-) create mode 100644 djangorestframework/tests/resource.py diff --git a/djangorestframework/authentication.py b/djangorestframework/authentication.py index dea19f916..97e5d9c5a 100644 --- a/djangorestframework/authentication.py +++ b/djangorestframework/authentication.py @@ -3,7 +3,7 @@ The ``authentication`` module provides a set of pluggable authentication classes Authentication behavior is provided by adding the ``AuthMixin`` class to a ``View`` . -The set of authentication methods which are used is then specified by setting +The set of authentication methods which are used is then specified by setting the ``authentication`` attribute on the ``View`` class, and listing a set of authentication classes. """ @@ -81,7 +81,7 @@ class UserLoggedInAuthenticaton(BaseAuthenticaton): """ def authenticate(self, request): - # TODO: Switch this back to request.POST, and let MultiPartParser deal with the consequences. + # TODO: Switch this back to request.POST, and let FormParser/MultiPartParser deal with the consequences. if getattr(request, 'user', None) and request.user.is_active: # If this is a POST request we enforce CSRF validation. if request.method.upper() == 'POST': diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index b80f5894c..65ebe1715 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -33,7 +33,7 @@ __all__ = ( class RequestMixin(object): """ - Mixin class to provide request parsing behaviour. + Mixin class to provide request parsing behavior. """ USE_FORM_OVERLOADING = True @@ -93,6 +93,13 @@ class RequestMixin(object): if content_length == 0: return None elif hasattr(request, 'read'): + # UPDATE BASED ON COMMENT BELOW: + # + # Yup, this was a bug in Django - fixed and waiting check in - see ticket 15785. + # http://code.djangoproject.com/ticket/15785 + # + # COMMENT: + # # It's not at all clear if this needs to be byte limited or not. # Maybe I'm just being dumb but it looks to me like there's some issues # with that in Django. @@ -117,8 +124,6 @@ class RequestMixin(object): #except (ValueError, TypeError): # content_length = 0 # self._stream = LimitedStream(request, content_length) - # - # UPDATE: http://code.djangoproject.com/ticket/15785 self._stream = request else: self._stream = StringIO(request.raw_post_data) @@ -290,11 +295,15 @@ class ResponseMixin(object): return resp + # TODO: This should be simpler now. + # Add a handles_response() to the renderer, then iterate through the + # acceptable media types, ordered by how specific they are, + # calling handles_response on each renderer. def _determine_renderer(self, request): """ Return the appropriate renderer for the output, given the client's 'Accept' header, and the content types that this mixin knows how to serve. - + See: RFC 2616, Section 14 - http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html """ @@ -321,7 +330,7 @@ class ResponseMixin(object): qvalue = Decimal('1.0') if len(components) > 1: - # Parse items that have a qvalue eg text/html;q=0.9 + # Parse items that have a qvalue eg 'text/html; q=0.9' try: (q, num) = components[-1].split('=') if q == 'q': @@ -356,10 +365,10 @@ class ResponseMixin(object): raise ErrorResponse(status.HTTP_406_NOT_ACCEPTABLE, {'detail': 'Could not satisfy the client\'s Accept header', - 'available_types': self.renderted_media_types}) + 'available_types': self.rendered_media_types}) @property - def renderted_media_types(self): + def rendered_media_types(self): """ Return an list of all the media types that this resource can render. """ diff --git a/djangorestframework/renderers.py b/djangorestframework/renderers.py index 73809b5ed..bda2d38e9 100644 --- a/djangorestframework/renderers.py +++ b/djangorestframework/renderers.py @@ -24,6 +24,7 @@ from urllib import quote_plus __all__ = ( 'BaseRenderer', + 'TemplateRenderer', 'JSONRenderer', 'DocumentingHTMLRenderer', 'DocumentingXHTMLRenderer', @@ -87,10 +88,12 @@ class DocumentingTemplateRenderer(BaseRenderer): template = None def _get_content(self, view, request, obj, media_type): - """Get the content as if it had been rendered by a non-documenting renderer. + """ + Get the content as if it had been rendered by a non-documenting renderer. (Typically this will be the content as it would have been if the Resource had been - requested with an 'Accept: */*' header, although with verbose style formatting if appropriate.)""" + requested with an 'Accept: */*' header, although with verbose style formatting if appropriate.) + """ # Find the first valid renderer and render the content. (Don't use another documenting renderer.) renderers = [renderer for renderer in view.renderers if not isinstance(renderer, DocumentingTemplateRenderer)] @@ -103,12 +106,14 @@ class DocumentingTemplateRenderer(BaseRenderer): return '[%d bytes of binary content]' return content - + def _get_form_instance(self, view): - """Get a form, possibly bound to either the input or output data. + """ + Get a form, possibly bound to either the input or output data. In the absence on of the Resource having an associated form then - provide a form that can be used to submit arbitrary content.""" + provide a form that can be used to submit arbitrary content. + """ # Get the form instance if we have one bound to the input form_instance = getattr(view, 'bound_form_instance', None) @@ -138,8 +143,10 @@ class DocumentingTemplateRenderer(BaseRenderer): def _get_generic_content_form(self, view): - """Returns a form that allows for arbitrary content types to be tunneled via standard HTML forms - (Which are typically application/x-www-form-urlencoded)""" + """ + Returns a form that allows for arbitrary content types to be tunneled via standard HTML forms + (Which are typically application/x-www-form-urlencoded) + """ # 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. @@ -197,8 +204,8 @@ class DocumentingTemplateRenderer(BaseRenderer): template = loader.get_template(self.template) context = RequestContext(self.view.request, { 'content': content, - 'resource': self.view, - 'request': self.view.request, + 'resource': self.view, # TODO: rename to view + 'request': self.view.request, # TODO: remove 'response': self.view.response, 'description': description, 'name': name, diff --git a/djangorestframework/resource.py b/djangorestframework/resource.py index 6b039059f..441786842 100644 --- a/djangorestframework/resource.py +++ b/djangorestframework/resource.py @@ -1,16 +1,89 @@ -from django.db.models import Model +from django.db import models from django.db.models.query import QuerySet from django.db.models.fields.related import RelatedField +from django.utils.encoding import smart_unicode import decimal import inspect import re + + +def _model_to_dict(instance, fields=None, exclude=None): + """ + This is a clone of Django's ``django.forms.model_to_dict`` except that it + doesn't coerce related objects into primary keys. + """ + opts = instance._meta + data = {} + for f in opts.fields + opts.many_to_many: + if not f.editable: + continue + if fields and not f.name in fields: + continue + if exclude and f.name in exclude: + continue + if isinstance(f, models.ForeignKey): + data[f.name] = getattr(instance, f.name) + else: + data[f.name] = f.value_from_object(instance) + return data + + +def _object_to_data(obj): + """ + Convert an object into a serializable representation. + """ + if isinstance(obj, dict): + # dictionaries + return dict([ (key, _object_to_data(val)) for key, val in obj.iteritems() ]) + if isinstance(obj, (tuple, list, set, QuerySet)): + # basic iterables + return [_object_to_data(item) for item in obj] + if isinstance(obj, models.Manager): + # Manager objects + ret = [_object_to_data(item) for item in obj.all()] + if isinstance(obj, models.Model): + # Model instances + return _object_to_data(_model_to_dict(obj)) + if isinstance(obj, decimal.Decimal): + # Decimals (force to string representation) + return str(obj) + if inspect.isfunction(obj) and not inspect.getargspec(obj)[0]: + # function with no args + return _object_to_data(obj()) + if inspect.ismethod(obj) and len(inspect.getargspec(obj)[0]) == 1: + # method with only a 'self' args + return _object_to_data(obj()) + + # fallback + return smart_unicode(obj, strings_only=True) + + # TODO: Replace this with new Serializer code based on Forms API. +#class Resource(object): +# def __init__(self, view): +# self.view = view +# +# def object_to_data(self, obj): +# pass +# +# def data_to_object(self, data, files): +# pass +# +#class FormResource(object): +# pass +# +#class ModelResource(object): +# pass + + class Resource(object): - """A Resource determines how an object maps to a serializable entity. - Objects that a resource can act on include plain Python object instances, Django Models, and Django QuerySets.""" + """ + A Resource determines how a python object maps to some serializable data. + Objects that a resource can act on include plain Python object instances, Django Models, and Django QuerySets. + """ # The model attribute refers to the Django Model which this Resource maps to. # (The Model's class, rather than an instance of the Model) @@ -50,7 +123,7 @@ class Resource(object): ret = thing elif isinstance(thing, decimal.Decimal): ret = str(thing) - elif isinstance(thing, Model): + elif isinstance(thing, models.Model): ret = _model(thing, fields=fields) #elif isinstance(thing, HttpResponse): TRC # raise HttpStatusCode(thing) diff --git a/djangorestframework/response.py b/djangorestframework/response.py index 9b3c5851b..72bc16c8b 100644 --- a/djangorestframework/response.py +++ b/djangorestframework/response.py @@ -1,11 +1,14 @@ from django.core.handlers.wsgi import STATUS_CODE_TEXT -__all__ =['Response', 'ErrorResponse'] +__all__ = ('Response', 'ErrorResponse') # TODO: remove raw_content/cleaned_content and just use content? class Response(object): - """An HttpResponse that may include content that hasn't yet been serialized.""" + """ + An HttpResponse that may include content that hasn't yet been serialized. + """ + def __init__(self, status=200, content=None, headers={}): self.status = status self.has_content_body = content is not None @@ -15,12 +18,18 @@ class Response(object): @property def status_text(self): - """Return reason text corresponding to our HTTP response status code. - Provided for convenience.""" + """ + Return reason text corresponding to our HTTP response status code. + Provided for convenience. + """ return STATUS_CODE_TEXT.get(self.status, '') class ErrorResponse(BaseException): - """An exception representing an HttpResponse that should be returned immediately.""" + """ + An exception representing an Response that should be returned immediately. + Any content should be serialized as-is, without being filtered. + """ + def __init__(self, status, content=None, headers={}): self.response = Response(status, content=content, headers=headers) diff --git a/djangorestframework/status.py b/djangorestframework/status.py index 8e95f1e4b..67704b8e9 100644 --- a/djangorestframework/status.py +++ b/djangorestframework/status.py @@ -1,7 +1,9 @@ -"""Descriptive HTTP status codes, for code readability. +""" +Descriptive HTTP status codes, for code readability. See RFC 2616 - Sec 10: http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html -Also, django.core.handlers.wsgi.STATUS_CODE_TEXT""" +Also see django.core.handlers.wsgi.STATUS_CODE_TEXT +""" # Verbose format HTTP_100_CONTINUE = 100 diff --git a/djangorestframework/templates/renderer.html b/djangorestframework/templates/renderer.html index 105ea0a2d..e213ecfa9 100644 --- a/djangorestframework/templates/renderer.html +++ b/djangorestframework/templates/renderer.html @@ -48,7 +48,7 @@

GET {{ name }}

GET - {% for media_type in resource.renderted_media_types %} + {% for media_type in resource.rendered_media_types %} {% with resource.ACCEPT_QUERY_PARAM|add:"="|add:media_type as param %} [{{ media_type }}] {% endwith %} diff --git a/djangorestframework/tests/resource.py b/djangorestframework/tests/resource.py new file mode 100644 index 000000000..0ed41951b --- /dev/null +++ b/djangorestframework/tests/resource.py @@ -0,0 +1,31 @@ +"""Tests for the resource module""" +from django.test import TestCase +from djangorestframework.resource import _object_to_data + +import datetime +import decimal + +class TestObjectToData(TestCase): + """Tests for the _object_to_data function""" + + def test_decimal(self): + """Decimals need to be converted to a string representation.""" + self.assertEquals(_object_to_data(decimal.Decimal('1.5')), '1.5') + + def test_function(self): + """Functions with no arguments should be called.""" + def foo(): + return 1 + self.assertEquals(_object_to_data(foo), 1) + + def test_method(self): + """Methods with only a ``self`` argument should be called.""" + class Foo(object): + def foo(self): + return 1 + self.assertEquals(_object_to_data(Foo().foo), 1) + + def test_datetime(self): + """datetime objects are left as-is.""" + now = datetime.datetime.now() + self.assertEquals(_object_to_data(now), now) \ No newline at end of file diff --git a/djangorestframework/validators.py b/djangorestframework/validators.py index d806a6146..bef85af77 100644 --- a/djangorestframework/validators.py +++ b/djangorestframework/validators.py @@ -31,20 +31,24 @@ class FormValidator(BaseValidator): def validate(self, content): - """Given some content as input return some cleaned, validated content. + """ + Given some content as input return some cleaned, validated content. Raises a ErrorResponse 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. On failure the ErrorResponse content is a dict which may contain 'errors' and 'field-errors' keys. If the '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}.""" + If the 'field-errors' key exists it is a dict of {field name as string: list of errors as strings}. + """ return self._validate(content) def _validate(self, content, allowed_extra_fields=()): - """Wrapped by validate to hide the extra_fields option that the ModelValidatorMixin uses. + """ + Wrapped by validate to hide the extra_fields option that the ModelValidatorMixin uses. extra_fields is a list of fields which are not defined by the form, but which we still - expect to see on the input.""" + expect to see on the input. + """ bound_form = self.get_bound_form(content) if bound_form is None: @@ -138,7 +142,8 @@ class ModelFormValidator(FormValidator): # TODO: test the different validation here to allow for get get_absolute_url to be supplied on input and not bork out # TODO: be really strict on fields - check they match in the handler methods. (this isn't a validator thing tho.) def validate(self, content): - """Given some content as input return some cleaned, validated content. + """ + Given some content as input return some cleaned, validated content. Raises a ErrorResponse with status code 400 (Bad Request) on failure. Validation is standard form or model form validation, @@ -148,7 +153,8 @@ class ModelFormValidator(FormValidator): On failure the ErrorResponse content is a dict which may contain 'errors' and 'field-errors' keys. If the '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}.""" + If the 'field-errors' key exists it is a dict of {field name as string: list of errors as strings}. + """ return self._validate(content, allowed_extra_fields=self._property_fields_set)