From 91bb86d6b040bbcf683e88f255ff8d1c96ba88e6 Mon Sep 17 00:00:00 2001 From: Tim Csitkovics Date: Tue, 21 Aug 2018 13:31:26 +0200 Subject: [PATCH 1/3] Do not lazy load forward relations If the forward relations are included, the renderer will break, so this optimization only works if it is not included. fixup! Do not lazy load forward relations fixup! Do not lazy load forward relations fixup! Do not lazy load forward relations fixup! Do not lazy load forward relations fixup! Do not lazy load forward relations Include test case Small refactoring Make compatible with Python 2.X fixup! Make compatible with Python 2.X Fix travis Use isort Implement suggested changes Make import backward compatible Make travis happy --- AUTHORS | 1 + CHANGELOG.md | 2 + example/tests/test_serializers.py | 76 ++++++++++++++++++++++++-- rest_framework_json_api/serializers.py | 54 ++++++++++-------- 4 files changed, 105 insertions(+), 28 deletions(-) diff --git a/AUTHORS b/AUTHORS index b28cc99c..f2525a0a 100644 --- a/AUTHORS +++ b/AUTHORS @@ -15,4 +15,5 @@ Oliver Sauder Raphael Cohen Roberto Barreda santiavenda +Tim Selman Yaniv Peer diff --git a/CHANGELOG.md b/CHANGELOG.md index 354c1b5c..af69d7d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ * Add optional [jsonapi-style](http://jsonapi.org/format/) sort filter backend. See [usage docs](docs/usage.md#filter-backends) * For naming consistency, renamed new `JsonApi`-prefix pagination classes to `JSONAPI`-prefix. * Deprecates `JsonApiPageNumberPagination` and `JsonApiLimitOffsetPagination` +* Fix for performance improvement for related `ModelSerializer` + v2.5.0 - Released July 11, 2018 diff --git a/example/tests/test_serializers.py b/example/tests/test_serializers.py index 66860b61..5dc5ce81 100644 --- a/example/tests/test_serializers.py +++ b/example/tests/test_serializers.py @@ -2,24 +2,39 @@ from django.test import TestCase from django.urls import reverse from django.utils import timezone - -from rest_framework_json_api.serializers import ResourceIdentifierObjectSerializer +from rest_framework.request import Request +from rest_framework.test import APIRequestFactory + +from rest_framework_json_api.serializers import ( + DateField, + ModelSerializer, + ResourceIdentifierObjectSerializer +) from rest_framework_json_api.utils import format_resource_type from example.models import Author, Blog, Entry +from example.serializers import BlogSerializer + +try: + from unittest import mock +except ImportError: + import mock +request_factory = APIRequestFactory() pytestmark = pytest.mark.django_db class TestResourceIdentifierObjectSerializer(TestCase): def setUp(self): self.blog = Blog.objects.create(name='Some Blog', tagline="It's a blog") + now = timezone.now() + self.entry = Entry.objects.create( blog=self.blog, headline='headline', body_text='body_text', - pub_date=timezone.now(), - mod_date=timezone.now(), + pub_date=now.date(), + mod_date=now.date(), n_comments=0, n_pingbacks=0, rating=3 @@ -30,6 +45,59 @@ def setUp(self): Author.objects.create(name=name, email='{}@example.org'.format(name)) ) + def test_forward_relationship_not_loaded_when_not_included(self): + to_representation_method = 'example.serializers.BlogSerializer.to_representation' + with mock.patch(to_representation_method) as mocked_serializer: + class EntrySerializer(ModelSerializer): + blog = BlogSerializer() + + class Meta: + model = Entry + fields = '__all__' + + request_without_includes = Request(request_factory.get('/')) + serializer = EntrySerializer(context={'request': request_without_includes}) + serializer.to_representation(self.entry) + + mocked_serializer.assert_not_called() + + def test_forward_relationship_optimization_correct_representation(self): + class EntrySerializer(ModelSerializer): + blog = BlogSerializer() + + class Meta: + model = Entry + fields = '__all__' + + request_without_includes = Request(request_factory.get('/')) + serializer = EntrySerializer(context={'request': request_without_includes}) + result = serializer.to_representation(self.entry) + + # Remove non deterministic fields + result.pop('created_at') + result.pop('modified_at') + + expected = dict( + [ + ('id', 1), + ('blog', dict([('type', 'blogs'), ('id', 1)])), + ('headline', 'headline'), + ('body_text', 'body_text'), + ('pub_date', DateField().to_representation(self.entry.pub_date)), + ('mod_date', DateField().to_representation(self.entry.mod_date)), + ('n_comments', 0), + ('n_pingbacks', 0), + ('rating', 3), + ('authors', + [ + dict([('type', 'authors'), ('id', '1')]), + dict([('type', 'authors'), ('id', '2')]), + dict([('type', 'authors'), ('id', '3')]), + dict([('type', 'authors'), ('id', '4')]), + dict([('type', 'authors'), ('id', '5')])])]) + + self.assertDictEqual(expected, result) + def test_data_in_correct_format_when_instantiated_with_blog_object(self): serializer = ResourceIdentifierObjectSerializer(instance=self.blog) diff --git a/rest_framework_json_api/serializers.py b/rest_framework_json_api/serializers.py index ffb77800..d525af15 100644 --- a/rest_framework_json_api/serializers.py +++ b/rest_framework_json_api/serializers.py @@ -182,35 +182,41 @@ def to_representation(self, instance): for field in readable_fields: try: - - if isinstance(field, ModelSerializer) and hasattr(field, field.source + "_id"): - attribute = getattr(instance, field.source + "_id") - if attribute is None: - ret[field.field_name] = None - continue - resource_type = get_resource_type_from_instance(field) - if resource_type: - ret[field.field_name] = OrderedDict([("type", resource_type), - ("id", attribute)]) - continue - - attribute = field.get_attribute(instance) + field_representation = self._get_field_representation(field, instance) + ret[field.field_name] = field_representation except SkipField: continue - # We skip `to_representation` for `None` values so that fields do - # not have to explicitly deal with that case. - # - # For related fields with `use_pk_only_optimization` we need to - # resolve the pk value. - check_for_none = attribute.pk if isinstance(attribute, PKOnlyObject) else attribute - if check_for_none is None: - ret[field.field_name] = None - else: - ret[field.field_name] = field.to_representation(attribute) - return ret + def _get_field_representation(self, field, instance): + request = self.context.get('request') + is_included = field.source in get_included_resources(request) + if not is_included and \ + isinstance(field, ModelSerializer) and \ + hasattr(instance, field.source + '_id'): + attribute = getattr(instance, field.source + '_id') + + if attribute is None: + return None + + resource_type = get_resource_type_from_serializer(field) + if resource_type: + return OrderedDict([('type', resource_type), ('id', attribute)]) + + attribute = field.get_attribute(instance) + + # We skip `to_representation` for `None` values so that fields do + # not have to explicitly deal with that case. + # + # For related fields with `use_pk_only_optimization` we need to + # resolve the pk value. + check_for_none = attribute.pk if isinstance(attribute, PKOnlyObject) else attribute + if check_for_none is None: + return None + else: + return field.to_representation(attribute) + class PolymorphicSerializerMetaclass(SerializerMetaclass): """ From c794f0b778433881b1db6fb00b633c418a7c4f24 Mon Sep 17 00:00:00 2001 From: Oliver Sauder Date: Tue, 4 Sep 2018 09:42:12 +0200 Subject: [PATCH 2/3] Clarify changelog entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index af69d7d6..34374593 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ * Add optional [jsonapi-style](http://jsonapi.org/format/) sort filter backend. See [usage docs](docs/usage.md#filter-backends) * For naming consistency, renamed new `JsonApi`-prefix pagination classes to `JSONAPI`-prefix. * Deprecates `JsonApiPageNumberPagination` and `JsonApiLimitOffsetPagination` -* Fix for performance improvement for related `ModelSerializer` +* Performance improvment when rendering relationships with `ModelSerializer` v2.5.0 - Released July 11, 2018 From b5b5ce74d76ca0a3c496115a35a20738bd75af56 Mon Sep 17 00:00:00 2001 From: Oliver Sauder Date: Tue, 4 Sep 2018 09:54:45 +0200 Subject: [PATCH 3/3] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34374593..cba4a852 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ * Add optional [jsonapi-style](http://jsonapi.org/format/) sort filter backend. See [usage docs](docs/usage.md#filter-backends) * For naming consistency, renamed new `JsonApi`-prefix pagination classes to `JSONAPI`-prefix. * Deprecates `JsonApiPageNumberPagination` and `JsonApiLimitOffsetPagination` -* Performance improvment when rendering relationships with `ModelSerializer` +* Performance improvement when rendering relationships with `ModelSerializer` v2.5.0 - Released July 11, 2018