]> git.ipfire.org Git - thirdparty/patchwork.git/commitdiff
REST: Use generic views instead of ViewSets
authorStephen Finucane <stephen@that.guru>
Thu, 24 Nov 2016 12:55:42 +0000 (12:55 +0000)
committerStephen Finucane <stephen@that.guru>
Fri, 23 Dec 2016 23:41:29 +0000 (23:41 +0000)
ViewSet provide an easy way to define an API, but they don't offer much
flexibility. To get them to work as expected, a lot of hacks were
required. Generic views provide a more verbose, but ultimately easier to
understand, version. Using generic views lets us remove the dependency
of drf-nested-routers, bringing us back down to two main dependencies.
It also lets us remove the AuthenticatedReadOnly permission class, as
the DRF provides a similar permission class that can be used with
generic views.

The main user facing change is that invalid methods, such as POST on an
endpoint that doesn't allow object creation, will now return a HTTP 405
(Method Not Allowed) error code rather than the HTTP 403 (Forbidden)
error code previously returned. This is the semantically correct option
and should have been used all along.

Signed-off-by: Stephen Finucane <stephen@that.guru>
Reviewed-by: Daniel Axtens <dja@axtens.net>
13 files changed:
patchwork/api/base.py
patchwork/api/check.py
patchwork/api/index.py [new file with mode: 0644]
patchwork/api/patch.py
patchwork/api/person.py
patchwork/api/project.py
patchwork/api/user.py
patchwork/settings/base.py
patchwork/tests/test_rest_api.py
patchwork/urls.py
requirements-dev.txt
requirements-prod.txt
tox.ini

index 3639ebe487f494661f795e764b40470cea2e440b..dbd8148d033f3563162ae5c1865dd55f68051b5d 100644 (file)
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 from django.conf import settings
+from django.shortcuts import get_object_or_404
 from rest_framework import permissions
 from rest_framework.pagination import PageNumberPagination
 from rest_framework.response import Response
-from rest_framework.viewsets import ModelViewSet
 
 
 class LinkHeaderPagination(PageNumberPagination):
@@ -53,11 +53,6 @@ class LinkHeaderPagination(PageNumberPagination):
 
 class PatchworkPermission(permissions.BasePermission):
     """This permission works for Project and Patch model objects"""
-    def has_permission(self, request, view):
-        if request.method in ('POST', 'DELETE'):
-            return False
-        return super(PatchworkPermission, self).has_permission(request, view)
-
     def has_object_permission(self, request, view, obj):
         # read only for everyone
         if request.method in permissions.SAFE_METHODS:
@@ -65,14 +60,15 @@ class PatchworkPermission(permissions.BasePermission):
         return obj.is_editable(request.user)
 
 
-class AuthenticatedReadOnly(permissions.BasePermission):
-    def has_permission(self, request, view):
-        authenticated = request.user.is_authenticated()
-        return authenticated and request.method in permissions.SAFE_METHODS
-
+class MultipleFieldLookupMixin(object):
+    """Enable multiple lookups fields."""
 
-class PatchworkViewSet(ModelViewSet):
-    pagination_class = LinkHeaderPagination
+    def get_object(self):
+        queryset = self.filter_queryset(self.get_queryset())
+        filter_kwargs = {}
+        for field_name, field in zip(
+                self.lookup_fields, self.lookup_url_kwargs):
+            if self.kwargs[field]:
+                filter_kwargs[field_name] = self.kwargs[field]
 
-    def get_queryset(self):
-        return self.serializer_class.Meta.model.objects.all()
+        return get_object_or_404(queryset, **filter_kwargs)
index 2fd681a1fe1d1b2a0a9f0a5034fe6eee5bd1f1ad..78c21414e55341652123da15f6344449e5de979d 100644 (file)
 # along with Patchwork; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
-from django.core.urlresolvers import reverse
 from rest_framework.exceptions import PermissionDenied
+from rest_framework.generics import ListCreateAPIView
+from rest_framework.generics import RetrieveAPIView
 from rest_framework.relations import HyperlinkedRelatedField
-from rest_framework.response import Response
 from rest_framework.serializers import CurrentUserDefault
 from rest_framework.serializers import HiddenField
