From 1dccbb089a6eb1f6be448efc0574cb09af207ec5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20G=C5=82=C3=B3wka?= Date: Sun, 3 May 2020 21:44:18 +0200 Subject: [PATCH] Reimplement SerializerMethodResourceRelatedField (#781) Fixes #639 - interface not consistent with `SerializerMethodField` Fixes #779 - no enforcement of `read_only` Fixes #780 - broken `parent` chain --- AUTHORS | 1 + CHANGELOG.md | 10 +++ docs/usage.md | 60 ++++++++++++- example/serializers.py | 10 +-- example/tests/test_parsers.py | 2 +- example/tests/test_relations.py | 4 - example/tests/test_serializers.py | 10 +-- .../unit/test_serializer_method_field.py | 66 ++++++++++++++ rest_framework_json_api/relations.py | 86 +++++++++++-------- 9 files changed, 190 insertions(+), 59 deletions(-) create mode 100644 example/tests/unit/test_serializer_method_field.py diff --git a/AUTHORS b/AUTHORS index 8ecb85d2..6e98ad71 100644 --- a/AUTHORS +++ b/AUTHORS @@ -28,3 +28,4 @@ Nathanael Gordon Charlie Allatson Joseba Mendivil Felix Viernickel +Tom Glowka diff --git a/CHANGELOG.md b/CHANGELOG.md index 85942770..59110062 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,16 @@ any parts of the framework not mentioned in the documentation should generally b * Avoid `AttributeError` for PUT and PATCH methods when using `APIView` +### Changed + +* `SerializerMethodResourceRelatedField` is now consistent with DRF `SerializerMethodField`: + * Pass `method_name` argument to specify method name. If no value is provided, it defaults to `get_{field_name}` + +### Deprecated + +* Deprecate `source` argument of `SerializerMethodResourceRelatedField`, use `method_name` instead + + ## [3.1.0] - 2020-02-08 ### Added diff --git a/docs/usage.md b/docs/usage.md index fc6aa6ad..7f34da98 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -586,10 +586,68 @@ class LineItemViewSet(viewsets.ModelViewSet): #### HyperlinkedRelatedField -`HyperlinkedRelatedField` has same functionality as `ResourceRelatedField` but does +`relations.HyperlinkedRelatedField` has same functionality as `ResourceRelatedField` but does not render `data`. Use this in case you only need links of relationships and want to lower payload and increase performance. +#### SerializerMethodResourceRelatedField + +`relations.SerializerMethodResourceRelatedField` combines behaviour of DRF `SerializerMethodField` and +`ResourceRelatedField`, so it accepts `method_name` together with `model` and links-related arguments. +`data` is rendered in `ResourceRelatedField` manner. + +```python +from rest_framework_json_api import serializers +from rest_framework_json_api.relations import SerializerMethodResourceRelatedField + +from myapp.models import Order, LineItem + + +class OrderSerializer(serializers.ModelSerializer): + class Meta: + model = Order + + line_items = SerializerMethodResourceRelatedField( + model=LineItem, + many=True, + method_name='get_big_line_items' + ) + + small_line_items = SerializerMethodResourceRelatedField( + model=LineItem, + many=True, + # default to method_name='get_small_line_items' + ) + + def get_big_line_items(self, instance): + return LineItem.objects.filter(order=instance).filter(amount__gt=1000) + + def get_small_line_items(self, instance): + return LineItem.objects.filter(order=instance).filter(amount__lte=1000) + +``` + +or using `related_link_*` with `HyperlinkedModelSerializer` + +```python +class OrderSerializer(serializers.HyperlinkedModelSerializer): + class Meta: + model = Order + + line_items = SerializerMethodResourceRelatedField( + model=LineItem, + many=True, + method_name='get_big_line_items', + related_link_view_name='order-lineitems-list', + related_link_url_kwarg='order_pk', + ) + + def get_big_line_items(self, instance): + return LineItem.objects.filter(order=instance).filter(amount__gt=1000) + +``` + + #### Related urls There is a nice way to handle "related" urls like `/orders/3/lineitems/` or `/orders/3/customer/`. diff --git a/example/serializers.py b/example/serializers.py index cc24efb0..2af5eb70 100644 --- a/example/serializers.py +++ b/example/serializers.py @@ -126,30 +126,24 @@ def __init__(self, *args, **kwargs): related_link_view_name='entry-suggested', related_link_url_kwarg='entry_pk', self_link_view_name='entry-relationships', - source='get_suggested', model=Entry, many=True, - read_only=True ) # many related hyperlinked from serializer suggested_hyperlinked = relations.SerializerMethodHyperlinkedRelatedField( related_link_view_name='entry-suggested', related_link_url_kwarg='entry_pk', self_link_view_name='entry-relationships', - source='get_suggested', model=Entry, many=True, - read_only=True ) # single related from serializer - featured = relations.SerializerMethodResourceRelatedField( - source='get_featured', model=Entry, read_only=True) + featured = relations.SerializerMethodResourceRelatedField(model=Entry) # single related hyperlinked from serializer featured_hyperlinked = relations.SerializerMethodHyperlinkedRelatedField( related_link_view_name='entry-featured', related_link_url_kwarg='entry_pk', self_link_view_name='entry-relationships', - source='get_featured', model=Entry, read_only=True ) @@ -229,8 +223,6 @@ class AuthorSerializer(serializers.ModelSerializer): related_link_view_name='author-related', self_link_view_name='author-relationships', model=Entry, - read_only=True, - source='get_first_entry' ) comments = relations.HyperlinkedRelatedField( related_link_view_name='author-related', diff --git a/example/tests/test_parsers.py b/example/tests/test_parsers.py index 3a2459c0..be2b2ce3 100644 --- a/example/tests/test_parsers.py +++ b/example/tests/test_parsers.py @@ -4,7 +4,7 @@ from django.conf.urls import url from django.test import TestCase, override_settings from django.urls import reverse -from rest_framework import views, status +from rest_framework import status, views from rest_framework.exceptions import ParseError from rest_framework.response import Response from rest_framework.test import APITestCase diff --git a/example/tests/test_relations.py b/example/tests/test_relations.py index 94db188a..ef1dfb02 100644 --- a/example/tests/test_relations.py +++ b/example/tests/test_relations.py @@ -290,16 +290,12 @@ class EntryModelSerializerWithHyperLinks(serializers.ModelSerializer): related_link_url_kwarg='entry_pk', self_link_view_name='entry-relationships', many=True, - read_only=True, - source='get_blog' ) comments = SerializerMethodHyperlinkedRelatedField( related_link_view_name='entry-comments', related_link_url_kwarg='entry_pk', self_link_view_name='entry-relationships', many=True, - read_only=True, - source='get_comments' ) class Meta: diff --git a/example/tests/test_serializers.py b/example/tests/test_serializers.py index 50a84f4d..5f277f2f 100644 --- a/example/tests/test_serializers.py +++ b/example/tests/test_serializers.py @@ -7,21 +7,17 @@ from rest_framework.request import Request from rest_framework.test import APIRequestFactory -from example.factories import ArtProjectFactory from rest_framework_json_api.serializers import ( DateField, ModelSerializer, ResourceIdentifierObjectSerializer, - empty, + empty ) from rest_framework_json_api.utils import format_resource_type +from example.factories import ArtProjectFactory from example.models import Author, Blog, Entry -from example.serializers import ( - BlogSerializer, - ProjectSerializer, - ArtProjectSerializer, -) +from example.serializers import ArtProjectSerializer, BlogSerializer, ProjectSerializer request_factory = APIRequestFactory() pytestmark = pytest.mark.django_db diff --git a/example/tests/unit/test_serializer_method_field.py b/example/tests/unit/test_serializer_method_field.py new file mode 100644 index 00000000..22935ebb --- /dev/null +++ b/example/tests/unit/test_serializer_method_field.py @@ -0,0 +1,66 @@ +from __future__ import absolute_import + +import pytest +from rest_framework import serializers + +from rest_framework_json_api.relations import SerializerMethodResourceRelatedField + +from example.models import Blog, Entry + + +def test_method_name_default(): + class BlogSerializer(serializers.ModelSerializer): + one_entry = SerializerMethodResourceRelatedField(model=Entry) + + class Meta: + model = Blog + fields = ['one_entry'] + + def get_one_entry(self, instance): + return Entry(id=100) + + serializer = BlogSerializer(instance=Blog()) + assert serializer.data['one_entry']['id'] == '100' + + +def test_method_name_custom(): + class BlogSerializer(serializers.ModelSerializer): + one_entry = SerializerMethodResourceRelatedField( + model=Entry, + method_name='get_custom_entry' + ) + + class Meta: + model = Blog + fields = ['one_entry'] + + def get_custom_entry(self, instance): + return Entry(id=100) + + serializer = BlogSerializer(instance=Blog()) + assert serializer.data['one_entry']['id'] == '100' + + +@pytest.mark.filterwarnings("ignore::DeprecationWarning") +def test_source(): + class BlogSerializer(serializers.ModelSerializer): + one_entry = SerializerMethodResourceRelatedField( + model=Entry, + source='get_custom_entry' + ) + + class Meta: + model = Blog + fields = ['one_entry'] + + def get_custom_entry(self, instance): + return Entry(id=100) + + serializer = BlogSerializer(instance=Blog()) + assert serializer.data['one_entry']['id'] == '100' + + +@pytest.mark.filterwarnings("error::DeprecationWarning") +def test_source_is_deprecated(): + with pytest.raises(DeprecationWarning): + SerializerMethodResourceRelatedField(model=Entry, source='get_custom_entry') diff --git a/rest_framework_json_api/relations.py b/rest_framework_json_api/relations.py index 9fbfb98f..eb7ff3a1 100644 --- a/rest_framework_json_api/relations.py +++ b/rest_framework_json_api/relations.py @@ -1,12 +1,12 @@ import json +import warnings from collections import OrderedDict -from collections.abc import Iterable import inflection from django.core.exceptions import ImproperlyConfigured from django.urls import NoReverseMatch from django.utils.translation import gettext_lazy as _ -from rest_framework.fields import MISSING_ERROR_MESSAGE, SkipField +from rest_framework.fields import MISSING_ERROR_MESSAGE, Field, SkipField from rest_framework.relations import MANY_RELATION_KWARGS from rest_framework.relations import ManyRelatedField as DRFManyRelatedField from rest_framework.relations import PrimaryKeyRelatedField, RelatedField @@ -347,51 +347,63 @@ def to_internal_value(self, data): return super(ResourceRelatedField, self).to_internal_value(data['id']) -class SerializerMethodResourceRelatedField(ResourceRelatedField): +class SerializerMethodFieldBase(Field): + def __init__(self, method_name=None, **kwargs): + if not method_name and kwargs.get('source'): + method_name = kwargs.pop('source') + warnings.warn(DeprecationWarning( + "'source' argument of {cls} is deprecated, use 'method_name' " + "as in SerializerMethodField".format(cls=self.__class__.__name__)), stacklevel=3) + self.method_name = method_name + kwargs['source'] = '*' + kwargs['read_only'] = True + super().__init__(**kwargs) + + def bind(self, field_name, parent): + default_method_name = 'get_{field_name}'.format(field_name=field_name) + if self.method_name is None: + self.method_name = default_method_name + super().bind(field_name, parent) + + def get_attribute(self, instance): + serializer_method = getattr(self.parent, self.method_name) + return serializer_method(instance) + + +class ManySerializerMethodResourceRelatedField(SerializerMethodFieldBase, ResourceRelatedField): + def __init__(self, child_relation=None, *args, **kwargs): + assert child_relation is not None, '`child_relation` is a required argument.' + self.child_relation = child_relation + super().__init__(**kwargs) + self.child_relation.bind(field_name='', parent=self) + + def to_representation(self, value): + return [self.child_relation.to_representation(item) for item in value] + + +class SerializerMethodResourceRelatedField(SerializerMethodFieldBase, ResourceRelatedField): """ Allows us to use serializer method RelatedFields with return querysets """ - def __new__(cls, *args, **kwargs): - """ - We override this because getting serializer methods - fails at the base class when many=True - """ - if kwargs.pop('many', False): - return cls.many_init(*args, **kwargs) - return super(ResourceRelatedField, cls).__new__(cls, *args, **kwargs) - def __init__(self, child_relation=None, *args, **kwargs): - model = kwargs.pop('model', None) - if child_relation is not None: - self.child_relation = child_relation - if model: - self.model = model - super(SerializerMethodResourceRelatedField, self).__init__(*args, **kwargs) + many_kwargs = [*MANY_RELATION_KWARGS, *LINKS_PARAMS, 'method_name', 'model'] + many_cls = ManySerializerMethodResourceRelatedField @classmethod def many_init(cls, *args, **kwargs): - list_kwargs = {k: kwargs.pop(k) for k in LINKS_PARAMS if k in kwargs} - list_kwargs['child_relation'] = cls(*args, **kwargs) - for key in kwargs.keys(): - if key in ('model',) + MANY_RELATION_KWARGS: + list_kwargs = {'child_relation': cls(**kwargs)} + for key in kwargs: + if key in cls.many_kwargs: list_kwargs[key] = kwargs[key] - return cls(**list_kwargs) + return cls.many_cls(**list_kwargs) - def get_attribute(self, instance): - # check for a source fn defined on the serializer instead of the model - if self.source and hasattr(self.parent, self.source): - serializer_method = getattr(self.parent, self.source) - if hasattr(serializer_method, '__call__'): - return serializer_method(instance) - return super(SerializerMethodResourceRelatedField, self).get_attribute(instance) - def to_representation(self, value): - if isinstance(value, Iterable): - base = super(SerializerMethodResourceRelatedField, self) - return [base.to_representation(x) for x in value] - return super(SerializerMethodResourceRelatedField, self).to_representation(value) +class ManySerializerMethodHyperlinkedRelatedField(SkipDataMixin, + ManySerializerMethodResourceRelatedField): + pass -class SerializerMethodHyperlinkedRelatedField(SkipDataMixin, SerializerMethodResourceRelatedField): - pass +class SerializerMethodHyperlinkedRelatedField(SkipDataMixin, + SerializerMethodResourceRelatedField): + many_cls = ManySerializerMethodHyperlinkedRelatedField