[PATCH 06/13] REST: Stop using ViewSets
Stephen Finucane
stephen at that.guru
Thu Nov 17 12:39:09 AEDT 2016
These are hacked a lot of to get them to work as we want. Use a more
verbose, but ultimately easier to parse, version. This lets us remove
the dependency of drf-nested-routers, bringing us back down to two main
dependencies. It also lets us remove the AuthenticedReadOnly permission
class, as the generic views enforce this for us.
The main user facing change is the use of 405 (Method Not Allowed)
errors when trying to use an invalid method, such as POST on the
project endpoint.
Signed-off-by: Stephen Finucane <stephen at that.guru>
Cc: Andy Doan <andy.doan at linaro.org>
---
patchwork/api/__init__.py | 27 +++++-------
patchwork/api/check.py | 76 +++++++++++++++++++--------------
patchwork/api/index.py | 36 ++++++++++++++++
patchwork/api/patch.py | 90 +++++++++++++++++++++++-----------------
patchwork/api/person.py | 30 ++++++++++----
patchwork/api/project.py | 53 ++++++++++++++---------
patchwork/api/user.py | 28 ++++++++++---
patchwork/settings/base.py | 5 ++-
patchwork/tests/test_rest_api.py | 80 ++++++++++++++++++-----------------
patchwork/urls.py | 63 +++++++++++++++++++---------
10 files changed, 315 insertions(+), 173 deletions(-)
create mode 100644 patchwork/api/index.py
diff --git a/patchwork/api/__init__.py b/patchwork/api/__init__.py
index e91a282..dbd8148 100644
--- a/patchwork/api/__init__.py
+++ b/patchwork/api/__init__.py
@@ -18,11 +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):
@@ -54,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:
@@ -66,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 d24b0c6..1ff9992 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 import PatchworkViewSet
+from patchwork.api 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', )
+ 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 bc4cb5c..737ada1 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 import PatchworkPermission
-from patchwork.api 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
@@ -55,39 +47,61 @@ class PatchSerializer(HyperlinkedModelSerializer):
return {x.name: getattr(instance, x.attr_name)
for x in instance.project.tags}
- 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."""
+
+ queryset = Patch.objects.all().with_tag_counts().prefetch_related(
+ 'check_set').select_related(
+ 'state', 'submitter', 'delegate').defer(
+ 'content', 'diff', 'headers')
+ permission_classes = (PatchworkPermission,)
+ serializer_class = PatchListSerializer
+
+class PatchDetail(RetrieveUpdateAPIView):
+ """Show a patch."""
-class PatchViewSet(PatchworkViewSet):
+ queryset = Patch.objects.all().with_tag_counts().prefetch_related(
+ 'check_set').select_related(
+ 'state', 'submitter', 'delegate')
permission_classes = (PatchworkPermission,)
- serializer_class = PatchSerializer
-
- def get_queryset(self):
- qs = super(PatchViewSet, self).get_queryset().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
+ serializer_class = PatchDetailSerializer
diff --git a/patchwork/api/person.py b/patchwork/api/person.py
index 7507fe5..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 import AuthenticatedReadOnly
-from patchwork.api 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 9e5d2aa..3319339 100644
--- a/patchwork/api/project.py
+++ b/patchwork/api/project.py
@@ -17,11 +17,12 @@
# along with Patchwork; if not, write to the Free Software
# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+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 import PatchworkPermission
-from patchwork.api import PatchworkViewSet
from patchwork.models import Project
@@ -35,26 +36,40 @@ 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):
+
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()
- 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)
+ def get_queryset(self):
+ query = Project.objects.all()
+
+ if 'pk' in self.kwargs:
+ try:
+ query.get(id=int(self.kwargs['pk']))
+ except (ValueError, Project.DoesNotExist):
+ query.get(linkname=self.kwargs['pk'])
+
+ # NOTE(stephenfin): We must do this to make sure the 'url'
+ # field is populated correctly
+ self.kwargs['pk'] = query[0].id
+
+ return query
+
+
+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 1da90ac..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 import AuthenticatedReadOnly
-from patchwork.api 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..35ac96a 100644
--- a/patchwork/settings/base.py
+++ b/patchwork/settings/base.py
@@ -140,7 +140,10 @@ except ImportError:
REST_FRAMEWORK = {
- 'DEFAULT_VERSIONING_CLASS': 'rest_framework.versioning.NamespaceVersioning'
+ 'DEFAULT_VERSIONING_CLASS': (
+ 'rest_framework.versioning.NamespaceVersioning'
+ ),
+ 'DEFAULT_PAGINATION_CLASS': 'patchwork.api.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 7644da9..7c29319 100644
--- a/patchwork/urls.py
+++ b/patchwork/urls.py
@@ -147,29 +147,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 += [
--
2.7.4
More information about the Patchwork
mailing list