-from rest_framework.serializers import ModelSerializer
+from rest_framework.serializers import HyperlinkedModelSerializer
+from rest_framework.serializers import HyperlinkedIdentityField
 
-from patchwork.api.base import PatchworkViewSet
+from patchwork.api.base import MultipleFieldLookupMixin
 from patchwork.models import Check
 from patchwork.models import Patch
 
@@ -38,10 +39,29 @@ class CurrentPatchDefault(object):
         return self.patch
 
 
-class CheckSerializer(ModelSerializer):
+class CheckHyperlinkedIdentityField(HyperlinkedIdentityField):
+
+    def get_url(self, obj, view_name, request, format):
+        # Unsaved objects will not yet have a valid URL.
+        if obj.pk is None:
+            return None
+
+        return self.reverse(
+            view_name,
+            kwargs={
+                'patch_id': obj.patch.id,
+                'check_id': obj.id,
+            },
+            request=request,
+            format=format,
+        )
+
+
+class CheckSerializer(HyperlinkedModelSerializer):
     user = HyperlinkedRelatedField(
-        'user-detail', read_only=True, default=CurrentUserDefault())
+        'api-user-detail', read_only=True, default=CurrentUserDefault())
     patch = HiddenField(default=CurrentPatchDefault())
+    url = CheckHyperlinkedIdentityField('api-check-detail')
 
     def run_validation(self, data):
         for val, label in Check.STATE_CHOICES:
@@ -53,45 +73,39 @@ class CheckSerializer(ModelSerializer):
     def to_representation(self, instance):
         data = super(CheckSerializer, self).to_representation(instance)
         data['state'] = instance.get_state_display()
-        # drf-nested doesn't handle HyperlinkedModelSerializers properly,
-        # so we have to put the url in by hand here.
-        url = self.context['request'].build_absolute_uri(reverse(
-            'api_1.0:patch-detail', args=[instance.patch.id]))
-        data['url'] = url + 'checks/%s/' % instance.id
         return data
 
     class Meta:
         model = Check
