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 <andy.doan@linaro.org>
Signed-off-by: Stephen Finucane <stephen.finucane@intel.com>

diff --git a/patchwork/models.py b/patchwork/models.py
index 4d454c7bc1e20d9065201ed3ef71266c7c5f44a3..632427317785fb13f4147e7354cc9b16558d2eb7 100644 (file)
--- 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 103280a4b641512d0fa884819aca0178a047f336..12a71991023153fe13106ef299a53b32a5bd3e3d 100644 (file)
--- 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 7e48fd8ea6aafa7bb385941f89ed59a4d0c55aeb..b2af52c38828488f29bc2ca12c07ec3e62d04e66 100644 (file)
--- 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 <test@example.com>\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 6e7f94e1c2bf88181147ff83a4af1f95add8543d..2ab0f97a51d001486834afbe28d6c83e87634c66 100644 (file)
--- 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')