From fe07f30df6fe263938b1f898ffffc354c4ff470c Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 1 Oct 2020 14:38:31 +0100 Subject: Remove 'PatchRelationSerializer' This wasn't writeable for reasons I haven't been able to figure out. However, it's not actually needed: the 'PatchSerializer' can do the job just fine, given enough information. This exposes a bug in DRF, which has been reported upstream [1]. While we wait for that fix, or some variant of it, to be merged, we must monkey patch the library. [1] https://github.com/encode/django-rest-framework/issues/7550 [2] https://github.com/encode/django-rest-framework/pull/7574 Signed-off-by: Stephen Finucane Reported-by: Ralf Ramsauer Closes: #379 Cc: Daniel Axtens Cc: Rohit Sarkar --- patchwork/api/__init__.py | 50 +++++++++++++++++++++++++++++++++++++++++++++++ patchwork/api/embedded.py | 28 +------------------------- patchwork/api/event.py | 7 ++++--- patchwork/api/patch.py | 22 ++++++++++----------- 4 files changed, 65 insertions(+), 42 deletions(-) diff --git a/patchwork/api/__init__.py b/patchwork/api/__init__.py index e69de29..efc0dd8 100644 --- a/patchwork/api/__init__.py +++ b/patchwork/api/__init__.py @@ -0,0 +1,50 @@ +# Patchwork - automated patch tracking system +# Copyright (C) 2020, Stephen Finucane +# +# SPDX-License-Identifier: GPL-2.0-or-later + +from rest_framework.fields import empty +from rest_framework.fields import get_attribute +from rest_framework.fields import SkipField +from rest_framework.relations import ManyRelatedField + + +# monkey patch django-rest-framework to work around issue #7550 [1] until #7574 +# [2] or some other variant lands +# +# [1] https://github.com/encode/django-rest-framework/issues/7550 +# [2] https://github.com/encode/django-rest-framework/pull/7574 + +def _get_attribute(self, instance): + # Can't have any relationships if not created + if hasattr(instance, 'pk') and instance.pk is None: + return [] + + try: + relationship = get_attribute(instance, self.source_attrs) + except (KeyError, AttributeError) as exc: + if self.default is not empty: + return self.get_default() + if self.allow_null: + return None + if not self.required: + raise SkipField() + msg = ( + 'Got {exc_type} when attempting to get a value for field ' + '`{field}` on serializer `{serializer}`.\nThe serializer ' + 'field might be named incorrectly and not match ' + 'any attribute or key on the `{instance}` instance.\n' + 'Original exception text was: {exc}.'.format( + exc_type=type(exc).__name__, + field=self.field_name, + serializer=self.parent.__class__.__name__, + instance=instance.__class__.__name__, + exc=exc + ) + ) + raise type(exc)(msg) + + return relationship.all() if hasattr(relationship, 'all') else relationship + + +ManyRelatedField.get_attribute = _get_attribute diff --git a/patchwork/api/embedded.py b/patchwork/api/embedded.py index 3f32bd4..7831697 100644 --- a/patchwork/api/embedded.py +++ b/patchwork/api/embedded.py @@ -12,9 +12,8 @@ nested fields. from collections import OrderedDict from rest_framework.serializers import CharField -from rest_framework.serializers import SerializerMethodField from rest_framework.serializers import PrimaryKeyRelatedField -from rest_framework.serializers import ValidationError +from rest_framework.serializers import SerializerMethodField from patchwork.api.base import BaseHyperlinkedModelSerializer from patchwork.api.base import CheckHyperlinkedIdentityField @@ -139,31 +138,6 @@ class PatchSerializer(SerializedRelatedField): } -class PatchRelationSerializer(BaseHyperlinkedModelSerializer): - """Hide the PatchRelation model, just show the list""" - patches = PatchSerializer(many=True, - style={'base_template': 'input.html'}) - - def to_internal_value(self, data): - if not isinstance(data, type([])): - raise ValidationError( - "Patch relations must be specified as a list of patch IDs" - ) - result = super(PatchRelationSerializer, self).to_internal_value( - {'patches': data} - ) - return result - - def to_representation(self, instance): - data = super(PatchRelationSerializer, self).to_representation(instance) - data = data['patches'] - return data - - class Meta: - model = models.PatchRelation - fields = ('patches',) - - class PersonSerializer(SerializedRelatedField): class _Serializer(BaseHyperlinkedModelSerializer): diff --git a/patchwork/api/event.py b/patchwork/api/event.py index 7ed9efb..75bf870 100644 --- a/patchwork/api/event.py +++ b/patchwork/api/event.py @@ -13,7 +13,6 @@ from rest_framework.serializers import SlugRelatedField from patchwork.api.embedded import CheckSerializer from patchwork.api.embedded import CoverSerializer from patchwork.api.embedded import PatchSerializer -from patchwork.api.embedded import PatchRelationSerializer from patchwork.api.embedded import ProjectSerializer from patchwork.api.embedded import SeriesSerializer from patchwork.api.embedded import UserSerializer @@ -34,8 +33,10 @@ class EventSerializer(ModelSerializer): current_delegate = UserSerializer() created_check = SerializerMethodField() created_check = CheckSerializer() - previous_relation = PatchRelationSerializer(read_only=True) - current_relation = PatchRelationSerializer(read_only=True) + previous_relation = PatchSerializer( + source='previous_relation.patches', many=True, default=None) + current_relation = PatchSerializer( + source='current_relation.patches', many=True, default=None) _category_map = { Event.CATEGORY_COVER_CREATED: ['cover'], diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py index d881504..f6cb276 100644 --- a/patchwork/api/patch.py +++ b/patchwork/api/patch.py @@ -10,7 +10,6 @@ import email.parser from django.core.exceptions import ValidationError from django.utils.text import slugify from django.utils.translation import gettext_lazy as _ -from rest_framework import status from rest_framework.exceptions import APIException from rest_framework.exceptions import PermissionDenied from rest_framework.generics import ListAPIView @@ -18,15 +17,16 @@ from rest_framework.generics import RetrieveUpdateAPIView from rest_framework.relations import RelatedField from rest_framework.reverse import reverse from rest_framework.serializers import SerializerMethodField +from rest_framework import status from patchwork.api.base import BaseHyperlinkedModelSerializer from patchwork.api.base import PatchworkPermission -from patchwork.api.filters import PatchFilterSet -from patchwork.api.embedded import PatchRelationSerializer +from patchwork.api.embedded import PatchSerializer from patchwork.api.embedded import PersonSerializer from patchwork.api.embedded import ProjectSerializer from patchwork.api.embedded import SeriesSerializer from patchwork.api.embedded import UserSerializer +from patchwork.api.filters import PatchFilterSet from patchwork.models import Patch from patchwork.models import PatchRelation from patchwork.models import State @@ -83,7 +83,8 @@ class PatchListSerializer(BaseHyperlinkedModelSerializer): check = SerializerMethodField() checks = SerializerMethodField() tags = SerializerMethodField() - related = PatchRelationSerializer() + related = PatchSerializer( + source='related.patches', many=True, default=[]) def get_web_url(self, instance): request = self.context.get('request') @@ -127,14 +128,11 @@ class PatchListSerializer(BaseHyperlinkedModelSerializer): data = super(PatchListSerializer, self).to_representation(instance) data['series'] = [data['series']] if data['series'] else [] - # stop the related serializer returning this patch in the list of - # related patches. Also make it return an empty list, not null/None - if 'related' in data: - if data['related']: - data['related'] = [p for p in data['related'] - if p['id'] != instance.id] - else: - data['related'] = [] + # Remove this patch from 'related' + if 'related' in data and data['related']: + data['related'] = [ + x for x in data['related'] if x['id'] != data['id'] + ] return data -- cgit v1.2.3