-        fields = ('patch', 'user', 'date', 'state', 'target_url',
+        fields = ('url', 'patch', 'user', 'date', 'state', 'target_url',
                   'context', 'description')
         read_only_fields = ('date',)
+        extra_kwargs = {
+            'url': {'view_name': 'api-check-detail'},
+        }
+
 
+class CheckMixin(object):
 
-class CheckViewSet(PatchworkViewSet):
+    queryset = Check.objects.prefetch_related('patch', 'user')
     serializer_class = CheckSerializer
 
-    def not_allowed(self, request, **kwargs):
-        raise PermissionDenied()
 
-    update = not_allowed
-    partial_update = not_allowed
-    destroy = not_allowed
+class CheckListCreate(CheckMixin, ListCreateAPIView):
+    """List or create checks."""
+
+    lookup_url_kwarg = 'patch_id'
 
-    def create(self, request, patch_pk):
-        p = Patch.objects.get(id=patch_pk)
+    def create(self, request, patch_id):
+        p = Patch.objects.get(id=patch_id)
         if not p.is_editable(request.user):
             raise PermissionDenied()
         request.patch = p
-        return super(CheckViewSet, self).create(request)
+        return super(CheckListCreate, self).create(request)
 
-    def list(self, request, patch_pk):
-        queryset = self.filter_queryset(self.get_queryset())
-        queryset = queryset.filter(patch=patch_pk)
 
-        page = self.paginate_queryset(queryset)
-        if page is not None:
-            serializer = self.get_serializer(page, many=True)
-            return self.get_paginated_response(serializer.data)
+class CheckDetail(CheckMixin, MultipleFieldLookupMixin, RetrieveAPIView):
+    """Show a check."""
 
-        serializer = self.get_serializer(queryset, many=True)
-        return Response(serializer.data)
+    lookup_url_kwargs = ('patch_id', 'check_id')
+    lookup_fields = ('patch_id', 'id')
diff --git a/patchwork/api/index.py b/patchwork/api/index.py
new file mode 100644 (file)
index 0000000..58aeb87
--- /dev/null
@@ -0,0 +1,36 @@
+# Patchwork - automated patch tracking system
+# Copyright (C) 2016 Linaro Corporation
+#
+# This file is part of the Patchwork package.
+#
+# Patchwork is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# Patchwork is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Patchwork; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+from django.core.urlresolvers import reverse
+from rest_framework.response import Response
+from rest_framework.views import APIView
+
+
+class IndexView(APIView):
+
+    def get(self, request, format=None):
+        return Response({
+            'projects': request.build_absolute_uri(
+                reverse('api-project-list')),
+            'users': request.build_absolute_uri(reverse('api-user-list')),
+            'people': request.build_absolute_uri(reverse('api-person-list')),
+            'patches': request.build_absolute_uri(reverse('api-patch-list')),
+            'covers': request.build_absolute_uri(reverse('api-cover-list')),
+            'series': request.build_absolute_uri(reverse('api-series-list')),
+        })
index 811ba1e947cedad2b377b6bbbb3e5bf9b14880ca..842745033d17da561911c9c2a1c4efb9f4682c3a 100644 (file)
 
 import email.parser
 
+from django.core.urlresolvers import reverse
 from rest_framework.serializers import HyperlinkedModelSerializer
-from rest_framework.serializers import ListSerializer
+from rest_framework.generics import ListAPIView
+from rest_framework.generics import RetrieveUpdateAPIView
 from rest_framework.serializers import SerializerMethodField
 
 from patchwork.api.base import PatchworkPermission
-from patchwork.api.base import PatchworkViewSet
 from patchwork.models import Patch
 
 
-class PatchListSerializer(ListSerializer):
-    """Semi hack to make the list of patches more efficient"""
-    def to_representation(self, data):
-        del self.child.fields['content']
-        del self.child.fields['headers']
-        del self.child.fields['diff']
-        return super(PatchListSerializer, self).to_representation(data)
-
-
-class PatchSerializer(HyperlinkedModelSerializer):
+class PatchListSerializer(HyperlinkedModelSerializer):
     mbox = SerializerMethodField()
     state = SerializerMethodField()
     tags = SerializerMethodField()
-    headers = SerializerMethodField()
     check = SerializerMethodField()
+    checks = SerializerMethodField()
 
     def get_state(self, instance):
         return instance.state.name
@@ -58,39 +50,65 @@ class PatchSerializer(HyperlinkedModelSerializer):
         else:
             return None
 
-    def get_headers(self, instance):
-        if instance.headers:
-            return
-        email.parser.Parser().parsestr(instance.headers, True)
-
     def get_check(self, instance):
         return instance.combined_check_state
 
-    def to_representation(self, instance):
-        data = super(PatchSerializer, self).to_representation(instance)
-        data['checks'] = data['url'] + 'checks/'
-        return data
+    def get_checks(self, instance):
+        return self.context.get('request').build_absolute_uri(
+            reverse('api-check-list', kwargs={'patch_id': instance.id}))
 
     class Meta:
         model = Patch
-        list_serializer_class = PatchListSerializer
         fields = ('url', 'project', 'msgid', 'date', 'name', 'commit_ref',
                   'pull_url', 'state', 'archived', 'hash', 'submitter',
-                  'delegate', 'mbox', 'check', 'checks', 'tags', 'headers',
-                  'content', 'diff')
-        read_only_fields = ('project', 'name', 'date', 'submitter', 'diff',
-                            'content', 'hash', 'msgid')
+                  'delegate', 'mbox', 'check', 'checks', 'tags')
+        read_only_fields = ('project', 'msgid', 'date', 'name', 'hash',
+                            'submitter', 'mbox', 'mbox', 'series', 'check',
+                            'checks', 'tags')
+        extra_kwargs = {
+            'url': {'view_name': 'api-patch-detail'},
+            'project': {'view_name': 'api-project-detail'},
+            'submitter': {'view_name': 'api-person-detail'},
+            'delegate': {'view_name': 'api-user-detail'},
+        }
+
+
+class PatchDetailSerializer(PatchListSerializer):
+    headers = SerializerMethodField()
+
+    def get_headers(self, patch):
+        if patch.headers:
+            return email.parser.Parser().parsestr(patch.headers, True)
+
+    class Meta:
+        model = Patch
+        fields = PatchListSerializer.Meta.fields + (
+            'headers', 'content', 'diff')
+        read_only_fields = PatchListSerializer.Meta.read_only_fields + (
+            'headers', 'content', 'diff')
+        extra_kwargs = PatchListSerializer.Meta.extra_kwargs
+
+
+class PatchList(ListAPIView):
+    """List patches."""
+
+    permission_classes = (PatchworkPermission,)
+    serializer_class = PatchListSerializer
+
+    def get_queryset(self):
+        return Patch.objects.all().with_tag_counts()\
+            .prefetch_related('check_set')\
+            .select_related('state', 'submitter', 'delegate')\
+            .defer('content', 'diff', 'headers')
+
 
