On 18 May 22:30, 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 <[email protected]>
Generally OK with this, but some questions before sign off. Stephen > --- > patchwork/rest_serializers.py | 32 +++++++++++++++-- > patchwork/tests/test_rest_api.py | 77 > +++++++++++++++++++++++++++++++++++++++- > patchwork/urls.py | 6 ++-- > patchwork/views/rest_api.py | 39 ++++++++++++++++++-- > requirements-test.txt | 1 + > 5 files changed, 147 insertions(+), 8 deletions(-) > > diff --git a/patchwork/rest_serializers.py b/patchwork/rest_serializers.py > index 9558d45..5f16954 100644 > --- a/patchwork/rest_serializers.py > +++ b/patchwork/rest_serializers.py > @@ -21,10 +21,11 @@ import email.parser > > from django.core.urlresolvers import reverse > > -from patchwork.models import Patch, Person, Project > +from patchwork.models import Check, Patch, Person, Project > > from rest_framework.serializers import ( > - ListSerializer, ModelSerializer) > + CurrentUserDefault, HiddenField, ListSerializer, ModelSerializer, > + PrimaryKeyRelatedField) > > > class PersonSerializer(ModelSerializer): > @@ -71,3 +72,30 @@ class PatchSerializer(ModelSerializer): > 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() > + return data > diff --git a/patchwork/tests/test_rest_api.py > b/patchwork/tests/test_rest_api.py > index 7107dfa..7e2545d 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) > > @@ -266,3 +266,78 @@ 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()) > + > + > [email protected](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 = '/api/1.0/patches/%d/checks/' % self.patch.id Can we use reverse here? > + 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_get(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) What happens if you pass in an invalid state? Should we test? > 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 1e03c68..b7b0c4b 100644 > --- a/patchwork/views/rest_api.py > +++ b/patchwork/views/rest_api.py > @@ -18,15 +18,17 @@ > # 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) > + ChecksSerializer, PatchSerializer, ProjectSerializer, PersonSerializer) > > 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): > @@ -107,7 +109,40 @@ 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') > + > +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 > -- > 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
