[PATCH v4 06/10] REST: Add Patches to the API
Finucane, Stephen
stephen.finucane at intel.com
Wed May 25 19:31:26 AEST 2016
On 20 May 14:17, Andy Doan wrote:
> This exposes patches 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)
>
> NOTE: Patch.save was overridden incorrectly and had to be
> fixed to work with DRF.
>
> Signed-off-by: Andy Doan <andy.doan at linaro.org>
So close :)
There seems to be an issue with the 'tags' field on this. I get the
following error message when I try to load a page with patches that
have associated tags.
ImproperlyConfigured at /api/1.0/patches/
Could not resolve URL for hyperlinked relationship using view name
"tag-detail". You may have failed to include the related model in
your API, or incorrectly configured the `lookup_field` attribute on
this field.
Seem like you need to override the 'tags' field to be a ListField?
Stephen
> ---
> patchwork/models.py | 4 +-
> patchwork/rest_serializers.py | 35 ++++++++-
> patchwork/tests/test_rest_api.py | 149 ++++++++++++++++++++++++++++++++++++++-
> patchwork/views/rest_api.py | 18 ++++-
> 4 files changed, 199 insertions(+), 7 deletions(-)
>
> diff --git a/patchwork/models.py b/patchwork/models.py
> index 4d454c7..6324273 100644
> --- a/patchwork/models.py
> +++ b/patchwork/models.py
> @@ -359,14 +359,14 @@ class Patch(Submission):
> for tag in tags:
> self._set_tag(tag, counter[tag])
>
> - def save(self):
> + def save(self, **kwargs):
> if not hasattr(self, 'state') or not self.state:
> self.state = get_default_initial_patch_state()
>
> if self.hash is None and self.diff is not None:
> self.hash = hash_patch(self.diff).hexdigest()
>
> - super(Patch, self).save()
> + super(Patch, self).save(**kwargs)
>
> self.refresh_tag_counts()
>
> diff --git a/patchwork/rest_serializers.py b/patchwork/rest_serializers.py
> index b7e476a..a232a49 100644
> --- a/patchwork/rest_serializers.py
> +++ b/patchwork/rest_serializers.py
> @@ -17,11 +17,14 @@
> # along with Patchwork; if not, write to the Free Software
> # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>
> +import email.parser
> +
> from django.contrib.auth.models import User
>
> -from rest_framework.serializers import HyperlinkedModelSerializer
> +from rest_framework.serializers import (
> + HyperlinkedModelSerializer, ListSerializer, SerializerMethodField)
>
> -from patchwork.models import Person, Project
> +from patchwork.models import Patch, Person, Project
>
>
> class UserSerializer(HyperlinkedModelSerializer):
> @@ -44,3 +47,31 @@ class PersonSerializer(HyperlinkedModelSerializer):
> class ProjectSerializer(HyperlinkedModelSerializer):
> class Meta:
> model = Project
> +
> +
> +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 Meta:
> + model = Patch
> + list_serializer_class = PatchListSerializer
> + read_only_fields = ('project', 'name', 'date', 'submitter', 'diff',
> + 'content', 'hash', 'msgid')
> + state = SerializerMethodField()
> +
> + def get_state(self, obj):
> + return obj.state.name
> +
> + def to_representation(self, instance):
> + data = super(PatchSerializer, self).to_representation(instance)
> + headers = data.get('headers')
> + if headers:
> + data['headers'] = email.parser.Parser().parsestr(headers, True)
> + return data
> diff --git a/patchwork/tests/test_rest_api.py b/patchwork/tests/test_rest_api.py
> index cf72e95..e70bd08 100644
> --- a/patchwork/tests/test_rest_api.py
> +++ b/patchwork/tests/test_rest_api.py
> @@ -25,8 +25,9 @@ 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
> +from patchwork.models import Patch, Project
> +from patchwork.tests.utils import (
> + defaults, create_maintainer, create_user, create_patches, make_msgid)
>
>
> @unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
> @@ -213,3 +214,147 @@ class TestUserAPI(APITestCase):
>
> resp = self.client.post(self.api_url(), {'email': 'foo at f.com'})
> self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
> +
> +
> + at unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
> +class TestPatchAPI(APITestCase):
> + fixtures = ['default_states']
> +
> + @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])
> +
> + def test_list_simple(self):
> + """Validate we can list a patch."""
> + patches = create_patches()
> + resp = self.client.get(self.api_url())
> + self.assertEqual(status.HTTP_200_OK, resp.status_code)
> + self.assertEqual(1, len(resp.data))
> + patch = resp.data[0]
> + self.assertEqual(patches[0].name, patch['name'])
> + self.assertNotIn('content', patch)
> + self.assertNotIn('headers', patch)
> + self.assertNotIn('diff', patch)
> +
> + # test while authenticated
> + user = create_user()
> + self.client.force_authenticate(user=user)
> + resp = self.client.get(self.api_url())
> + self.assertEqual(status.HTTP_200_OK, resp.status_code)
> + self.assertEqual(1, len(resp.data))
> + patch = resp.data[0]
> + self.assertEqual(patches[0].name, patch['name'])
> +
> + def test_detail(self):
> + """Validate we can get a specific project."""
> + patches = create_patches()
> + resp = self.client.get(self.api_url(patches[0].id))
> + self.assertEqual(status.HTTP_200_OK, resp.status_code)
> + self.assertEqual(patches[0].name, resp.data['name'])
> + self.assertIn(TestProjectAPI.api_url(patches[0].project.id),
> + resp.data['project'])
> + self.assertEqual(patches[0].msgid, resp.data['msgid'])
> + self.assertEqual(patches[0].diff, resp.data['diff'])
> + self.assertIn(TestPersonAPI.api_url(patches[0].submitter.id),
> + resp.data['submitter'])
> + self.assertEqual(patches[0].state.name, resp.data['state'])
> +
> + def test_anonymous_writes(self):
> + """Ensure anonymous "write" operations are rejected."""
> + patches = create_patches()
> + patch_url = self.api_url(patches[0].id)
> + resp = self.client.get(patch_url)
> + patch = resp.data
> + patch['msgid'] = 'foo'
> + patch['name'] = 'this should fail'
> +
> + # create
> + resp = self.client.post(self.api_url(), patch)
> + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
> + # update
> + resp = self.client.patch(patch_url, {'name': 'foo'})
> + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
> + # delete
> + resp = self.client.delete(patch_url)
> + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
> +
> + def test_anonymous_create(self):
> + """Ensure anonymous "POST" operations are rejected."""
> + patches = create_patches()
> + patch_url = self.api_url(patches[0].id)
> + resp = self.client.get(patch_url)
> + patch = resp.data
> + patch['msgid'] = 'foo'
> + patch['name'] = 'this should fail'
> +
> + resp = self.client.post(self.api_url(), patch)
> + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
> +
> + def test_anonymous_update(self):
> + """Ensure anonymous "PATCH" operations are rejected."""
> + patches = create_patches()
> + patch_url = self.api_url(patches[0].id)
> + resp = self.client.get(patch_url)
> + patch = resp.data
> + patch['msgid'] = 'foo'
> + patch['name'] = 'this should fail'
> +
> + resp = self.client.patch(patch_url, {'name': 'foo'})
> + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
> +
> + def test_anonymous_delete(self):
> + """Ensure anonymous "DELETE" operations are rejected."""
> + patches = create_patches()
> + patch_url = self.api_url(patches[0].id)
> + resp = self.client.get(patch_url)
> +
> + resp = self.client.delete(patch_url)
> + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
> +
> + def test_create(self):
> + """Ensure creations are rejected."""
> + create_patches()
> + patch = {
> + 'project': defaults.project.id,
> + 'submitter': defaults.patch_author_person.id,
> + 'msgid': make_msgid(),
> + 'name': 'test-create-patch',
> + 'diff': 'patch diff',
> + }
> +
> + user = create_maintainer(defaults.project)
> + user.is_superuser = True
> + user.save()
> + self.client.force_authenticate(user=user)
> + resp = self.client.post(self.api_url(), patch)
> + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
> +
> + def test_update(self):
> + """Ensure updates can be performed maintainers."""
> + patches = create_patches()
> +
> + # A maintainer can update
> + user = create_maintainer(defaults.project)
> + self.client.force_authenticate(user=user)
> + resp = self.client.patch(self.api_url(patches[0].id), {'state': 2})
> + 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(patches[0].id), {'state': 2})
> + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
> +
> + def test_delete(self):
> + """Ensure deletions are rejected."""
> + patches = create_patches()
> +
> + user = create_maintainer(defaults.project)
> + user.is_superuser = True
> + user.save()
> + self.client.force_authenticate(user=user)
> + 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())
> diff --git a/patchwork/views/rest_api.py b/patchwork/views/rest_api.py
> index 2101037..2a152ed 100644
> --- a/patchwork/views/rest_api.py
> +++ b/patchwork/views/rest_api.py
> @@ -20,7 +20,7 @@
> from django.conf import settings
>
> from patchwork.rest_serializers import (
> - ProjectSerializer, PersonSerializer, UserSerializer)
> + PatchSerializer, ProjectSerializer, PersonSerializer, UserSerializer)
>
> from rest_framework import permissions
> from rest_framework.pagination import PageNumberPagination
> @@ -80,6 +80,21 @@ class PatchworkViewSet(ModelViewSet):
> return self.serializer_class.Meta.model.objects.all()
>
>
> +class PatchViewSet(PatchworkViewSet):
> + permission_classes = (PatchworkPermission,)
> + serializer_class = PatchSerializer
> +
> + def get_queryset(self):
> + qs = super(PatchViewSet, self).get_queryset(
> + ).prefetch_related(
> + 'check_set', 'tags'
> + ).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
> +
> +
> class PeopleViewSet(PatchworkViewSet):
> permission_classes = (AuthenticatedReadOnly, )
> serializer_class = PersonSerializer
> @@ -100,6 +115,7 @@ class ProjectViewSet(PatchworkViewSet):
>
>
> router = DefaultRouter()
> +router.register('patches', PatchViewSet, 'patch')
> router.register('people', PeopleViewSet, 'person')
> router.register('projects', ProjectViewSet, 'project')
> router.register('user', UserViewSet, 'user')
> --
> 2.7.4
>
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
More information about the Patchwork
mailing list