+class PatchDetail(RetrieveUpdateAPIView):
+    """Show a patch."""
 
-class PatchViewSet(PatchworkViewSet):
     permission_classes = (PatchworkPermission,)
-    serializer_class = PatchSerializer
+    serializer_class = PatchDetailSerializer
 
     def get_queryset(self):
-        qs = super(PatchViewSet, self).get_queryset().with_tag_counts()\
+        return Patch.objects.all().with_tag_counts()\
             .prefetch_related('check_set')\
             .select_related('state', 'submitter', 'delegate')
-        if 'pk' not in self.kwargs:
-            # we are doing a listing, we don't need these fields
-            qs = qs.defer('content', 'diff', 'headers')
-        return qs
index fe1e3b766496dd51e34c7807a8c7de3f95e18aac..c84cff5949c8869ee94d850ae5d9f95d4aa41b92 100644 (file)
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 from rest_framework.serializers import HyperlinkedModelSerializer
+from rest_framework.generics import ListAPIView
+from rest_framework.generics import RetrieveAPIView
+from rest_framework.permissions import IsAuthenticated
 
-from patchwork.api.base import AuthenticatedReadOnly
-from patchwork.api.base import PatchworkViewSet
 from patchwork.models import Person
 
 
@@ -28,12 +29,27 @@ class PersonSerializer(HyperlinkedModelSerializer):
     class Meta:
         model = Person
         fields = ('url', 'name', 'email', 'user')
+        read_only_fields = fields
+        extra_kwargs = {
+            'url': {'view_name': 'api-person-detail'},
+            'user': {'view_name': 'api-user-detail'},
+        }
 
 
-class PeopleViewSet(PatchworkViewSet):
-    permission_classes = (AuthenticatedReadOnly,)
+class PersonMixin(object):
+
+    queryset = Person.objects.prefetch_related('user')
+    permission_classes = (IsAuthenticated,)
     serializer_class = PersonSerializer
 
-    def get_queryset(self):
-        qs = super(PeopleViewSet, self).get_queryset()
-        return qs.prefetch_related('user')
+
+class PersonList(PersonMixin, ListAPIView):
+    """List users."""
+
+    pass
+
+
+class PersonDetail(PersonMixin, RetrieveAPIView):
+    """Show a user."""
+
+    pass
index af2aea0417a0c6479d0982493e3a9c331f5e34fe..14bc73d63a8375b416d5b65a4aac04f396183959 100644 (file)
 # along with Patchwork; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
+from django.shortcuts import get_object_or_404
+from rest_framework.generics import ListAPIView
+from rest_framework.generics import RetrieveUpdateAPIView
 from rest_framework.serializers import CharField
 from rest_framework.serializers import HyperlinkedModelSerializer
 
 from patchwork.api.base import PatchworkPermission
-from patchwork.api.base import PatchworkViewSet
 from patchwork.models import Project
 
 
@@ -34,26 +36,44 @@ class ProjectSerializer(HyperlinkedModelSerializer):
         model = Project
         fields = ('url', 'name', 'link_name', 'list_id', 'list_email',
                   'web_url', 'scm_url', 'webscm_url')
+        read_only_fields = ('name', 'maintainers')
+        extra_kwargs = {
+            'url': {'view_name': 'api-project-detail'},
+        }
 
 
-class ProjectViewSet(PatchworkViewSet):
+class ProjectMixin(object):
+
+    queryset = Project.objects.all()
     permission_classes = (PatchworkPermission,)
     serializer_class = ProjectSerializer
 
-    def _handle_linkname(self, pk):
-        '''Make it easy for users to list by project-id or linkname'''
-        qs = self.get_queryset()
+    def get_object(self):
+        queryset = self.filter_queryset(self.get_queryset())
+
+        assert 'pk' in self.kwargs
+
         try:
