Correct allow_null behaviour when required=False (#5888)

* Revert "Non-required fields with 'allow_null=True' should not imply a default value (#5639)"
    This reverts commit 905a557.
    Closes #5708

* Add test for allow_null + required=False
    Ref #5708: allow_null should imply default=None, even for non-required fields.

* Re-order allow_null and default in field docs
    default is prior to allow_null. allow_null implies an outgoing default=None.

* Adjust allow_null note.

Author: carltongibson

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, 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`
+
 ### `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.

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 '

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):
@@ -486,12 +478,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: