Stephen Finucane <[email protected]> writes: > 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. >
Heaps better, thank you! Regards, Daniel > Signed-off-by: Stephen Finucane <[email protected]> > --- > 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': '[email protected]'}) > - 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': '[email protected]'}) > - 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': '[email protected]'}) > - 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': '[email protected]'}) > - 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 > > _______________________________________________ > Patchwork mailing list > [email protected] > https://lists.ozlabs.org/listinfo/patchwork _______________________________________________ Patchwork mailing list [email protected] https://lists.ozlabs.org/listinfo/patchwork
