Consider the renderer class when generating the schema

[MissiaL]

Describe the bug
When generating a scheme, the renderer_classes is not used. This leads to a problem when the serializer is wrapped in an additional list. We do not use pagination in the viewset.

To Reproduce
Our example

class EnvelopedJSONRenderer(UnicodeJsonRenderer):
    def render(self, data, accepted_media_type=None, renderer_context=None):
        if renderer_context and renderer_context['response'].status_code < 400:
            if data and 'data' in data:
                # data is already enveloped (eg after pagination)
                enveloped_data = {'status': 'ok', **data}
            else:
                enveloped_data = {'status': 'ok', 'data': data}
        else:
            enveloped_data = {'status': 'error', 'error': data}
        return super().render(enveloped_data, accepted_media_type, renderer_context)

class ResponseWithStatus(serializers.Serializer):
    status = serializers.ChoiceField(choices=['ok', 'error'])


class ApiSerializer(ResponseWithStatus):
    data = NotImplemented

@memoize
def list_of(serializer_cls):
    return type(
        f'ApiList{serializer_cls.__name__}',
        (ApiSerializer,),
        {'data': serializer_cls(many=True)},
    )

@method_decorator(
    extend_schema(
        tags=['favorites'],
        responses={
            '200': list_of(FavoriteSerializer),
            '401': ResponseWithStatusAndError,
        },
        operation_id='get_favorites',
    ),
    'list',
)
class FavoritesViewSet(
    mixins.CreateModelMixin,
    mixins.DestroyModelMixin,
    mixins.ListModelMixin,
    GenericViewSet,
):
    lookup_field = 'article_uid'
    permission_classes = [IsAuthenticated]
    serializer_class = FavoriteSerializer
    renderer_classes = (EnvelopedJSONRenderer,)
    queryset = Favorite.objects.all().select_related('article')

After that we got schema:

[{
  "status": "ok",
  "data": [
    {
      "article_path": "string"
    }
  ]
}]

Expected behavior
We need to get the right scheme without wrapped list

{
  "status": "ok",
  "data": [
    {
      "article_path": "string"
    }
  ]
}

I also want to say that the drf_yasg package correctly processes this situation. Maybe you can see the implementation

[tfranzel]

that is an awesomely detailed issue 👍

When generating a scheme, the renderer_classes is not used.

that is correct. there is no (reasonable) way of introspecting the magic in the render method body. you have to notify spectacular somewhere before that. no way around it.

I also want to say that the drf_yasg package correctly processes this situation. Maybe you can see the implementation

that might be a lucky accident... its a 50/50 thing

            '200': list_of(FavoriteSerializer),

so far so good.

    return type(
        f'ApiList{serializer_cls.__name__}',
        (ApiSerializer,),
        {'data': serializer_cls(many=True)},
    )

that does not look like fun 😄

