On Sat, 2019-12-07 at 17:46 +0100, Mete Polat wrote: > View relations and add/update/delete them as a maintainer. Maintainers > can only create relations of submissions (patches/cover letters) which > are part of a project they maintain. > > New REST API urls: > api/relations/ > api/relations/<relation_id>/ > > Co-authored-by: Daniel Axtens <d...@axtens.net> > Signed-off-by: Mete Polat <metepolat2...@gmail.com>
Why did you choose to expose this as a separate API rather than as a field on the '/patches' resource? While a 'Series' objet has enough separate metadata to warrant a separate '/series' resource, a 'SubmissionRelation' object is basically just a container. Including a 'related_patches' field on the detailed patch view would seem like more than enough detail for me, anyway, and unless there's a reason not to do this, I'd like to see it done that way. Is it possible? Stephen PS: I could have sworn I had asked this before, but I can't find any mails about it so maybe I didn't. Please tell me to RTML (read the mailing list) if so > --- > Optimize db queries: > I have spent quite a lot of time in optimizing the db queries for the REST > API > (thanks for the tip with the Django toolbar). Daniel stated that > prefetch_related is possibly hitting the database for every relation when > prefetching submissions but it turns out that we can tell Django to use a > statement like: > SELECT * > FROM `patchwork_patch` > INNER JOIN `patchwork_submission` > ON (`patchwork_patch`.`submission_ptr_id` = > `patchwork_submission`.`id`) > WHERE `patchwork_patch`.`submission_ptr_id` IN > (LIST_OF_ALL_SUBMISSION_IDS) > > We do the same for `patchwork_coverletter`. > This means we only hit the db two times for casting _all_ submissions to a > patch or cover-letter. > > Prefetching submissions__project eliminates similar and duplicate queries > that are used to determine whether a logged in user is at least maintainer > of one submission's project. > > docs/api/schemas/latest/patchwork.yaml | 273 +++++++++++++++++ > docs/api/schemas/patchwork.j2 | 285 ++++++++++++++++++ > docs/api/schemas/v1.2/patchwork.yaml | 273 +++++++++++++++++ > patchwork/api/embedded.py | 39 +++ > patchwork/api/index.py | 1 + > patchwork/api/relation.py | 121 ++++++++ > patchwork/models.py | 6 + > patchwork/tests/api/test_relation.py | 181 +++++++++++ > patchwork/tests/utils.py | 15 + > patchwork/urls.py | 11 + > ...submission-relations-c96bb6c567b416d8.yaml | 10 + > 11 files changed, 1215 insertions(+) > create mode 100644 patchwork/api/relation.py > create mode 100644 patchwork/tests/api/test_relation.py > create mode 100644 > releasenotes/notes/add-submission-relations-c96bb6c567b416d8.yaml > > diff --git a/docs/api/schemas/latest/patchwork.yaml > b/docs/api/schemas/latest/patchwork.yaml > index a5e235be936d..7dd24fd700d5 100644 > --- a/docs/api/schemas/latest/patchwork.yaml > +++ b/docs/api/schemas/latest/patchwork.yaml > @@ -1039,6 +1039,188 @@ paths: > $ref: '#/components/schemas/Error' > tags: > - series > + /api/relations/: > + get: > + description: List relations. > + operationId: relations_list > + parameters: > + - $ref: '#/components/parameters/Page' > + - $ref: '#/components/parameters/PageSize' > + - $ref: '#/components/parameters/Order' > + responses: > + '200': > + description: '' > + headers: > + Link: > + $ref: '#/components/headers/Link' > + content: > + application/json: > + schema: > + type: array > + items: > + $ref: '#/components/schemas/Relation' > + tags: > + - relations > + post: > + description: Create a relation. > + operationId: relations_create > + security: > + - basicAuth: [] > + - apiKeyAuth: [] > + requestBody: > + $ref: '#/components/requestBodies/Relation' > + responses: > + '201': > + description: '' > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Relation' > + '400': > + description: Invalid Request > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '403': > + description: Forbidden > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '404': > + description: Not found > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '409': > + description: Conflict > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + tags: > + - checks > + /api/relations/{id}/: > + get: > + description: Show a relation. > + operationId: relation_read > + parameters: > + - in: path > + name: id > + description: A unique integer value identifying this relation. > + required: true > + schema: > + title: ID > + type: integer > + responses: > + '200': > + description: '' > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Relation' > + '404': > + description: Not found > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '409': > + description: Conflict > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + tags: > + - relations > + patch: > + description: Update a relation (partial). > + operationId: relations_partial_update > + security: > + - basicAuth: [] > + - apiKeyAuth: [] > + parameters: > + - in: path > + name: id > + description: A unique integer value identifying this relation. > + required: true > + schema: > + title: ID > + type: integer > + requestBody: > + $ref: '#/components/requestBodies/Relation' > + responses: > + '200': > + description: '' > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Relation' > + '400': > + description: Bad request > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '403': > + description: Forbidden > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '404': > + description: Not found > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + tags: > + - relations > + put: > + description: Update a relation. > + operationId: relations_update > + security: > + - basicAuth: [] > + - apiKeyAuth: [] > + parameters: > + - in: path > + name: id > + description: A unique integer value identifying this relation. > + required: true > + schema: > + title: ID > + type: integer > + requestBody: > + $ref: '#/components/requestBodies/Relation' > + responses: > + '200': > + description: '' > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Relation' > + '400': > + description: Bad request > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '403': > + description: Forbidden > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '404': > + description: Not found > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + tags: > + - relations > /api/users/: > get: > description: List users. > @@ -1314,6 +1496,18 @@ components: > application/x-www-form-urlencoded: > schema: > $ref: '#/components/schemas/User' > + Relation: > + required: true > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/RelationUpdate' > + multipart/form-data: > + schema: > + $ref: '#/components/schemas/RelationUpdate' > + application/x-www-form-urlencoded: > + schema: > + $ref: '#/components/schemas/RelationUpdate' > schemas: > Index: > type: object > @@ -1358,6 +1552,11 @@ components: > type: string > format: uri > readOnly: true > + relations: > + title: Relations URL > + type: string > + format: uri > + readOnly: true > Bundle: > required: > - name > @@ -1943,6 +2142,14 @@ components: > title: Delegate > type: integer > nullable: true > + RelationUpdate: > + type: object > + properties: > + submissions: > + title: Submission IDs > + type: array > + items: > + type: integer > Person: > type: object > properties: > @@ -2133,6 +2340,30 @@ components: > $ref: '#/components/schemas/PatchEmbedded' > readOnly: true > uniqueItems: true > + Relation: > + type: object > + properties: > + id: > + title: ID > + type: integer > + url: > + title: URL > + type: string > + format: uri > + readOnly: true > + by: > + type: object > + title: By > + readOnly: true > + allOf: > + - $ref: '#/components/schemas/UserEmbedded' > + submissions: > + title: Submissions > + type: array > + items: > + $ref: '#/components/schemas/SubmissionEmbedded' > + readOnly: true > + uniqueItems: true > User: > type: object > properties: > @@ -2211,6 +2442,48 @@ components: > maxLength: 255 > minLength: 1 > readOnly: true > + SubmissionEmbedded: > + type: object > + properties: > + id: > + title: ID > + type: integer > + readOnly: true > + url: > + title: URL > + type: string > + format: uri > + readOnly: true > + web_url: > + title: Web URL > + type: string > + format: uri > + readOnly: true > + msgid: > + title: Message ID > + type: string > + readOnly: true > + minLength: 1 > + list_archive_url: > + title: List archive URL > + type: string > + readOnly: true > + nullable: true > + date: > + title: Date > + type: string > + format: iso8601 > + readOnly: true > + name: > + title: Name > + type: string > + readOnly: true > + minLength: 1 > + mbox: > + title: Mbox > + type: string > + format: uri > + readOnly: true > CoverLetterEmbedded: > type: object > properties: > diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2 > index 196d78466b55..a034029accf9 100644 > --- a/docs/api/schemas/patchwork.j2 > +++ b/docs/api/schemas/patchwork.j2 > @@ -1048,6 +1048,190 @@ paths: > $ref: '#/components/schemas/Error' > tags: > - series > +{% if version >= (1, 2) %} > + /api/{{ version_url }}relations/: > + get: > + description: List relations. > + operationId: relations_list > + parameters: > + - $ref: '#/components/parameters/Page' > + - $ref: '#/components/parameters/PageSize' > + - $ref: '#/components/parameters/Order' > + responses: > + '200': > + description: '' > + headers: > + Link: > + $ref: '#/components/headers/Link' > + content: > + application/json: > + schema: > + type: array > + items: > + $ref: '#/components/schemas/Relation' > + tags: > + - relations > + post: > + description: Create a relation. > + operationId: relations_create > + security: > + - basicAuth: [] > + - apiKeyAuth: [] > + requestBody: > + $ref: '#/components/requestBodies/Relation' > + responses: > + '201': > + description: '' > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Relation' > + '400': > + description: Invalid Request > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '403': > + description: Forbidden > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '404': > + description: Not found > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '409': > + description: Conflict > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + tags: > + - checks > + /api/{{ version_url }}relations/{id}/: > + get: > + description: Show a relation. > + operationId: relation_read > + parameters: > + - in: path > + name: id > + description: A unique integer value identifying this relation. > + required: true > + schema: > + title: ID > + type: integer > + responses: > + '200': > + description: '' > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Relation' > + '404': > + description: Not found > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '409': > + description: Conflict > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + tags: > + - relations > + patch: > + description: Update a relation (partial). > + operationId: relations_partial_update > + security: > + - basicAuth: [] > + - apiKeyAuth: [] > + parameters: > + - in: path > + name: id > + description: A unique integer value identifying this relation. > + required: true > + schema: > + title: ID > + type: integer > + requestBody: > + $ref: '#/components/requestBodies/Relation' > + responses: > + '200': > + description: '' > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Relation' > + '400': > + description: Bad request > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '403': > + description: Forbidden > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '404': > + description: Not found > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + tags: > + - relations > + put: > + description: Update a relation. > + operationId: relations_update > + security: > + - basicAuth: [] > + - apiKeyAuth: [] > + parameters: > + - in: path > + name: id > + description: A unique integer value identifying this relation. > + required: true > + schema: > + title: ID > + type: integer > + requestBody: > + $ref: '#/components/requestBodies/Relation' > + responses: > + '200': > + description: '' > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Relation' > + '400': > + description: Bad request > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '403': > + description: Forbidden > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '404': > + description: Not found > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + tags: > + - relations > +{% endif %} > /api/{{ version_url }}users/: > get: > description: List users. > @@ -1325,6 +1509,20 @@ components: > application/x-www-form-urlencoded: > schema: > $ref: '#/components/schemas/User' > +{% if version >= (1, 2) %} > + Relation: > + required: true > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/RelationUpdate' > + multipart/form-data: > + schema: > + $ref: '#/components/schemas/RelationUpdate' > + application/x-www-form-urlencoded: > + schema: > + $ref: '#/components/schemas/RelationUpdate' > +{% endif %} > schemas: > Index: > type: object > @@ -1369,6 +1567,13 @@ components: > type: string > format: uri > readOnly: true > +{% if version >= (1, 2) %} > + relations: > + title: Relations URL > + type: string > + format: uri > + readOnly: true > +{% endif %} > Bundle: > required: > - name > @@ -1981,6 +2186,16 @@ components: > title: Delegate > type: integer > nullable: true > +{% if version >= (1, 2) %} > + RelationUpdate: > + type: object > + properties: > + submissions: > + title: Submission IDs > + type: array > + items: > + type: integer > +{% endif %} > Person: > type: object > properties: > @@ -2177,6 +2392,32 @@ components: > $ref: '#/components/schemas/PatchEmbedded' > readOnly: true > uniqueItems: true > +{% if version >= (1, 2) %} > + Relation: > + type: object > + properties: > + id: > + title: ID > + type: integer > + url: > + title: URL > + type: string > + format: uri > + readOnly: true > + by: > + type: object > + title: By > + readOnly: true > + allOf: > + - $ref: '#/components/schemas/UserEmbedded' > + submissions: > + title: Submissions > + type: array > + items: > + $ref: '#/components/schemas/SubmissionEmbedded' > + readOnly: true > + uniqueItems: true > +{% endif %} > User: > type: object > properties: > @@ -2255,6 +2496,50 @@ components: > maxLength: 255 > minLength: 1 > readOnly: true > +{% if version >= (1, 2) %} > + SubmissionEmbedded: > + type: object > + properties: > + id: > + title: ID > + type: integer > + readOnly: true > + url: > + title: URL > + type: string > + format: uri > + readOnly: true > + web_url: > + title: Web URL > + type: string > + format: uri > + readOnly: true > + msgid: > + title: Message ID > + type: string > + readOnly: true > + minLength: 1 > + list_archive_url: > + title: List archive URL > + type: string > + readOnly: true > + nullable: true > + date: > + title: Date > + type: string > + format: iso8601 > + readOnly: true > + name: > + title: Name > + type: string > + readOnly: true > + minLength: 1 > + mbox: > + title: Mbox > + type: string > + format: uri > + readOnly: true > +{% endif %} > CoverLetterEmbedded: > type: object > properties: > diff --git a/docs/api/schemas/v1.2/patchwork.yaml > b/docs/api/schemas/v1.2/patchwork.yaml > index d7b4d2957cff..99425e968881 100644 > --- a/docs/api/schemas/v1.2/patchwork.yaml > +++ b/docs/api/schemas/v1.2/patchwork.yaml > @@ -1039,6 +1039,188 @@ paths: > $ref: '#/components/schemas/Error' > tags: > - series > + /api/1.2/relations/: > + get: > + description: List relations. > + operationId: relations_list > + parameters: > + - $ref: '#/components/parameters/Page' > + - $ref: '#/components/parameters/PageSize' > + - $ref: '#/components/parameters/Order' > + responses: > + '200': > + description: '' > + headers: > + Link: > + $ref: '#/components/headers/Link' > + content: > + application/json: > + schema: > + type: array > + items: > + $ref: '#/components/schemas/Relation' > + tags: > + - relations > + post: > + description: Create a relation. > + operationId: relations_create > + security: > + - basicAuth: [] > + - apiKeyAuth: [] > + requestBody: > + $ref: '#/components/requestBodies/Relation' > + responses: > + '201': > + description: '' > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Relation' > + '400': > + description: Invalid Request > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '403': > + description: Forbidden > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '404': > + description: Not found > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '409': > + description: Conflict > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + tags: > + - checks > + /api/1.2/relations/{id}/: > + get: > + description: Show a relation. > + operationId: relation_read > + parameters: > + - in: path > + name: id > + description: A unique integer value identifying this relation. > + required: true > + schema: > + title: ID > + type: integer > + responses: > + '200': > + description: '' > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Relation' > + '404': > + description: Not found > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '409': > + description: Conflict > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + tags: > + - relations > + patch: > + description: Update a relation (partial). > + operationId: relations_partial_update > + security: > + - basicAuth: [] > + - apiKeyAuth: [] > + parameters: > + - in: path > + name: id > + description: A unique integer value identifying this relation. > + required: true > + schema: > + title: ID > + type: integer > + requestBody: > + $ref: '#/components/requestBodies/Relation' > + responses: > + '200': > + description: '' > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Relation' > + '400': > + description: Bad request > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '403': > + description: Forbidden > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '404': > + description: Not found > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + tags: > + - relations > + put: > + description: Update a relation. > + operationId: relations_update > + security: > + - basicAuth: [] > + - apiKeyAuth: [] > + parameters: > + - in: path > + name: id > + description: A unique integer value identifying this relation. > + required: true > + schema: > + title: ID > + type: integer > + requestBody: > + $ref: '#/components/requestBodies/Relation' > + responses: > + '200': > + description: '' > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Relation' > + '400': > + description: Bad request > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '403': > + description: Forbidden > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + '404': > + description: Not found > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/Error' > + tags: > + - relations > /api/1.2/users/: > get: > description: List users. > @@ -1314,6 +1496,18 @@ components: > application/x-www-form-urlencoded: > schema: > $ref: '#/components/schemas/User' > + Relation: > + required: true > + content: > + application/json: > + schema: > + $ref: '#/components/schemas/RelationUpdate' > + multipart/form-data: > + schema: > + $ref: '#/components/schemas/RelationUpdate' > + application/x-www-form-urlencoded: > + schema: > + $ref: '#/components/schemas/RelationUpdate' > schemas: > Index: > type: object > @@ -1358,6 +1552,11 @@ components: > type: string > format: uri > readOnly: true > + relations: > + title: Relations URL > + type: string > + format: uri > + readOnly: true > Bundle: > required: > - name > @@ -1943,6 +2142,14 @@ components: > title: Delegate > type: integer > nullable: true > + RelationUpdate: > + type: object > + properties: > + submissions: > + title: Submission IDs > + type: array > + items: > + type: integer > Person: > type: object > properties: > @@ -2133,6 +2340,30 @@ components: > $ref: '#/components/schemas/PatchEmbedded' > readOnly: true > uniqueItems: true > + Relation: > + type: object > + properties: > + id: > + title: ID > + type: integer > + url: > + title: URL > + type: string > + format: uri > + readOnly: true > + by: > + type: object > + title: By > + readOnly: true > + allOf: > + - $ref: '#/components/schemas/UserEmbedded' > + submissions: > + title: Submissions > + type: array > + items: > + $ref: '#/components/schemas/SubmissionEmbedded' > + readOnly: true > + uniqueItems: true > User: > type: object > properties: > @@ -2211,6 +2442,48 @@ components: > maxLength: 255 > minLength: 1 > readOnly: true > + SubmissionEmbedded: > + type: object > + properties: > + id: > + title: ID > + type: integer > + readOnly: true > + url: > + title: URL > + type: string > + format: uri > + readOnly: true > + web_url: > + title: Web URL > + type: string > + format: uri > + readOnly: true > + msgid: > + title: Message ID > + type: string > + readOnly: true > + minLength: 1 > + list_archive_url: > + title: List archive URL > + type: string > + readOnly: true > + nullable: true > + date: > + title: Date > + type: string > + format: iso8601 > + readOnly: true > + name: > + title: Name > + type: string > + readOnly: true > + minLength: 1 > + mbox: > + title: Mbox > + type: string > + format: uri > + readOnly: true > CoverLetterEmbedded: > type: object > properties: > diff --git a/patchwork/api/embedded.py b/patchwork/api/embedded.py > index de4f31165ee7..0fba291b62b8 100644 > --- a/patchwork/api/embedded.py > +++ b/patchwork/api/embedded.py > @@ -102,6 +102,45 @@ class CheckSerializer(SerializedRelatedField): > } > > > +def _upgrade_instance(instance): > + if hasattr(instance, 'patch'): > + return instance.patch > + else: > + return instance.coverletter > + > + > +class SubmissionSerializer(SerializedRelatedField): > + > + class _Serializer(BaseHyperlinkedModelSerializer): > + """We need to 'upgrade' or specialise the submission to the relevant > + subclass, so we can't use the mixins. This is gross but can go away > + once we flatten the models.""" > + url = SerializerMethodField() > + web_url = SerializerMethodField() > + mbox = SerializerMethodField() > + > + def get_url(self, instance): > + instance = _upgrade_instance(instance) > + request = self.context.get('request') > + return > request.build_absolute_uri(instance.get_absolute_api_url()) > + > + def get_web_url(self, instance): > + instance = _upgrade_instance(instance) > + request = self.context.get('request') > + return request.build_absolute_uri(instance.get_absolute_url()) > + > + def get_mbox(self, instance): > + instance = _upgrade_instance(instance) > + request = self.context.get('request') > + return request.build_absolute_uri(instance.get_mbox_url()) > + > + class Meta: > + model = models.Submission > + fields = ('id', 'url', 'web_url', 'msgid', 'list_archive_url', > + 'date', 'name', 'mbox') > + read_only_fields = fields > + > + > class CoverLetterSerializer(SerializedRelatedField): > > class _Serializer(MboxMixin, WebURLMixin, > BaseHyperlinkedModelSerializer): > diff --git a/patchwork/api/index.py b/patchwork/api/index.py > index 45485c9106f6..cf1845393835 100644 > --- a/patchwork/api/index.py > +++ b/patchwork/api/index.py > @@ -21,4 +21,5 @@ class IndexView(APIView): > 'series': reverse('api-series-list', request=request), > 'events': reverse('api-event-list', request=request), > 'bundles': reverse('api-bundle-list', request=request), > + 'relations': reverse('api-relation-list', request=request), > }) > diff --git a/patchwork/api/relation.py b/patchwork/api/relation.py > new file mode 100644 > index 000000000000..37640d62e9cc > --- /dev/null > +++ b/patchwork/api/relation.py > @@ -0,0 +1,121 @@ > +# Patchwork - automated patch tracking system > +# Copyright (C) 2019, Bayerische Motoren Werke Aktiengesellschaft (BMW AG) > +# > +# SPDX-License-Identifier: GPL-2.0-or-later > + > +from rest_framework import permissions > +from rest_framework import status > +from rest_framework.exceptions import PermissionDenied, APIException > +from rest_framework.generics import GenericAPIView > +from rest_framework.generics import ListCreateAPIView > +from rest_framework.generics import RetrieveUpdateDestroyAPIView > +from rest_framework.serializers import ModelSerializer > + > +from patchwork.api.base import PatchworkPermission > +from patchwork.api.embedded import SubmissionSerializer > +from patchwork.api.embedded import UserSerializer > +from patchwork.models import SubmissionRelation > + > + > +class MaintainerPermission(PatchworkPermission): > + > + def has_permission(self, request, view): > + if request.method in permissions.SAFE_METHODS: > + return True > + > + # Prevent showing an HTML POST form in the browseable API for logged > in > + # users who are not maintainers. > + return len(request.user.maintains) > 0 > + > + def has_object_permission(self, request, view, relation): > + if request.method in permissions.SAFE_METHODS: > + return True > + > + maintains = request.user.maintains > + submissions = relation.submissions.all() > + # user has to be maintainer of every project a submission is part of > + return self.check_user_maintains_all(maintains, submissions) > + > + @staticmethod > + def check_user_maintains_all(maintains, submissions): > + if any(s.project not in maintains for s in submissions): > + detail = 'At least one submission is part of a project you are ' > \ > + 'not maintaining.' > + raise PermissionDenied(detail=detail) > + return True > + > + > +class SubmissionConflict(APIException): > + status_code = status.HTTP_409_CONFLICT > + default_detail = 'At least one submission is already part of another ' \ > + 'relation. You have to explicitly remove a submission ' > \ > + 'from its existing relation before moving it to this > one.' > + > + > +class SubmissionRelationSerializer(ModelSerializer): > + by = UserSerializer(read_only=True) > + submissions = SubmissionSerializer(many=True) > + > + def create(self, validated_data): > + submissions = validated_data['submissions'] > + if any(submission.related_id is not None > + for submission in submissions): > + raise SubmissionConflict() > + return super(SubmissionRelationSerializer, > self).create(validated_data) > + > + def update(self, instance, validated_data): > + submissions = validated_data['submissions'] > + if any(submission.related_id is not None and > + submission.related_id != instance.id > + for submission in submissions): > + raise SubmissionConflict() > + return super(SubmissionRelationSerializer, self) \ > + .update(instance, validated_data) > + > + class Meta: > + model = SubmissionRelation > + fields = ('id', 'url', 'by', 'submissions',) > + read_only_fields = ('url', 'by', ) > + extra_kwargs = { > + 'url': {'view_name': 'api-relation-detail'}, > + } > + > + > +class SubmissionRelationMixin(GenericAPIView): > + serializer_class = SubmissionRelationSerializer > + permission_classes = (MaintainerPermission,) > + > + def initial(self, request, *args, **kwargs): > + user = request.user > + if not hasattr(user, 'maintains'): > + if user.is_authenticated: > + user.maintains = user.profile.maintainer_projects.all() > + else: > + user.maintains = [] > + super(SubmissionRelationMixin, self).initial(request, *args, > **kwargs) > + > + def get_queryset(self): > + return SubmissionRelation.objects.all() \ > + .select_related('by') \ > + .prefetch_related('submissions__patch', > + 'submissions__coverletter', > + 'submissions__project') > + > + > +class SubmissionRelationList(SubmissionRelationMixin, ListCreateAPIView): > + ordering = 'id' > + ordering_fields = ['id'] > + > + def perform_create(self, serializer): > + # has_object_permission() is not called when creating a new relation. > + # Check whether user is maintainer of every project a submission is > + # part of > + maintains = self.request.user.maintains > + submissions = serializer.validated_data['submissions'] > + MaintainerPermission.check_user_maintains_all(maintains, submissions) > + serializer.save(by=self.request.user) > + > + > +class SubmissionRelationDetail(SubmissionRelationMixin, > + RetrieveUpdateDestroyAPIView): > + pass > diff --git a/patchwork/models.py b/patchwork/models.py > index a92203b24ff2..9ae3370e896b 100644 > --- a/patchwork/models.py > +++ b/patchwork/models.py > @@ -415,6 +415,9 @@ class CoverLetter(Submission): > kwargs={'project_id': self.project.linkname, > 'msgid': self.url_msgid}) > > + def get_absolute_api_url(self): > + return reverse('api-cover-detail', kwargs={'pk': self.id}) > + > def get_mbox_url(self): > return reverse('cover-mbox', > kwargs={'project_id': self.project.linkname, > @@ -604,6 +607,9 @@ class Patch(Submission): > kwargs={'project_id': self.project.linkname, > 'msgid': self.url_msgid}) > > + def get_absolute_api_url(self): > + return reverse('api-patch-detail', kwargs={'pk': self.id}) > + > def get_mbox_url(self): > return reverse('patch-mbox', > kwargs={'project_id': self.project.linkname, > diff --git a/patchwork/tests/api/test_relation.py > b/patchwork/tests/api/test_relation.py > new file mode 100644 > index 000000000000..5b1a04f13670 > --- /dev/null > +++ b/patchwork/tests/api/test_relation.py > @@ -0,0 +1,181 @@ > +# Patchwork - automated patch tracking system > +# Copyright (C) 2019, Bayerische Motoren Werke Aktiengesellschaft (BMW AG) > +# > +# SPDX-License-Identifier: GPL-2.0-or-later > + > +import unittest > + > +import six > +from django.conf import settings > +from django.urls import reverse > + > +from patchwork.tests.api import utils > +from patchwork.tests.utils import create_cover > +from patchwork.tests.utils import create_maintainer > +from patchwork.tests.utils import create_patches > +from patchwork.tests.utils import create_project > +from patchwork.tests.utils import create_relation > +from patchwork.tests.utils import create_user > + > +if settings.ENABLE_REST_API: > + from rest_framework import status > + > + > +class UserType: > + ANONYMOUS = 1 > + NON_MAINTAINER = 2 > + MAINTAINER = 3 > + > + > +@unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API') > +class TestRelationAPI(utils.APITestCase): > + fixtures = ['default_tags'] > + > + @staticmethod > + def api_url(item=None): > + kwargs = {} > + if item is None: > + return reverse('api-relation-list', kwargs=kwargs) > + kwargs['pk'] = item > + return reverse('api-relation-detail', kwargs=kwargs) > + > + def request_restricted(self, method, user_type): > + """Assert post/delete/patch requests on the relation API.""" > + assert method in ['post', 'delete', 'patch'] > + > + # setup > + > + project = create_project() > + maintainer = create_maintainer(project) > + > + if user_type == UserType.ANONYMOUS: > + expected_status = status.HTTP_403_FORBIDDEN > + elif user_type == UserType.NON_MAINTAINER: > + expected_status = status.HTTP_403_FORBIDDEN > + self.client.force_authenticate(user=create_user()) > + elif user_type == UserType.MAINTAINER: > + if method == 'post': > + expected_status = status.HTTP_201_CREATED > + elif method == 'delete': > + expected_status = status.HTTP_204_NO_CONTENT > + else: > + expected_status = status.HTTP_200_OK > + self.client.force_authenticate(user=maintainer) > + else: > + raise ValueError > + > + resource_id = None > + req = None > + > + if method == 'delete': > + resource_id = create_relation(project=project, by=maintainer).id > + elif method == 'post': > + patch_ids = [p.id for p in create_patches(2, project=project)] > + req = {'submissions': patch_ids} > + elif method == 'patch': > + resource_id = create_relation(project=project, by=maintainer).id > + patch_ids = [p.id for p in create_patches(2, project=project)] > + req = {'submissions': patch_ids} > + else: > + raise ValueError > + > + # request > + > + resp = getattr(self.client, method)(self.api_url(resource_id), req) > + > + # check > + > + self.assertEqual(expected_status, resp.status_code) > + > + if resp.status_code in range(status.HTTP_200_OK, > + status.HTTP_204_NO_CONTENT): > + self.assertRequest(req, resp.data) > + > + def assertRequest(self, request, resp): > + if request.get('id'): > + self.assertEqual(request['id'], resp['id']) > + send_ids = request['submissions'] > + resp_ids = [s['id'] for s in resp['submissions']] > + six.assertCountEqual(self, resp_ids, send_ids) > + > + def assertSerialized(self, obj, resp): > + self.assertEqual(obj.id, resp['id']) > + exp_ids = [s.id for s in obj.submissions.all()] > + act_ids = [s['id'] for s in resp['submissions']] > + six.assertCountEqual(self, exp_ids, act_ids) > + > + def test_list_empty(self): > + """List relation when none are present.""" > + resp = self.client.get(self.api_url()) > + self.assertEqual(status.HTTP_200_OK, resp.status_code) > + self.assertEqual(0, len(resp.data)) > + > + @utils.store_samples('relation-list') > + def test_list(self): > + """List relations.""" > + relation = create_relation() > + > + resp = self.client.get(self.api_url()) > + self.assertEqual(status.HTTP_200_OK, resp.status_code) > + self.assertEqual(1, len(resp.data)) > + self.assertSerialized(relation, resp.data[0]) > + > + def test_detail(self): > + """Show relation.""" > + relation = create_relation() > + > + resp = self.client.get(self.api_url(relation.id)) > + self.assertEqual(status.HTTP_200_OK, resp.status_code) > + self.assertSerialized(relation, resp.data) > + > + @utils.store_samples('relation-create-error-forbidden') > + def test_create_anonymous(self): > + self.request_restricted('post', UserType.ANONYMOUS) > + > + def test_create_non_maintainer(self): > + self.request_restricted('post', UserType.NON_MAINTAINER) > + > + @utils.store_samples('relation-create') > + def test_create_maintainer(self): > + self.request_restricted('post', UserType.MAINTAINER) > + > + @utils.store_samples('relation-update-error-forbidden') > + def test_update_anonymous(self): > + self.request_restricted('patch', UserType.ANONYMOUS) > + > + def test_update_non_maintainer(self): > + self.request_restricted('patch', UserType.NON_MAINTAINER) > + > + @utils.store_samples('relation-update') > + def test_update_maintainer(self): > + self.request_restricted('patch', UserType.MAINTAINER) > + > + @utils.store_samples('relation-delete-error-forbidden') > + def test_delete_anonymous(self): > + self.request_restricted('delete', UserType.ANONYMOUS) > + > + def test_delete_non_maintainer(self): > + self.request_restricted('delete', UserType.NON_MAINTAINER) > + > + @utils.store_samples('relation-update') > + def test_delete_maintainer(self): > + self.request_restricted('delete', UserType.MAINTAINER) > + > + def test_submission_conflict(self): > + project = create_project() > + maintainer = create_maintainer(project) > + self.client.force_authenticate(user=maintainer) > + relation = create_relation(by=maintainer, project=project) > + submission_ids = [s.id for s in relation.submissions.all()] > + > + # try to create a new relation with a new submission (cover) and > + # submissions already bound to another relation > + cover = create_cover(project=project) > + submission_ids.append(cover.id) > + req = {'submissions': submission_ids} > + resp = self.client.post(self.api_url(), req) > + self.assertEqual(status.HTTP_409_CONFLICT, resp.status_code) > + > + # try to patch relation > + resp = self.client.patch(self.api_url(relation.id), req) > + self.assertEqual(status.HTTP_200_OK, resp.status_code) > diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py > index 577183d0986c..ffe90976233e 100644 > --- a/patchwork/tests/utils.py > +++ b/patchwork/tests/utils.py > @@ -16,6 +16,7 @@ from patchwork.models import Check > from patchwork.models import Comment > from patchwork.models import CoverLetter > from patchwork.models import Patch > +from patchwork.models import SubmissionRelation > from patchwork.models import Person > from patchwork.models import Project > from patchwork.models import Series > @@ -347,3 +348,17 @@ def create_covers(count=1, **kwargs): > kwargs (dict): Overrides for various cover letter fields > """ > return _create_submissions(create_cover, count, **kwargs) > + > + > +def create_relation(count_patches=2, by=None, **kwargs): > + if not by: > + project = create_project() > + kwargs['project'] = project > + by = create_maintainer(project) > + relation = SubmissionRelation.objects.create(by=by) > + values = { > + 'related': relation > + } > + values.update(kwargs) > + create_patches(count_patches, **values) > + return relation > diff --git a/patchwork/urls.py b/patchwork/urls.py > index dcdcfb49e67e..92095f62c7b9 100644 > --- a/patchwork/urls.py > +++ b/patchwork/urls.py > @@ -187,6 +187,7 @@ if settings.ENABLE_REST_API: > from patchwork.api import patch as api_patch_views # noqa > from patchwork.api import person as api_person_views # noqa > from patchwork.api import project as api_project_views # noqa > + from patchwork.api import relation as api_relation_views # noqa > from patchwork.api import series as api_series_views # noqa > from patchwork.api import user as api_user_views # noqa > > @@ -256,9 +257,19 @@ if settings.ENABLE_REST_API: > name='api-cover-comment-list'), > ] > > + api_1_2_patterns = [ > + url(r'^relations/$', > + api_relation_views.SubmissionRelationList.as_view(), > + name='api-relation-list'), > + url(r'^relations/(?P<pk>[^/]+)/$', > + api_relation_views.SubmissionRelationDetail.as_view(), > + name='api-relation-detail'), > + ] > + > urlpatterns += [ > url(r'^api/(?:(?P<version>(1.0|1.1|1.2))/)?', include(api_patterns)), > url(r'^api/(?:(?P<version>(1.1|1.2))/)?', include(api_1_1_patterns)), > + url(r'^api/(?:(?P<version>1.2)/)?', include(api_1_2_patterns)), > > # token change > url(r'^user/generate-token/$', user_views.generate_token, > diff --git > a/releasenotes/notes/add-submission-relations-c96bb6c567b416d8.yaml > b/releasenotes/notes/add-submission-relations-c96bb6c567b416d8.yaml > new file mode 100644 > index 000000000000..cb877991cd55 > --- /dev/null > +++ b/releasenotes/notes/add-submission-relations-c96bb6c567b416d8.yaml > @@ -0,0 +1,10 @@ > +--- > +features: > + - | > + Submissions (cover letters or patches) can now be related to other ones > + (e.g. revisions). Relations can be set via the REST API by maintainers > + (currently only for submissions of projects they maintain) > +api: > + - | > + Relations are available via ``/relations/`` and > + ``/relations/{relationID}/`` endpoints. _______________________________________________ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork