NullEnum reference generated for nullable enumerated fields

[rvinzent]

Describe the bug
When using a nullable ChoiceField field, a weird NullEnum reference is generated, and this causes issues with auto-generated client modules. While it renders a correct payload in the Swagger UI, it does not play nicely with code generation tools.

my_enum_field:
  nullable: true
  oneOf:
    - $ref: '#/components/schemas/MyEnum'
    - $ref: '#/components/schemas/NullEnum

To Reproduce

class MyEnum(models.TextChoices):
    VALUE = "Value"

class MySerialzer(serializers.Serializer):
    my_enum_field = serializers.ChoiceField(MyEnum.choices, allow_null=True)

Expected behavior

No NullEnum reference should be generated, and the following should be produced.

my_enum_field:
  nullable: true
  $ref: '#/components/schemas/MyEnum'

[tfranzel]

hi @rvinzent, this has come up before. According to the Open API documentation, this is actually the proper way to do it. the NullEnum is only there to prevent having 4 variations of MyEnum. The important part is to have null in the enum list as well as nullable: true, as nullable : true only caters to the allowed types (a.k.a. type: [string, null]) and not allowed values.

https://swagger.io/docs/specification/data-models/enums/

https://github.com/OAI/OpenAPI-Specification/blob/master/proposals/003_Clarify-Nullable.md#if-a-schema-specifies-nullable-true-and-enum-1-2-3-does-that-schema-allow-null-values-see-1900

we also found out that some client generator have trouble with this "correct" modelling. for that case we created a setting to disable the null/blank enums (#185, 05e4b90): 'ENUM_ADD_EXPLICIT_BLANK_NULL_CHOICE': False

does that make sense?

[rvinzent]

Yes! Thank you for the explanation and the fix!

[idesoto-rover]

Hi, the problem with setting ENUM_ADD_EXPLICIT_BLANK_NULL_CHOICE as False is that (as its name implies) the explicit null option is no longer added, which in theory is not correct. Do you think it should be possible to configure drf-spectacular to generate enum types that include the null option but as a single definition (without using oneOf)? Also, instead of generating the 4 variations that include null and blank, it could only generate the variations that are actually used in the API.

Thanks

[tfranzel]

hi @idesoto-rover, so we support the correct way (True), the wrong way (False). i suppose you ask this due to client generation. which client target do you use? the generator targets are very picky with this stuff. have you evaluated that the single definition actually produces the desired outcome? i did some testing myself back then but i don't have notes on it anymore.

[idesoto-rover]

Hi, the issue I'm having is with a validation library that fails to validate this correctly because a valid value is matching both branches of the oneOf. I think it's an issue with the validation library since it shouldn't be matching a NullEnum with a valid enum value, that's why I didn't open an issue in this project and instead just commented here, but in this case having one enum type that contains the null option would be correct OpenAPI and fix the issue. I'm not familiar with the problems with client generation so I'm not sure it would also fix that. If it doesn't fix those issues, then it's probably not worth changing anything, but I thought I would suggest it just in case it helps.

[valentijnscholten]

To me it would more sense if with ENUM_ADD_EXPLICIT_BLANK_NULL_CHOICE: False that a null value would be added to the remaining Enum. Now it basically results in a non-nullable enum field.

[tfranzel]

@valentijnscholten i'm not sure what you mean exactly. with True a null value is added via "mixin".

• True means that null is added via oneOf. that is what the name suggests.
• False means there is no null choice, which is not optimal but some client generators choke with the oneOf.

otherwise you would have to generate 4 different variations of the same Enum. you cannot simply add null to the Enum and be done with it. if you have another field with the same Enum that does not allow being nulled, you have a wrong schema.

we could add another option to generate those 4 but it wouldn't be pretty either.

[valentijnscholten]

Yeah, the code I am working with doesn't understand OneOf nor AllOf nor AnyOf, so I was hoping just to get a simple enum / list of values, but it looks like even with only one possible enum I'll get AllOf.

Curious: Why would it need 4 variations, I can only think of 2. One with null one without?

[tfranzel]

those are the 4 possible variations that DRF allows out of the box:

• Enum
• Enum + "" (blank)
• Enum + null
• Enum + "" (blank) + null

and the naming would even be more horrendous: StatusEnum, StatusNullEnum, StatusBlankEnum, StatusNullBlankEnum.

and adding stuff on demand is not a good idea either. lets say XEnum is only used once with null. for brevity we would call this one XEnum. then somebody add a field where XEnum is used without nullable. then the semantics of XEnum would change and another XNullEnum would need to be added. that would silently break your generated client. not a good idea.

[valentijnscholten]

What I meant was that the most compatible way is to not use components at all for enums. Could result in a bit of duplication, but might be preferable for some, or even needed to make client generation (or validation as mentioned above) work.

I understand that the current way is "the official way", just dealing with some code here that can't handle it.

[tfranzel]

if you don't want the enum extraction functionality you can simply turn it off. this painstakingly created postprocessing hook is purely optional and not part of the core generator.

    # Postprocessing functions that run at the end of schema generation.
    # must satisfy interface result = hook(generator, request, public, result)
    'POSTPROCESSING_HOOKS': [
        'drf_spectacular.hooks.postprocess_schema_enums'
    ],

just disable the hook. that should give you what you asked for.

    'POSTPROCESSING_HOOKS': []

[valentijnscholten]

this painstakingly created postprocessing hook is purely optional and not part of the core generator.
Haha, don't blame the messenger ;-)

Thanks, I didn't realize we could turn it off. But it helps a lot.