Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Annotate @extend_schema to accept a list of integers #1174

Closed
johnthagen opened this issue Feb 15, 2024 · 5 comments · Fixed by #1181
Closed

Annotate @extend_schema to accept a list of integers #1174

johnthagen opened this issue Feb 15, 2024 · 5 comments · Fixed by #1181

Comments

@johnthagen
Copy link
Contributor

I'd roughly like to express that my API endpoint directly accepts a list of integers (in my case they will be a list of IDs in the POST body to operate on). The input to the endpoint would look like:

[1, 2, 3, 4]

What I roughly would like to do is:

from rest_framework.fields import IntegerField, ListField

class IdListField(ListField):
    child = IntegerField()

@extend_schema(request=IdListField)
def f():
    ...

but a ListField isn't an actual Serializer.

I also tried

@extend_schema(request=list[int])
def f():
    ...

But both print out the warning

Warning [MyViewSet]: could not resolve request body for POST /api/. Defaulting to generic free-form object. (Maybe annotate a Serializer class?)

I think this is similar to Pydantic'c concept of a RootModel

Is there a way to express this to drf-spectacular?

@tfranzel
Copy link
Owner

Hey John, I was always reluctant to add this because I feared people would abuse it and shoot themselves in the foot instead of properly using many=True (where applicable) and relying on the automatic list detection.

This came of course at the expense of this very valid use-case. You may have noticed it already works for basic types, just not list (e.g. uuid, str, float, etc). So the functionality is already there in principle.

I'll have a look at it again, because I'm not sure anymore if that was an overly cautious decision or not.

@johnthagen
Copy link
Contributor Author

You may have noticed it already works for basic types

Yep, we've used it for bytes where it worked great, which is why I initially tried list[int]. I agree that you are wise to be cautious about deduplicating the whole system also with type hints. In this case DRF can't express a top type in a Serializer that isn't a dict so it might make sense to allow this to allow full flexibility in what can be expressed?

@johnthagen
Copy link
Contributor Author

@tfranzel In my case I simply went ahead and created a serializer with a single ids field that accepts a list of ints. To me, this is sufficient and does also provide validation, which wouldn't be possible for only a list[int] since DRF doesn't support creating a serializer to represent that.

So for me, I'm personally okay to close this issue if you are, since this is somewhat a fundamental limitation of DRF itself.

@kozlek
Copy link

kozlek commented Mar 13, 2024

Hi,

Thanks for your time on this issue, that's exactly what I was looking for ! 🤗
Based on your MR, I would be able to use list[int], which is great !

As of today, my team is enforcing the use of serializers before returning back data.
As mentioned by @johnthagen, list[int] is not supported by DRF using serializers, so we're using the following to reproduce the behaviour of a serializer.

MyResponseSerializerField = serializers.ListField(child=serializers.Integer())

def view():
    data_to_return = [0, 1, 2]
    serialized_data = MyResponseSerializerField.to_representation(data_to_return)
    return Response(data=serialized_data)

It would be awesome to reuse MyResponseSerializerField with drf-spectacular:

MyResponseSerializerField = serializers.ListField(child=serializers.Integer())

@extend_schema(responses={200: MyResponseSerializerField})
def view():
    data_to_return = [0, 1, 2]
    serialized_data = MyResponseSerializerField.to_representation(data_to_return)
    return Response(data=serialized_data)

As of today, we're using this :

@extend_schema(
    responses={
        200: AutoSchema()._map_serializer_field(MyResponseSerializerField, "response"),
    },
)

It works well, but it's not very nice.

Do you think it's possible to accept serializer fields in responses ?
I'd be happy to help with a PR 🙂

@kozlek
Copy link

kozlek commented Mar 15, 2024

I figured out my previous proposition was not working well for many spectacular use cases, especially because of the "list" mechanism that is a condition for many features to work (filters, examples, ...).

As pointed by @johnthagen, the limitation comes from DRF so I opted for a DRF-side fix.

from __future__ import annotations

import typing

from drf_spectacular.extensions import OpenApiSerializerExtension
from rest_framework import serializers

if typing.TYPE_CHECKING:
    from drf_spectacular.openapi import AutoSchema
    from drf_spectacular.utils import Direction, _SchemaType


class FieldSerializer(serializers.Serializer):
    child: serializers.Field = None

    def to_internal_value(self, data):
        return self.fields["child"].to_internal_value(data)

    def to_representation(self, instance):
        return self.fields["child"].to_representation(instance)


class FieldSerializerOpenApiExtension(OpenApiSerializerExtension):
    target_class = FieldSerializer
    match_subclasses = True

    def map_serializer(self, auto_schema: AutoSchema, direction: Direction) -> _SchemaType:
        return auto_schema._map_serializer_field(self.target.fields["child"], direction=direction)

I'm now able to declare serializers with unique fields, and so to leverage the many=True which helps proper detection. Serializer usage is also more natural.

class MyIdsSerializer(FieldSerializer):
    child = serializers.IntegerField()


@extend_schema(responses={200: MyIdsSerializer(many=True)})
def view():
    data = [0, 1, 2]
    serialized_data = MyIdsSerializer(data, many=True)
    return Response(data=serialized_data)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants