[PATCH v3 08/16] REST: Use generic views instead of ViewSets
Stephen Finucane
stephen at that.guru
Sat Nov 26 05:18:27 AEDT 2016
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 at that.guru>
---
v3:
- Override 'get_queryset' for '/patch' endpoints, resolving an issue
under Python 3.4
- Correctly raise 404 for invalid project id and linkname (Andrew
Donnellan)
- Remove drf-nested-routers dependency (Andrew Donnellan)
---
patchwork/api/base.py | 26 +++++-------
patchwork/api/check.py | 74 ++++++++++++++++++++--------------
patchwork/api/index.py | 36 +++++++++++++++++
patchwork/api/patch.py | 86 ++++++++++++++++++++++++----------------
patchwork/api/person.py | 30 ++++++++++----
patchwork/api/project.py | 56 +++++++++++++++++---------
patchwork/api/user.py | 28 ++++++++++---
patchwork/settings/base.py | 4 +-
patchwork/tests/test_rest_api.py | 80 ++++++++++++++++++++-----------------
patchwork/urls.py | 63 ++++++++++++++++++++---------
requirements-dev.txt | 1 -
requirements-prod.txt | 1 -
tox.ini | 1 -
13 files changed, 317 insertions(+), 169 deletions(-)
create mode 100644 patchwork/api/index.py
diff --git a/patchwork/api/base.py b/patchwork/api/base.py
index 3639ebe..dbd8148 100644
--- a/patchwork/api/base.py
+++ b/patchwork/api/base.py
@@ -18,10 +18,10 @@
# 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)
diff --git a/patchwork/api/check.py b/patchwork/api/check.py
index 2fd681a..78c2141 100644
--- a/patchwork/api/check.py
+++ b/patchwork/api/check.py
@@ -17,15 +17,16 @@
# 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
index 0000000..58aeb87
--- /dev/null
+++ b/patchwork/api/index.py
@@ -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')),
+ })
diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
index 811ba1e..8427450 100644
--- a/patchwork/api/patch.py
+++ b/patchwork/api/patch.py
@@ -19,30 +19,22 @@
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
diff --git a/patchwork/api/person.py b/patchwork/api/person.py
index fe1e3b7..c84cff5 100644
--- a/patchwork/api/person.py
+++ b/patchwork/api/person.py
@@ -18,9 +18,10 @@
# 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
diff --git a/patchwork/api/project.py b/patchwork/api/project.py
index af2aea0..14bc73d 100644
--- a/patchwork/api/project.py
+++ b/patchwork/api/project.py
@@ -17,11 +17,13 @@
# 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
diff --git a/patchwork/api/user.py b/patchwork/api/user.py
index 3a4ae1a..8fe3e74 100644
--- a/patchwork/api/user.py
+++ b/patchwork/api/user.py
@@ -18,18 +18,36 @@
# 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
diff --git a/patchwork/settings/base.py b/patchwork/settings/base.py
index a32710f..b7b10c3 100644
--- a/patchwork/settings/base.py
+++ b/patchwork/settings/base.py
@@ -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',
}
#
diff --git a/patchwork/tests/test_rest_api.py b/patchwork/tests/test_rest_api.py
index ddce4d7..469fd26 100644
--- a/patchwork/tests/test_rest_api.py
+++ b/patchwork/tests/test_rest_api.py
@@ -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 at 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 at 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 at 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 at 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())
diff --git a/patchwork/urls.py b/patchwork/urls.py
index 6671837..56db24c 100644
--- a/patchwork/urls.py
+++ b/patchwork/urls.py
@@ -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 += [
diff --git a/requirements-dev.txt b/requirements-dev.txt
index fdfe00c..0f8f08b 100644
--- a/requirements-dev.txt
+++ b/requirements-dev.txt
@@ -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
diff --git a/requirements-prod.txt b/requirements-prod.txt
index 57e6fce..64d4339 100644
--- a/requirements-prod.txt
+++ b/requirements-prod.txt
@@ -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 d68588a..c5a9349 100644
--- 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
--
2.7.4
More information about the Patchwork
mailing list