Schema fails to build with serializer method fields that reference an overridden list serializer

[mwhansen]

Describe the bug
Sorry the title is not the greatest! When annotating a SerializerMethodField with a list serializer where the child is overridden with @extend_schema_field, then the schema fails to build, erroring out with

schema = None, meta = {'readOnly': True}

    def append_meta(schema, meta):
>       return safe_ref({**schema, **meta})
E       TypeError: 'NoneType' object is not a mapping

To Reproduce

We can update test_extend_schema.py with the following to reproduce the error:

class InlineSerializer(serializers.Serializer):
    inline_b = serializers.BooleanField()
    inline_i = serializers.IntegerField()


class OtherInlineSerializer(InlineSerializer):
    pass


@extend_schema_field(OtherInlineSerializer)
class InlineWithOverrideSerializer(serializers.Serializer):
    def to_representation(self, object):
        return {
            'inline_b': bool(object.inline_b),
            'inline_i': int(object.inline_i),
        }


class ErrorDetailSerializer(serializers.Serializer):
    ...
    field_n = serializers.SerializerMethodField()

    ...
    @extend_schema_field(InlineWithOverrideSerializer(many=True))
    def get_field_n(self, object):
        return InlineWithOverrideSerializer([], many=True).data  # pragma: no cover

This is happening since we get a call trace like:

1. _map_response_type_hint is called while processing the SerializerMethodField
2. self._unwrap_list_serializer(field, direction) is called on on InlineWithOverrideSerializer(many=True)
3. _unwrap_list_serializer on InlineWithOverrideSerializer()
4. self.resolve_serializer(...) is called on InlineWithOverrideSerializer()
5. component.schema = self._map_serializer(serializer, direction) is called to build the schema from the serializer fields directly;
6. Since component.schema is None, this is component discarded and ResolvedComponent(None, None) is returned from resolve_serializer
7. This will eventually cause schema=None to be returned to the SerializerMethodField handling
8. That handling will call append_meta(None, meta) which causes the error.

Note: This does not happen if we do @extend_schema_field(InlineWithOverrideSerializer) (without the many=True.)

It seems like we should check for overrides and extensions during the _unwrap_list_serializer, but I'm not sure the best place as putting _map_serializer_field within _unwrap_list_serializer causes some additional changes like:

E                board:
E                  type: array
E                  items:
E       -            $ref: '#/components/schemas/Person'
E       +            allOf:
E       +            - $ref: '#/components/schemas/Person'
E       +            readOnly: true
E                  readOnly: true

Expected behavior
I would expect the schema to build and the schema for the SerializerMethodField to be an array of OtherInlineSerializer components.

[tfranzel]

Hi @mwhansen,

first of all thanks for the detailed explanation. This is such a specific circumstance that I don't think I would have found it without it. Apparently this is only broken when extend_schema_field is chained more than once while also having many=True at the same time. As matter of fact this would have silently yielded the wrong schema if InlineWithOverrideSerializer wouldn't have been empty.

With that construction you snug by all the override resolution guards and - as you rightly noted - we then walk past the override in _unwrap_list_serializer and step down into the wrong execution path.

causes some additional changes like:

Yes I noticed. Although that change is functionally identical, I'm not sure yet if we want to have that.

I have to think about how to resolve this optimally.

[mwhansen]

Thanks for taking a look! It's not a block now since we can manually use the schema directly rather than through the indirection.

[uroybd]

Interesting. I thought the error is due to the fact that this decorator works on a Serializer class, not an instance of a Serializer.

Can we solve this by passing a second optional parameter to this function to determine if it should be a list or not instead of the Serializer itself?

[tfranzel]

@uroybd the decorators take both serializer class and serializer instance. Instance is usually used to signal a list withXSerializer(many=True). If you pass a class, spectacular will just do XSerializer() (non-list unless it's for def list()) internally.

Adding a extra parameter for this case would be weird, as this behavior is consistent over the whole API.