Reimplement SerializerMethodResourceRelatedField (#781)

Fixes #639 - interface not consistent with `SerializerMethodField`
Fixes #779 - no enforcement of `read_only`
Fixes #780 - broken `parent` chain

Author: glowka

AUTHORS

@@ -28,3 +28,4 @@ Nathanael Gordon <nathanael.l.gordon@gmail.com>
 Charlie Allatson <charles.allatson@gmail.com>
 Joseba Mendivil <git@jma.email>
 Felix Viernickel <felix@gedankenspieler.org>
+Tom Glowka <glowka.tom@gmail.com>

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

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/`.

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',

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

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:

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

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')

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