datamodel-code-generator icon indicating copy to clipboard operation
datamodel-code-generator copied to clipboard

codegen for nullable Enum throws AttributeError

Open mybr4inhurts opened this issue 4 years ago • 7 comments
trafficstars

Hi,

we have encountered a problem when trying to generate code for nullable Enums. This bug is present since at least version 0.7.3.

Describe the bug When generating code from an OpenAPI 3.0.3 spec with a nullable enum for my python project, the datamodel-codegen throws an AttributeError 'NoneType'" object has no attribute 'isIdentifier' though it should be possible to model nullable enums:

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

To Reproduce

Example schema v1.yaml:

openapi: 3.0.3
info:
  version: 0.1.1
  title: testing-enums
paths:
  /product/{product_id}/security-class:
    get:
      description: get the securityClass of a product
      parameters:
      - in: path
        name: product_id
        required: true
        schema:
          type: str
      responses:
        '200':
          description: A Uuid that corresponds to a running task
            content:
              application/json:
                schema:
                  $ref: '#/components/schemas/SecurityClass'
components:
  schemas:
    SecurityClass:
      type: string
      nullable: true
      enum:
      - RC1
      - RC1N
      - RC2
      - RC2N
      - RC3
      - RC4
      - null

Used commandline:

$ datamodel-codegen --disable-timestamp --use-default --strict-nullable --input ~/v1.yaml --output ~/my-project/models/scheduler_v1.py'

Expected behavior The models are generated correctly something like:

class SecurityClass:
   __root__: Optional[SecurityClassEnum] = None

class SecurityClassEnum(Enum):
    RC1 = "RC1"
    RC1N = "RC1N"
    RC2 = "RC2"
    RC2N = "RC2N"
    RC3 = "RC3"
    RC4 = "RC4"

Version:

  • OS: Ubuntu x64 20.04
  • Python version: 3.9
  • datamodel-code-generator version: 0.8.1

Additional context Add any other context about the problem here.

mybr4inhurts avatar Feb 19 '21 15:02 mybr4inhurts

Thank you for creating this issue. I don't expect the pattern. OK, I will fix it soon.

koxudaxi avatar Feb 20 '21 14:02 koxudaxi

@mybr4inhurts I relased a new version as 0.8.2 The problem is fixed on the version.

koxudaxi avatar Feb 21 '21 01:02 koxudaxi

Wow, that was fast! Thanks for that, you are awesome! 👍 💪 The model is generated according to my suggestion now...though it was only a suggestion. I'm nowhere near an openapi expert. 😅 However, sadly I have encountered a pydantic bug with these new models: "null" is not an allowed value for security_class.

After digging down the rabbithole I found an inconsistency in the model creation process. If you define the enum inside of the class that uses it, the models are created correctly. If you instead create a disctinct enum class and use it via ref, the models are generated without the Optional typehint.

how to reproduce

This builds corerrct models with Optional fields:

MyObject:
  type: object
  patameters:
    my_nullable_enum:
      type: string
      nullable: true
      enum:
        - foo
        - bar

will result in correct models:

class MyNullableEnum(Enum):  # not 100% sure with the naming translation here but that's not the point
  foo = "foo"
  bar = "bar"


class MyObject:
   my_nullable_enum: Optional[MyNullableEnum]

However if you use a ref to address your nullable enum, the "my_nullable_enum" field is not Optional anymore:

MyObject:
  type: object
  patameters:
    my_nullable_enum:
      $ref: "#/components/schema/MyNullableEnum"

MyNullableEnum:
  type: string
  nullable: true
  enum:
    - foo
    - bar

generated models:

class My_Nullable_Enum(Enum):
  foo = "foo"
  bar = "bar"


class MyObject:
   my_nullable_enum: MyNullableEnum  # Optional is missing here

so when instanciating a model according to the spec:

MyObject(my_nullable_enum=None)

you will of course encounter a pydantic ValidationError

E   pydantic.error_wrappers.ValidationError: 1 validation error for MyObject
E   my_nullable_enum
E     none is not an allowed value (type=type_error.none.not_allowed)

Can you confirm that or have I maybe screwed up my toolset?

mybr4inhurts avatar Feb 22 '21 11:02 mybr4inhurts

Update: Nevermind, I had an error in my openApi.yaml...an unwanted requirements entry had sneaked itsself into my code...

So the model generation from enums works as expected.

The problem with a "nullable enum" still exists though. It is currently not possible to model an Enum that can have "null" oder None as Field. My suggestion is not quite providing a solution to the problem, because you cannot instanciate the enum properly without evaluating the constructor parameters as first.

To stick with the SecurityClass example:

MyProduct(security_class=None) 

will result in a pydantic ValidationError because if you boil down the code you have to make a decision at one point: Do I instanciate the SecurityClassEnum Object or not.

It is not possible to instanciate a "nullable Enum" with "None" as a valid Enum option.

I suspect this is a pydantic problem because Enums are always of a particular datatype - str in our case. "None" instead is not a string (per definition) so the model itsself may be inconsistent for pydantic. I am not sure how this could be solved...

What you ultimately need, would be something like this (which is not possible, bc you cannot assign to None):

SecurityClass(Enum):
  RC1 = "RC1"
  RC2 = "RC2"
  None = "null"

so an instanciation of SecurityClass(None) would be possible

but what you can do is:

