Fixing some of the last blocking issues

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)
 
 

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
         })
         

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))

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})
-        

examples/blogpost/urls.py

 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<key>[^/]+)/$', BlogPostInstance.as_view(), name='blog-post'),
-    url(r'^(?P<blogpost>[^/]+)/comments/$', Comments.as_view(), name='comments'),
-    url(r'^(?P<blogpost>[^/]+)/comments/(?P<id>[^/]+)/$', CommentInstance.as_view(), name='comment'),
-)
+    url(r'^$', ListOrCreateModelView.as_view(resource=BlogPostResource), name='blog-posts-root'),
+    url(r'^(?P<key>[^/]+)/$', InstanceModelView.as_view(resource=BlogPostResource)),
+    url(r'^(?P<blogpost>[^/]+)/comments/$', ListOrCreateModelView.as_view(resource=CommentResource), name='comments'),
+    url(r'^(?P<blogpost>[^/]+)/comments/(?P<id>[^/]+)/$', InstanceModelView.as_view(resource=CommentResource)),
+)
\ No newline at end of file

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()
+
 

examples/modelresourceexample/urls.py

 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)),
 )

examples/modelresourceexample/views.py

-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

examples/objectstore/views.py

 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)

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)

examples/resourceexample/views.py

 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"""

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')}]