drf-spectacular: Dataclass Extension does not avoid serializer name collisions

Describe the bug This is how we get_name for a Dataclass Serializer self.target.dataclass_definition.dataclass_type.__name__ This will lead to collisions when you have multiple dataclasses with the same name or different serializers that use the same dataclass as the base To Reproduce

class MySerializer(DataclassSerializer):
    class Meta:
        dataclass = MyDataclass
class MyOtherSerializer(DataclassSerializer):
    my_field = serializers.CharField()
    class Meta:
        dataclass = MyDataclass

Issue: There is no collision avoidance when it comes to using dataclass serializers.

Expected behavior I’m not sure how collisions are avoided with normal serializers (I think a postfix of some kind is applied), but I would expect a similar collision avoidance scheme to be used if the extension is unable to provide a unique component name.

Some ideas:

Prefix with some combination of the dataclass module, the serializer module, the serializer name, the serializer ref name
Generate some unique postfix if the name is already present
Allow support for ref_name in Dataclass itself, or dataclass serializer (currently ignored)
Add a get unique name field to the OpenApiDataclassSerializerExtensions and let the extension determine how to get a unique name for the component

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 15 (6 by maintainers)

Commits related to this issue

name overrides for rest_framework_dataclasses #839 — committed to tfranzel/drf-spectacular by tfranzel 2 years ago
Merge pull request #864 from tfranzel/dataclass_improvements name overrides for rest_framework_dataclasses #839 — committed to tfranzel/drf-spectacular by tfranzel 2 years ago

Most upvoted comments

Sry @ford-carta but this was no simple answer and so I had to set aside extra time.

Issue: There is no collision avoidance when it comes to using dataclass serializers.

There is no collision avoidance for Serializer either. We only have collision avoidance for the enum postprocessing hook.

As a matter of fact this is more complicated to do consistently than meets the eye. That much I learned writing that avoidance. The discovery order may matter and you can’t just fix an added class as we don’t know which one was there first. So for avoidance you must change names of all affected classes that share a collision, which might break e.g. a generated client. For that exact reason we emit warnings/errors if serializer collisions are detected, leaving it to the user to come up with a sensible resolution.

Thus, this problem seems to be introduced by and specific to the drf-dataclasses extension, and it probably doesn’t make sense to introduce a generic solution for this.

I think so too.

though that’ll be a backward compatibility break as component names might change.

Yes, that would be a problem and we need to make a very strong argument for that.

I got to admit that I am not a user of this library and so I have not thoroughly considered all cases in depth or rather in what different ways this library can be used.

Aside from a breaking change, I think it would make sense to consider Meta.ref_name as a name override as we do this also for regular serializers. I see no harm in supporting this, and it would be more consistent imho.

As for changing the default naming scheme, I will look into djangorestframework-dataclasses more thoroughly and get a more informed opinion. I don’t want to change it haphazardly.

And now that I think about it, nested dataclasses is probably another shortcoming of the current implementation.

tfranzel on Nov 21, 2022

Thanks for working on this! Really helped out my team. Our schema is 23k lines long, we love the tool. We implemented a view decorator to do DDD like Fast API

  @exposed()
  def post(self, request, body: BodyDomain, query_params: QueryParamsDomain) -> List[ResponseDomain]
      # return domain object, serialization boilerplate taken care of

ford-carta on Feb 9, 2023