From bc5955f0316c9fb10ae60322ff0f03acd25482c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristi=20V=C3=AEjdea?= Date: Thu, 13 Jun 2019 20:01:33 +0300 Subject: [PATCH 1/4] Allow nested write of non-relational fields --- rest_framework/serializers.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 01f34298b4..56a7c7b264 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -773,7 +773,7 @@ def errors(self): # ModelSerializer & HyperlinkedModelSerializer # -------------------------------------------- -def raise_errors_on_nested_writes(method_name, serializer, validated_data): +def raise_errors_on_nested_writes(method_name, serializer, validated_data, model_field_info): """ Give explicit errors when users attempt to pass writable nested data. @@ -800,6 +800,7 @@ def raise_errors_on_nested_writes(method_name, serializer, validated_data): assert not any( isinstance(field, BaseSerializer) and (field.source in validated_data) and + (field.source in model_field_info.relations) and isinstance(validated_data[field.source], (list, dict)) for field in serializer._writable_fields ), ( @@ -915,14 +916,14 @@ def create(self, validated_data): If you want to support writable nested relationships you'll need to write an explicit `.create()` method. """ - raise_errors_on_nested_writes('create', self, validated_data) - ModelClass = self.Meta.model + info = model_meta.get_field_info(ModelClass) + + raise_errors_on_nested_writes('create', self, validated_data, info) # Remove many-to-many relationships from validated_data. # They are not valid arguments to the default `.create()` method, # as they require that the instance has already been saved. - info = model_meta.get_field_info(ModelClass) many_to_many = {} for field_name, relation_info in info.relations.items(): if relation_info.to_many and (field_name in validated_data): @@ -959,8 +960,8 @@ def create(self, validated_data): return instance def update(self, instance, validated_data): - raise_errors_on_nested_writes('update', self, validated_data) info = model_meta.get_field_info(instance) + raise_errors_on_nested_writes('update', self, validated_data, info) # Simply set each attribute on the instance, and then save it. # Note that unlike `.create()` we don't need to treat many-to-many From 2d52a0f1806866bd3e0315391d1a8d6cb91ab5c7 Mon Sep 17 00:00:00 2001 From: Konstantinos Tselepakis Date: Fri, 6 Sep 2019 00:05:47 +0300 Subject: [PATCH 2/4] Fixed dotted write of non-relational fields, plus the related tests --- rest_framework/serializers.py | 12 +++++++ tests/test_serializer_nested.py | 55 +++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 56a7c7b264..67b29d53ab 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -819,9 +819,21 @@ def raise_errors_on_nested_writes(method_name, serializer, validated_data, model # class UserSerializer(ModelSerializer): # ... # address = serializer.CharField('profile.address') + # + # Though, we can have a dotted field if it is not expressing a model relation. + # + # For example: + # + # class NonRelationalPersonModel(models.Model): + # profile = JSONField() + # + # class UserSerializer(ModelSerializer): + # ... + # address = serializer.CharField('profile.address') assert not any( len(field.source_attrs) > 1 and (field.source_attrs[0] in validated_data) and + (field.source_attrs[0] in model_field_info.relations) and isinstance(validated_data[field.source_attrs[0]], (list, dict)) for field in serializer._writable_fields ), ( diff --git a/tests/test_serializer_nested.py b/tests/test_serializer_nested.py index ab30fad223..92f2084df8 100644 --- a/tests/test_serializer_nested.py +++ b/tests/test_serializer_nested.py @@ -4,6 +4,9 @@ from django.test import TestCase from rest_framework import serializers +from rest_framework.compat import postgres_fields +from rest_framework.serializers import raise_errors_on_nested_writes +from rest_framework.utils import model_meta class TestNestedSerializer: @@ -302,3 +305,55 @@ class Meta: 'serializer `tests.test_serializer_nested.DottedAddressSerializer`, ' 'or set `read_only=True` on dotted-source serializer fields.' ) + + +@pytest.mark.skipif('not postgres_fields') +class TestNestedNonRelationalFieldWrite: + """ + Test that raise_errors_on_nested_writes does not raise `AssertionError` when the + model field is not a relation. + """ + + def test_nested_serializer_create_and_update(self): + class NonRelationalPersonModel(models.Model): + """Model declaring a postgres JSONField""" + data = postgres_fields.JSONField() + + class NonRelationalPersonDataSerializer(serializers.Serializer): + occupation = serializers.CharField() + + class NonRelationalPersonSerializer(serializers.ModelSerializer): + data = NonRelationalPersonDataSerializer() + + class Meta: + model = NonRelationalPersonModel + fields = ['data'] + + serializer = NonRelationalPersonSerializer(data={'data': {'occupation': 'developer'}}) + assert serializer.is_valid() + assert serializer.validated_data == {'data': {'occupation': 'developer'}} + ModelClass = serializer.Meta.model + info = model_meta.get_field_info(ModelClass) + raise_errors_on_nested_writes('create', serializer, serializer.validated_data, info) + raise_errors_on_nested_writes('update', serializer, serializer.validated_data, info) + + def test_dotted_source_field_create_and_update(self): + class NonRelationalPersonModel(models.Model): + """Model declaring a postgres JSONField""" + data = postgres_fields.JSONField() + + class DottedNonRelationalPersonSerializer(serializers.ModelSerializer): + occupation = serializers.CharField(source='data.occupation') + + class Meta: + model = NonRelationalPersonModel + fields = ['occupation'] + + serializer = DottedNonRelationalPersonSerializer(data={'occupation': 'developer'}) + assert serializer.is_valid() + assert serializer.validated_data == {'data': {'occupation': 'developer'}} + ModelClass = serializer.Meta.model + info = model_meta.get_field_info(ModelClass) + raise_errors_on_nested_writes('create', serializer, serializer.validated_data, info) + raise_errors_on_nested_writes('update', serializer, serializer.validated_data, info) + assert serializer.data == {'occupation': 'developer'} From 3801ef7c6bc50acffc71d876e00530192cdae645 Mon Sep 17 00:00:00 2001 From: Konstantinos Tselepakis Date: Fri, 6 Sep 2019 08:36:20 +0300 Subject: [PATCH 3/4] Minor fixes * no need to change `raise_errors_on_nested_writes` signature * declare nested-model outside of the test class --- rest_framework/serializers.py | 12 ++++++------ tests/test_serializer_nested.py | 28 +++++++++++----------------- 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 67b29d53ab..a45f2b7d8e 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -773,7 +773,7 @@ def errors(self): # ModelSerializer & HyperlinkedModelSerializer # -------------------------------------------- -def raise_errors_on_nested_writes(method_name, serializer, validated_data, model_field_info): +def raise_errors_on_nested_writes(method_name, serializer, validated_data): """ Give explicit errors when users attempt to pass writable nested data. @@ -791,6 +791,8 @@ def raise_errors_on_nested_writes(method_name, serializer, validated_data, model * Silently ignore the nested part of the update. * Automatically create a profile instance. """ + ModelClass = serializer.Meta.model + model_field_info = model_meta.get_field_info(ModelClass) # Ensure we don't have a writable nested field. For example: # @@ -820,9 +822,7 @@ def raise_errors_on_nested_writes(method_name, serializer, validated_data, model # ... # address = serializer.CharField('profile.address') # - # Though, we can have a dotted field if it is not expressing a model relation. - # - # For example: + # Though, non-relational fields (e.g., JSONField) are acceptable. For example: # # class NonRelationalPersonModel(models.Model): # profile = JSONField() @@ -931,7 +931,7 @@ def create(self, validated_data): ModelClass = self.Meta.model info = model_meta.get_field_info(ModelClass) - raise_errors_on_nested_writes('create', self, validated_data, info) + raise_errors_on_nested_writes('create', self, validated_data) # Remove many-to-many relationships from validated_data. # They are not valid arguments to the default `.create()` method, @@ -973,7 +973,7 @@ def create(self, validated_data): def update(self, instance, validated_data): info = model_meta.get_field_info(instance) - raise_errors_on_nested_writes('update', self, validated_data, info) + raise_errors_on_nested_writes('update', self, validated_data) # Simply set each attribute on the instance, and then save it. # Note that unlike `.create()` we don't need to treat many-to-many diff --git a/tests/test_serializer_nested.py b/tests/test_serializer_nested.py index 92f2084df8..a614e05d13 100644 --- a/tests/test_serializer_nested.py +++ b/tests/test_serializer_nested.py @@ -6,7 +6,6 @@ from rest_framework import serializers from rest_framework.compat import postgres_fields from rest_framework.serializers import raise_errors_on_nested_writes -from rest_framework.utils import model_meta class TestNestedSerializer: @@ -307,7 +306,13 @@ class Meta: ) -@pytest.mark.skipif('not postgres_fields') +if postgres_fields: + class NonRelationalPersonModel(models.Model): + """Model declaring a postgres JSONField""" + data = postgres_fields.JSONField() + + +@pytest.mark.skipif(not postgres_fields, reason='psycopg2 is not installed') class TestNestedNonRelationalFieldWrite: """ Test that raise_errors_on_nested_writes does not raise `AssertionError` when the @@ -315,9 +320,6 @@ class TestNestedNonRelationalFieldWrite: """ def test_nested_serializer_create_and_update(self): - class NonRelationalPersonModel(models.Model): - """Model declaring a postgres JSONField""" - data = postgres_fields.JSONField() class NonRelationalPersonDataSerializer(serializers.Serializer): occupation = serializers.CharField() @@ -332,15 +334,10 @@ class Meta: serializer = NonRelationalPersonSerializer(data={'data': {'occupation': 'developer'}}) assert serializer.is_valid() assert serializer.validated_data == {'data': {'occupation': 'developer'}} - ModelClass = serializer.Meta.model - info = model_meta.get_field_info(ModelClass) - raise_errors_on_nested_writes('create', serializer, serializer.validated_data, info) - raise_errors_on_nested_writes('update', serializer, serializer.validated_data, info) + raise_errors_on_nested_writes('create', serializer, serializer.validated_data) + raise_errors_on_nested_writes('update', serializer, serializer.validated_data) def test_dotted_source_field_create_and_update(self): - class NonRelationalPersonModel(models.Model): - """Model declaring a postgres JSONField""" - data = postgres_fields.JSONField() class DottedNonRelationalPersonSerializer(serializers.ModelSerializer): occupation = serializers.CharField(source='data.occupation') @@ -352,8 +349,5 @@ class Meta: serializer = DottedNonRelationalPersonSerializer(data={'occupation': 'developer'}) assert serializer.is_valid() assert serializer.validated_data == {'data': {'occupation': 'developer'}} - ModelClass = serializer.Meta.model - info = model_meta.get_field_info(ModelClass) - raise_errors_on_nested_writes('create', serializer, serializer.validated_data, info) - raise_errors_on_nested_writes('update', serializer, serializer.validated_data, info) - assert serializer.data == {'occupation': 'developer'} + raise_errors_on_nested_writes('create', serializer, serializer.validated_data) + raise_errors_on_nested_writes('update', serializer, serializer.validated_data) From 4ce363ac800f06f0f7c9c0735e30e523ea495fb0 Mon Sep 17 00:00:00 2001 From: Ryan P Kilby Date: Fri, 6 Sep 2019 10:50:50 -0700 Subject: [PATCH 4/4] Revert minor line changes --- rest_framework/serializers.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index a45f2b7d8e..279898d082 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -928,14 +928,14 @@ def create(self, validated_data): If you want to support writable nested relationships you'll need to write an explicit `.create()` method. """ - ModelClass = self.Meta.model - info = model_meta.get_field_info(ModelClass) - raise_errors_on_nested_writes('create', self, validated_data) + ModelClass = self.Meta.model + # Remove many-to-many relationships from validated_data. # They are not valid arguments to the default `.create()` method, # as they require that the instance has already been saved. + info = model_meta.get_field_info(ModelClass) many_to_many = {} for field_name, relation_info in info.relations.items(): if relation_info.to_many and (field_name in validated_data): @@ -972,8 +972,8 @@ def create(self, validated_data): return instance def update(self, instance, validated_data): - info = model_meta.get_field_info(instance) raise_errors_on_nested_writes('update', self, validated_data) + info = model_meta.get_field_info(instance) # Simply set each attribute on the instance, and then save it. # Note that unlike `.create()` we don't need to treat many-to-many