Null/blank ModelSerializer fields are not required in responses with COMPONENT_SPLIT_REQUEST=True

[salomvary]

Describe the bug

When component splitting is turned on, ModelSerializer fields that correspond to null=True or blank=True model fields are not marked as required in the response components, despite DRF's default behavior of always including these fields in the JSON response when serializing model instances, even if the field's value is None or empty string.

To Reproduce

diff --git a/tests/test_split.py b/tests/test_split.py
index 33ade7f..f9f9945 100644
--- a/tests/test_split.py
+++ b/tests/test_split.py
@@ -13,6 +13,9 @@ class PNM1(models.Model):
 class PNM2(models.Model):
     field_relation = models.ForeignKey(PNM1, on_delete=models.CASCADE)
     field_non_blank = models.CharField(max_length=5)
+    field_blank_null = models.CharField(max_length=5, blank=True, null=True)
+    field_blank = models.CharField(max_length=5, blank=True, null=False)
+    field_null = models.CharField(max_length=5, blank=False, null=True)
 
 
 class XSerializer(serializers.ModelSerializer):
@@ -47,6 +50,18 @@ def test_nested_partial_on_split_request_false(no_warnings, django_transforms):
     )
 
 
+def test_serialize_blank():
+    assert {
+               "id": None,
+               "field_relation": None,
+               "field_relation_partial": None,
+               "field_non_blank": "test",
+               "field_blank_null": None,
+               "field_blank": "",
+               "field_null": None,
+           } == YSerializer(PNM2(field_non_blank="test")).data
+
+
 @mock.patch('drf_spectacular.settings.spectacular_settings.COMPONENT_SPLIT_REQUEST', True)
 def test_nested_partial_on_split_request_true(no_warnings, django_transforms):
     # with split request, behaves like above, however response schemas are always unpatched.

And

diff --git a/tests/test_split_request_true.yml b/tests/test_split_request_true.yml
index a77c0a2..4ebd249 100644
--- a/tests/test_split_request_true.yml
+++ b/tests/test_split_request_true.yml
@@ -77,6 +77,18 @@ components:
           type: string
           minLength: 1
           maxLength: 5
+        field_blank_null:
+          type: string
+          nullable: true
+          maxLength: 5
+        field_blank:
+          type: string
+          maxLength: 5
+        field_null:
+          type: string
+          nullable: true
+          minLength: 1
+          maxLength: 5
     X:
       type: object
       properties:
@@ -108,7 +120,21 @@ components:
         field_non_blank:
           type: string
           maxLength: 5
+        field_blank_null:
+          type: string
+          nullable: true
+          maxLength: 5
+        field_blank:
+          type: string
+          maxLength: 5
+        field_null:
+          type: string
+          nullable: true
+          maxLength: 5
       required:
+      - field_blank
+      - field_blank_null
+      - field_null
       - field_non_blank
       - field_relation
       - field_relation_partial
@@ -124,7 +150,22 @@ components:
           type: string
           minLength: 1
           maxLength: 5
+        field_blank_null:
+          type: string
+          nullable: true
+          maxLength: 5
+        field_blank:
+          type: string
+          maxLength: 5
+        field_null:
+          type: string
+          nullable: true
+          minLength: 1
+          maxLength: 5
       required:
+      - field_blank
+      - field_blank_null
+      - field_null
       - field_non_blank
       - field_relation
       - field_relation_partial

Fails with:

FAILED          [100%]
tests/test_split.py:64 (test_nested_partial_on_split_request_true)
no_warnings = <_pytest.capture.CaptureFixture object at 0x1136b1040>
django_transforms = [<function django_transforms.<locals>.integer_field_sqlite at 0x1137f31a0>]

    @mock.patch('drf_spectacular.settings.spectacular_settings.COMPONENT_SPLIT_REQUEST', True)
    def test_nested_partial_on_split_request_true(no_warnings, django_transforms):
        # with split request, behaves like above, however response schemas are always unpatched.
        # nested request serializers are only affected by their manual partial flag and not due to PATCH.
>       assert_schema(
            generate_schema('x', XViewset),
            'tests/test_split_request_true.yml',
            transforms=django_transforms,
        )

test_split.py:69: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
__init__.py:46: in assert_schema
    assert_equal(generated, expected)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

