From patchwork Fri May 20 19:17:09 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andy Doan X-Patchwork-Id: 624608 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3rBHmd1rymz9t5g for ; Sat, 21 May 2016 05:19:13 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b=kUKHJRla; dkim-atps=neutral Received: from ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 3rBHmc5JxSzDqGp for ; Sat, 21 May 2016 05:19:12 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b=kUKHJRla; dkim-atps=neutral X-Original-To: patchwork@lists.ozlabs.org Delivered-To: patchwork@lists.ozlabs.org Received: from mail-oi0-x235.google.com (mail-oi0-x235.google.com [IPv6:2607:f8b0:4003:c06::235]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3rBHkb6FClzDqDK for ; Sat, 21 May 2016 05:17:27 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b=kUKHJRla; dkim-atps=neutral Received: by mail-oi0-x235.google.com with SMTP id b65so49877668oia.1 for ; Fri, 20 May 2016 12:17:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=CGEjVfuDxWEqIu3jBxQXWwezmkb9Cv9VNyrNE6zMC1k=; b=kUKHJRlaccpoe2cB9tWdkCI6Ti2OTuIYGzzgRIOy4f8B4rP4PrkMzGbMZ+qmmAILkw 8QgMFj5azyBVqogZ8Tb7UGHSn9+mSJFdoLYqCBZvqQw6zizV/OkGLIqctQ/xAgZyaXeU O4KUn5M8nqapX9wJxd8/TYEd6PLQiRjfcX1z4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=CGEjVfuDxWEqIu3jBxQXWwezmkb9Cv9VNyrNE6zMC1k=; b=EMXTSG866bTJsxvBzWXKT4NlH/Jcc+s9W7wFd+b/E3Ge00EAW3OcRiP4rwH5AIlpfQ luxjE3ppEcwHzSOSp6O8vCCFvusrY1DpWBHmZolyAeC6Pf8lP6/15YHMH6vTZhMX1ttz XBbKZ4Iy2YnuOxvCJO5yznHQ72kEc6ekXydpB6HPHlau85LjGhgRTbsxKW2vUFb9flpz DnhvFmt2ioeSH8VDXViGzZxNEDQZwNJ5B5hrTGQTruIVdF6ZpNs/mWpzlISpwzgNzg6Q Kafp+a8jB9XPwh0SYRBMZGGxivxO3YX5N/YSsajEHArxPpigC5E/beyTJF74Fi1rgJKm SQrg== X-Gm-Message-State: AOPr4FWWRqisG6ICdepydQ3YRQRJw8MNQERo8BL0LMXToCXgbRuAkGbUVdShTdnbcYMTiPU+ X-Received: by 10.157.20.202 with SMTP id r10mr3184869otr.59.1463771845405; Fri, 20 May 2016 12:17:25 -0700 (PDT) Received: from doanac-xps.local (cpe-70-117-126-139.austin.res.rr.com. [70.117.126.139]) by smtp.gmail.com with ESMTPSA id l65sm5731844otc.28.2016.05.20.12.17.24 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 20 May 2016 12:17:24 -0700 (PDT) From: Andy Doan To: patchwork@lists.ozlabs.org Subject: [PATCH v4 07/10] REST: Add Patch Checks to the API Date: Fri, 20 May 2016 14:17:09 -0500 Message-Id: <1463771832-23025-8-git-send-email-andy.doan@linaro.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1463771832-23025-1-git-send-email-andy.doan@linaro.org> References: <1463771832-23025-1-git-send-email-andy.doan@linaro.org> X-BeenThere: patchwork@lists.ozlabs.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Patchwork development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Patchwork" 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 --- 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 + 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()) + + +@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