Various clean up and lots of docs

2 files changed, 222 insertions, 83 deletions

diff --git a/rest_framework/generics.py b/rest_framework/generics.py
index ae03060b..3440c01d 100644
--- a/rest_framework/generics.py
+++ b/rest_framework/generics.py
@@ -18,21 +18,35 @@ class GenericAPIView(views.APIView):
     Base class for all other generic views.
     """
 
+    # You'll need to either set these attributes,
+    # or override `get_queryset`/`get_serializer_class`.
     queryset = None
     serializer_class = None
 
-    # Shortcut which may be used in place of `queryset`/`serializer_class`
-    model = None
+    # If you want to use object lookups other than pk, set this attribute.
+    lookup_field = 'pk'
 
-    filter_backend = api_settings.FILTER_BACKEND
+    # Pagination settings
     paginate_by = api_settings.PAGINATE_BY
     paginate_by_param = api_settings.PAGINATE_BY_PARAM
     pagination_serializer_class = api_settings.DEFAULT_PAGINATION_SERIALIZER_CLASS
-    model_serializer_class = api_settings.DEFAULT_MODEL_SERIALIZER_CLASS
     page_kwarg = 'page'
-    lookup_field = 'pk'
+
+    # The filter backend class to use for queryset filtering
+    filter_backend = api_settings.FILTER_BACKEND
+
+    # Determines if the view will return 200 or 404 responses for empty lists.
     allow_empty = True
 
+    # This shortcut may be used instead of setting either (or both)
+    # of the `queryset`/`serializer_class` attributes, although using
+    # the explicit style is generally preferred.
+    model = None
+
+    # If the `model` shortcut is used instead of `serializer_class`, then the
+    # serializer class will be constructed using this class as the base.
+    model_serializer_class = api_settings.DEFAULT_MODEL_SERIALIZER_CLASS
+
     ######################################
     # These are pending deprecation...
 
@@ -61,7 +75,7 @@ class GenericAPIView(views.APIView):
         return serializer_class(instance, data=data, files=files,
                                 many=many, partial=partial, context=context)
 
-    def get_pagination_serializer(self, page=None):
+    def get_pagination_serializer(self, page):
         """
         Return a serializer instance to use with paginated data.
         """
@@ -73,32 +87,15 @@ class GenericAPIView(views.APIView):
         context = self.get_serializer_context()
         return pagination_serializer_class(instance=page, context=context)
 
-    def get_paginate_by(self, queryset=None):
-        """
-        Return the size of pages to use with pagination.
-
-        If `PAGINATE_BY_PARAM` is set it will attempt to get the page size
-        from a named query parameter in the url, eg. ?page_size=100
-
-        Otherwise defaults to using `self.paginate_by`.
-        """
-        if self.paginate_by_param:
-            query_params = self.request.QUERY_PARAMS
-            try:
-                return int(query_params[self.paginate_by_param])
-            except (KeyError, ValueError):
-                pass
-
-        return self.paginate_by
-
     def paginate_queryset(self, queryset, page_size, paginator_class=Paginator):
         """
         Paginate a queryset.
         """
         paginator = paginator_class(queryset, page_size,
                                     allow_empty_first_page=self.allow_empty)
-        page_kwarg = self.page_kwarg
-        page = self.kwargs.get(page_kwarg) or self.request.GET.get(page_kwarg) or 1
+        page_kwarg = self.kwargs.get(self.page_kwarg)
+        page_query_param = self.request.QUERY_PARAMS.get(self.page_kwarg)
+        page = page_kwarg or page_query_param or 1
         try:
             page_number = int(page)
         except ValueError:
@@ -133,6 +130,27 @@ class GenericAPIView(views.APIView):
     ### The following methods provide default implementations
     ### that you may want to override for more complex cases.
 
+    def get_paginate_by(self, queryset=None):
+        """
+        Return the size of pages to use with pagination.
+
+        If `PAGINATE_BY_PARAM` is set it will attempt to get the page size
+        from a named query parameter in the url, eg. ?page_size=100
+
+        Otherwise defaults to using `self.paginate_by`.
+        """
+        if queryset is not None:
+            pass  # TODO: Deprecation warning
+
+        if self.paginate_by_param:
+            query_params = self.request.QUERY_PARAMS
+            try:
+                return int(query_params[self.paginate_by_param])
+            except (KeyError, ValueError):
+                pass
+
+        return self.paginate_by
+
     def get_serializer_class(self):
         """
         Return the class to use for the serializer.