actual = "openapi: 3.0.3\ninfo:\n  title: ''\n  version: 0.0.0\npaths:\n  /x/{id}/:\n    put:\n      operationId: x_update\n   ...n      type: http\n      scheme: basic\n    cookieAuth:\n      type: apiKey\n      in: cookie\n      name: sessionid\n"
expected = "openapi: 3.0.3\ninfo:\n  title: ''\n  version: 0.0.0\npaths:\n  /x/{id}/:\n    put:\n      operationId: x_update\n   ...n      type: http\n      scheme: basic\n    cookieAuth:\n      type: apiKey\n      in: cookie\n      name: sessionid\n"

    def assert_equal(actual, expected):
        if not isinstance(actual, str):
            actual = json.dumps(actual, indent=4)
        if not isinstance(expected, str):
            expected = json.dumps(expected, indent=4)
        diff = difflib.unified_diff(
            expected.splitlines(True),
            actual.splitlines(True),
        )
        diff = ''.join(diff)
>       assert actual == expected and not diff, diff
E       AssertionError: --- 
E       +++ 
E       @@ -132,9 +132,6 @@
E                  nullable: true
E                  maxLength: 5
E              required:
E       -      - field_blank
E       -      - field_blank_null
E       -      - field_null
E              - field_non_blank
E              - field_relation
E              - field_relation_partial
E       @@ -163,9 +160,6 @@
E                  minLength: 1
E                  maxLength: 5
E              required:
E       -      - field_blank
E       -      - field_blank_null
E       -      - field_null
E              - field_non_blank
E              - field_relation
E              - field_relation_partial

__init__.py:61: AssertionError

Expected behavior

All ModelSerializer fields should be marked as required. There should however be some way of opting out from this, in case for instance toRepresentation is overridden to exclude some fields.

[salomvary]

I wonder if this is (also) a bug in DRF. Documentation for fields/required says:

required
[...]
Setting this to False also allows the object attribute or dictionary key to be omitted from output when serializing the instance. If the key is not present it will simply not be included in the output representation.
[...]
Defaults to True. If you're using Model Serializer default value will be False if you have specified blank=True or default or null=True at your field in your Model.

In my understanding, this means that blank=True or null=True makes a field required=False and that should omit these fields during serialization. That makes DRF Spectacular's behavior match the documentation but not the actual DRF behavior.

Edit: I even found a test in DRF that seems to contradict the documentation. Here is an optional field not being omitted during serialization: https://github.com/encode/django-rest-framework/blob/master/tests/test_serializer.py#L567C1-L567C96

    def test_default_for_allow_null(self):
        """
        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)

        # allow_null=True should imply default=None when serializing:
        assert Serializer({'foo': None}).data == {'foo': None, 'bar': None, 'optional': None, }

[tfranzel]

Hi, this is a convoluted issue and easy to misinterpret. Let me give some ground work:

We only use what DRF gives us (because we cannot differentiate your choices from DRFs default choices). We are not inventing new logic here. If DRF (or you) mark the serializer field as required, we mark it as required. Also, you have to live with the Model -> ModelSerializer conversion logic, as it will never ever be changed. You can, for yourself, mark fields required and still do null=True to make it "explicit". It is just not what ModelSerializer will do by itself.

drf-spectacular/drf_spectacular/openapi.py

Lines 1053 to 1058 in 9bf8e92

	add_to_required = (
	field.required
	or (schema.get('readOnly') and not spectacular_settings.COMPONENT_NO_READ_ONLY_REQUIRED)
	)
	if add_to_required:
	required.add(field.field_name)

It is more informative to look at the generated ModelSerializer instead of the given Model properties, due to the discussed conversion logic we have no control over.

---

Also, we have added a heuristic with the readOnly, as usually all readOnly fields are present on response, but as you can see, this can be disabled if desired. Actually we have a couple of settings to customize the behavior around this topic.

In a perfect world there would be required_on_request and required_on_response. In practice, (imho) required refers more to the request side, but all of this has no rigorous definition so we try to go the most sensible route.

---

All ModelSerializer fields should be marked as required. There should however be some way of opting out from this, in case for instance toRepresentation is overridden to exclude some fields.

We cannot look into the toRepresentation, but you can exclude fields via decorator:

drf-spectacular/drf_spectacular/utils.py

Lines 598 to 601 in 9bf8e92

	def extend_schema_serializer(
	many: Optional[bool] = None,
	exclude_fields: Optional[Sequence[str]] = None,
	deprecate_fields: Optional[Sequence[str]] = None,

[tfranzel]

2 more things:

• As the docs state, since you are allowed to omit fields in the "response dict" of ModelSerializer, marking them required in the response schema could make the schema incorrect.

• If unclear, it is safer to mark a field non-required as it allows both cases. Marking it required is a stricter statement that might turn out wrong.