[PATCH v3 03/10] REST: Add Projects to the API
Finucane, Stephen
stephen.finucane at intel.com
Thu May 19 17:03:10 AEST 2016
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.doan at linaro.org>
> Inspired-by: Damien Lespiau <damien.lespiau at intel.com>
Reviewed-by: Stephen Finucane <stephen.finucane at 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
> +
> +
> + at 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 at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
More information about the Patchwork
mailing list