-            qs.get(id=pk)
-        except (self.serializer_class.Meta.model.DoesNotExist, ValueError):
-            # probably a non-numeric value which means we are going by linkname
-            self.kwargs = {'linkname': pk}  # try and lookup by linkname
-            self.lookup_field = 'linkname'
-
-    def retrieve(self, request, pk=None):
-        self._handle_linkname(pk)
-        return super(ProjectViewSet, self).retrieve(request, pk)
-
-    def partial_update(self, request, pk=None):
-        self._handle_linkname(pk)
-        return super(ProjectViewSet, self).partial_update(request, pk)
+            obj = queryset.get(id=int(self.kwargs['pk']))
+        except (ValueError, Project.DoesNotExist):
+            obj = get_object_or_404(queryset, linkname=self.kwargs['pk'])
+
+        # NOTE(stephenfin): We must do this to make sure the 'url'
+        # field is populated correctly
+        self.kwargs['pk'] = obj.id
+
+        self.check_object_permissions(self.request, obj)
+
+        return obj
+
+
+class ProjectList(ProjectMixin, ListAPIView):
+    """List projects."""
+
+    pass
+
+
+class ProjectDetail(ProjectMixin, RetrieveUpdateAPIView):
+    """Show a project."""
+
+    pass
index 3a4ae1a609dae1711aa0d05d5665784aaa553fae..8fe3e74ad9a82e6b0f39b37efcc90a1446471eee 100644 (file)
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 from django.contrib.auth.models import User
+from rest_framework.generics import ListAPIView
+from rest_framework.generics import RetrieveAPIView
+from rest_framework.permissions import IsAuthenticated
 from rest_framework.serializers import HyperlinkedModelSerializer
 
-from patchwork.api.base import AuthenticatedReadOnly
-from patchwork.api.base import PatchworkViewSet
-
 
 class UserSerializer(HyperlinkedModelSerializer):
+
     class Meta:
         model = User
         fields = ('url', 'username', 'first_name', 'last_name', 'email')
+        extra_kwargs = {
+            'url': {'view_name': 'api-user-detail'},
+        }
+
 
+class UserMixin(object):
 
-class UserViewSet(PatchworkViewSet):
-    permission_classes = (AuthenticatedReadOnly,)
+    queryset = User.objects.all()
+    permission_classes = (IsAuthenticated,)
     serializer_class = UserSerializer
+
+
+class UserList(UserMixin, ListAPIView):
+    """List users."""
+
+    pass
+
+
+class UserDetail(UserMixin, RetrieveAPIView):
+    """Show a user."""
+
+    pass
index a32710f1d02bb24ff527c4444300950a8bf11215..b7b10c37dbba89708b2ecb5bf8ae6030b0f8f347 100644 (file)
@@ -140,7 +140,9 @@ except ImportError:
 
 
 REST_FRAMEWORK = {
-    'DEFAULT_VERSIONING_CLASS': 'rest_framework.versioning.NamespaceVersioning'
+    'DEFAULT_VERSIONING_CLASS':
+        'rest_framework.versioning.NamespaceVersioning',
+    'DEFAULT_PAGINATION_CLASS': 'patchwork.api.base.LinkHeaderPagination',
 }
 
 #
index ddce4d75310d03dd3df96a4d4caf9c50c28e9a8e..469fd267950c142217e7a7157d5a45d0fe266bd8 100644 (file)
@@ -48,8 +48,8 @@ class TestProjectAPI(APITestCase):
     @staticmethod
     def api_url(item=None):
         if item is None:
-            return reverse('api_1.0:project-list')
-        return reverse('api_1.0:project-detail', args=[item])
+            return reverse('api-project-list')
+        return reverse('api-project-detail', args=[item])
 
     def test_list_simple(self):
         """Validate we can list the default test project."""
@@ -89,7 +89,7 @@ class TestProjectAPI(APITestCase):
         resp = self.client.post(
             self.api_url(),
             {'linkname': 'l', 'name': 'n', 'listid': 'l', 'listemail': 'e'})
-        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
     def test_anonymous_update(self):
         """Ensure anonymous "PATCH" operations are rejected."""
@@ -104,7 +104,7 @@ class TestProjectAPI(APITestCase):
         project = create_project()
 
         resp = self.client.delete(self.api_url(project.id))
-        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
     def test_create(self):
         """Ensure creations are rejected."""
@@ -117,7 +117,7 @@ class TestProjectAPI(APITestCase):
         resp = self.client.post(
             self.api_url(),
             {'linkname': 'l', 'name': 'n', 'listid': 'l', 'listemail': 'e'})
-        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
     def test_update(self):
         """Ensure updates can be performed maintainers."""
@@ -147,7 +147,7 @@ class TestProjectAPI(APITestCase):
         user.save()
         self.client.force_authenticate(user=user)
         resp = self.client.delete(self.api_url(project.id))
-        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
         self.assertEqual(1, Project.objects.all().count())
 
 
@@ -157,8 +157,8 @@ class TestPersonAPI(APITestCase):
     @staticmethod
     def api_url(item=None):
         if item is None:
-            return reverse('api_1.0:person-list')
-        return reverse('api_1.0:person-detail', args=[item])
+            return reverse('api-person-list')
+        return reverse('api-person-detail', args=[item])
 
     def test_anonymous_list(self):
         """The API should reject anonymous users."""
@@ -196,14 +196,14 @@ class TestPersonAPI(APITestCase):
         self.client.force_authenticate(user=user)
 
         resp = self.client.delete(self.api_url(user.id))
-        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
         resp = self.client.patch(self.api_url(user.id),
                                  {'email': 'foo@f.com'})
-        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
         resp = self.client.post(self.api_url(), {'email': 'foo@f.com'})
-        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
 
 @unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
@@ -212,8 +212,8 @@ class TestUserAPI(APITestCase):
     @staticmethod
     def api_url(item=None):
         if item is None:
-            return reverse('api_1.0:user-list')
-        return reverse('api_1.0:user-detail', args=[item])
+            return reverse('api-user-list')
+        return reverse('api-user-detail', args=[item])
 
     def test_anonymous_list(self):
         """The API should reject anonymous users."""
@@ -239,13 +239,13 @@ class TestUserAPI(APITestCase):
         self.client.force_authenticate(user=user)
 
         resp = self.client.delete(self.api_url(1))
-        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
         resp = self.client.patch(self.api_url(1), {'email': 'foo@f.com'})
-        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
         resp = self.client.post(self.api_url(), {'email': 'foo@f.com'})
-        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
 
 @unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
@@ -255,8 +255,8 @@ class TestPatchAPI(APITestCase):
     @staticmethod
     def api_url(item=None):
         if item is None:
-            return reverse('api_1.0:patch-list')
-        return reverse('api_1.0:patch-detail', args=[item])
+            return reverse('api-patch-list')
+        return reverse('api-patch-detail', args=[item])
 
     def test_list_simple(self):
         """Validate we can list a patch."""
@@ -265,6 +265,8 @@ class TestPatchAPI(APITestCase):
         self.assertEqual(0, len(resp.data))
 
         patch_obj = create_patch()
+
+        # anonymous user
         resp = self.client.get(self.api_url())
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertEqual(1, len(resp.data))
@@ -274,7 +276,7 @@ class TestPatchAPI(APITestCase):
         self.assertNotIn('headers', patch_rsp)
         self.assertNotIn('diff', patch_rsp)
 
-        # test while authenticated
+        # authenticated user
         user = create_user()
         self.client.force_authenticate(user=user)
         resp = self.client.get(self.api_url())
@@ -284,7 +286,7 @@ class TestPatchAPI(APITestCase):
         self.assertEqual(patch_obj.name, patch_rsp['name'])
 
     def test_detail(self):
-        """Validate we can get a specific project."""
+        """Validate we can get a specific patch."""
         patch = create_patch()
 
         resp = self.client.get(self.api_url(patch.id))
@@ -318,7 +320,7 @@ class TestPatchAPI(APITestCase):
         }
 
         resp = self.client.post(self.api_url(), patch)
-        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
     def test_anonymous_update(self):
         """Ensure anonymous "PATCH" operations are rejected."""
@@ -339,7 +341,7 @@ class TestPatchAPI(APITestCase):
         patch_url = self.api_url(patch.id)
 
         resp = self.client.delete(patch_url)