@@ -202,6 +220,7 @@ class GenericAPIView(views.APIView):
             # TODO: Deprecation warning
             filter_kwargs = {self.slug_field: slug}
         else:
+            # TODO: Fix error message
             raise AttributeError("Generic detail view %s must be called with "
                                  "either an object pk or a slug."
                                  % self.__class__.__name__)
@@ -216,6 +235,9 @@ class GenericAPIView(views.APIView):
     ########################
     ### The following are placeholder methods,
     ### and are intended to be overridden.
+    ###
+    ### The are not called by GenericAPIView directly,
+    ### but are used by the mixin methods.
 
     def pre_save(self, obj):
         """
diff --git a/rest_framework/routers.py b/rest_framework/routers.py
index afc51f3b..febb02b3 100644
--- a/rest_framework/routers.py
+++ b/rest_framework/routers.py
@@ -1,81 +1,198 @@
+"""
+Routers provide a convenient and consistent way of automatically
+determining the URL conf for your API.
+
+They are used by simply instantiating a Router class, and then registering
+all the required ViewSets with that router.
+
+For example, you might have a `urls.py` that looks something like this:
+
+    router = routers.DefaultRouter()
+    router.register('users', UserViewSet, 'user')
+    router.register('accounts', AccountViewSet, 'account')
+
+    urlpatterns = router.urls
+"""
 from django.conf.urls import url, patterns
+from rest_framework.decorators import api_view
+from rest_framework.response import Response
+from rest_framework.reverse import reverse
+from rest_framework.urlpatterns import format_suffix_patterns
 
 
 class BaseRouter(object):
     def __init__(self):
         self.registry = []
 
-    def register(self, prefix, viewset, base_name):
-        self.registry.append((prefix, viewset, base_name))
+    def register(self, prefix, viewset, basename):
+        self.registry.append((prefix, viewset, basename))
 
-    def get_urlpatterns(self):
-        raise NotImplemented('get_urlpatterns must be overridden')
+    def get_urls(self):
+        raise NotImplemented('get_urls must be overridden')
 
     @property
-    def urlpatterns(self):
-        if not hasattr(self, '_urlpatterns'):
-            self._urlpatterns = patterns('', *self.get_urlpatterns())
-        return self._urlpatterns
-
-
-class DefaultRouter(BaseRouter):
-    route_list = [
-        (r'$', {
-            'get': 'list',
-            'post': 'create'
-        }, 'list'),
-        (r'(?P<pk>[^/]+)/$', {
-            'get': 'retrieve',
-            'put': 'update',
-            'patch': 'partial_update',
-            'delete': 'destroy'
-        }, 'detail'),
+    def urls(self):
+        if not hasattr(self, '_urls'):
+            self._urls = patterns('', *self.get_urls())
+        return self._urls
+
+
+class SimpleRouter(BaseRouter):
+    routes = [
+        # List route.
+        (
+            r'^{prefix}/$',
+            {
+                'get': 'list',
+                'post': 'create'
+            },
+            '{basename}-list'
+        ),
+        # Detail route.
+        (
+            r'^{prefix}/{lookup}/$',
+            {
+                'get': 'retrieve',
+                'put': 'update',
+                'patch': 'partial_update',
+                'delete': 'destroy'
+            },
+            '{basename}-detail'
+        ),
+        # Dynamically generated routes.
+        # Generated using @action or @link decorators on methods of the viewset.
+        (
+            r'^{prefix}/{lookup}/{methodname}/$',
+            {
+                '{httpmethod}': '{methodname}',
+            },
+            '{basename}-{methodname}'
+        ),
     ]
-    extra_routes = r'(?P<pk>[^/]+)/%s/$'
-    name_format = '%s-%s'
 
-    def get_urlpatterns(self):
+    def get_routes(self, viewset):
+        """
+        Augment `self.routes` with any dynamically generated routes.
+
+        Returns a list of 4-tuples, of the form:
+        `(url_format, method_map, name_format, extra_kwargs)`
+        """
+
+        # Determine any `@action` or `@link` decorated methods on the viewset
+        dynamic_routes = {}
+        for methodname in dir(viewset):
+            attr = getattr(viewset, methodname)
+            httpmethod = getattr(attr, 'bind_to_method', None)
+            if httpmethod:
+                dynamic_routes[httpmethod] = methodname
+
         ret = []
-        for prefix, viewset, base_name in self.registry:
-            # Bind regular views
-            if not getattr(viewset, '_is_viewset', False):
-                regex = prefix
-                view = viewset
-                name = base_name
-                ret.append(url(regex, view, name=name))
-                continue
+        for url_format, method_map, name_format in self.routes:
+            if method_map == {'{httpmethod}': '{methodname}'}:
+                # Dynamic routes
+                for httpmethod, methodname in dynamic_routes.items():
+                    extra_kwargs = getattr(viewset, methodname).kwargs
+                    ret.append((
+                        url_format.replace('{methodname}', methodname),
+                        {httpmethod: methodname},
+                        name_format.replace('{methodname}', methodname),
+                        extra_kwargs
+                    ))
+            else:
+                # Standard route
+                extra_kwargs = {}
+                ret.append((url_format, method_map, name_format, extra_kwargs))
 
-            # Bind standard CRUD routes
-            for suffix, action_mapping, action_name in self.route_list:
+        return ret
 
-                # Only actions which actually exist on the viewset will be bound
-                bound_actions = {}
-                for method, action in action_mapping.items():
-                    if hasattr(viewset, action):
-                        bound_actions[method] = action
+    def get_method_map(self, viewset, method_map):
+        """
+        Given a viewset, and a mapping of http methods to actions,
+        return a new mapping which only includes any mappings that
+        are actually implemented by the viewset.
+        """
+        bound_methods = {}
+        for method, action in method_map.items():
+            if hasattr(viewset, action):
+                bound_methods[method] = action
+        return bound_methods
+
+    def get_lookup_regex(self, viewset):
+        """
+        Given a viewset, return the portion of URL regex that is used
+        to match against a single instance.
+        """
+        base_regex = '(?P<{lookup_field}>[^/]+)'
+        lookup_field = getattr(viewset, 'lookup_field', 'pk')
+        return base_regex.format(lookup_field=lookup_field)
+
+    def get_urls(self):
+        """
+        Use the registered viewsets to generate a list of URL patterns.
+        """
+        ret = []
 
-                # Build the url pattern
-                regex = prefix + suffix
-                view = viewset.as_view(bound_actions, name_suffix=action_name)
-                name = self.name_format % (base_name, action_name)
-                ret.append(url(regex, view, name=name))
+        for prefix, viewset, basename in self.registry:
+            lookup = self.get_lookup_regex(viewset)
+            routes = self.get_routes(viewset)
 
-            # Bind any extra `@action` or `@link` routes
-            for action_name in dir(viewset):
-                func = getattr(viewset, action_name)
-                http_method = getattr(func, 'bind_to_method', None)
+            for url_format, method_map, name_format, extra_kwargs in routes:
 
-                # Skip if this is not an @action or @link method
-                if not http_method:
+                # Only actions which actually exist on the viewset will be bound
+                method_map = self.get_method_map(viewset, method_map)
+                if not method_map:
                     continue
 
-                suffix = self.extra_routes % action_name
-
                 # Build the url pattern
-                regex = prefix + suffix
-                view = viewset.as_view({http_method: action_name}, **func.kwargs)
-                name = self.name_format % (base_name, action_name)
+                regex = url_format.format(prefix=prefix, lookup=lookup)
+                view = viewset.as_view(method_map, **extra_kwargs)
+                name = name_format.format(basename=basename)
                 ret.append(url(regex, view, name=name))
 
-        # Return a list of url patterns
         return ret
+
+
+class DefaultRouter(SimpleRouter):
+    """
+    The default router extends the SimpleRouter, but also adds in a default
+    API root view, and adds format suffix patterns to the URLs.
+    """
+    include_root_view = True
+    include_format_suffixes = True
+
+    def get_api_root_view(self):
+        """
+        Return a view to use as the API root.
+        """
+        api_root_dict = {}
+        list_name = self.routes[0][-1]
+        for prefix, viewset, basename in self.registry:
+            api_root_dict[prefix] = list_name.format(basename=basename)
+
+        @api_view(('GET',))
+        def api_root(request, format=None):
+            ret = {}
+            for key, url_name in api_root_dict.items():
+                ret[key] = reverse(url_name, request=request, format=format)
+            return Response(ret)
+
+        return api_root
+
+    def get_urls(self):
+        """
+        Generate the list of URL patterns, including a default root view
+        for the API, and appending `.json` style format suffixes.
+        """
+        urls = []
+
+        if self.include_root_view:
+            root_url = url(r'^$', self.get_api_root_view(), name='api-root')
+            urls.append(root_url)
+
+        default_urls = super(DefaultRouter, self).get_urls()
+        urls.extend(default_urls)
+
+        if self.include_format_suffixes:
+            urls = format_suffix_patterns(urls)
+
+        return urls