Do not merge Enums of same shape

[jooola]

Describe the bug

In our project, we have multiple enum that have the same shape but with different purposes and sometimes different names.

Some enums share the same name even if they are name spaced in the Models classes. I was wondering if there was a way to suffix the enum names with the parent class if any, instead of having to override the names using ENUM_NAME_OVERRIDES:

class SomeModel(models.Model):
    class Kind(models.IntegerChoices):
        FILE = 0, "File"
        STREAM = 1, "Stream"
        BLOCK = 2, "Block"

    kind = models.SmallIntegerField(
        choices=Kind.choices,
    )

class SomeOtherModel(models.Model):
    class Kind(models.IntegerChoices):
        RADIO = 0, "Radio"
        RANGE = 1, "Range
        CHECKBOX = 2, "Checkbox"

    kind = models.SmallIntegerField(
        choices=Kind.choices,
    )

We found out that if a enum have the same shape than another, the enum definition will be reused in the openapi schema. While keeping the name of the first Enum. This can cause confusion, and weird schema changes if one of them is edited.

To Reproduce https://github.com/libretime/libretime/blob/5505222df632ff8f311d19056db0619501b083cf/api/libretime_api/settings/_internal.py#L166-L179 The following 2 enums are merged together:

• https://github.com/libretime/libretime/blob/5505222df632ff8f311d19056db0619501b083cf/api/libretime_api/storage/models/file.py#L20-L23
• https://github.com/libretime/libretime/blob/5505222df632ff8f311d19056db0619501b083cf/api/libretime_api/schedule/models/playlist.py#L36-L39

Resulting schema bug: https://github.com/libretime/libretime/blob/e97b06496a42d7163eaca5593776cf136f2c1876/api/schema.yml#L5406-L5416

Expected behavior

I expect to have each enum to have there own name and definition, with an automatic parent class prefix if possible to prevent name collision.

[tfranzel]

Hi,

Some enums share the same name even if they are name spaced in the Models classes.

The fact that they are nested inside the models is irrelevant to spectacular. Everybody does this differently and it is not a reliable source of information in general.

All naming is derived from serializer names and serializer field names. It is coincidental if that aligns with your model / model field names. Due to separation of concerns we do not use model names at all.

I was wondering if there was a way to suffix the enum names with the parent class if any,

On naming collision we do attempt to resolve them with a prefix taken from the serializer name. However, this is only done if that resolution is guaranteed to be correct.

We found out that if a enum have the same shape than another, the enum definition will be reused in the openapi schema

That sounds a bit weird to me. The shape is resused only if the serializer field name is identical. Otherwise a duplicate shape is created and and a warning is emitted that draws attention to that potential duplication.

I expect to have each enum to have there own name and definition, with an automatic parent class prefix if possible to prevent name collision.

There should be no breaking collisions. Issues should be highlighted by a warning.

I get that this appears weird, but the problem is that DRF/Django does not allow for a full view on the code. The choices class name is lost and not available anymore when we get our turn. All of the inadequacies can be traced back to information lost in the DRF/Django architecture. Afais, ENUM_NAME_OVERRIDES is the only way to fix those issues short of changing DRF/Django.

With all that said, is there still a bug?

[tfranzel]

Another thing I just saw. There is a gap in what OpenAPI can expose regarding enums. As of 3.1 there is still no structured way to have a key,description tuple for enums, though there have been ongoing discussions about that.

Some people use the description string for documentation. I know this might be a bit frustrating but it is the state of the ecosystem right now.

[jooola]

Ok thanks for the explanation, I was hoping to polish our schema, but we can work with that.

Last question, from what I understand, we have to wait for jsonschema to implement enum descriptions/documentation, which should trickle down to openapi. At this point, will django and/or drf consider adding a way to see such details ? This should allow drf-spectacular to implement a way to extend these details ?

Thanks for your work !

[jooola]

Hi @tfranzel,

With the recent addition of descriptions to the enums, I now clearly see that some of the enums are merged together (or overwritten).

Adding an explicit name in the settings does not fix my issue and an error is raised in the schema generation logs.

I wonder if we can use the description when checking if an enum is duplicate or not ?

See https://github.com/libretime/libretime/pull/2423/commits/c454fe936a8ee37959e2337a652c7bbe61057627

jo@jofix: renovate/main-drf-spectacular-0.x ↓1↑3 ⚑13 ~/git/github.com/libretime/libretime/api $ make schema
.venv/bin/libretime-api spectacular --file schema.yml
provided config filepath '/etc/libretime/config.yml' is not a file
Error: ENUM_NAME_OVERRIDES has duplication issues. Encountered multiple names for the same choice set. Enum naming might be unexpected.

Schema generation summary:
Warnings: 0 (0 unique)
Errors:   1 (1 unique)

if command -v npx > /dev/null; then npx prettier --write schema.yml; fi
schema.yml 718ms

If I change the order of the ENUM_NAME_OVERRIDES items, I end up replacing the old enum with the new one and the description is wrong.

Sorry to dig out this old ticket, but I think this is still the same issue.

[tfranzel]

Hi @jooola, don't worry about the resurrection.

I think before it wasn't a bug in practice because SomeModel.Kind.values() == SomeOtherModel.Kind.values(). So the set was identical, even though the labels were different. Until now we discarded the labels so it made basically no difference in the schema. Imperfection, yes, bug, debatable.

However, now with the added description, this actually became a bug imho. That is because even though the values are still identical, you get a mismatch/override for the labels.

I'm actually trying to make Django (and then DRF) retain the Choice information: https://github.com/django/django/pull/16629. This will help improve name generation, although your case still would produce a name conflict due the same nested name. Maybe we should also account for this common idiom, if we can detect that nesting situation.

[sshishov]

@tfranzel I guess your MR in django regarding choices has been merged. Can we expect to have it working now?

[tfranzel]

nope, my PR had 2 parts. The relevant part where class information is retained was rejected and only a minor convenience feature actually made it. Nothing we can do about it without upstream changes.

[gabn88]

This is also biting us now. Is there currently a work-around?

Just to be sure, we have two integerchoices

StatusChoices(Integerchoices):
    ACTIVE = 1, _('Active')
    INACTIVE = 2, _('Inactive)

IntensityChoices(IntegerChoices):
    LOW = 1, _('low') 
    HIGH = 2, _('high')

These are both merged into one with the same description/labels: IntensityEnum. Nothing to be done about that?