-        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
     def test_create(self):
         """Ensure creations are rejected."""
@@ -359,7 +361,7 @@ class TestPatchAPI(APITestCase):
         }
 
         resp = self.client.post(self.api_url(), patch)
-        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
     def test_update(self):
         """Ensure updates can be performed by maintainers."""
@@ -391,7 +393,7 @@ class TestPatchAPI(APITestCase):
         self.client.force_authenticate(user=user)
 
         resp = self.client.delete(self.api_url(patch.id))
-        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
         self.assertEqual(1, Patch.objects.all().count())
 
 
@@ -399,13 +401,17 @@ class TestPatchAPI(APITestCase):
 class TestCheckAPI(APITestCase):
     fixtures = ['default_tags']
 
+    def api_url(self, item=None):
+        if item is None:
+            return reverse('api-check-list', args=[self.patch.id])
+        return reverse('api-check-detail', kwargs={
+            'patch_id': self.patch.id, 'check_id': item.id})
+
     def setUp(self):
         super(TestCheckAPI, self).setUp()
         project = create_project()
         self.user = create_maintainer(project)
         self.patch = create_patch(project=project)
-        self.urlbase = reverse('api_1.0:patch-detail', args=[self.patch.id])
-        self.urlbase += 'checks/'
 
     def _create_check(self):
         values = {
@@ -416,13 +422,13 @@ class TestCheckAPI(APITestCase):
 
     def test_list_simple(self):
         """Validate we can list checks on a patch."""
-        resp = self.client.get(self.urlbase)
+        resp = self.client.get(self.api_url())
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertEqual(0, len(resp.data))
 
         check_obj = self._create_check()
 
-        resp = self.client.get(self.urlbase)
+        resp = self.client.get(self.api_url())
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertEqual(1, len(resp.data))
         check_rsp = resp.data[0]
@@ -434,7 +440,7 @@ class TestCheckAPI(APITestCase):
     def test_detail(self):
         """Validate we can get a specific check."""
         check = self._create_check()
-        resp = self.client.get(self.urlbase + str(check.id) + '/')
+        resp = self.client.get(self.api_url(check))
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertEqual(check.target_url, resp.data['target_url'])
 
@@ -446,13 +452,13 @@ class TestCheckAPI(APITestCase):
         self.client.force_authenticate(user=self.user)
 
         # update
-        resp = self.client.patch(
-            self.urlbase + str(check.id) + '/', {'target_url': 'fail'})
-        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+        resp = self.client.patch(self.api_url(check),
+                                 {'target_url': 'fail'})
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
         # delete
-        resp = self.client.delete(self.urlbase + str(check.id) + '/')
-        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+        resp = self.client.delete(self.api_url(check))
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
     def test_create(self):
         """Ensure creations can be performed by user of patch."""
@@ -464,13 +470,13 @@ class TestCheckAPI(APITestCase):
         }
 
         self.client.force_authenticate(user=self.user)
-        resp = self.client.post(self.urlbase, check)
+        resp = self.client.post(self.api_url(), check)
         self.assertEqual(status.HTTP_201_CREATED, resp.status_code)
         self.assertEqual(1, Check.objects.all().count())
 
         user = create_user()
         self.client.force_authenticate(user=user)
-        resp = self.client.post(self.urlbase, check)
+        resp = self.client.post(self.api_url(), check)
         self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
 
     def test_create_invalid(self):
@@ -483,6 +489,6 @@ class TestCheckAPI(APITestCase):
         }
 
         self.client.force_authenticate(user=self.user)
-        resp = self.client.post(self.urlbase, check)
+        resp = self.client.post(self.api_url(), check)
         self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code)
         self.assertEqual(0, Check.objects.all().count())
index 66718376d74193c81eadeef3d85d49ace38ebe91..56db24ccdb61b567e678709535bb4cf2fcf3c30c 100644 (file)
@@ -152,29 +152,54 @@ if settings.ENABLE_REST_API:
         raise RuntimeError(
             'djangorestframework must be installed to enable the REST API.')
 