@method_decorator(
    extend_schema(

does that not work without the @method_decorator?. i can see that the @extend_schema(methods=[FOO]) scoping breaks here for list`. interesting.

i believe i found the problem. it is with AutoSchema._is_list_view. pretty sure you found the only way to get a list false positive. i have to think about this.

[tsouvarev]

does that not work without the @method_decorator?

It works if there is method to decorate in declared class (not inherited from base class). This code was copied from viewset without one.

there is no (reasonable) way of introspecting the magic in the render method body. you have to notify spectacular somewhere before that. no way around it.

Well, DRF has found a way around the same problem with pagination using get_paginated_response_schema. You can ask for similar method on renderer

[tfranzel]

It works if there is method to decorate in declared class (not inherited from base class). This code was copied from viewset without one.

the idea in the beginning was that this is also supposed to work for

@extend_schema(..., methods=['POST'])
class XViewset(mixins.CreateModelMixin,GenericViewSet):
   serializer_class = foo
   queryset = bar

but it breaks atm on the GET duality for retrieve/list currently.

Well, DRF has found a way around the same problem with pagination using get_paginated_response_schema. You can ask for similar method on renderer

yes, they found a way there and we already use it in spectacular. would be possible for renderer classes too, but in theory the renderer is supposed to be transparent. at least that is my understanding. i would cautiously guess that this would have only a minor chance of getting merged.

@MissiaL: all the wrapping and all stuff that is going on there does not matter i think. its just that you try to massage an explicit list operation to a retrieve. afais this is the core of the problem:

drf-spectacular/drf_spectacular/openapi.py

Lines 99 to 100 in 97ce18e

	if hasattr(self.view, 'action'):
	return self.view.action == 'list'

[tsouvarev]

in theory the renderer is supposed to be transparent

What do you mean by that? Seems like renderer is default place to put envelopes around data, because other formats like XML will make their own envelope.

its just that you try to massage an explicit list operation to a retrieve

I think that real problem here is that we pass already enveloped serializer to extend_schema_field and spectacular tries to wrap it once more (and in a bad way since we use specific envelope). Is there any way to avoid it?

[tfranzel]

What do you mean by that? Seems like renderer is default place to put envelopes around data, because other formats like XML will make their own envelope.

you make a good point there. i could be totally wrong about this. I just have not encountered DRF renderer modifications like this. though i seen this outside of python before. would be awesome if anyone volunteers to do a PR at DRF for this.

the extend_schema_field is not in the example, but i would guess it does not make a big difference here. here is a minimal test that, i believe, captures the essence of the problem

def test_viweset_list_coerced_to_retrieve(no_warnings):
    class XSerializer(serializers.Serializer):
        x = serializers.IntegerField()

    class XViewset(mixins.ListModelMixin, viewsets.GenericViewSet):
        @extend_schema(responses={200: XSerializer()})
        def list(self, request, *args, **kwargs):
            return super().retrieve(request, *args, **kwargs)

    schema = generate_schema('x', viewset=XViewset)

    operation = schema['paths']['/x/']['get']
    # FAILS
    assert operation['responses']['200']['content']['application/json']['schema']['type'] == 'object'

we had a similar problem in #20. it is still a heuristic. plus you cannot actively do Serializer(many=False) which would provide the necessary signal. i can only see some hack to solve this. if we would just change that one heuristic check above other things would break.

also i have not found out whether yasg has this right by accident or it's just a glitch, because their heuristic is very similar.

would be happy for any suggestions here.

[MissiaL]

would be happy for any suggestions here.

What if you use the new parameter in extend_schema like response_paginator and pass paginator_class

Example:

class ResponsePaginator(LimitOffsetPagination ):
        def get_paginated_response_schema(self, schema):
            return schema['items']

@extend_schema(responses={200: XSerializer()}, response_paginator=ResponsePaginator)
    def list(self, request, *args, **kwargs):
        return super().retrieve(request, *args, **kwargs)

[tfranzel]

i see what you getting at there. is see 3 the problem with that

• it would apply to all responses in the dict (200, 400, etc). that might work for you but does not necessarily apply to everyone.
• response_paginator might be misleading, as it does something it is not originally intended to do.
• the mechanics are not 100% clear. does this parameter coerce the serializer into a list operation? at least that would have to be the case here to make it do what you want. is that always desirable? not sure about that.

[MissiaL]

I agree with you. This is not explicit 😔

Maybe we can add some other parameter that would change the scheme for receiving a response?

[tsouvarev]

I see two approaches here: make heuristics better or give a switch to disable them (because right now they break our scheme and give no way to make it right).
To make heuristics better you can check whether renderer has method like get_rendered_scheme and use it accordingly if it's there. You don't really need support from DRF for that, since end user can subclass pagination and add such method (it's not elegant, but it lets us build correct scheme).

[ghost]

i think the heuristic cannot be improved here. its simply missing one bit of information. in theory serializers would need 3 init states:

1. many=True (override default)
2. many=False (override default)
3. many=None (do heuristic)

but only state 1. and 3. exist. the missing 2. is not achievable due to DRF serializer mechanics.

i will come up with something. probably another decorator. here, you would use it like this decorator(many=False)(type(x,x,x)).

[tfranzel]

b814fa2 should at least get rid of that ugly three param type()

[tfranzel]

added mechanics for explicit serializer defaults override. wasn't all that bad.

caveat: pagination will not work anymore on those endpoints. that would need to be manually handled then. i guess you can't have you cake AND eat it after all.

this is how i achieved your list_of, but your could also use the decorator as i said above.

drf-spectacular/tests/test_regressions.py

Lines 502 to 513 in df22119

	def enveloper(serializer_class, list):
	@extend_schema_serializer(many=False)
	class EnvelopeSerializer(serializers.Serializer):
	status = serializers.BooleanField()
	data = XSerializer(many=list)
	class Meta:
	ref_name = 'Enveloped{}{}'.format(
	serializer_class.__name__.replace("Serializer", ""),
	"List" if list else "",
	)
	return EnvelopeSerializer

[MissiaL]

It works perfect! Thanks!
Could you release a new version?

[tfranzel]

sure thing. please test the versioning #22 and let me know. i'd like to include both things into the next release.