[PATCH v3 08/16] REST: Use generic views instead of ViewSets
Daniel Axtens
dja at axtens.net
Wed Nov 30 09:14:16 AEDT 2016
Stephen Finucane <stephen at that.guru> 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 <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
>
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
More information about the Patchwork
mailing list