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

False positive duplicate Serializer warning when DataclassSerializer used both as top level Serializer and nested within another dataclass #1288

Open
johnthagen opened this issue Sep 5, 2024 · 1 comment

Comments

@johnthagen
Copy link
Contributor

johnthagen commented Sep 5, 2024

Describe the bug

For code reuse, it's often useful to expose endpoints for the same data at different layers within the overall tree of data. If a DataclassSerializer is used for both a parent @dataclass and a child @dataclass a warning is printed.

To Reproduce

from dataclasses import dataclass

from drf_spectacular.utils import extend_schema
from rest_framework.decorators import api_view
from rest_framework.request import Request
from rest_framework.response import Response
from rest_framework_dataclasses.serializers import DataclassSerializer


@dataclass
class Person:
    name: str
    age: int


@dataclass
class Party:
    person: Person
    num_persons: int


class PartySerializer(DataclassSerializer[Party]):
    class Meta:
        dataclass = Party


class PersonSerializer(DataclassSerializer[Person]):
    class Meta:
        dataclass = Person


@extend_schema(responses=PartySerializer)
@api_view()
def party(request: Request) -> Response:
    pass


@extend_schema(responses=PersonSerializer)
@api_view()
def person(request: Request) -> Response:
    pass

Results in a warning when the OpenAPI spec is queried:

views/api/test.py: Warning [person > PersonSerializer]: 
Encountered 2 components with identical names "Person" and different classes <class 'views.api.test.PersonSerializer'> and <class 'rest_framework_dataclasses.serializers.DataclassSerializer'>. 
This will very likely result in an incorrect schema. Try renaming one.

But, in this case, we do want to represent the same original @dataclass in both endpoints and have them generate the same Person object in the OpenAPI spec (i.e. we wouldn't want to generate two types in the API when really we mean a single type that we want clients to use interchangeably).

Also, as far as I can tell, the actual OpenAPI spec is generated correctly, the warning is the only issue. But of course if this is not documented/tested behavior of drf-spectacular that would give me pause, thus why I wanted to report the issue.

The code in question is located at

if query_class != registry_class and not suppress_collision_warning:
warn(
f'Encountered 2 components with identical names "{component.name}" and '
f'different classes {query_class} and {registry_class}. This will very '
f'likely result in an incorrect schema. Try renaming one.'
)

Expected behavior

No warning printed.

Environment

  • Python 3.11.8
drf-spectacular                 0.27.2
djangorestframework-dataclasses 1.3.1
djangorestframework             3.15.2
@johnthagen johnthagen changed the title False positive duplicate Serializer warning when DataclassSerializer used both as top level and nested Serializer False positive duplicate Serializer warning when DataclassSerializer used both as top level Serialized and nested within another dataclass Sep 5, 2024
@johnthagen johnthagen changed the title False positive duplicate Serializer warning when DataclassSerializer used both as top level Serialized and nested within another dataclass False positive duplicate Serializer warning when DataclassSerializer used both as top level Serializer and nested within another dataclass Sep 5, 2024
@tfranzel
Copy link
Owner

tfranzel commented Sep 5, 2024

Hi @johnthagen,

I see the problem. In the beginning when we had only actual serializers this made perfect sense. If you have a different serializer class under the same name that is likely gonna be an issue. Here, Person will get converted to a serializer twice (one directly and one indirectly) and those are different, albeit functional identical classes and so the check give a false positive.

The object check in the registry would need to compare the Person dataclass instead of the generated serializer to make this correct. I think we can do this.

tfranzel added a commit that referenced this issue Sep 7, 2024
parametrize component registry identity #1288
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

No branches or pull requests

2 participants