[PATCH v4 07/10] REST: Add Patch Checks to the API
Finucane, Stephen
stephen.finucane at intel.com
Wed May 25 22:49:26 AEST 2016
On 20 May 14:17, Andy Doan wrote:
> This exports patch checks via the REST API.
>
> The drf-nested-routers package is used to handle the fact Checks are
> nested under a Patch.
>
> Security Constraints:
> * Anyone (logged in or not) can read all objects.
> * No one can update/delete objects.
> * Project maintainers and patch owners may create objects.
>
> Signed-off-by: Andy Doan <andy.doan at linaro.org>
One request, then we're done.
> ---
> patchwork/rest_serializers.py | 35 ++++++++++++++-
> patchwork/tests/test_rest_api.py | 92 +++++++++++++++++++++++++++++++++++++++-
> patchwork/urls.py | 6 +--
> patchwork/views/rest_api.py | 40 ++++++++++++++++-
> requirements-test.txt | 1 +
> 5 files changed, 166 insertions(+), 8 deletions(-)
>
> diff --git a/patchwork/rest_serializers.py b/patchwork/rest_serializers.py
> index a232a49..94ac469 100644
> --- a/patchwork/rest_serializers.py
> +++ b/patchwork/rest_serializers.py
> @@ -22,9 +22,11 @@ import email.parser
> from django.contrib.auth.models import User
>
> from rest_framework.serializers import (
> - HyperlinkedModelSerializer, ListSerializer, SerializerMethodField)
> + CurrentUserDefault, HiddenField, HyperlinkedModelSerializer,
> + ListSerializer, ModelSerializer, PrimaryKeyRelatedField,
> + SerializerMethodField)
>
> -from patchwork.models import Patch, Person, Project
> +from patchwork.models import Check, Patch, Person, Project
>
>
> class UserSerializer(HyperlinkedModelSerializer):
> @@ -71,7 +73,36 @@ class PatchSerializer(HyperlinkedModelSerializer):
>
> def to_representation(self, instance):
> data = super(PatchSerializer, self).to_representation(instance)
> + data['checks'] = data['url'] + 'checks/'
> headers = data.get('headers')
> if headers:
> data['headers'] = email.parser.Parser().parsestr(headers, True)
> return data
> +
> +
> +class CurrentPatchDefault(object):
> + def set_context(self, serializer_field):
> + self.patch = serializer_field.context['request'].patch
> +
> + def __call__(self):
> + return self.patch
> +
> +
> +class ChecksSerializer(ModelSerializer):
> + class Meta:
> + model = Check
> + user = PrimaryKeyRelatedField(read_only=True, default=CurrentUserDefault())
> + patch = HiddenField(default=CurrentPatchDefault())
> +
> + def run_validation(self, data):
> + for val, label in Check.STATE_CHOICES:
> + if label == data['state']:
> + data['state'] = val
> + break
> + return super(ChecksSerializer, self).run_validation(data)
> +
> + def to_representation(self, instance):
> + data = super(ChecksSerializer, self).to_representation(instance)
> + data['state'] = instance.get_state_display()
> + data['user'] = instance.user.username
nit: s/user/username/. Also, should we add a 'user_url' link, as
suggested for 'people' endpoint?
> + return data
> diff --git a/patchwork/tests/test_rest_api.py b/patchwork/tests/test_rest_api.py
> index e70bd08..2be3ed5 100644
> --- a/patchwork/tests/test_rest_api.py
> +++ b/patchwork/tests/test_rest_api.py
> @@ -25,7 +25,7 @@ from django.core.urlresolvers import reverse
> from rest_framework import status
> from rest_framework.test import APITestCase
>
> -from patchwork.models import Patch, Project
> +from patchwork.models import Check, Patch, Project
> from patchwork.tests.utils import (
> defaults, create_maintainer, create_user, create_patches, make_msgid)
>
> @@ -358,3 +358,93 @@ class TestPatchAPI(APITestCase):
> resp = self.client.delete(self.api_url(patches[0].id))
> self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
> self.assertEqual(1, Patch.objects.all().count())
> +
> +
> + at unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
> +class TestCheckAPI(APITestCase):
> + fixtures = ['default_states']
> +
> + def setUp(self):
> + super(TestCheckAPI, self).setUp()
> + self.patch = create_patches()[0]
> + self.urlbase = reverse('api_1.0:patch-detail', args=[self.patch.id])
> + self.urlbase += 'checks/'
> + defaults.project.save()
> + self.user = create_maintainer(defaults.project)
> +
> + def create_check(self):
> + return Check.objects.create(patch=self.patch, user=self.user,
> + state=Check.STATE_WARNING, target_url='t',
> + description='d', context='c')
> +
> + def test_list_simple(self):
> + """Validate we can list checks on a patch."""
> + resp = self.client.get(self.urlbase)
> + self.assertEqual(status.HTTP_200_OK, resp.status_code)
> + self.assertEqual(0, len(resp.data))
> +
> + c = self.create_check()
> + resp = self.client.get(self.urlbase)
> + self.assertEqual(status.HTTP_200_OK, resp.status_code)
> + self.assertEqual(1, len(resp.data))
> + check = resp.data[0]
> + self.assertEqual(c.get_state_display(), check['state'])
> + self.assertEqual(c.target_url, check['target_url'])
> + self.assertEqual(c.context, check['context'])
> + self.assertEqual(c.description, check['description'])
> +
> + def test_detail(self):
> + """Validate we can get a specific check."""
> + c = self.create_check()
> + resp = self.client.get(self.urlbase + str(c.id) + '/')
> + self.assertEqual(status.HTTP_200_OK, resp.status_code)
> + self.assertEqual(c.target_url, resp.data['target_url'])
> +
> + def test_update_delete(self):
> + """Ensure updates and deletes aren't allowed"""
> + c = self.create_check()
> +
> + self.user.is_superuser = True
> + self.user.save()
> + self.client.force_authenticate(user=self.user)
> +
> + # update
> + resp = self.client.patch(
> + self.urlbase + str(c.id) + '/', {'target_url': 'fail'})
> + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
> + # delete
> + resp = self.client.delete(self.urlbase + str(c.id) + '/')
> + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
> +
> + def test_create(self):
> + """Ensure creations can be performed by user of patch."""
> + check = {
> + 'state': 'success',
> + 'target_url': 'http://t.co',
> + 'description': 'description',
> + 'context': 'context',
> + }
> +
> + self.client.force_authenticate(user=self.user)
> + resp = self.client.post(self.urlbase, 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)
> + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
> +
> + def test_create_invalid(self):
> + """Ensure we handle invalid check states"""
> + check = {
> + 'state': 'this-is-not-a-valid-state',
> + 'target_url': 'http://t.co',
> + 'description': 'description',
> + 'context': 'context',
> + }
> +
> + self.client.force_authenticate(user=self.user)
> + resp = self.client.post(self.urlbase, 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 f6763d4..c97b422 100644
> --- a/patchwork/urls.py
> +++ b/patchwork/urls.py
> @@ -142,10 +142,10 @@ if settings.ENABLE_REST_API:
> if 'rest_framework' not in settings.INSTALLED_APPS:
> raise RuntimeError(
> 'djangorestframework must be installed to enable the REST API.')
> - import patchwork.views.rest_api
> + from patchwork.views.rest_api import router, patches_router
> urlpatterns += [
> - url(r'^api/1.0/', include(
> - patchwork.views.rest_api.router.urls, namespace='api_1.0')),
> + 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')),
> ]
>
> # redirect from old urls
> diff --git a/patchwork/views/rest_api.py b/patchwork/views/rest_api.py
> index 2a152ed..34c5161 100644
> --- a/patchwork/views/rest_api.py
> +++ b/patchwork/views/rest_api.py
> @@ -18,15 +18,18 @@
> # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>
> from django.conf import settings
> -
> +from patchwork.models import Patch
> from patchwork.rest_serializers import (
> - PatchSerializer, ProjectSerializer, PersonSerializer, UserSerializer)
> + ChecksSerializer, PatchSerializer, ProjectSerializer, PersonSerializer,
> + UserSerializer)
>
> from rest_framework import permissions
> +from rest_framework.exceptions import PermissionDenied
> from rest_framework.pagination import PageNumberPagination
> from rest_framework.response import Response
> from rest_framework.routers import DefaultRouter
> from rest_framework.viewsets import ModelViewSet
> +from rest_framework_nested.routers import NestedSimpleRouter
>
>
> class LinkHeaderPagination(PageNumberPagination):
> @@ -114,8 +117,41 @@ class ProjectViewSet(PatchworkViewSet):
> serializer_class = ProjectSerializer
>
>
> +class ChecksViewSet(PatchworkViewSet):
> + serializer_class = ChecksSerializer
> +
> + def not_allowed(self, request, **kwargs):
> + raise PermissionDenied()
> +
> + update = not_allowed
> + partial_update = not_allowed
> + destroy = not_allowed
> +
> + def create(self, request, patch_pk):
> + p = Patch.objects.get(id=patch_pk)
> + if not p.is_editable(request.user):
> + raise PermissionDenied()
> + request.patch = p
> + return super(ChecksViewSet, 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)
> +
> + serializer = self.get_serializer(queryset, many=True)
> + return Response(serializer.data)
> +
> +
> router = DefaultRouter()
> router.register('patches', PatchViewSet, 'patch')
> router.register('people', PeopleViewSet, 'person')
> router.register('projects', ProjectViewSet, 'project')
> router.register('user', UserViewSet, 'user')
> +
> +patches_router = NestedSimpleRouter(router, r'patches', lookup='patch')
> +patches_router.register(r'checks', ChecksViewSet, base_name='patch-checks')
> diff --git a/requirements-test.txt b/requirements-test.txt
> index b5f976c..cfc242f 100644
> --- a/requirements-test.txt
> +++ b/requirements-test.txt
> @@ -3,3 +3,4 @@ django-debug-toolbar==1.4
> python-dateutil>2.0,<3.0
> selenium>2.0,<3.0
> djangorestframework>=3.3,<3.4
> +drf-nested-routers>=0.11.1,<0.12
s/0.11.1/0.11/?
> --
> 2.7.4
>
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
More information about the Patchwork
mailing list