SecurityClass(Enum):
  RC1 = "RC1"
  RC2 = "RC2"
  null = None

which results in:

>>> SecurityClass(None)
<SecurityClass.null: None>

You would have to compensate in the code though to properly deal with SecurityClass.null...

Maybe some else has a good idea how to approach this problem properly?

mybr4inhurts avatar Feb 22 '21 12:02 mybr4inhurts

@mybr4inhurts Thank you for testing the new version. I'm happy that you can clear the first problem 🎉

OpenAPI and Python are not in the same context. It means we may not resolve the problem perfectly.

You would have to compensate in the code though to properly deal with SecurityClass.null... Maybe some else has a good idea how to approach this problem properly?

I feel a workaround with __root__ is not bad. But, I can implement SecurityClass.null. A lot of users haven't used nullable enum yet. we can change behavior. Or adding the CLI option. I'm a little worried the null attribute isn't standard in Python. 🤔 I don't prefer injecting an individual style like null for this library.

If you use the Python version3.8 then, you can use typing.Literal for enum. I show you an example because It may be one of the workarounds.

Command

$  datamodel-codegen --input openapi.yaml --output models.py --input-file-type=openapi --enum-field-as-literal all --target-python-version 3.8

openapi.yaml

openapi: 3.0.3
info:
  version: 1.0.0
  title: testapi
  license:
    name: proprietary
servers: []
paths: {}
components:
  schemas:
    MyObject:
      type: object
      properties:
        my_nullable_enum:
          type: string
          nullable: true
          enum:
            - foo
            - bar

models.py

# generated by datamodel-codegen:
#   filename:  openapi.yaml
#   timestamp: 2021-02-22T17:38:54+00:00

from __future__ import annotations

from typing import Literal, Optional

from pydantic import BaseModel


class MyObject(BaseModel):
    my_nullable_enum: Optional[Literal['foo', 'bar']] = None

Notes

You need mypy or another linter to check the type. And, the option don't support root enum model like MyNullableEnum

components:
  schemas:
    MyObject:
      type: object
      properties:
        my_nullable_enum:
          $ref: "#/components/schema/MyNullableEnum"
    MyNullableEnum:
      type: string
      nullable: true
      enum:
        - foo
        - bar

I hope to hear ideas from users.

koxudaxi avatar Feb 22 '21 18:02 koxudaxi

@koxudaxi Thank you for your response. Your suggestion with using the literal option is great but I personally feel having classes is just a bit more explicit.

I agree, the null option is not a good solution but it was my desperate try to come up with...something, I guess. 😅 I am also fine with working with __root__ and I really don't see a better alternative right now. 🤷‍♂️

OpenAPI and Python are not in the same context. It means we may not resolve the problem perfectly.

Yes, that is the problem after all, I agree.

I feel there still is a bug hidden

Now after testing a bit more I have found that the datamodel codegen does not fully respect the openapi definition for nullable enums, yet. 😟

It is currently possible to model a nullable enum like in your example (without the null element) which would not validate against e.g. spectral.

A nullable enum has to have both, the nullable flag and an enum element null to be valid against all constraints, like mentioned here: 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

Also the codegen would create different models for either with or without the null element.

invalid OpenAPI.yaml

openapi.yaml

this should not validate and throw an error

components:
  schemas:
    MyObject:
      type: object
      properties:
        my_nullable_enum:
          $ref: "#/components/schema/MyNullableEnum"
    MyNullableEnum:
      type: string
      nullable: true
      enum:
        - foo
        - bar

will give you:

models.py

class MyNullableEnum(Enum):
    foo = "foo"
    bar = "bar"

class MyObject(BaseModel):
    my_nullable_enum: Optional[MyNullableEnum] = None

This would make an instanciation of your enum with None impossible which would violate the above mentioned nullable constraints, if I understand that correctly:

foo = MyNullableEnum(None)
> ValueError: None is not a valid MyNullableEnum

valid OpenAPI.yaml

If you add the null element to the nullable enum definition:

openapi.yaml

components:
  schemas:
    MyObject:
      type: object
      properties:
        my_nullable_enum:
          $ref: "#/components/schema/MyNullableEnum"
    MyNullableEnum:
      type: string
      nullable: true
      enum:
        - foo
        - bar
        - null

the codegen will give you the following:

models.py

class MyNullableEnumEnum(Enum):
    foo = "foo"
    bar = "bar"

class MyNullableEnum:
   __root__: Optional[MyNullableEnumEnum] = None

class MyObject:
    my_nullable_enum: Optional[MyNullableEnum] = None

This will enable you to at least instanciate MyNullableEnum(__root__=None). It is not quite an Enum instanciation but I think this is as close as you can get right now.

mybr4inhurts avatar Feb 23 '21 09:02 mybr4inhurts

@mybr4inhurts

Also the codegen would create different models for either with or without the null element.

Yes, I expect this behavior. I don't want to break current behavior because this code-generator is used in a lot of projects.

I implemented nullable-enum only for nullable == true and type == string. The previous version of the generator couldn't generate a valid Enum class with the pattern. (You said the problem on the head of this issue.) It means we didn't break the behavior and fixed it.

But, I agree with your suggestion. The generator should tell about invalid OpenAPI Schema to users. What do you think to show a warning message when nullable is true when null is no in the enum?

koxudaxi avatar Feb 23 '21 17:02 koxudaxi