On 18 May 22:30, Andy Doan wrote: > This exports projects via the REST API. > > Security Constraints: > * Anyone (logged in or not) can read all objects. > * No one can create/delete objects. > * Project maintainers are allowed to update (ie "patch" > attributes) > > Signed-off-by: Andy Doan <andy.d...@linaro.org> > Inspired-by: Damien Lespiau <damien.lesp...@intel.com>
Reviewed-by: Stephen Finucane <stephen.finuc...@intel.com> Re-reviewed due to changes in pagination. Some nits, but nothing that'll affect the API output (so all fixable later). Thanks for the hard work here. Stephen > --- > patchwork/rest_serializers.py | 27 +++++++++ > patchwork/settings/base.py | 1 + > patchwork/tests/test_rest_api.py | 116 > +++++++++++++++++++++++++++++++++++++++ > patchwork/urls.py | 3 +- > patchwork/views/rest_api.py | 58 +++++++++++++++++++- > 5 files changed, 202 insertions(+), 3 deletions(-) > create mode 100644 patchwork/rest_serializers.py > create mode 100644 patchwork/tests/test_rest_api.py > > diff --git a/patchwork/rest_serializers.py b/patchwork/rest_serializers.py > new file mode 100644 > index 0000000..b399d79 > --- /dev/null > +++ b/patchwork/rest_serializers.py > @@ -0,0 +1,27 @@ > +# 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 patchwork.models import Project > + > +from rest_framework.serializers import ModelSerializer > + > + > +class ProjectSerializer(ModelSerializer): > + class Meta: > + model = Project > diff --git a/patchwork/settings/base.py b/patchwork/settings/base.py > index 14f3209..d07dbaa 100644 > --- a/patchwork/settings/base.py > +++ b/patchwork/settings/base.py > @@ -158,6 +158,7 @@ ENABLE_XMLRPC = False > > # Set to True to enable the Patchwork REST API > ENABLE_REST_API = False > +REST_RESULTS_PER_PAGE = 30 > > # Set to True to enable redirections or URLs from previous versions > # of patchwork > diff --git a/patchwork/tests/test_rest_api.py > b/patchwork/tests/test_rest_api.py > new file mode 100644 > index 0000000..0dc1fe6 > --- /dev/null > +++ b/patchwork/tests/test_rest_api.py > @@ -0,0 +1,116 @@ > +# 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 > + > +import unittest > + > +from django.conf import settings > +from django.core.urlresolvers import reverse > + > +from rest_framework import status > +from rest_framework.test import APITestCase > + > +from patchwork.models import Project > +from patchwork.tests.utils import defaults, create_maintainer, create_user > + > + > +@unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API') > +class TestProjectAPI(APITestCase): > + fixtures = ['default_states'] > + apibase = reverse('api_1.0:project-list') > + > + @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]) > + > + def test_list_simple(self): > + """Validate we can list the default test project.""" > + defaults.project.save() > + resp = self.client.get(self.api_url()) > + self.assertEqual(status.HTTP_200_OK, resp.status_code) > + self.assertEqual(1, len(resp.data)) > + proj = resp.data[0] > + self.assertEqual(defaults.project.linkname, proj['linkname']) > + self.assertEqual(defaults.project.name, proj['name']) > + self.assertEqual(defaults.project.listid, proj['listid']) > + > + def test_get(self): nit: Could we call this 'test_detail' instead: this is the convention that I'm used to. > + """Validate we can get a specific project.""" > + defaults.project.save() > + resp = self.client.get(self.api_url(1)) > + self.assertEqual(status.HTTP_200_OK, resp.status_code) > + self.assertEqual(defaults.project.name, resp.data['name']) > + > + def test_anonymous_writes(self): > + """Ensure anonymous "write" operations are rejected.""" > + defaults.project.save() > + # create > + resp = self.client.post( > + self.api_url(), > + {'linkname': 'l', 'name': 'n', 'listid': 'l', 'listemail': 'e'}) > + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) > + # update > + resp = self.client.patch(self.api_url(1), {'linkname': 'foo'}) > + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) > + # delete > + resp = self.client.delete(self.api_url(1)) > + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) nit: This could probably be split and the components merged into test_(create|update|delete). > + def test_create(self): > + """Ensure creations are rejected.""" > + defaults.project.save() > + > + user = create_maintainer(defaults.project) > + user.is_superuser = True > + user.save() > + self.client.force_authenticate(user=user) > + resp = self.client.post( > + self.api_url(), > + {'linkname': 'l', 'name': 'n', 'listid': 'l', 'listemail': 'e'}) > + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) > + > + def test_update(self): > + """Ensure updates can be performed maintainers.""" > + defaults.project.save() > + > + # A maintainer can update > + user = create_maintainer(defaults.project) > + self.client.force_authenticate(user=user) > + resp = self.client.patch(self.api_url(1), {'linkname': 'TEST'}) > + self.assertEqual(status.HTTP_200_OK, resp.status_code) > + > + # A normal user can't > + user = create_user() > + self.client.force_authenticate(user=user) > + resp = self.client.patch(self.api_url(1), {'linkname': 'TEST'}) > + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) > + > + def test_delete(self): > + """Ensure deletions are rejected.""" > + defaults.project.save() > + > + # Even an admin can't remove a project > + user = create_maintainer(defaults.project) > + user.is_superuser = True > + user.save() > + 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(1, Project.objects.all().count()) > diff --git a/patchwork/urls.py b/patchwork/urls.py > index 3f0fcbf..f6763d4 100644 > --- a/patchwork/urls.py > +++ b/patchwork/urls.py > @@ -144,7 +144,8 @@ if settings.ENABLE_REST_API: > 'djangorestframework must be installed to enable the REST API.') > import patchwork.views.rest_api > urlpatterns += [ > - url(r'^api/1.0/', include(patchwork.views.rest_api.router.urls)), > + url(r'^api/1.0/', include( > + patchwork.views.rest_api.router.urls, namespace='api_1.0')), > ] > > # redirect from old urls > diff --git a/patchwork/views/rest_api.py b/patchwork/views/rest_api.py > index 5436ed6..4938e78 100644 > --- a/patchwork/views/rest_api.py > +++ b/patchwork/views/rest_api.py > @@ -17,6 +17,60 @@ > # 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 import routers > +from django.conf import settings > > -router = routers.DefaultRouter() > +from patchwork.models import Project > +from patchwork.rest_serializers import ProjectSerializer > + > +from rest_framework import permissions > +from rest_framework.pagination import PageNumberPagination > +from rest_framework.response import Response > +from rest_framework.routers import DefaultRouter > +from rest_framework.viewsets import ModelViewSet > + > + > +class LinkHeaderPagination(PageNumberPagination): > + # https://tools.ietf.org/html/rfc5988#section-5 > + # https://developer.github.com/guides/traversing-with-pagination/ nit: This should form part of a docstring, IMO. > + page_size = settings.REST_RESULTS_PER_PAGE nit: This is already a DRF-provided setting that we can override: http://www.django-rest-framework.org/api-guide/settings/#page_size However, Given the deprecation of other similar parameters, I'm fine to stick with our own parameter (those settings might be worth reading though, in case there's anything cool there :)). > + page_size_query_param = 'per_page' > + > + def get_paginated_response(self, data): > + next_url = self.get_next_link() > + previous_url = self.get_previous_link() > + > + link = '' > + if next_url is not None and previous_url is not None: > + link = '<{next_url}>; rel="next", <{previous_url}>; rel="prev"' > + elif next_url is not None: > + link = '<{next_url}>; rel="next"' > + elif previous_url is not None: > + link = '<{previous_url}>; rel="prev"' > + link = link.format(next_url=next_url, previous_url=previous_url) > + headers = {'Link': link} if link else {} > + return Response(data, headers=headers) Yay! > +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: > + return True > + return obj.is_editable(request.user) > + > + > +class ProjectViewSet(ModelViewSet): > + permission_classes = (PatchworkPermission, ) > + queryset = Project.objects.all() > + serializer_class = ProjectSerializer > + pagination_class = LinkHeaderPagination > + > + > +router = DefaultRouter() > +router.register('projects', ProjectViewSet, 'project') > -- > 2.7.4 > > _______________________________________________ > Patchwork mailing list > Patchwork@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/patchwork _______________________________________________ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork