tfranzel / drf-spectacular

Sane and flexible OpenAPI 3 schema generation for Django REST framework.
https://drf-spectacular.readthedocs.io
BSD 3-Clause "New" or "Revised" License
2.39k stars 265 forks source link

warnings when using django-restql NestedField #289

Closed sommelon closed 3 years ago

sommelon commented 3 years ago

Hello, I mentioned previously that I'm using django-restql for nested serializers. I have to define my serializers like this

class BankAccountSerializer(DynamicFieldsMixin, NestedModelSerializer):
    bank_address = NestedField(AddressSerializer, required=False, allow_null=True, accept_pk_only=True)

The problem is, that the schema gets generated incorrectly. It repeats the same nested serializer for every NestedField there is.

{
  "id": 0,
  "contact_info": {
    "id": 0,
    "street": "string",
    "city": "string",
    "postcode": "string",
    "country": "AF",
    "state": "string",
    "address_line": "string",
    "latitude": "string",
    "longitude": "string"
  },
  "business_detail": {
    "id": 0,
    "street": "string",
    "city": "string",
    "postcode": "string",
    "country": "AF",
    "state": "string",
    "address_line": "string",
    "latitude": "string",
    "longitude": "string"
  },
...
}

In the example above, my AddressSerializer is replacing the ContactInfoSerializer and BusinessDetailSerializer. I think it's because the AddressSerializer is used in my first view in the project. To fix this, I added ref_name to each serializer. It works, but I get a lot of warnings.

Warning #34: Encountered 2 components with identical names "PermissionGroup" and different classes <class 'apps.core.users.serializers.PermissionGroupSerializer'> and <class 'django_restql.fields.NestedFieldWraper.<locals>.NestedSerializer'>. This will very likely result in an incorrect schema. Try renaming one.

Their implementation uses a wrapper that returns a serializer.

Is there a way I can get rid of the warnings and, possibly, the need to define ref_name on every serializer?

tfranzel commented 3 years ago

hi @sommelon, most warning don't have an impact on the schema. they are just supposed to signal that something is fishy.

we do not support django-restql at this moment. nobody tells you not to use drf-spectacular, but ther are no guarantees and your mileage may vary. the highly dynamic nature may prove tricky to properly map. it may be possible to get quite far with writing OpenApiSerializerFieldExtension and OpenApiSerializerExtension. we did something similar with django-polymorphic. currently i don't have the time to implement this (or even try to). feel free to upstream this though, if you attempt to do it.

sommelon commented 3 years ago

The problem I have with the warnings is that it's hard to see if something new is being reported (errors or a different kind of warning).

I will try to fix it using an extension, thanks.

sommelon commented 3 years ago

I tried SerializerField and Serializer extensions, but couldn't get them to work. Instead I overrode _get_serializer_name() in my custom schema. Not the best solution, but it will do it for me.

    def _get_serializer_name(self, serializer, direction):
        name = super()._get_serializer_name(serializer, direction)
        if serializer.__class__.__name__ == 'NestedSerializer':
            name += 'Nested' + str(randint(1,99999))
        return name

It's generating new components with different names every time, so I will use it only when I want to check for new warnings.

sommelon commented 2 years ago

I ended up monkey-patching the drf_spectacular.plumbing.warn name in the projects __init__.py.

import drf_spectacular.plumbing
import drf_spectacular.drainage

def __suppressed_warn(msg):
    if 'components with identical' in msg and '<locals>.Nested' in msg:
        return
    drf_spectacular.drainage.warn(msg)

drf_spectacular.plumbing.warn = __suppressed_warn