From 7bb92d3ba56e284b7f604fe7b787cbe068e22148 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Tue, 20 Mar 2018 13:43:52 +0100 Subject: [PATCH 1/4] Add test for allow_null + required=False Ref #5708: allow_null should imply default=None, even for non-required fields. flake8 --- tests/test_serializer.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/test_serializer.py b/tests/test_serializer.py index 120632572d..c077e7f622 100644 --- a/tests/test_serializer.py +++ b/tests/test_serializer.py @@ -486,12 +486,16 @@ class Serializer(serializers.Serializer): assert Serializer({'nested': {'a': '3', 'b': {'c': '4'}}}).data == {'nested': {'a': '3', 'c': '4'}} def test_default_for_allow_null(self): - # allow_null=True should imply default=None + """ + Without an explicit default, allow_null implies default=None when serializing. #5518 #5708 + """ class Serializer(serializers.Serializer): foo = serializers.CharField() bar = serializers.CharField(source='foo.bar', allow_null=True) + optional = serializers.CharField(required=False, allow_null=True) - assert Serializer({'foo': None}).data == {'foo': None, 'bar': None} + # allow_null=True should imply default=None when serialising: + assert Serializer({'foo': None}).data == {'foo': None, 'bar': None, 'optional': None, } class TestCacheSerializerData: From 8d2ac40658a9b7eb81495fe384e11ba8becbe1af Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Tue, 20 Mar 2018 12:26:51 +0100 Subject: [PATCH 2/4] Revert "Non-required fields with 'allow_null=True' should not imply a default value (#5639)" This reverts commit 905a5579dfaa717ad4210ddd285267bb9bbb9a43. Closes #5708 --- rest_framework/fields.py | 4 ++-- tests/test_serializer.py | 8 -------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/rest_framework/fields.py b/rest_framework/fields.py index ad710b9678..58e28ed4ce 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -442,10 +442,10 @@ def get_attribute(self, instance): except (KeyError, AttributeError) as exc: if self.default is not empty: return self.get_default() - if not self.required: - raise SkipField() 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 ' diff --git a/tests/test_serializer.py b/tests/test_serializer.py index c077e7f622..c44e830d0e 100644 --- a/tests/test_serializer.py +++ b/tests/test_serializer.py @@ -384,14 +384,6 @@ def create(self, validated_data): serializer.save() assert serializer.data == {'included': 'abc'} - def test_not_required_output_for_allow_null_field(self): - class ExampleSerializer(serializers.Serializer): - omitted = serializers.CharField(required=False, allow_null=True) - included = serializers.CharField() - - serializer = ExampleSerializer({'included': 'abc'}) - assert 'omitted' not in serializer.data - class TestDefaultOutput: def setup(self): From e5034b20016d9b513ad09ffea2dd1d0f7ffdad30 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Tue, 20 Mar 2018 13:46:05 +0100 Subject: [PATCH 3/4] Re-order allow_null and default in field docs default is prior to allow_null. allow_null implies an outgoing default=None. --- docs/api-guide/fields.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/api-guide/fields.md b/docs/api-guide/fields.md index 78fb3f83eb..69a769c798 100644 --- a/docs/api-guide/fields.md +++ b/docs/api-guide/fields.md @@ -41,14 +41,6 @@ Setting this to `False` also allows the object attribute or dictionary key to be Defaults to `True`. -### `allow_null` - -Normally an error will be raised if `None` is passed to a serializer field. Set this keyword argument to `True` if `None` should be considered a valid value. - -Note that setting this argument to `True` will imply a default value of `null` for serialization output, but does not imply a default for input deserialization. - -Defaults to `False` - ### `default` If set, this gives the default value that will be used for the field if no input value is supplied. If not set the default behaviour is to not populate the attribute at all. @@ -61,6 +53,14 @@ When serializing the instance, default will be used if the the object attribute Note that setting a `default` value implies that the field is not required. Including both the `default` and `required` keyword arguments is invalid and will raise an error. +### `allow_null` + +Normally an error will be raised if `None` is passed to a serializer field. Set this keyword argument to `True` if `None` should be considered a valid value. + +Note that setting this argument to `True` will imply a default value of `null` for serialization output, but does not imply a default for input deserialization. + +Defaults to `False` + ### `source` The name of the attribute that will be used to populate the field. May be a method that only takes a `self` argument, such as `URLField(source='get_absolute_url')`, or may use dotted notation to traverse attributes, such as `EmailField(source='user.email')`. When serializing fields with dotted notation, it may be necessary to provide a `default` value if any object is not present or is empty during attribute traversal. From 758b797ceb1f94a28295b45198eab90a586ae19d Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Tue, 20 Mar 2018 13:48:15 +0100 Subject: [PATCH 4/4] Adjust allow_null note. --- docs/api-guide/fields.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api-guide/fields.md b/docs/api-guide/fields.md index 69a769c798..6c68a5b818 100644 --- a/docs/api-guide/fields.md +++ b/docs/api-guide/fields.md @@ -57,7 +57,7 @@ Note that setting a `default` value implies that the field is not required. Incl Normally an error will be raised if `None` is passed to a serializer field. Set this keyword argument to `True` if `None` should be considered a valid value. -Note that setting this argument to `True` will imply a default value of `null` for serialization output, but does not imply a default for input deserialization. +Note that, without an explicit `default`, setting this argument to `True` will imply a `default` value of `null` for serialization output, but does not imply a default for input deserialization. Defaults to `False`