From: Andy Doan Date: Thu, 16 Jun 2016 21:13:21 +0000 (-0500) Subject: REST: Add Patches to the API X-Git-Tag: v2.0.0-rc1~339 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ca2781a8b464e6014415d7375762379066bde1cb;p=thirdparty%2Fpatchwork.git REST: Add Patches to the API 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 Signed-off-by: Stephen Finucane --- diff --git a/patchwork/models.py b/patchwork/models.py index 4d454c7b..63242731 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 103280a4..12a71991 100644 --- a/patchwork/rest_serializers.py +++ b/patchwork/rest_serializers.py @@ -17,12 +17,15 @@ # 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.relations import HyperlinkedRelatedField -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 URLSerializer(HyperlinkedModelSerializer): @@ -60,3 +63,36 @@ class ProjectSerializer(HyperlinkedModelSerializer): data['list_email'] = data.pop('listemail') data['list_id'] = data.pop('listid') return data + + +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(URLSerializer): + class Meta: + model = Patch + list_serializer_class = PatchListSerializer + read_only_fields = ('project', 'name', 'date', 'submitter', 'diff', + 'content', 'hash', 'msgid') + # there's no need to expose an entire "tags" endpoint, so we custom + # render this field + exclude = ('tags',) + 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 is not None: + data['headers'] = email.parser.Parser().parsestr(headers, True) + data['tags'] = [{'name': x.tag.name, 'count': x.count} + for x in instance.patchtag_set.all()] + return data diff --git a/patchwork/tests/test_rest_api.py b/patchwork/tests/test_rest_api.py index 7e48fd8e..b2af52c3 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') @@ -212,3 +213,141 @@ class TestUserAPI(APITestCase): resp = self.client.post(self.api_url(), {'email': 'foo@f.com'}) self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) + + +@unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API') +class TestPatchAPI(APITestCase): + fixtures = ['default_states', 'default_tags'] + + def setUp(self): + self.patches = create_patches() + + @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.""" + 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(self.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(self.patches[0].name, patch['name']) + + def test_detail(self): + """Validate we can get a specific project.""" + resp = self.client.get(self.api_url(self.patches[0].id)) + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual(self.patches[0].name, resp.data['name']) + self.assertIn(TestProjectAPI.api_url(self.patches[0].project.id), + resp.data['project_url']) + self.assertEqual(self.patches[0].msgid, resp.data['msgid']) + self.assertEqual(self.patches[0].diff, resp.data['diff']) + self.assertIn(TestPersonAPI.api_url(self.patches[0].submitter.id), + resp.data['submitter_url']) + self.assertEqual(self.patches[0].state.name, resp.data['state']) + + def test_detail_tags(self): + # defaults.project is remembered between TestCases and .save() is + # called which just updates the object. This leaves the potential for + # the @cached_property project.tags to be invalid, so we have to + # invalidate this cached value before doing tag operations: + del defaults.project.tags + + self.patches[0].content = 'Reviewed-by: Test User \n' + self.patches[0].save() + resp = self.client.get(self.api_url(self.patches[0].id)) + tags = resp.data['tags'] + self.assertEqual(1, len(tags)) + self.assertEqual(1, tags[0]['count']) + self.assertEqual('Reviewed-by', tags[0]['name']) + + def test_anonymous_create(self): + """Ensure anonymous "POST" operations are rejected.""" + patch = { + 'project': defaults.project.id, + 'submitter': defaults.patch_author_person.id, + 'msgid': make_msgid(), + 'name': 'test-create-patch', + 'diff': 'patch diff', + } + + 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.""" + patch_url = self.api_url(self.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.""" + patch_url = self.api_url(self.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.""" + 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.""" + # A maintainer can update + user = create_maintainer(defaults.project) + self.client.force_authenticate(user=user) + resp = self.client.patch(self.api_url(self.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(self.patches[0].id), + {'state': 2}) + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) + + def test_delete(self): + """Ensure deletions are rejected.""" + user = create_maintainer(defaults.project) + user.is_superuser = True + user.save() + self.client.force_authenticate(user=user) + resp = self.client.delete(self.api_url(self.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 6e7f94e1..2ab0f97a 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 ( - PersonSerializer, ProjectSerializer, UserSerializer) + PatchSerializer, PersonSerializer, ProjectSerializer, UserSerializer) from rest_framework import permissions from rest_framework.pagination import PageNumberPagination @@ -99,7 +99,23 @@ class ProjectViewSet(PatchworkViewSet): serializer_class = ProjectSerializer +class PatchViewSet(PatchworkViewSet): + permission_classes = (PatchworkPermission,) + serializer_class = PatchSerializer + + def get_queryset(self): + qs = super(PatchViewSet, self).get_queryset( + ).prefetch_related( + 'check_set', 'patchtag_set' + ).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 + + router = DefaultRouter() +router.register('patches', PatchViewSet, 'patch') router.register('people', PeopleViewSet, 'person') router.register('projects', ProjectViewSet, 'project') router.register('users', UserViewSet, 'user')