Message ID | 20190908223147.25789-3-stephen@that.guru |
---|---|
State | Accepted |
Headers | show |
Series | Add writeable 'bundles' API | expand |
Stephen Finucane <stephen@that.guru> writes: > Allow users to create a new bundle, change the name, public flag and > patches of an existing bundle, and delete an existing bundle. > > Some small nits with existing tests are resolved. > > Signed-off-by: Stephen Finucane <stephen@that.guru> > --- > docs/api/schemas/latest/patchwork.yaml | 170 +++++++++++++++- > docs/api/schemas/patchwork.j2 | 181 +++++++++++++++++- > docs/api/schemas/v1.0/patchwork.yaml | 5 +- > docs/api/schemas/v1.1/patchwork.yaml | 5 +- > docs/api/schemas/v1.2/patchwork.yaml | 170 +++++++++++++++- > patchwork/api/bundle.py | 84 +++++++- > patchwork/models.py | 11 ++ > patchwork/tests/api/test_bundle.py | 118 +++++++++++- > patchwork/tests/api/utils.py | 16 +- > ...pdate-bundle-via-api-2946d8c4e730d545.yaml | 4 + > 10 files changed, 737 insertions(+), 27 deletions(-) > create mode 100644 releasenotes/notes/update-bundle-via-api-2946d8c4e730d545.yaml > > diff --git a/docs/api/schemas/latest/patchwork.yaml b/docs/api/schemas/latest/patchwork.yaml > index 45a61180..e6c6bb4a 100644 > --- a/docs/api/schemas/latest/patchwork.yaml > +++ b/docs/api/schemas/latest/patchwork.yaml > @@ -1,5 +1,6 @@ > # DO NOT EDIT THIS FILE. It is generated from a template. Changes should be > -# proposed against the template. > +# proposed against the template and updated files generated using the > +# 'generate_schema.py' tool > --- > openapi: '3.0.0' > info: > @@ -72,6 +73,35 @@ paths: > $ref: '#/components/schemas/Bundle' > tags: > - bundles > + post: > + description: Create a bundle. > + operationId: bundles_create > + security: > + - basicAuth: [] > + - apiKeyAuth: [] > + requestBody: > + $ref: '#/components/requestBodies/Bundle' > + responses: > + '201': > + description: '' > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Bundle' > + '400': > + description: Invalid Request > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/ErrorBundleCreateUpdate' > + '403': > + description: Forbidden > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + tags: > + - bundles > /api/bundles/{id}/: > get: > description: Show a bundle. > @@ -99,6 +129,92 @@ paths: > $ref: '#/components/schemas/Error' > tags: > - bundles > + patch: > + description: Update a bundle (partial). > + operationId: bundles_partial_update > + security: > + - basicAuth: [] > + - apiKeyAuth: [] > + parameters: > + - in: path > + name: id > + description: A unique integer value identifying this bundle. > + required: true > + schema: > + title: ID > + type: integer > + requestBody: > + $ref: '#/components/requestBodies/Bundle' > + responses: > + '200': > + description: '' > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Bundle' > + '400': > + description: Bad request > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/ErrorBundleCreateUpdate' > + '403': > + description: Forbidden > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '404': > + description: Not found > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + tags: > + - bundles > + put: > + description: Update a bundle. > + operationId: bundles_update > + security: > + - basicAuth: [] > + - apiKeyAuth: [] > + parameters: > + - in: path > + name: id > + description: A unique integer value identifying this bundle. > + required: true > + schema: > + title: ID > + type: integer > + requestBody: > + $ref: '#/components/requestBodies/Bundle' > + responses: > + '200': > + description: '' > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Bundle' > + '400': > + description: Bad request > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/ErrorBundleCreateUpdate' > + '403': > + description: Forbidden > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '404': > + description: Not found > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + tags: > + - bundles > /api/covers/: > get: > description: List cover letters. > @@ -1131,6 +1247,18 @@ components: > schema: > type: string > requestBodies: > + Bundle: > + required: true > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/BundleCreateUpdate' > + multipart/form-data: > + schema: > + $ref: '#/components/schemas/BundleCreateUpdate' > + application/x-www-form-urlencoded: > + schema: > + $ref: '#/components/schemas/BundleCreateUpdate' > Check: > required: true > content: > @@ -1251,10 +1379,10 @@ components: > allOf: > - $ref: '#/components/schemas/UserEmbedded' > patches: > + title: Patches > type: array > items: > $ref: '#/components/schemas/PatchEmbedded' > - readOnly: true > uniqueItems: true > public: > title: Public > @@ -1264,6 +1392,25 @@ components: > type: string > format: uri > readOnly: true > + BundleCreateUpdate: > + type: object > + required: > + - name > + properties: > + name: > + title: Name > + type: string > + minLength: 1 > + maxLength: 50 > + patches: > + title: Patches > + type: array > + items: > + type: integer > + uniqueItems: true > + public: > + title: Public > + type: boolean > Check: > type: object > properties: > @@ -1961,6 +2108,7 @@ components: > cover_letter: > $ref: '#/components/schemas/CoverLetterEmbedded' > patches: > + title: Patches > type: array > items: > $ref: '#/components/schemas/PatchEmbedded' > @@ -2307,6 +2455,24 @@ components: > title: Detail > type: string > readOnly: true > + ErrorBundleCreateUpdate: > + type: object > + properties: > + name: > + title: Name > + type: string > + minLength: 1 > + maxLength: 50 > + patches: > + title: Patches > + type: array > + items: > + $ref: '#/components/schemas/PatchEmbedded' > + readOnly: true > + uniqueItems: true > + public: > + title: Public > + type: boolean > ErrorCheckCreate: > type: object > properties: > diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2 > index 843981f8..5f7510da 100644 > --- a/docs/api/schemas/patchwork.j2 > +++ b/docs/api/schemas/patchwork.j2 > @@ -1,6 +1,7 @@ > {# You can obviously ignore the below when editing this template #} > # DO NOT EDIT THIS FILE. It is generated from a template. Changes should be > -# proposed against the template. > +# proposed against the template and updated files generated using the > +# 'generate_schema.py' tool > --- > openapi: '3.0.0' > info: > @@ -73,6 +74,37 @@ paths: > $ref: '#/components/schemas/Bundle' > tags: > - bundles > +{% if version >= (1, 2) %} > + post: > + description: Create a bundle. > + operationId: bundles_create > + security: > + - basicAuth: [] > + - apiKeyAuth: [] > + requestBody: > + $ref: '#/components/requestBodies/Bundle' > + responses: > + '201': > + description: '' > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Bundle' > + '400': > + description: Invalid Request > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/ErrorBundleCreateUpdate' > + '403': > + description: Forbidden > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + tags: > + - bundles > +{% endif %} > /api/{{ version_url }}bundles/{id}/: > get: > description: Show a bundle. > @@ -100,6 +132,94 @@ paths: > $ref: '#/components/schemas/Error' > tags: > - bundles > +{% if version >= (1, 2) %} > + patch: > + description: Update a bundle (partial). > + operationId: bundles_partial_update > + security: > + - basicAuth: [] > + - apiKeyAuth: [] > + parameters: > + - in: path > + name: id > + description: A unique integer value identifying this bundle. > + required: true > + schema: > + title: ID > + type: integer > + requestBody: > + $ref: '#/components/requestBodies/Bundle' > + responses: > + '200': > + description: '' > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Bundle' > + '400': > + description: Bad request > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/ErrorBundleCreateUpdate' > + '403': > + description: Forbidden > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '404': > + description: Not found > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + tags: > + - bundles > + put: > + description: Update a bundle. > + operationId: bundles_update > + security: > + - basicAuth: [] > + - apiKeyAuth: [] > + parameters: > + - in: path > + name: id > + description: A unique integer value identifying this bundle. > + required: true > + schema: > + title: ID > + type: integer > + requestBody: > + $ref: '#/components/requestBodies/Bundle' > + responses: > + '200': > + description: '' > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Bundle' > + '400': > + description: Bad request > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/ErrorBundleCreateUpdate' > + '403': > + description: Forbidden > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '404': > + description: Not found > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + tags: > + - bundles > +{% endif %} > /api/{{ version_url }}covers/: > get: > description: List cover letters. > @@ -1132,6 +1252,20 @@ components: > schema: > type: string > requestBodies: > +{% if version >= (1, 2) %} > + Bundle: > + required: true > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/BundleCreateUpdate' > + multipart/form-data: > + schema: > + $ref: '#/components/schemas/BundleCreateUpdate' > + application/x-www-form-urlencoded: > + schema: > + $ref: '#/components/schemas/BundleCreateUpdate' > +{% endif %} > Check: > required: true > content: > @@ -1254,10 +1388,13 @@ components: > allOf: > - $ref: '#/components/schemas/UserEmbedded' > patches: > + title: Patches > type: array > items: > $ref: '#/components/schemas/PatchEmbedded' > +{% if version < (1, 2) %} > readOnly: true > +{% endif %} > uniqueItems: true > public: > title: Public > @@ -1267,6 +1404,27 @@ components: > type: string > format: uri > readOnly: true > +{% if version >= (1, 2) %} > + BundleCreateUpdate: > + type: object > + required: > + - name > + properties: > + name: > + title: Name > + type: string > + minLength: 1 > + maxLength: 50 > + patches: > + title: Patches > + type: array > + items: > + type: integer > + uniqueItems: true > + public: > + title: Public > + type: boolean > +{% endif %} > Check: > type: object > properties: > @@ -1988,6 +2146,7 @@ components: > cover_letter: > $ref: '#/components/schemas/CoverLetterEmbedded' > patches: > + title: Patches > type: array > items: > $ref: '#/components/schemas/PatchEmbedded' > @@ -2346,6 +2505,26 @@ components: > title: Detail > type: string > readOnly: true > +{% if version >= (1, 2) %} > + ErrorBundleCreateUpdate: > + type: object > + properties: > + name: > + title: Name > + type: string > + minLength: 1 > + maxLength: 50 > + patches: > + title: Patches > + type: array > + items: > + $ref: '#/components/schemas/PatchEmbedded' > + readOnly: true > + uniqueItems: true > + public: > + title: Public > + type: boolean > +{% endif %} > ErrorCheckCreate: > type: object > properties: > diff --git a/docs/api/schemas/v1.0/patchwork.yaml b/docs/api/schemas/v1.0/patchwork.yaml > index 02f3a156..cafef5ee 100644 > --- a/docs/api/schemas/v1.0/patchwork.yaml > +++ b/docs/api/schemas/v1.0/patchwork.yaml > @@ -1,5 +1,6 @@ > # DO NOT EDIT THIS FILE. It is generated from a template. Changes should be > -# proposed against the template. > +# proposed against the template and updated files generated using the > +# 'generate_schema.py' tool > --- > openapi: '3.0.0' > info: > @@ -1246,6 +1247,7 @@ components: > allOf: > - $ref: '#/components/schemas/UserEmbedded' > patches: > + title: Patches > type: array > items: > $ref: '#/components/schemas/PatchEmbedded' > @@ -1877,6 +1879,7 @@ components: > cover_letter: > $ref: '#/components/schemas/CoverLetterEmbedded' > patches: > + title: Patches > type: array > items: > $ref: '#/components/schemas/PatchEmbedded' > diff --git a/docs/api/schemas/v1.1/patchwork.yaml b/docs/api/schemas/v1.1/patchwork.yaml > index 0c086eda..9d45d058 100644 > --- a/docs/api/schemas/v1.1/patchwork.yaml > +++ b/docs/api/schemas/v1.1/patchwork.yaml > @@ -1,5 +1,6 @@ > # DO NOT EDIT THIS FILE. It is generated from a template. Changes should be > -# proposed against the template. > +# proposed against the template and updated files generated using the > +# 'generate_schema.py' tool > --- > openapi: '3.0.0' > info: > @@ -1251,6 +1252,7 @@ components: > allOf: > - $ref: '#/components/schemas/UserEmbedded' > patches: > + title: Patches > type: array > items: > $ref: '#/components/schemas/PatchEmbedded' > @@ -1928,6 +1930,7 @@ components: > cover_letter: > $ref: '#/components/schemas/CoverLetterEmbedded' > patches: > + title: Patches > type: array > items: > $ref: '#/components/schemas/PatchEmbedded' > diff --git a/docs/api/schemas/v1.2/patchwork.yaml b/docs/api/schemas/v1.2/patchwork.yaml > index 3a96aa3a..5c03bf1e 100644 > --- a/docs/api/schemas/v1.2/patchwork.yaml > +++ b/docs/api/schemas/v1.2/patchwork.yaml > @@ -1,5 +1,6 @@ > # DO NOT EDIT THIS FILE. It is generated from a template. Changes should be > -# proposed against the template. > +# proposed against the template and updated files generated using the > +# 'generate_schema.py' tool > --- > openapi: '3.0.0' > info: > @@ -72,6 +73,35 @@ paths: > $ref: '#/components/schemas/Bundle' > tags: > - bundles > + post: > + description: Create a bundle. > + operationId: bundles_create > + security: > + - basicAuth: [] > + - apiKeyAuth: [] > + requestBody: > + $ref: '#/components/requestBodies/Bundle' > + responses: > + '201': > + description: '' > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Bundle' > + '400': > + description: Invalid Request > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/ErrorBundleCreateUpdate' > + '403': > + description: Forbidden > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + tags: > + - bundles > /api/1.2/bundles/{id}/: > get: > description: Show a bundle. > @@ -99,6 +129,92 @@ paths: > $ref: '#/components/schemas/Error' > tags: > - bundles > + patch: > + description: Update a bundle (partial). > + operationId: bundles_partial_update > + security: > + - basicAuth: [] > + - apiKeyAuth: [] > + parameters: > + - in: path > + name: id > + description: A unique integer value identifying this bundle. > + required: true > + schema: > + title: ID > + type: integer > + requestBody: > + $ref: '#/components/requestBodies/Bundle' > + responses: > + '200': > + description: '' > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Bundle' > + '400': > + description: Bad request > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/ErrorBundleCreateUpdate' > + '403': > + description: Forbidden > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '404': > + description: Not found > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + tags: > + - bundles > + put: > + description: Update a bundle. > + operationId: bundles_update > + security: > + - basicAuth: [] > + - apiKeyAuth: [] > + parameters: > + - in: path > + name: id > + description: A unique integer value identifying this bundle. > + required: true > + schema: > + title: ID > + type: integer > + requestBody: > + $ref: '#/components/requestBodies/Bundle' > + responses: > + '200': > + description: '' > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Bundle' > + '400': > + description: Bad request > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/ErrorBundleCreateUpdate' > + '403': > + description: Forbidden > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '404': > + description: Not found > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + tags: > + - bundles > /api/1.2/covers/: > get: > description: List cover letters. > @@ -1131,6 +1247,18 @@ components: > schema: > type: string > requestBodies: > + Bundle: > + required: true > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/BundleCreateUpdate' > + multipart/form-data: > + schema: > + $ref: '#/components/schemas/BundleCreateUpdate' > + application/x-www-form-urlencoded: > + schema: > + $ref: '#/components/schemas/BundleCreateUpdate' > Check: > required: true > content: > @@ -1251,10 +1379,10 @@ components: > allOf: > - $ref: '#/components/schemas/UserEmbedded' > patches: > + title: Patches > type: array > items: > $ref: '#/components/schemas/PatchEmbedded' > - readOnly: true > uniqueItems: true > public: > title: Public > @@ -1264,6 +1392,25 @@ components: > type: string > format: uri > readOnly: true > + BundleCreateUpdate: > + type: object > + required: > + - name > + properties: > + name: > + title: Name > + type: string > + minLength: 1 > + maxLength: 50 > + patches: > + title: Patches > + type: array > + items: > + type: integer > + uniqueItems: true > + public: > + title: Public > + type: boolean > Check: > type: object > properties: > @@ -1961,6 +2108,7 @@ components: > cover_letter: > $ref: '#/components/schemas/CoverLetterEmbedded' > patches: > + title: Patches > type: array > items: > $ref: '#/components/schemas/PatchEmbedded' > @@ -2307,6 +2455,24 @@ components: > title: Detail > type: string > readOnly: true > + ErrorBundleCreateUpdate: > + type: object > + properties: > + name: > + title: Name > + type: string > + minLength: 1 > + maxLength: 50 > + patches: > + title: Patches > + type: array > + items: > + $ref: '#/components/schemas/PatchEmbedded' > + readOnly: true > + uniqueItems: true > + public: > + title: Public > + type: boolean > ErrorCheckCreate: > type: object > properties: > diff --git a/patchwork/api/bundle.py b/patchwork/api/bundle.py > index 2dec70d1..c5885aae 100644 > --- a/patchwork/api/bundle.py > +++ b/patchwork/api/bundle.py > @@ -4,9 +4,12 @@ > # SPDX-License-Identifier: GPL-2.0-or-later > > from django.db.models import Q > -from rest_framework.generics import ListAPIView > -from rest_framework.generics import RetrieveAPIView > +from rest_framework import exceptions > +from rest_framework.generics import ListCreateAPIView > +from rest_framework.generics import RetrieveUpdateDestroyAPIView > +from rest_framework import permissions > from rest_framework.serializers import SerializerMethodField > +from rest_framework.serializers import ValidationError > > from patchwork.api.base import BaseHyperlinkedModelSerializer > from patchwork.api.base import PatchworkPermission > @@ -14,16 +17,52 @@ from patchwork.api.filters import BundleFilterSet > from patchwork.api.embedded import PatchSerializer > from patchwork.api.embedded import ProjectSerializer > from patchwork.api.embedded import UserSerializer > +from patchwork.api import utils > from patchwork.models import Bundle > > > +class BundlePermission(permissions.BasePermission): > + """Ensure the API version, if configured, is >= v1.2. > + > + Bundle creation/updating was only added in API v1.2 and we don't want to > + change behavior in older API versions. > + """ > + def has_permission(self, request, view): > + # read-only permission for everything > + if request.method in permissions.SAFE_METHODS: > + return True > + > + if not utils.has_version(request, '1.2'): > + raise exceptions.MethodNotAllowed(request.method) > + > + if request.method == 'POST' and ( > + not request.user or not request.user.is_authenticated): > + return False > + > + # we have more to do but we can't do that until we have an object > + return True > + > + def has_object_permission(self, request, view, obj): > + if (request.user and > + request.user.is_authenticated and > + request.user == obj.owner): Just checking that this doesn't have the same User vs UserProfile thing we had with delegation? I haven't checked in great detail, I just saw a user comparison and thought it best to ask! > + return True > + > + if not obj.public: > + # if the bundle isn't public, we don't want to leak the fact that > + # it exists > + raise exceptions.NotFound A++ > + > + return request.method in permissions.SAFE_METHODS > + > + > class BundleSerializer(BaseHyperlinkedModelSerializer): > > web_url = SerializerMethodField() > project = ProjectSerializer(read_only=True) > mbox = SerializerMethodField() > owner = UserSerializer(read_only=True) > - patches = PatchSerializer(many=True, read_only=True) > + patches = PatchSerializer(many=True) > > def get_web_url(self, instance): > request = self.context.get('request') > @@ -33,11 +72,35 @@ class BundleSerializer(BaseHyperlinkedModelSerializer): > request = self.context.get('request') > return request.build_absolute_uri(instance.get_mbox_url()) > > + def create(self, validated_data): > + patches = validated_data.pop('patches') > + instance = super(BundleSerializer, self).create(validated_data) > + instance.overwrite_patches(patches) > + return instance > + > + def update(self, instance, validated_data): > + patches = validated_data.pop('patches') > + instance = super(BundleSerializer, self).update( > + instance, validated_data) > + instance.overwrite_patches(patches) > + return instance > + > + def validate(self, data): > + if not data.get('patches'): > + raise ValidationError('Bundles cannot be empty') > + > + if len(set([p.project.id for p in data['patches']])) > 1: > + raise ValidationError('Patches must belong to the same project') > + > + data['project'] = data['patches'][0].project > + > + return super(BundleSerializer, self).validate(data) > + > class Meta: > model = Bundle > fields = ('id', 'url', 'web_url', 'project', 'name', 'owner', > 'patches', 'public', 'mbox') > - read_only_fields = ('owner', 'patches', 'mbox') > + read_only_fields = ('project', 'owner', 'mbox') > versioned_fields = { > '1.1': ('web_url', ), > } > @@ -48,7 +111,7 @@ class BundleSerializer(BaseHyperlinkedModelSerializer): > > class BundleMixin(object): > > - permission_classes = (PatchworkPermission,) > + permission_classes = [PatchworkPermission & BundlePermission] > serializer_class = BundleSerializer > > def get_queryset(self): > @@ -63,16 +126,19 @@ class BundleMixin(object): > .select_related('owner', 'project') > > > -class BundleList(BundleMixin, ListAPIView): > - """List bundles.""" > +class BundleList(BundleMixin, ListCreateAPIView): > + """List or create bundles.""" > > filter_class = filterset_class = BundleFilterSet > search_fields = ('name',) > ordering_fields = ('id', 'name', 'owner') > ordering = 'id' > > + def perform_create(self, serializer): > + serializer.save(owner=self.request.user) > + Likewise and throughout the patch w/ user types. Apart from that, I think this is absolutely a Good Idea and I hope to do some testing of it soon. Regards, Daniel > > -class BundleDetail(BundleMixin, RetrieveAPIView): > - """Show a bundle.""" > +class BundleDetail(BundleMixin, RetrieveUpdateDestroyAPIView): > + """Show, update or delete a bundle.""" > > pass > diff --git a/patchwork/models.py b/patchwork/models.py > index 32d1b3c2..631de85d 100644 > --- a/patchwork/models.py > +++ b/patchwork/models.py > @@ -788,6 +788,11 @@ class Bundle(models.Model): > patches = models.ManyToManyField(Patch, through='BundlePatch') > public = models.BooleanField(default=False) > > + def is_editable(self, user): > + if not user.is_authenticated: > + return False > + return user == self.owner > + > def ordered_patches(self): > return self.patches.order_by('bundlepatch__order') > > @@ -806,6 +811,12 @@ class Bundle(models.Model): > return BundlePatch.objects.create(bundle=self, patch=patch, > order=max_order + 1) > > + def overwrite_patches(self, patches): > + BundlePatch.objects.filter(bundle=self).delete() > + > + for patch in patches: > + self.append_patch(patch) > + > def get_absolute_url(self): > return reverse('bundle-detail', kwargs={ > 'username': self.owner.username, > diff --git a/patchwork/tests/api/test_bundle.py b/patchwork/tests/api/test_bundle.py > index 303c500c..a3a0c113 100644 > --- a/patchwork/tests/api/test_bundle.py > +++ b/patchwork/tests/api/test_bundle.py > @@ -8,9 +8,11 @@ import unittest > from django.conf import settings > from django.urls import reverse > > +from patchwork.models import Bundle > from patchwork.tests.api import utils > from patchwork.tests.utils import create_bundle > from patchwork.tests.utils import create_maintainer > +from patchwork.tests.utils import create_patch > from patchwork.tests.utils import create_project > from patchwork.tests.utils import create_user > > @@ -42,12 +44,15 @@ class TestBundleAPI(utils.APITestCase): > > # nested fields > > - self.assertEqual(bundle_obj.patches.count(), > - len(bundle_json['patches'])) > self.assertEqual(bundle_obj.owner.id, > bundle_json['owner']['id']) > self.assertEqual(bundle_obj.project.id, > bundle_json['project']['id']) > + self.assertEqual(bundle_obj.patches.count(), > + len(bundle_json['patches'])) > + for patch_obj, patch_json in zip( > + bundle_obj.patches.all(), bundle_json['patches']): > + self.assertEqual(patch_obj.id, patch_json['id']) > > def test_list_empty(self): > """List bundles when none are present.""" > @@ -179,18 +184,117 @@ class TestBundleAPI(utils.APITestCase): > self.assertIn('url', resp.data) > self.assertNotIn('web_url', resp.data) > > - def test_create_update_delete(self): > - """Ensure creates, updates and deletes aren't allowed""" > + def _test_create_update(self, authenticate=True): > + user = create_user() > + project = create_project() > + patch_a = create_patch(project=project) > + patch_b = create_patch(project=project) > + > + if authenticate: > + self.client.force_authenticate(user=user) > + > + return user, project, patch_a, patch_b > + > + @utils.store_samples('bundle-create-error-forbidden') > + def test_create_anonymous(self): > + """Create a bundle when not signed in. > + > + Ensure creations can only be performed by signed in users. > + """ > + user, project, patch_a, patch_b = self._test_create_update( > + authenticate=False) > + bundle = { > + 'name': 'test-bundle', > + 'public': True, > + 'patches': [patch_a.id, patch_b.id], > + } > + > + resp = self.client.post(self.api_url(), bundle) > + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) > + > + @utils.store_samples('bundle-create') > + def test_create(self): > + """Validate we can create a new bundle.""" > + user, project, patch_a, patch_b = self._test_create_update() > + bundle = { > + 'name': 'test-bundle', > + 'public': True, > + 'patches': [patch_a.id, patch_b.id], > + } > + > + resp = self.client.post(self.api_url(), bundle) > + self.assertEqual(status.HTTP_201_CREATED, resp.status_code) > + self.assertEqual(1, Bundle.objects.all().count()) > + self.assertSerialized(Bundle.objects.first(), resp.data) > + > + @utils.store_samples('bundle-update-not-found') > + def test_update_anonymous(self): > + """Update an existing bundle when not signed in. > + > + Ensure updates can only be performed by signed in users. > + """ > + user, project, patch_a, patch_b = self._test_create_update( > + authenticate=False) > + bundle = create_bundle(owner=user, project=project) > + > + resp = self.client.patch(self.api_url(bundle.id), { > + 'name': 'hello-bundle', 'patches': [patch_a.id, patch_b.id]}) > + self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code) > + > + @utils.store_samples('bundle-update') > + def test_update(self): > + """Validate we can update an existing bundle.""" > + user, project, patch_a, patch_b = self._test_create_update() > + bundle = create_bundle(owner=user, project=project) > + > + resp = self.client.patch(self.api_url(bundle.id), { > + 'name': 'hello-bundle', 'patches': [patch_a.id, patch_b.id]}) > + self.assertEqual(status.HTTP_200_OK, resp.status_code) > + self.assertEqual(1, Bundle.objects.all().count()) > + self.assertEqual(len(resp.data['patches']), 2) > + self.assertEqual(resp.data['name'], 'hello-bundle') > + > + @utils.store_samples('bundle-delete-not-found') > + def test_delete_anonymous(self): > + """Delete a bundle when not signed in. > + > + Ensure deletions can only be performed when signed in. > + """ > + user, project, patch_a, patch_b = self._test_create_update( > + authenticate=False) > + bundle = create_bundle(owner=user, project=project) > + > + resp = self.client.delete(self.api_url(bundle.id)) > + self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code) > + > + @utils.store_samples('bundle-delete') > + def test_delete(self): > + """Validate we can delete an existing bundle.""" > + user = create_user() > + bundle = create_bundle(owner=user) > + > + self.client.force_authenticate(user=user) > + > + resp = self.client.delete(self.api_url(bundle.id)) > + self.assertEqual(status.HTTP_204_NO_CONTENT, resp.status_code) > + self.assertEqual(0, Bundle.objects.all().count()) > + > + def test_create_update_delete_version_1_1(self): > + """Ensure creates, updates and deletes aren't allowed with old API.""" > user = create_maintainer() > user.is_superuser = True > user.save() > self.client.force_authenticate(user=user) > > - resp = self.client.post(self.api_url(), {'email': 'foo@f.com'}) > + resp = self.client.post(self.api_url(version='1.1'), {'name': 'test'}, > + validate_schema=False) > self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) > > - resp = self.client.patch(self.api_url(user.id), {'email': 'foo@f.com'}) > + resp = self.client.patch(self.api_url(1, version='1.1'), > + {'name': 'test'}, > + validate_schema=False) > self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) > > - resp = self.client.delete(self.api_url(1)) > + resp = self.client.delete(self.api_url(1, version='1.1'), > + validate_schema=False) > self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) > diff --git a/patchwork/tests/api/utils.py b/patchwork/tests/api/utils.py > index 0c232d04..ce83ce2b 100644 > --- a/patchwork/tests/api/utils.py > +++ b/patchwork/tests/api/utils.py > @@ -112,44 +112,52 @@ class APIClient(BaseAPIClient): > self.factory = APIRequestFactory() > > def get(self, path, data=None, follow=False, **extra): > + validate_schema = extra.pop('validate_schema', True) > request = self.factory.get( > path, data=data, SERVER_NAME='example.com', **extra) > response = super(APIClient, self).get( > path, data=data, follow=follow, SERVER_NAME='example.com', **extra) > - validator.validate_data(path, request, response) > + if validate_schema: > + validator.validate_data(path, request, response) > return response > > def post(self, path, data=None, format=None, content_type=None, > follow=False, **extra): > + validate_schema = extra.pop('validate_schema', True) > request = self.factory.post( > path, data=data, format='json', content_type=content_type, > SERVER_NAME='example.com', **extra) > response = super(APIClient, self).post( > path, data=data, format='json', content_type=content_type, > follow=follow, SERVER_NAME='example.com', **extra) > - validator.validate_data(path, request, response) > + if validate_schema: > + validator.validate_data(path, request, response) > return response > > def put(self, path, data=None, format=None, content_type=None, > follow=False, **extra): > + validate_schema = extra.pop('validate_schema', True) > request = self.factory.put( > path, data=data, format='json', content_type=content_type, > SERVER_NAME='example.com', **extra) > response = super(APIClient, self).put( > path, data=data, format='json', content_type=content_type, > follow=follow, SERVER_NAME='example.com', **extra) > - validator.validate_data(path, request, response) > + if validate_schema: > + validator.validate_data(path, request, response) > return response > > def patch(self, path, data=None, format=None, content_type=None, > follow=False, **extra): > + validate_schema = extra.pop('validate_schema', True) > request = self.factory.patch( > path, data=data, format='json', content_type=content_type, > SERVER_NAME='example.com', **extra) > response = super(APIClient, self).patch( > path, data=data, format='json', content_type=content_type, > follow=follow, SERVER_NAME='example.com', **extra) > - validator.validate_data(path, request, response) > + if validate_schema: > + validator.validate_data(path, request, response) > return response > > > diff --git a/releasenotes/notes/update-bundle-via-api-2946d8c4e730d545.yaml b/releasenotes/notes/update-bundle-via-api-2946d8c4e730d545.yaml > new file mode 100644 > index 00000000..bfa1ef55 > --- /dev/null > +++ b/releasenotes/notes/update-bundle-via-api-2946d8c4e730d545.yaml > @@ -0,0 +1,4 @@ > +--- > +api: > + - | > + Bundles can now be created, updated and deleted via the REST API. > -- > 2.21.0 > > _______________________________________________ > Patchwork mailing list > Patchwork@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/patchwork
On Wed, 2019-10-02 at 11:50 +1000, Daniel Axtens wrote: > Stephen Finucane <stephen@that.guru> writes: > > > Allow users to create a new bundle, change the name, public flag and > > patches of an existing bundle, and delete an existing bundle. > > > > Some small nits with existing tests are resolved. > > > > Signed-off-by: Stephen Finucane <stephen@that.guru> > > --- > > docs/api/schemas/latest/patchwork.yaml | 170 +++++++++++++++- > > docs/api/schemas/patchwork.j2 | 181 +++++++++++++++++- > > docs/api/schemas/v1.0/patchwork.yaml | 5 +- > > docs/api/schemas/v1.1/patchwork.yaml | 5 +- > > docs/api/schemas/v1.2/patchwork.yaml | 170 +++++++++++++++- > > patchwork/api/bundle.py | 84 +++++++- > > patchwork/models.py | 11 ++ > > patchwork/tests/api/test_bundle.py | 118 +++++++++++- > > patchwork/tests/api/utils.py | 16 +- > > ...pdate-bundle-via-api-2946d8c4e730d545.yaml | 4 + > > 10 files changed, 737 insertions(+), 27 deletions(-) > > create mode 100644 releasenotes/notes/update-bundle-via-api-2946d8c4e730d545.yaml > > > > diff --git a/docs/api/schemas/latest/patchwork.yaml b/docs/api/schemas/latest/patchwork.yaml > > index 45a61180..e6c6bb4a 100644 > > --- a/docs/api/schemas/latest/patchwork.yaml > > +++ b/docs/api/schemas/latest/patchwork.yaml [snip] > > diff --git a/patchwork/api/bundle.py b/patchwork/api/bundle.py > > index 2dec70d1..c5885aae 100644 > > --- a/patchwork/api/bundle.py > > +++ b/patchwork/api/bundle.py > > @@ -4,9 +4,12 @@ > > # SPDX-License-Identifier: GPL-2.0-or-later > > > > from django.db.models import Q > > -from rest_framework.generics import ListAPIView > > -from rest_framework.generics import RetrieveAPIView > > +from rest_framework import exceptions > > +from rest_framework.generics import ListCreateAPIView > > +from rest_framework.generics import RetrieveUpdateDestroyAPIView > > +from rest_framework import permissions > > from rest_framework.serializers import SerializerMethodField > > +from rest_framework.serializers import ValidationError > > > > from patchwork.api.base import BaseHyperlinkedModelSerializer > > from patchwork.api.base import PatchworkPermission > > @@ -14,16 +17,52 @@ from patchwork.api.filters import BundleFilterSet > > from patchwork.api.embedded import PatchSerializer > > from patchwork.api.embedded import ProjectSerializer > > from patchwork.api.embedded import UserSerializer > > +from patchwork.api import utils > > from patchwork.models import Bundle > > > > > > +class BundlePermission(permissions.BasePermission): > > + """Ensure the API version, if configured, is >= v1.2. > > + > > + Bundle creation/updating was only added in API v1.2 and we don't want to > > + change behavior in older API versions. > > + """ > > + def has_permission(self, request, view): > > + # read-only permission for everything > > + if request.method in permissions.SAFE_METHODS: > > + return True > > + > > + if not utils.has_version(request, '1.2'): > > + raise exceptions.MethodNotAllowed(request.method) > > + > > + if request.method == 'POST' and ( > > + not request.user or not request.user.is_authenticated): > > + return False > > + > > + # we have more to do but we can't do that until we have an object > > + return True > > + > > + def has_object_permission(self, request, view, obj): > > + if (request.user and > > + request.user.is_authenticated and > > + request.user == obj.owner): > > Just checking that this doesn't have the same User vs UserProfile thing > we had with delegation? I haven't checked in great detail, I just saw a > user comparison and thought it best to ask! No, we're good here since Bundle.user points to the User object, not the UserProfile object. > > + return True > > + > > + if not obj.public: > > + # if the bundle isn't public, we don't want to leak the fact that > > + # it exists > > + raise exceptions.NotFound > > A++ > > > + > > + return request.method in permissions.SAFE_METHODS > > + > > + > > class BundleSerializer(BaseHyperlinkedModelSerializer): > > > > web_url = SerializerMethodField() > > project = ProjectSerializer(read_only=True) > > mbox = SerializerMethodField() > > owner = UserSerializer(read_only=True) > > - patches = PatchSerializer(many=True, read_only=True) > > + patches = PatchSerializer(many=True) > > > > def get_web_url(self, instance): > > request = self.context.get('request') > > @@ -33,11 +72,35 @@ class BundleSerializer(BaseHyperlinkedModelSerializer): > > request = self.context.get('request') > > return request.build_absolute_uri(instance.get_mbox_url()) > > > > + def create(self, validated_data): > > + patches = validated_data.pop('patches') > > + instance = super(BundleSerializer, self).create(validated_data) > > + instance.overwrite_patches(patches) > > + return instance > > + > > + def update(self, instance, validated_data): > > + patches = validated_data.pop('patches') > > + instance = super(BundleSerializer, self).update( > > + instance, validated_data) > > + instance.overwrite_patches(patches) > > + return instance > > + > > + def validate(self, data): > > + if not data.get('patches'): > > + raise ValidationError('Bundles cannot be empty') > > + > > + if len(set([p.project.id for p in data['patches']])) > 1: > > + raise ValidationError('Patches must belong to the same project') > > + > > + data['project'] = data['patches'][0].project > > + > > + return super(BundleSerializer, self).validate(data) > > + > > class Meta: > > model = Bundle > > fields = ('id', 'url', 'web_url', 'project', 'name', 'owner', > > 'patches', 'public', 'mbox') > > - read_only_fields = ('owner', 'patches', 'mbox') > > + read_only_fields = ('project', 'owner', 'mbox') > > versioned_fields = { > > '1.1': ('web_url', ), > > } > > @@ -48,7 +111,7 @@ class BundleSerializer(BaseHyperlinkedModelSerializer): > > > > class BundleMixin(object): > > > > - permission_classes = (PatchworkPermission,) > > + permission_classes = [PatchworkPermission & BundlePermission] > > serializer_class = BundleSerializer > > > > def get_queryset(self): > > @@ -63,16 +126,19 @@ class BundleMixin(object): > > .select_related('owner', 'project') > > > > > > -class BundleList(BundleMixin, ListAPIView): > > - """List bundles.""" > > +class BundleList(BundleMixin, ListCreateAPIView): > > + """List or create bundles.""" > > > > filter_class = filterset_class = BundleFilterSet > > search_fields = ('name',) > > ordering_fields = ('id', 'name', 'owner') > > ordering = 'id' > > > > + def perform_create(self, serializer): > > + serializer.save(owner=self.request.user) > > + > Likewise and throughout the patch w/ user types. > > Apart from that, I think this is absolutely a Good Idea and I hope to do > some testing of it soon. > > Regards, > Daniel > > > > > -class BundleDetail(BundleMixin, RetrieveAPIView): > > - """Show a bundle.""" > > +class BundleDetail(BundleMixin, RetrieveUpdateDestroyAPIView): > > + """Show, update or delete a bundle.""" > > > > pass > > diff --git a/patchwork/models.py b/patchwork/models.py > > index 32d1b3c2..631de85d 100644 > > --- a/patchwork/models.py > > +++ b/patchwork/models.py > > @@ -788,6 +788,11 @@ class Bundle(models.Model): > > patches = models.ManyToManyField(Patch, through='BundlePatch') > > public = models.BooleanField(default=False) > > > > + def is_editable(self, user): > > + if not user.is_authenticated: > > + return False > > + return user == self.owner > > + > > def ordered_patches(self): > > return self.patches.order_by('bundlepatch__order') > > > > @@ -806,6 +811,12 @@ class Bundle(models.Model): > > return BundlePatch.objects.create(bundle=self, patch=patch, > > order=max_order + 1) > > > > + def overwrite_patches(self, patches): > > + BundlePatch.objects.filter(bundle=self).delete() > > + > > + for patch in patches: > > + self.append_patch(patch) > > + > > def get_absolute_url(self): > > return reverse('bundle-detail', kwargs={ > > 'username': self.owner.username, > > diff --git a/patchwork/tests/api/test_bundle.py b/patchwork/tests/api/test_bundle.py > > index 303c500c..a3a0c113 100644 > > --- a/patchwork/tests/api/test_bundle.py > > +++ b/patchwork/tests/api/test_bundle.py > > @@ -8,9 +8,11 @@ import unittest > > from django.conf import settings > > from django.urls import reverse > > > > +from patchwork.models import Bundle > > from patchwork.tests.api import utils > > from patchwork.tests.utils import create_bundle > > from patchwork.tests.utils import create_maintainer > > +from patchwork.tests.utils import create_patch > > from patchwork.tests.utils import create_project > > from patchwork.tests.utils import create_user > > > > @@ -42,12 +44,15 @@ class TestBundleAPI(utils.APITestCase): > > > > # nested fields > > > > - self.assertEqual(bundle_obj.patches.count(), > > - len(bundle_json['patches'])) > > self.assertEqual(bundle_obj.owner.id, > > bundle_json['owner']['id']) > > self.assertEqual(bundle_obj.project.id, > > bundle_json['project']['id']) > > + self.assertEqual(bundle_obj.patches.count(), > > + len(bundle_json['patches'])) > > + for patch_obj, patch_json in zip( > > + bundle_obj.patches.all(), bundle_json['patches']): > > + self.assertEqual(patch_obj.id, patch_json['id']) > > > > def test_list_empty(self): > > """List bundles when none are present.""" > > @@ -179,18 +184,117 @@ class TestBundleAPI(utils.APITestCase): > > self.assertIn('url', resp.data) > > self.assertNotIn('web_url', resp.data) > > > > - def test_create_update_delete(self): > > - """Ensure creates, updates and deletes aren't allowed""" > > + def _test_create_update(self, authenticate=True): > > + user = create_user() > > + project = create_project() > > + patch_a = create_patch(project=project) > > + patch_b = create_patch(project=project) > > + > > + if authenticate: > > + self.client.force_authenticate(user=user) > > + > > + return user, project, patch_a, patch_b > > + > > + @utils.store_samples('bundle-create-error-forbidden') > > + def test_create_anonymous(self): > > + """Create a bundle when not signed in. > > + > > + Ensure creations can only be performed by signed in users. > > + """ > > + user, project, patch_a, patch_b = self._test_create_update( > > + authenticate=False) > > + bundle = { > > + 'name': 'test-bundle', > > + 'public': True, > > + 'patches': [patch_a.id, patch_b.id], > > + } > > + > > + resp = self.client.post(self.api_url(), bundle) > > + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) > > + > > + @utils.store_samples('bundle-create') > > + def test_create(self): > > + """Validate we can create a new bundle.""" > > + user, project, patch_a, patch_b = self._test_create_update() > > + bundle = { > > + 'name': 'test-bundle', > > + 'public': True, > > + 'patches': [patch_a.id, patch_b.id], > > + } > > + > > + resp = self.client.post(self.api_url(), bundle) > > + self.assertEqual(status.HTTP_201_CREATED, resp.status_code) > > + self.assertEqual(1, Bundle.objects.all().count()) > > + self.assertSerialized(Bundle.objects.first(), resp.data) > > + > > + @utils.store_samples('bundle-update-not-found') > > + def test_update_anonymous(self): > > + """Update an existing bundle when not signed in. > > + > > + Ensure updates can only be performed by signed in users. > > + """ > > + user, project, patch_a, patch_b = self._test_create_update( > > + authenticate=False) > > + bundle = create_bundle(owner=user, project=project) > > + > > + resp = self.client.patch(self.api_url(bundle.id), { > > + 'name': 'hello-bundle', 'patches': [patch_a.id, patch_b.id]}) > > + self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code) > > + > > + @utils.store_samples('bundle-update') > > + def test_update(self): > > + """Validate we can update an existing bundle.""" > > + user, project, patch_a, patch_b = self._test_create_update() > > + bundle = create_bundle(owner=user, project=project) > > + > > + resp = self.client.patch(self.api_url(bundle.id), { > > + 'name': 'hello-bundle', 'patches': [patch_a.id, patch_b.id]}) > > + self.assertEqual(status.HTTP_200_OK, resp.status_code) > > + self.assertEqual(1, Bundle.objects.all().count()) > > + self.assertEqual(len(resp.data['patches']), 2) > > + self.assertEqual(resp.data['name'], 'hello-bundle') > > + > > + @utils.store_samples('bundle-delete-not-found') > > + def test_delete_anonymous(self): > > + """Delete a bundle when not signed in. > > + > > + Ensure deletions can only be performed when signed in. > > + """ > > + user, project, patch_a, patch_b = self._test_create_update( > > + authenticate=False) > > + bundle = create_bundle(owner=user, project=project) > > + > > + resp = self.client.delete(self.api_url(bundle.id)) > > + self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code) > > + > > + @utils.store_samples('bundle-delete') > > + def test_delete(self): > > + """Validate we can delete an existing bundle.""" > > + user = create_user() > > + bundle = create_bundle(owner=user) > > + > > + self.client.force_authenticate(user=user) > > + > > + resp = self.client.delete(self.api_url(bundle.id)) > > + self.assertEqual(status.HTTP_204_NO_CONTENT, resp.status_code) > > + self.assertEqual(0, Bundle.objects.all().count()) > > + > > + def test_create_update_delete_version_1_1(self): > > + """Ensure creates, updates and deletes aren't allowed with old API.""" > > user = create_maintainer() > > user.is_superuser = True > > user.save() > > self.client.force_authenticate(user=user) > > > > - resp = self.client.post(self.api_url(), {'email': 'foo@f.com'}) > > + resp = self.client.post(self.api_url(version='1.1'), {'name': 'test'}, > > + validate_schema=False) > > self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) > > > > - resp = self.client.patch(self.api_url(user.id), {'email': 'foo@f.com'}) > > + resp = self.client.patch(self.api_url(1, version='1.1'), > > + {'name': 'test'}, > > + validate_schema=False) > > self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) > > > > - resp = self.client.delete(self.api_url(1)) > > + resp = self.client.delete(self.api_url(1, version='1.1'), > > + validate_schema=False) > > self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) > > diff --git a/patchwork/tests/api/utils.py b/patchwork/tests/api/utils.py > > index 0c232d04..ce83ce2b 100644 > > --- a/patchwork/tests/api/utils.py > > +++ b/patchwork/tests/api/utils.py > > @@ -112,44 +112,52 @@ class APIClient(BaseAPIClient): > > self.factory = APIRequestFactory() > > > > def get(self, path, data=None, follow=False, **extra): > > + validate_schema = extra.pop('validate_schema', True) > > request = self.factory.get( > > path, data=data, SERVER_NAME='example.com', **extra) > > response = super(APIClient, self).get( > > path, data=data, follow=follow, SERVER_NAME='example.com', **extra) > > - validator.validate_data(path, request, response) > > + if validate_schema: > > + validator.validate_data(path, request, response) > > return response > > > > def post(self, path, data=None, format=None, content_type=None, > > follow=False, **extra): > > + validate_schema = extra.pop('validate_schema', True) > > request = self.factory.post( > > path, data=data, format='json', content_type=content_type, > > SERVER_NAME='example.com', **extra) > > response = super(APIClient, self).post( > > path, data=data, format='json', content_type=content_type, > > follow=follow, SERVER_NAME='example.com', **extra) > > - validator.validate_data(path, request, response) > > + if validate_schema: > > + validator.validate_data(path, request, response) > > return response > > > > def put(self, path, data=None, format=None, content_type=None, > > follow=False, **extra): > > + validate_schema = extra.pop('validate_schema', True) > > request = self.factory.put( > > path, data=data, format='json', content_type=content_type, > > SERVER_NAME='example.com', **extra) > > response = super(APIClient, self).put( > > path, data=data, format='json', content_type=content_type, > > follow=follow, SERVER_NAME='example.com', **extra) > > - validator.validate_data(path, request, response) > > + if validate_schema: > > + validator.validate_data(path, request, response) > > return response > > > > def patch(self, path, data=None, format=None, content_type=None, > > follow=False, **extra): > > + validate_schema = extra.pop('validate_schema', True) > > request = self.factory.patch( > > path, data=data, format='json', content_type=content_type, > > SERVER_NAME='example.com', **extra) > > response = super(APIClient, self).patch( > > path, data=data, format='json', content_type=content_type, > > follow=follow, SERVER_NAME='example.com', **extra) > > - validator.validate_data(path, request, response) > > + if validate_schema: > > + validator.validate_data(path, request, response) > > return response > > > > > > diff --git a/releasenotes/notes/update-bundle-via-api-2946d8c4e730d545.yaml b/releasenotes/notes/update-bundle-via-api-2946d8c4e730d545.yaml > > new file mode 100644 > > index 00000000..bfa1ef55 > > --- /dev/null > > +++ b/releasenotes/notes/update-bundle-via-api-2946d8c4e730d545.yaml > > @@ -0,0 +1,4 @@ > > +--- > > +api: > > + - | > > + Bundles can now be created, updated and deleted via the REST API. > > -- > > 2.21.0
diff --git a/docs/api/schemas/latest/patchwork.yaml b/docs/api/schemas/latest/patchwork.yaml index 45a61180..e6c6bb4a 100644 --- a/docs/api/schemas/latest/patchwork.yaml +++ b/docs/api/schemas/latest/patchwork.yaml @@ -1,5 +1,6 @@ # DO NOT EDIT THIS FILE. It is generated from a template. Changes should be -# proposed against the template. +# proposed against the template and updated files generated using the +# 'generate_schema.py' tool --- openapi: '3.0.0' info: @@ -72,6 +73,35 @@ paths: $ref: '#/components/schemas/Bundle' tags: - bundles + post: + description: Create a bundle. + operationId: bundles_create + security: + - basicAuth: [] + - apiKeyAuth: [] + requestBody: + $ref: '#/components/requestBodies/Bundle' + responses: + '201': + description: '' + content: + application/json: + schema: + $ref: '#/components/schemas/Bundle' + '400': + description: Invalid Request + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorBundleCreateUpdate' + '403': + description: Forbidden + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + tags: + - bundles /api/bundles/{id}/: get: description: Show a bundle. @@ -99,6 +129,92 @@ paths: $ref: '#/components/schemas/Error' tags: - bundles + patch: + description: Update a bundle (partial). + operationId: bundles_partial_update + security: + - basicAuth: [] + - apiKeyAuth: [] + parameters: + - in: path + name: id + description: A unique integer value identifying this bundle. + required: true + schema: + title: ID + type: integer + requestBody: + $ref: '#/components/requestBodies/Bundle' + responses: + '200': + description: '' + content: + application/json: + schema: + $ref: '#/components/schemas/Bundle' + '400': + description: Bad request + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorBundleCreateUpdate' + '403': + description: Forbidden + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + '404': + description: Not found + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + tags: + - bundles + put: + description: Update a bundle. + operationId: bundles_update + security: + - basicAuth: [] + - apiKeyAuth: [] + parameters: + - in: path + name: id + description: A unique integer value identifying this bundle. + required: true + schema: + title: ID + type: integer + requestBody: + $ref: '#/components/requestBodies/Bundle' + responses: + '200': + description: '' + content: + application/json: + schema: + $ref: '#/components/schemas/Bundle' + '400': + description: Bad request + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorBundleCreateUpdate' + '403': + description: Forbidden + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + '404': + description: Not found + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + tags: + - bundles /api/covers/: get: description: List cover letters. @@ -1131,6 +1247,18 @@ components: schema: type: string requestBodies: + Bundle: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/BundleCreateUpdate' + multipart/form-data: + schema: + $ref: '#/components/schemas/BundleCreateUpdate' + application/x-www-form-urlencoded: + schema: + $ref: '#/components/schemas/BundleCreateUpdate' Check: required: true content: @@ -1251,10 +1379,10 @@ components: allOf: - $ref: '#/components/schemas/UserEmbedded' patches: + title: Patches type: array items: $ref: '#/components/schemas/PatchEmbedded' - readOnly: true uniqueItems: true public: title: Public @@ -1264,6 +1392,25 @@ components: type: string format: uri readOnly: true + BundleCreateUpdate: + type: object + required: + - name + properties: + name: + title: Name + type: string + minLength: 1 + maxLength: 50 + patches: + title: Patches + type: array + items: + type: integer + uniqueItems: true + public: + title: Public + type: boolean Check: type: object properties: @@ -1961,6 +2108,7 @@ components: cover_letter: $ref: '#/components/schemas/CoverLetterEmbedded' patches: + title: Patches type: array items: $ref: '#/components/schemas/PatchEmbedded' @@ -2307,6 +2455,24 @@ components: title: Detail type: string readOnly: true + ErrorBundleCreateUpdate: + type: object + properties: + name: + title: Name + type: string + minLength: 1 + maxLength: 50 + patches: + title: Patches + type: array + items: + $ref: '#/components/schemas/PatchEmbedded' + readOnly: true + uniqueItems: true + public: + title: Public + type: boolean ErrorCheckCreate: type: object properties: diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2 index 843981f8..5f7510da 100644 --- a/docs/api/schemas/patchwork.j2 +++ b/docs/api/schemas/patchwork.j2 @@ -1,6 +1,7 @@ {# You can obviously ignore the below when editing this template #} # DO NOT EDIT THIS FILE. It is generated from a template. Changes should be -# proposed against the template. +# proposed against the template and updated files generated using the +# 'generate_schema.py' tool --- openapi: '3.0.0' info: @@ -73,6 +74,37 @@ paths: $ref: '#/components/schemas/Bundle' tags: - bundles +{% if version >= (1, 2) %} + post: + description: Create a bundle. + operationId: bundles_create + security: + - basicAuth: [] + - apiKeyAuth: [] + requestBody: + $ref: '#/components/requestBodies/Bundle' + responses: + '201': + description: '' + content: + application/json: + schema: + $ref: '#/components/schemas/Bundle' + '400': + description: Invalid Request + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorBundleCreateUpdate' + '403': + description: Forbidden + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + tags: + - bundles +{% endif %} /api/{{ version_url }}bundles/{id}/: get: description: Show a bundle. @@ -100,6 +132,94 @@ paths: $ref: '#/components/schemas/Error' tags: - bundles +{% if version >= (1, 2) %} + patch: + description: Update a bundle (partial). + operationId: bundles_partial_update + security: + - basicAuth: [] + - apiKeyAuth: [] + parameters: + - in: path + name: id + description: A unique integer value identifying this bundle. + required: true + schema: + title: ID + type: integer + requestBody: + $ref: '#/components/requestBodies/Bundle' + responses: + '200': + description: '' + content: + application/json: + schema: + $ref: '#/components/schemas/Bundle' + '400': + description: Bad request + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorBundleCreateUpdate' + '403': + description: Forbidden + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + '404': + description: Not found + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + tags: + - bundles + put: + description: Update a bundle. + operationId: bundles_update + security: + - basicAuth: [] + - apiKeyAuth: [] + parameters: + - in: path + name: id + description: A unique integer value identifying this bundle. + required: true + schema: + title: ID + type: integer + requestBody: + $ref: '#/components/requestBodies/Bundle' + responses: + '200': + description: '' + content: + application/json: + schema: + $ref: '#/components/schemas/Bundle' + '400': + description: Bad request + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorBundleCreateUpdate' + '403': + description: Forbidden + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + '404': + description: Not found + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + tags: + - bundles +{% endif %} /api/{{ version_url }}covers/: get: description: List cover letters. @@ -1132,6 +1252,20 @@ components: schema: type: string requestBodies: +{% if version >= (1, 2) %} + Bundle: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/BundleCreateUpdate' + multipart/form-data: + schema: + $ref: '#/components/schemas/BundleCreateUpdate' + application/x-www-form-urlencoded: + schema: + $ref: '#/components/schemas/BundleCreateUpdate' +{% endif %} Check: required: true content: @@ -1254,10 +1388,13 @@ components: allOf: - $ref: '#/components/schemas/UserEmbedded' patches: + title: Patches type: array items: $ref: '#/components/schemas/PatchEmbedded' +{% if version < (1, 2) %} readOnly: true +{% endif %} uniqueItems: true public: title: Public @@ -1267,6 +1404,27 @@ components: type: string format: uri readOnly: true +{% if version >= (1, 2) %} + BundleCreateUpdate: + type: object + required: + - name + properties: + name: + title: Name + type: string + minLength: 1 + maxLength: 50 + patches: + title: Patches + type: array + items: + type: integer + uniqueItems: true + public: + title: Public + type: boolean +{% endif %} Check: type: object properties: @@ -1988,6 +2146,7 @@ components: cover_letter: $ref: '#/components/schemas/CoverLetterEmbedded' patches: + title: Patches type: array items: $ref: '#/components/schemas/PatchEmbedded' @@ -2346,6 +2505,26 @@ components: title: Detail type: string readOnly: true +{% if version >= (1, 2) %} + ErrorBundleCreateUpdate: + type: object + properties: + name: + title: Name + type: string + minLength: 1 + maxLength: 50 + patches: + title: Patches + type: array + items: + $ref: '#/components/schemas/PatchEmbedded' + readOnly: true + uniqueItems: true + public: + title: Public + type: boolean +{% endif %} ErrorCheckCreate: type: object properties: diff --git a/docs/api/schemas/v1.0/patchwork.yaml b/docs/api/schemas/v1.0/patchwork.yaml index 02f3a156..cafef5ee 100644 --- a/docs/api/schemas/v1.0/patchwork.yaml +++ b/docs/api/schemas/v1.0/patchwork.yaml @@ -1,5 +1,6 @@ # DO NOT EDIT THIS FILE. It is generated from a template. Changes should be -# proposed against the template. +# proposed against the template and updated files generated using the +# 'generate_schema.py' tool --- openapi: '3.0.0' info: @@ -1246,6 +1247,7 @@ components: allOf: - $ref: '#/components/schemas/UserEmbedded' patches: + title: Patches type: array items: $ref: '#/components/schemas/PatchEmbedded' @@ -1877,6 +1879,7 @@ components: cover_letter: $ref: '#/components/schemas/CoverLetterEmbedded' patches: + title: Patches type: array items: $ref: '#/components/schemas/PatchEmbedded' diff --git a/docs/api/schemas/v1.1/patchwork.yaml b/docs/api/schemas/v1.1/patchwork.yaml index 0c086eda..9d45d058 100644 --- a/docs/api/schemas/v1.1/patchwork.yaml +++ b/docs/api/schemas/v1.1/patchwork.yaml @@ -1,5 +1,6 @@ # DO NOT EDIT THIS FILE. It is generated from a template. Changes should be -# proposed against the template. +# proposed against the template and updated files generated using the +# 'generate_schema.py' tool --- openapi: '3.0.0' info: @@ -1251,6 +1252,7 @@ components: allOf: - $ref: '#/components/schemas/UserEmbedded' patches: + title: Patches type: array items: $ref: '#/components/schemas/PatchEmbedded' @@ -1928,6 +1930,7 @@ components: cover_letter: $ref: '#/components/schemas/CoverLetterEmbedded' patches: + title: Patches type: array items: $ref: '#/components/schemas/PatchEmbedded' diff --git a/docs/api/schemas/v1.2/patchwork.yaml b/docs/api/schemas/v1.2/patchwork.yaml index 3a96aa3a..5c03bf1e 100644 --- a/docs/api/schemas/v1.2/patchwork.yaml +++ b/docs/api/schemas/v1.2/patchwork.yaml @@ -1,5 +1,6 @@ # DO NOT EDIT THIS FILE. It is generated from a template. Changes should be -# proposed against the template. +# proposed against the template and updated files generated using the +# 'generate_schema.py' tool --- openapi: '3.0.0' info: @@ -72,6 +73,35 @@ paths: $ref: '#/components/schemas/Bundle' tags: - bundles + post: + description: Create a bundle. + operationId: bundles_create + security: + - basicAuth: [] + - apiKeyAuth: [] + requestBody: + $ref: '#/components/requestBodies/Bundle' + responses: + '201': + description: '' + content: + application/json: + schema: + $ref: '#/components/schemas/Bundle' + '400': + description: Invalid Request + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorBundleCreateUpdate' + '403': + description: Forbidden + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + tags: + - bundles /api/1.2/bundles/{id}/: get: description: Show a bundle. @@ -99,6 +129,92 @@ paths: $ref: '#/components/schemas/Error' tags: - bundles + patch: + description: Update a bundle (partial). + operationId: bundles_partial_update + security: + - basicAuth: [] + - apiKeyAuth: [] + parameters: + - in: path + name: id + description: A unique integer value identifying this bundle. + required: true + schema: + title: ID + type: integer + requestBody: + $ref: '#/components/requestBodies/Bundle' + responses: + '200': + description: '' + content: + application/json: + schema: + $ref: '#/components/schemas/Bundle' + '400': + description: Bad request + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorBundleCreateUpdate' + '403': + description: Forbidden + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + '404': + description: Not found + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + tags: + - bundles + put: + description: Update a bundle. + operationId: bundles_update + security: + - basicAuth: [] + - apiKeyAuth: [] + parameters: + - in: path + name: id + description: A unique integer value identifying this bundle. + required: true + schema: + title: ID + type: integer + requestBody: + $ref: '#/components/requestBodies/Bundle' + responses: + '200': + description: '' + content: + application/json: + schema: + $ref: '#/components/schemas/Bundle' + '400': + description: Bad request + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorBundleCreateUpdate' + '403': + description: Forbidden + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + '404': + description: Not found + content: + application/json: + schema: + $ref: '#/components/schemas/Error' + tags: + - bundles /api/1.2/covers/: get: description: List cover letters. @@ -1131,6 +1247,18 @@ components: schema: type: string requestBodies: + Bundle: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/BundleCreateUpdate' + multipart/form-data: + schema: + $ref: '#/components/schemas/BundleCreateUpdate' + application/x-www-form-urlencoded: + schema: + $ref: '#/components/schemas/BundleCreateUpdate' Check: required: true content: @@ -1251,10 +1379,10 @@ components: allOf: - $ref: '#/components/schemas/UserEmbedded' patches: + title: Patches type: array items: $ref: '#/components/schemas/PatchEmbedded' - readOnly: true uniqueItems: true public: title: Public @@ -1264,6 +1392,25 @@ components: type: string format: uri readOnly: true + BundleCreateUpdate: + type: object + required: + - name + properties: + name: + title: Name + type: string + minLength: 1 + maxLength: 50 + patches: + title: Patches + type: array + items: + type: integer + uniqueItems: true + public: + title: Public + type: boolean Check: type: object properties: @@ -1961,6 +2108,7 @@ components: cover_letter: $ref: '#/components/schemas/CoverLetterEmbedded' patches: + title: Patches type: array items: $ref: '#/components/schemas/PatchEmbedded' @@ -2307,6 +2455,24 @@ components: title: Detail type: string readOnly: true + ErrorBundleCreateUpdate: + type: object + properties: + name: + title: Name + type: string + minLength: 1 + maxLength: 50 + patches: + title: Patches + type: array + items: + $ref: '#/components/schemas/PatchEmbedded' + readOnly: true + uniqueItems: true + public: + title: Public + type: boolean ErrorCheckCreate: type: object properties: diff --git a/patchwork/api/bundle.py b/patchwork/api/bundle.py index 2dec70d1..c5885aae 100644 --- a/patchwork/api/bundle.py +++ b/patchwork/api/bundle.py @@ -4,9 +4,12 @@ # SPDX-License-Identifier: GPL-2.0-or-later from django.db.models import Q -from rest_framework.generics import ListAPIView -from rest_framework.generics import RetrieveAPIView +from rest_framework import exceptions +from rest_framework.generics import ListCreateAPIView +from rest_framework.generics import RetrieveUpdateDestroyAPIView +from rest_framework import permissions from rest_framework.serializers import SerializerMethodField +from rest_framework.serializers import ValidationError from patchwork.api.base import BaseHyperlinkedModelSerializer from patchwork.api.base import PatchworkPermission @@ -14,16 +17,52 @@ from patchwork.api.filters import BundleFilterSet from patchwork.api.embedded import PatchSerializer from patchwork.api.embedded import ProjectSerializer from patchwork.api.embedded import UserSerializer +from patchwork.api import utils from patchwork.models import Bundle +class BundlePermission(permissions.BasePermission): + """Ensure the API version, if configured, is >= v1.2. + + Bundle creation/updating was only added in API v1.2 and we don't want to + change behavior in older API versions. + """ + def has_permission(self, request, view): + # read-only permission for everything + if request.method in permissions.SAFE_METHODS: + return True + + if not utils.has_version(request, '1.2'): + raise exceptions.MethodNotAllowed(request.method) + + if request.method == 'POST' and ( + not request.user or not request.user.is_authenticated): + return False + + # we have more to do but we can't do that until we have an object + return True + + def has_object_permission(self, request, view, obj): + if (request.user and + request.user.is_authenticated and + request.user == obj.owner): + return True + + if not obj.public: + # if the bundle isn't public, we don't want to leak the fact that + # it exists + raise exceptions.NotFound + + return request.method in permissions.SAFE_METHODS + + class BundleSerializer(BaseHyperlinkedModelSerializer): web_url = SerializerMethodField() project = ProjectSerializer(read_only=True) mbox = SerializerMethodField() owner = UserSerializer(read_only=True) - patches = PatchSerializer(many=True, read_only=True) + patches = PatchSerializer(many=True) def get_web_url(self, instance): request = self.context.get('request') @@ -33,11 +72,35 @@ class BundleSerializer(BaseHyperlinkedModelSerializer): request = self.context.get('request') return request.build_absolute_uri(instance.get_mbox_url()) + def create(self, validated_data): + patches = validated_data.pop('patches') + instance = super(BundleSerializer, self).create(validated_data) + instance.overwrite_patches(patches) + return instance + + def update(self, instance, validated_data): + patches = validated_data.pop('patches') + instance = super(BundleSerializer, self).update( + instance, validated_data) + instance.overwrite_patches(patches) + return instance + + def validate(self, data): + if not data.get('patches'): + raise ValidationError('Bundles cannot be empty') + + if len(set([p.project.id for p in data['patches']])) > 1: + raise ValidationError('Patches must belong to the same project') + + data['project'] = data['patches'][0].project + + return super(BundleSerializer, self).validate(data) + class Meta: model = Bundle fields = ('id', 'url', 'web_url', 'project', 'name', 'owner', 'patches', 'public', 'mbox') - read_only_fields = ('owner', 'patches', 'mbox') + read_only_fields = ('project', 'owner', 'mbox') versioned_fields = { '1.1': ('web_url', ), } @@ -48,7 +111,7 @@ class BundleSerializer(BaseHyperlinkedModelSerializer): class BundleMixin(object): - permission_classes = (PatchworkPermission,) + permission_classes = [PatchworkPermission & BundlePermission] serializer_class = BundleSerializer def get_queryset(self): @@ -63,16 +126,19 @@ class BundleMixin(object): .select_related('owner', 'project') -class BundleList(BundleMixin, ListAPIView): - """List bundles.""" +class BundleList(BundleMixin, ListCreateAPIView): + """List or create bundles.""" filter_class = filterset_class = BundleFilterSet search_fields = ('name',) ordering_fields = ('id', 'name', 'owner') ordering = 'id' + def perform_create(self, serializer): + serializer.save(owner=self.request.user) + -class BundleDetail(BundleMixin, RetrieveAPIView): - """Show a bundle.""" +class BundleDetail(BundleMixin, RetrieveUpdateDestroyAPIView): + """Show, update or delete a bundle.""" pass diff --git a/patchwork/models.py b/patchwork/models.py index 32d1b3c2..631de85d 100644 --- a/patchwork/models.py +++ b/patchwork/models.py @@ -788,6 +788,11 @@ class Bundle(models.Model): patches = models.ManyToManyField(Patch, through='BundlePatch') public = models.BooleanField(default=False) + def is_editable(self, user): + if not user.is_authenticated: + return False + return user == self.owner + def ordered_patches(self): return self.patches.order_by('bundlepatch__order') @@ -806,6 +811,12 @@ class Bundle(models.Model): return BundlePatch.objects.create(bundle=self, patch=patch, order=max_order + 1) + def overwrite_patches(self, patches): + BundlePatch.objects.filter(bundle=self).delete() + + for patch in patches: + self.append_patch(patch) + def get_absolute_url(self): return reverse('bundle-detail', kwargs={ 'username': self.owner.username, diff --git a/patchwork/tests/api/test_bundle.py b/patchwork/tests/api/test_bundle.py index 303c500c..a3a0c113 100644 --- a/patchwork/tests/api/test_bundle.py +++ b/patchwork/tests/api/test_bundle.py @@ -8,9 +8,11 @@ import unittest from django.conf import settings from django.urls import reverse +from patchwork.models import Bundle from patchwork.tests.api import utils from patchwork.tests.utils import create_bundle from patchwork.tests.utils import create_maintainer +from patchwork.tests.utils import create_patch from patchwork.tests.utils import create_project from patchwork.tests.utils import create_user @@ -42,12 +44,15 @@ class TestBundleAPI(utils.APITestCase): # nested fields - self.assertEqual(bundle_obj.patches.count(), - len(bundle_json['patches'])) self.assertEqual(bundle_obj.owner.id, bundle_json['owner']['id']) self.assertEqual(bundle_obj.project.id, bundle_json['project']['id']) + self.assertEqual(bundle_obj.patches.count(), + len(bundle_json['patches'])) + for patch_obj, patch_json in zip( + bundle_obj.patches.all(), bundle_json['patches']): + self.assertEqual(patch_obj.id, patch_json['id']) def test_list_empty(self): """List bundles when none are present.""" @@ -179,18 +184,117 @@ class TestBundleAPI(utils.APITestCase): self.assertIn('url', resp.data) self.assertNotIn('web_url', resp.data) - def test_create_update_delete(self): - """Ensure creates, updates and deletes aren't allowed""" + def _test_create_update(self, authenticate=True): + user = create_user() + project = create_project() + patch_a = create_patch(project=project) + patch_b = create_patch(project=project) + + if authenticate: + self.client.force_authenticate(user=user) + + return user, project, patch_a, patch_b + + @utils.store_samples('bundle-create-error-forbidden') + def test_create_anonymous(self): + """Create a bundle when not signed in. + + Ensure creations can only be performed by signed in users. + """ + user, project, patch_a, patch_b = self._test_create_update( + authenticate=False) + bundle = { + 'name': 'test-bundle', + 'public': True, + 'patches': [patch_a.id, patch_b.id], + } + + resp = self.client.post(self.api_url(), bundle) + self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) + + @utils.store_samples('bundle-create') + def test_create(self): + """Validate we can create a new bundle.""" + user, project, patch_a, patch_b = self._test_create_update() + bundle = { + 'name': 'test-bundle', + 'public': True, + 'patches': [patch_a.id, patch_b.id], + } + + resp = self.client.post(self.api_url(), bundle) + self.assertEqual(status.HTTP_201_CREATED, resp.status_code) + self.assertEqual(1, Bundle.objects.all().count()) + self.assertSerialized(Bundle.objects.first(), resp.data) + + @utils.store_samples('bundle-update-not-found') + def test_update_anonymous(self): + """Update an existing bundle when not signed in. + + Ensure updates can only be performed by signed in users. + """ + user, project, patch_a, patch_b = self._test_create_update( + authenticate=False) + bundle = create_bundle(owner=user, project=project) + + resp = self.client.patch(self.api_url(bundle.id), { + 'name': 'hello-bundle', 'patches': [patch_a.id, patch_b.id]}) + self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code) + + @utils.store_samples('bundle-update') + def test_update(self): + """Validate we can update an existing bundle.""" + user, project, patch_a, patch_b = self._test_create_update() + bundle = create_bundle(owner=user, project=project) + + resp = self.client.patch(self.api_url(bundle.id), { + 'name': 'hello-bundle', 'patches': [patch_a.id, patch_b.id]}) + self.assertEqual(status.HTTP_200_OK, resp.status_code) + self.assertEqual(1, Bundle.objects.all().count()) + self.assertEqual(len(resp.data['patches']), 2) + self.assertEqual(resp.data['name'], 'hello-bundle') + + @utils.store_samples('bundle-delete-not-found') + def test_delete_anonymous(self): + """Delete a bundle when not signed in. + + Ensure deletions can only be performed when signed in. + """ + user, project, patch_a, patch_b = self._test_create_update( + authenticate=False) + bundle = create_bundle(owner=user, project=project) + + resp = self.client.delete(self.api_url(bundle.id)) + self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code) + + @utils.store_samples('bundle-delete') + def test_delete(self): + """Validate we can delete an existing bundle.""" + user = create_user() + bundle = create_bundle(owner=user) + + self.client.force_authenticate(user=user) + + resp = self.client.delete(self.api_url(bundle.id)) + self.assertEqual(status.HTTP_204_NO_CONTENT, resp.status_code) + self.assertEqual(0, Bundle.objects.all().count()) + + def test_create_update_delete_version_1_1(self): + """Ensure creates, updates and deletes aren't allowed with old API.""" user = create_maintainer() user.is_superuser = True user.save() self.client.force_authenticate(user=user) - resp = self.client.post(self.api_url(), {'email': 'foo@f.com'}) + resp = self.client.post(self.api_url(version='1.1'), {'name': 'test'}, + validate_schema=False) self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) - resp = self.client.patch(self.api_url(user.id), {'email': 'foo@f.com'}) + resp = self.client.patch(self.api_url(1, version='1.1'), + {'name': 'test'}, + validate_schema=False) self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) - resp = self.client.delete(self.api_url(1)) + resp = self.client.delete(self.api_url(1, version='1.1'), + validate_schema=False) self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code) diff --git a/patchwork/tests/api/utils.py b/patchwork/tests/api/utils.py index 0c232d04..ce83ce2b 100644 --- a/patchwork/tests/api/utils.py +++ b/patchwork/tests/api/utils.py @@ -112,44 +112,52 @@ class APIClient(BaseAPIClient): self.factory = APIRequestFactory() def get(self, path, data=None, follow=False, **extra): + validate_schema = extra.pop('validate_schema', True) request = self.factory.get( path, data=data, SERVER_NAME='example.com', **extra) response = super(APIClient, self).get( path, data=data, follow=follow, SERVER_NAME='example.com', **extra) - validator.validate_data(path, request, response) + if validate_schema: + validator.validate_data(path, request, response) return response def post(self, path, data=None, format=None, content_type=None, follow=False, **extra): + validate_schema = extra.pop('validate_schema', True) request = self.factory.post( path, data=data, format='json', content_type=content_type, SERVER_NAME='example.com', **extra) response = super(APIClient, self).post( path, data=data, format='json', content_type=content_type, follow=follow, SERVER_NAME='example.com', **extra) - validator.validate_data(path, request, response) + if validate_schema: + validator.validate_data(path, request, response) return response def put(self, path, data=None, format=None, content_type=None, follow=False, **extra): + validate_schema = extra.pop('validate_schema', True) request = self.factory.put( path, data=data, format='json', content_type=content_type, SERVER_NAME='example.com', **extra) response = super(APIClient, self).put( path, data=data, format='json', content_type=content_type, follow=follow, SERVER_NAME='example.com', **extra) - validator.validate_data(path, request, response) + if validate_schema: + validator.validate_data(path, request, response) return response def patch(self, path, data=None, format=None, content_type=None, follow=False, **extra): + validate_schema = extra.pop('validate_schema', True) request = self.factory.patch( path, data=data, format='json', content_type=content_type, SERVER_NAME='example.com', **extra) response = super(APIClient, self).patch( path, data=data, format='json', content_type=content_type, follow=follow, SERVER_NAME='example.com', **extra) - validator.validate_data(path, request, response) + if validate_schema: + validator.validate_data(path, request, response) return response diff --git a/releasenotes/notes/update-bundle-via-api-2946d8c4e730d545.yaml b/releasenotes/notes/update-bundle-via-api-2946d8c4e730d545.yaml new file mode 100644 index 00000000..bfa1ef55 --- /dev/null +++ b/releasenotes/notes/update-bundle-via-api-2946d8c4e730d545.yaml @@ -0,0 +1,4 @@ +--- +api: + - | + Bundles can now be created, updated and deleted via the REST API.
Allow users to create a new bundle, change the name, public flag and patches of an existing bundle, and delete an existing bundle. Some small nits with existing tests are resolved. Signed-off-by: Stephen Finucane <stephen@that.guru> --- docs/api/schemas/latest/patchwork.yaml | 170 +++++++++++++++- docs/api/schemas/patchwork.j2 | 181 +++++++++++++++++- docs/api/schemas/v1.0/patchwork.yaml | 5 +- docs/api/schemas/v1.1/patchwork.yaml | 5 +- docs/api/schemas/v1.2/patchwork.yaml | 170 +++++++++++++++- patchwork/api/bundle.py | 84 +++++++- patchwork/models.py | 11 ++ patchwork/tests/api/test_bundle.py | 118 +++++++++++- patchwork/tests/api/utils.py | 16 +- ...pdate-bundle-via-api-2946d8c4e730d545.yaml | 4 + 10 files changed, 737 insertions(+), 27 deletions(-) create mode 100644 releasenotes/notes/update-bundle-via-api-2946d8c4e730d545.yaml