-    from rest_framework.routers import DefaultRouter
-    from rest_framework_nested.routers import NestedSimpleRouter
-
-    from patchwork.api.check import CheckViewSet
-    from patchwork.api.patch import PatchViewSet
-    from patchwork.api.person import PeopleViewSet
-    from patchwork.api.project import ProjectViewSet
-    from patchwork.api.user import UserViewSet
-
-    router = DefaultRouter()
-    router.register('patches', PatchViewSet, 'patch')
-    router.register('people', PeopleViewSet, 'person')
-    router.register('projects', ProjectViewSet, 'project')
-    router.register('users', UserViewSet, 'user')
-
-    patches_router = NestedSimpleRouter(router, r'patches', lookup='patch')
-    patches_router.register(r'checks', CheckViewSet, base_name='patch-checks')
+    from patchwork.api import check as api_check_views
+    from patchwork.api import index as api_index_views
+    from patchwork.api import patch as api_patch_views
+    from patchwork.api import person as api_person_views
+    from patchwork.api import project as api_project_views
+    from patchwork.api import user as api_user_views
+
+    api_patterns = [
+        url(r'^$',
+            api_index_views.IndexView.as_view(),
+            name='api-index'),
+        url(r'^users/$',
+            api_user_views.UserList.as_view(),
+            name='api-user-list'),
+        url(r'^users/(?P<pk>[^/]+)/$',
+            api_user_views.UserDetail.as_view(),
+            name='api-user-detail'),
+        url(r'^people/$',
+            api_person_views.PersonList.as_view(),
+            name='api-person-list'),
+        url(r'^people/(?P<pk>[^/]+)/$',
+            api_person_views.PersonDetail.as_view(),
+            name='api-person-detail'),
+        url(r'^patches/$',
+            api_patch_views.PatchList.as_view(),
+            name='api-patch-list'),
+        url(r'^patches/(?P<pk>[^/]+)/$',
+            api_patch_views.PatchDetail.as_view(),
+            name='api-patch-detail'),
+        url(r'^patches/(?P<patch_id>[^/]+)/checks/$',
+            api_check_views.CheckListCreate.as_view(),
+            name='api-check-list'),
+        url(r'^patches/(?P<patch_id>[^/]+)/checks/(?P<check_id>[^/]+)/$',
+            api_check_views.CheckDetail.as_view(),
+            name='api-check-detail'),
+        url(r'^projects/$',
+            api_project_views.ProjectList.as_view(),
+            name='api-project-list'),
+        url(r'^projects/(?P<pk>[^/]+)/$',
+            api_project_views.ProjectDetail.as_view(),
+            name='api-project-detail'),
+    ]
 
     urlpatterns += [
-        url(r'^api/1.0/', include(router.urls, namespace='api_1.0')),
-        url(r'^api/1.0/', include(patches_router.urls, namespace='api_1.0')),
+        url(r'^api/1.0/', include(api_patterns)),
     ]
 
+
 # redirect from old urls
 if settings.COMPAT_REDIR:
     urlpatterns += [
index fdfe00cb63d0adc98c21c413a5895925bb57ebb2..0f8f08bcb728647b90aa16f8793be3961765c851 100644 (file)
@@ -1,4 +1,3 @@
 Django>=1.8,<1.11
 djangorestframework>=3.5,<3.6
-drf-nested-routers>=0.11.1,<0.12
 -r requirements-test.txt
index 57e6fce2d52604c664276739f7e7b2836abe9300..64d4339b67b319c7cd8abbafb8c993dd9bc7da0a 100644 (file)
@@ -1,5 +1,4 @@
 Django>=1.8,<1.11
 djangorestframework>=3.5,<3.6
-drf-nested-routers>=0.11.1,<0.12
 psycopg2>2.6,<2.7
 sqlparse
diff --git a/tox.ini b/tox.ini
index d68588a3d8f8925f1d477da804e6cf4ddddd830b..c5a934963f3c005bbf50d638da6a9029096e3d5e 100644 (file)
--- a/tox.ini
+++ b/tox.ini
@@ -14,7 +14,6 @@ deps =
     django19: django>=1.9,<1.10
     django110: django>=1.10,<1.11
     django{18,19,110}: djangorestframework>=3.5,<3.6
-    drf-nested-routers>=0.11.1,<0.12
 setenv =
     DJANGO_SETTINGS_MODULE = patchwork.settings.dev
     PYTHONDONTWRITEBYTECODE = 1