[PATCH 3/3] Remove 'PatchRelationSerializer'
Stephen Finucane
stephen at that.guru
Sun Oct 4 22:10:02 AEDT 2020
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 <stephen at that.guru>
Reported-by: Ralf Ramsauer <ralf.ramsauer at oth-regensburg.de>
Closes: #379
Cc: Daniel Axtens <dja at axtens.net>
Cc: Rohit Sarkar <rohitsarkar5398 at gmail.com>
---
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 patchwork/api/__init__.py patchwork/api/__init__.py
index e69de29b..efc0dd89 100644
--- patchwork/api/__init__.py
+++ patchwork/api/__init__.py
@@ -0,0 +1,50 @@
+# Patchwork - automated patch tracking system
+# Copyright (C) 2020, Stephen Finucane <stephen at that.guru>
+#
+# 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 patchwork/api/embedded.py patchwork/api/embedded.py
index 3f32bd42..78316979 100644
--- patchwork/api/embedded.py
+++ 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 patchwork/api/event.py patchwork/api/event.py
index 7ed9efb1..75bf8708 100644
--- patchwork/api/event.py
+++ 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 patchwork/api/patch.py patchwork/api/patch.py
index d881504f..f6cb276d 100644
--- patchwork/api/patch.py
+++ 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
--
2.25.4
More information about the Patchwork
mailing list