diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index 76c2208df..278d4d4da 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -516,7 +516,7 @@ class CreateModelMixin(object): instance.save() headers = {} if hasattr(instance, 'get_absolute_url'): - headers['Location'] = instance.get_absolute_url() + headers['Location'] = self.resource(self).url(instance) return Response(status.HTTP_201_CREATED, instance, headers) @@ -569,10 +569,27 @@ class ListModelMixin(object): """ Behavior to list a set of model instances on GET requests """ + + # NB. Not obvious to me if it would be better to set this on the resource? + # + # Presumably it's more useful to have on the view, because that way you can + # have multiple views across different querysets mapping to the same resource. + # + # Perhaps it ought to be: + # + # 1) View.queryset + # 2) if None fall back to Resource.queryset + # 3) if None fall back to Resource.model.objects.all() + # + # Any feedback welcomed. queryset = None def get(self, request, *args, **kwargs): queryset = self.queryset if self.queryset else self.resource.model.objects.all() + ordering = getattr(self.resource, 'ordering', None) + if ordering: + args = as_tuple(ordering) + queryset = queryset.order_by(*args) return queryset.filter(**kwargs) diff --git a/djangorestframework/renderers.py b/djangorestframework/renderers.py index 371b5ef0e..112736d2f 100644 --- a/djangorestframework/renderers.py +++ b/djangorestframework/renderers.py @@ -251,8 +251,8 @@ class DocumentingTemplateRenderer(BaseRenderer): 'form': form_instance, 'login_url': login_url, 'logout_url': logout_url, - 'ACCEPT_PARAM': self.view._ACCEPT_QUERY_PARAM, - 'METHOD_PARAM': self.view._METHOD_PARAM, + 'ACCEPT_PARAM': getattr(self.view, '_ACCEPT_QUERY_PARAM', None), + 'METHOD_PARAM': getattr(self.view, '_METHOD_PARAM', None), 'ADMIN_MEDIA_PREFIX': settings.ADMIN_MEDIA_PREFIX }) diff --git a/djangorestframework/resources.py b/djangorestframework/resources.py index 19218b35c..8a9373cfc 100644 --- a/djangorestframework/resources.py +++ b/djangorestframework/resources.py @@ -5,6 +5,9 @@ from django.db.models.query import QuerySet from django.db.models.fields.related import RelatedField from django.utils.encoding import smart_unicode +from djangorestframework.response import ErrorResponse +from djangorestframework.utils import as_tuple + import decimal import inspect import re @@ -12,6 +15,10 @@ import re # TODO: _IgnoreFieldException +# Map model classes to resource classes +#_model_to_resource = {} + + def _model_to_dict(instance, resource=None): """ Given a model instance, return a ``dict`` representing the model. @@ -179,18 +186,31 @@ class FormResource(Resource): return self._validate(data, files) - def _validate(self, data, files, allowed_extra_fields=()): + def _validate(self, data, files, allowed_extra_fields=(), fake_data=None): """ 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. """ + + # We'd like nice error messages even if no content is supplied. + # Typically if an empty dict is given to a form Django will + # return .is_valid() == False, but .errors == {} + # + # To get around this case we revalidate with some fake data. + if fake_data: + data[fake_data] = '_fake_data' + allowed_extra_fields = allowed_extra_fields + ('_fake_data',) + bound_form = self.get_bound_form(data, files) if bound_form is None: return data self.view.bound_form_instance = bound_form + + data = data and data or {} + files = files and files or {} seen_fields_set = set(data.keys()) form_fields_set = set(bound_form.fields.keys()) @@ -198,6 +218,7 @@ class FormResource(Resource): # In addition to regular validation we also ensure no additional fields are being passed in... unknown_fields = seen_fields_set - (form_fields_set | allowed_extra_fields_set) + unknown_fields = unknown_fields - set(('csrfmiddlewaretoken',)) # Check using both regular validation, and our stricter no additional fields rule if bound_form.is_valid() and not unknown_fields: @@ -216,6 +237,13 @@ class FormResource(Resource): detail = {} if not bound_form.errors and not unknown_fields: + # is_valid() was False, but errors was empty. + # If we havn't already done so attempt revalidation with some fake data + # to force django to give us an errors dict. + if fake_data is None: + return self._validate(data, files, allowed_extra_fields, '_fake_data') + + # If we've already set fake_dict and we're still here, fallback gracefully. detail = {u'errors': [u'No content was supplied.']} else: @@ -256,12 +284,29 @@ class FormResource(Resource): return self.form() + +#class _RegisterModelResource(type): +# """ +# Auto register new ModelResource classes into ``_model_to_resource`` +# """ +# def __new__(cls, name, bases, dct): +# resource_cls = type.__new__(cls, name, bases, dct) +# model_cls = dct.get('model', None) +# if model_cls: +# _model_to_resource[model_cls] = resource_cls +# return resource_cls + + + class ModelResource(FormResource): """ Resource class that uses forms for validation and otherwise falls back to a model form if no form is set. Also provides a get_bound_form() method which may be used by some renderers. """ - + + # Auto-register new ModelResource classes into _model_to_resource + #__metaclass__ = _RegisterModelResource + """ The form class that should be used for request validation. If set to ``None`` then the default model form validation will be used. @@ -314,7 +359,7 @@ class ModelResource(FormResource): return self._validate(data, files, allowed_extra_fields=self._property_fields_set) - def get_bound_form(self, content=None): + def get_bound_form(self, data=None, files=None): """ Given some content return a ``Form`` instance bound to that content. @@ -334,11 +379,11 @@ class ModelResource(FormResource): #fields = tuple(self._model_fields_set) # Instantiate the ModelForm as appropriate - if content and isinstance(content, models.Model): + if data and isinstance(data, models.Model): # Bound to an existing model instance return OnTheFlyModelForm(instance=content) - elif content is not None: - return OnTheFlyModelForm(content) + elif data is not None: + return OnTheFlyModelForm(data, files) return OnTheFlyModelForm() # Both form and model not set? Okay bruv, whatevs... @@ -364,7 +409,19 @@ class ModelResource(FormResource): # pattern = tuple_item[1] # Note: defaults = tuple_item[2] for django >= 1.3 for result, params in possibility: - instance_attrs = dict([ (param, getattr(instance, param)) for param in params if hasattr(instance, param) ]) + + #instance_attrs = dict([ (param, getattr(instance, param)) for param in params if hasattr(instance, param) ]) + + instance_attrs = {} + for param in params: + if not hasattr(instance, param): + continue + attr = getattr(instance, param) + if isinstance(attr, models.Model): + instance_attrs[param] = attr.pk + else: + instance_attrs[param] = attr + try: return reverse(self.view_callable[0], kwargs=instance_attrs) except NoReverseMatch: @@ -393,7 +450,7 @@ class ModelResource(FormResource): isinstance(getattr(self.model, attr, None), property) and not attr.startswith('_')) - if fields: + if self.fields: return property_fields & set(as_tuple(self.fields)) - return property_fields - set(as_tuple(self.exclude)) + return property_fields.union(set(as_tuple(self.include))) - set(as_tuple(self.exclude)) diff --git a/examples/blogpost/models.py b/examples/blogpost/models.py index 3489c596b..c4925a15a 100644 --- a/examples/blogpost/models.py +++ b/examples/blogpost/models.py @@ -21,26 +21,10 @@ class BlogPost(models.Model): created = models.DateTimeField(auto_now_add=True) slug = models.SlugField(editable=False, default='') - class Meta: - ordering = ('created',) - - @models.permalink - def get_absolute_url(self): - return ('blog-post', (), {'key': self.key}) - - @property - @models.permalink - def comments_url(self): - """Link to a resource which lists all comments for this blog post.""" - return ('comments', (), {'blogpost': self.key}) - - def __unicode__(self): - return self.title - def save(self, *args, **kwargs): self.slug = slugify(self.title) super(self.__class__, self).save(*args, **kwargs) - for obj in self.__class__.objects.order_by('-pk')[MAX_POSTS:]: + for obj in self.__class__.objects.order_by('-created')[MAX_POSTS:]: obj.delete() @@ -51,16 +35,3 @@ class Comment(models.Model): rating = models.IntegerField(blank=True, null=True, choices=RATING_CHOICES, help_text='How did you rate this post?') created = models.DateTimeField(auto_now_add=True) - class Meta: - ordering = ('created',) - - @models.permalink - def get_absolute_url(self): - return ('comment', (), {'blogpost': self.blogpost.key, 'id': self.id}) - - @property - @models.permalink - def blogpost_url(self): - """Link to the blog post resource which this comment corresponds to.""" - return ('blog-post', (), {'key': self.blogpost.key}) - diff --git a/examples/blogpost/urls.py b/examples/blogpost/urls.py index 1306b0d7d..130363b17 100644 --- a/examples/blogpost/urls.py +++ b/examples/blogpost/urls.py @@ -1,9 +1,28 @@ from django.conf.urls.defaults import patterns, url -from blogpost.views import BlogPosts, BlogPostInstance, Comments, CommentInstance +from django.core.urlresolvers import reverse + +from djangorestframework.views import ListOrCreateModelView, InstanceModelView +from djangorestframework.resources import ModelResource + +from blogpost.models import BlogPost, Comment + +class BlogPostResource(ModelResource): + model = BlogPost + fields = ('created', 'title', 'slug', 'content', 'url', 'comments') + ordering = ('-created',) + + def comments(self, instance): + return reverse('comments', kwargs={'blogpost': instance.key}) + +class CommentResource(ModelResource): + model = Comment + fields = ('username', 'comment', 'created', 'rating', 'url', 'blogpost') + ordering = ('-created',) + urlpatterns = patterns('', - url(r'^$', BlogPosts.as_view(), name='blog-posts'), - url(r'^(?P[^/]+)/$', BlogPostInstance.as_view(), name='blog-post'), - url(r'^(?P[^/]+)/comments/$', Comments.as_view(), name='comments'), - url(r'^(?P[^/]+)/comments/(?P[^/]+)/$', CommentInstance.as_view(), name='comment'), -) + url(r'^$', ListOrCreateModelView.as_view(resource=BlogPostResource), name='blog-posts-root'), + url(r'^(?P[^/]+)/$', InstanceModelView.as_view(resource=BlogPostResource)), + url(r'^(?P[^/]+)/comments/$', ListOrCreateModelView.as_view(resource=CommentResource), name='comments'), + url(r'^(?P[^/]+)/comments/(?P[^/]+)/$', InstanceModelView.as_view(resource=CommentResource)), +) \ No newline at end of file diff --git a/examples/modelresourceexample/models.py b/examples/modelresourceexample/models.py index 160475243..ff0179c88 100644 --- a/examples/modelresourceexample/models.py +++ b/examples/modelresourceexample/models.py @@ -7,17 +7,13 @@ class MyModel(models.Model): bar = models.IntegerField(help_text='Must be an integer.') baz = models.CharField(max_length=32, help_text='Free text. Max length 32 chars.') created = models.DateTimeField(auto_now_add=True) - - class Meta: - ordering = ('created',) def save(self, *args, **kwargs): - """For the purposes of the sandbox, limit the maximum number of stored models.""" + """ + For the purposes of the sandbox limit the maximum number of stored models. + """ super(MyModel, self).save(*args, **kwargs) while MyModel.objects.all().count() > MAX_INSTANCES: - MyModel.objects.all()[0].delete() - - @models.permalink - def get_absolute_url(self): - return ('my-model-resource', (self.pk,)) + MyModel.objects.all().order_by('-created')[0].delete() + diff --git a/examples/modelresourceexample/urls.py b/examples/modelresourceexample/urls.py index 53d950cd6..5860c807f 100644 --- a/examples/modelresourceexample/urls.py +++ b/examples/modelresourceexample/urls.py @@ -1,7 +1,14 @@ from django.conf.urls.defaults import patterns, url -from modelresourceexample.views import MyModelRootResource, MyModelResource +from djangorestframework.views import ListOrCreateModelView, InstanceModelView +from djangorestframework.resources import ModelResource +from modelresourceexample.models import MyModel + +class MyModelResource(ModelResource): + model = MyModel + fields = ('foo', 'bar', 'baz', 'url') + ordering = ('created',) urlpatterns = patterns('modelresourceexample.views', - url(r'^$', MyModelRootResource.as_view(), name='my-model-root-resource'), - url(r'^([0-9]+)/$', MyModelResource.as_view(), name='my-model-resource'), + url(r'^$', ListOrCreateModelView.as_view(resource=MyModelResource), name='model-resource-root'), + url(r'^([0-9]+)/$', InstanceModelView.as_view(resource=MyModelResource)), ) diff --git a/examples/modelresourceexample/views.py b/examples/modelresourceexample/views.py index 5495a2931..e69de29bb 100644 --- a/examples/modelresourceexample/views.py +++ b/examples/modelresourceexample/views.py @@ -1,16 +0,0 @@ -from djangorestframework.modelresource import InstanceModelResource, ListOrCreateModelResource -from modelresourceexample.models import MyModel - -FIELDS = ('foo', 'bar', 'baz', 'absolute_url') - -class MyModelRootResource(ListOrCreateModelResource): - """A create/list resource for MyModel. - Available for both authenticated and anonymous access for the purposes of the sandbox.""" - model = MyModel - fields = FIELDS - -class MyModelResource(InstanceModelResource): - """A read/update/delete resource for MyModel. - Available for both authenticated and anonymous access for the purposes of the sandbox.""" - model = MyModel - fields = FIELDS diff --git a/examples/objectstore/views.py b/examples/objectstore/views.py index 2e353e08c..076e59416 100644 --- a/examples/objectstore/views.py +++ b/examples/objectstore/views.py @@ -1,7 +1,7 @@ from django.conf import settings from django.core.urlresolvers import reverse -from djangorestframework.resource import Resource +from djangorestframework.views import BaseView from djangorestframework.response import Response from djangorestframework import status @@ -15,55 +15,69 @@ MAX_FILES = 10 def remove_oldest_files(dir, max_files): - """Remove the oldest files in a directory 'dir', leaving at most 'max_files' remaining. - We use this to limit the number of resources in the sandbox.""" + """ + Remove the oldest files in a directory 'dir', leaving at most 'max_files' remaining. + We use this to limit the number of resources in the sandbox. + """ filepaths = [os.path.join(dir, file) for file in os.listdir(dir) if not file.startswith('.')] ctime_sorted_paths = [item[0] for item in sorted([(path, os.path.getctime(path)) for path in filepaths], key=operator.itemgetter(1), reverse=True)] [os.remove(path) for path in ctime_sorted_paths[max_files:]] -class ObjectStoreRoot(Resource): - """Root of the Object Store API. - Allows the client to get a complete list of all the stored objects, or to create a new stored object.""" - allowed_methods = anon_allowed_methods = ('GET', 'POST') +class ObjectStoreRoot(BaseView): + """ + Root of the Object Store API. + Allows the client to get a complete list of all the stored objects, or to create a new stored object. + """ - def get(self, request, auth): - """Return a list of all the stored object URLs. (Ordered by creation time, newest first)""" + def get(self, request): + """ + Return a list of all the stored object URLs. (Ordered by creation time, newest first) + """ 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] - def post(self, request, auth, content): - """Create a new stored object, with a unique key.""" + def post(self, request): + """ + Create a new stored object, with a unique key. + """ key = str(uuid.uuid1()) pathname = os.path.join(OBJECT_STORE_DIR, key) - pickle.dump(content, open(pathname, 'wb')) + pickle.dump(self.CONTENT, open(pathname, 'wb')) remove_oldest_files(OBJECT_STORE_DIR, MAX_FILES) - return Response(status.HTTP_201_CREATED, content, {'Location': reverse('stored-object', kwargs={'key':key})}) + return Response(status.HTTP_201_CREATED, self.CONTENT, {'Location': reverse('stored-object', kwargs={'key':key})}) -class StoredObject(Resource): - """Represents a stored object. - The object may be any picklable content.""" - allowed_methods = anon_allowed_methods = ('GET', 'PUT', 'DELETE') +class StoredObject(BaseView): + """ + Represents a stored object. + The object may be any picklable content. + """ - def get(self, request, auth, key): - """Return a stored object, by unpickling the contents of a locally stored file.""" + def get(self, request, key): + """ + Return a stored object, by unpickling the contents of a locally stored file. + """ 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')) - def put(self, request, auth, content, key): - """Update/create a stored object, by pickling the request content to a locally stored file.""" + def put(self, request, key): + """ + Update/create a stored object, by pickling the request content to a locally stored file. + """ pathname = os.path.join(OBJECT_STORE_DIR, key) - pickle.dump(content, open(pathname, 'wb')) - return content + pickle.dump(self.CONTENT, open(pathname, 'wb')) + return self.CONTENT - def delete(self, request, auth, key): - """Delete a stored object, by removing it's pickled file.""" + def delete(self, request): + """ + Delete a stored object, by removing it's pickled file. + """ pathname = os.path.join(OBJECT_STORE_DIR, key) if not os.path.exists(pathname): return Response(status.HTTP_404_NOT_FOUND) diff --git a/examples/pygments_api/views.py b/examples/pygments_api/views.py index 253b09070..e6bfae489 100644 --- a/examples/pygments_api/views.py +++ b/examples/pygments_api/views.py @@ -2,9 +2,10 @@ from __future__ import with_statement # for python 2.5 from django.conf import settings from django.core.urlresolvers import reverse -from djangorestframework.resource import Resource +from djangorestframework.resources import FormResource from djangorestframework.response import Response from djangorestframework.renderers import BaseRenderer +from djangorestframework.views import BaseView from djangorestframework import status from pygments.formatters import HtmlFormatter @@ -17,39 +18,60 @@ import os import uuid import operator -# We need somewhere to store the code that we highlight +# We need somewhere to store the code snippets that we highlight HIGHLIGHTED_CODE_DIR = os.path.join(settings.MEDIA_ROOT, 'pygments') MAX_FILES = 10 + def list_dir_sorted_by_ctime(dir): - """Return a list of files sorted by creation time""" + """ + Return a list of files sorted by creation time + """ filepaths = [os.path.join(dir, file) for file in os.listdir(dir) if not file.startswith('.')] - return [item[0] for item in sorted([(path, os.path.getctime(path)) for path in filepaths], - key=operator.itemgetter(1), reverse=False)] + return [item[0] for item in sorted( [(path, os.path.getctime(path)) for path in filepaths], + key=operator.itemgetter(1), reverse=False) ] + def remove_oldest_files(dir, max_files): - """Remove the oldest files in a directory 'dir', leaving at most 'max_files' remaining. - We use this to limit the number of resources in the sandbox.""" + """ + Remove the oldest files in a directory 'dir', leaving at most 'max_files' remaining. + We use this to limit the number of resources in the sandbox. + """ [os.remove(path) for path in list_dir_sorted_by_ctime(dir)[max_files:]] class HTMLRenderer(BaseRenderer): - """Basic renderer which just returns the content without any further serialization.""" + """ + Basic renderer which just returns the content without any further serialization. + """ media_type = 'text/html' -class PygmentsRoot(Resource): - """This example demonstrates a simple RESTful Web API aound the awesome pygments library. - This top level resource is used to create highlighted code snippets, and to list all the existing code snippets.""" + +class PygmentsFormResource(FormResource): + """ + """ form = PygmentsForm + +class PygmentsRoot(BaseView): + """ + This example demonstrates a simple RESTful Web API aound the awesome pygments library. + This top level resource is used to create highlighted code snippets, and to list all the existing code snippets. + """ + resource = PygmentsFormResource + def get(self, request): - """Return a list of all currently existing snippets.""" + """ + 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] def post(self, request): - """Create a new highlighed snippet and return it's location. - For the purposes of the sandbox example, also ensure we delete the oldest snippets if we have > MAX_FILES.""" + """ + Create a new highlighed snippet and return it's location. + For the purposes of the sandbox example, also ensure we delete the oldest snippets if we have > MAX_FILES. + """ unique_id = str(uuid.uuid1()) pathname = os.path.join(HIGHLIGHTED_CODE_DIR, unique_id) @@ -66,20 +88,26 @@ class PygmentsRoot(Resource): return Response(status.HTTP_201_CREATED, headers={'Location': reverse('pygments-instance', args=[unique_id])}) -class PygmentsInstance(Resource): - """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.""" +class PygmentsInstance(BaseView): + """ + 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,) def get(self, request, unique_id): - """Return the highlighted snippet.""" + """ + Return the highlighted snippet. + """ 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() def delete(self, request, unique_id): - """Delete the highlighted snippet.""" + """ + Delete the highlighted snippet. + """ pathname = os.path.join(HIGHLIGHTED_CODE_DIR, unique_id) if not os.path.exists(pathname): return Response(status.HTTP_404_NOT_FOUND) diff --git a/examples/resourceexample/views.py b/examples/resourceexample/views.py index 911fd467b..70d96891b 100644 --- a/examples/resourceexample/views.py +++ b/examples/resourceexample/views.py @@ -1,20 +1,33 @@ from django.core.urlresolvers import reverse -from djangorestframework.resource import Resource +from djangorestframework.views import BaseView +from djangorestframework.resources import FormResource from djangorestframework.response import Response from djangorestframework import status from resourceexample.forms import MyForm -class ExampleResource(Resource): - """A basic read-only resource that points to 3 other resources.""" +class MyFormValidation(FormResource): + """ + A resource which applies form validation on the input. + """ + form = MyForm + + +class ExampleResource(BaseView): + """ + A basic read-only resource that points to 3 other resources. + """ def get(self, request): return {"Some other resources": [reverse('another-example-resource', kwargs={'num':num}) for num in range(3)]} -class AnotherExampleResource(Resource): - """A basic GET-able/POST-able resource.""" - form = MyForm # Optional form validation on input (Applies in this case the POST method, but can also apply to PUT) + +class AnotherExampleResource(BaseView): + """ + A basic GET-able/POST-able resource. + """ + resource = MyFormValidation def get(self, request, num): """Handle GET requests""" diff --git a/examples/sandbox/views.py b/examples/sandbox/views.py index 78b722caf..d5b59284b 100644 --- a/examples/sandbox/views.py +++ b/examples/sandbox/views.py @@ -27,8 +27,8 @@ class Sandbox(BaseView): def get(self, request): return [{'name': 'Simple Resource example', 'url': reverse('example-resource')}, - {'name': 'Simple ModelResource example', 'url': reverse('my-model-root-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')}, {'name': 'Code highlighting API', 'url': reverse('pygments-root')}, - {'name': 'Blog posts API', 'url': reverse('blog-posts')}] + {'name': 'Blog posts API', 'url': reverse('blog-posts-root')}]