Skip to content

Optional enum fields are generated with the wrong type #311

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

Closed
frjonsen opened this issue Jan 22, 2021 · 3 comments
Closed

Optional enum fields are generated with the wrong type #311

frjonsen opened this issue Jan 22, 2021 · 3 comments
Labels
🐞bug Something isn't working

Comments

@frjonsen
Copy link

frjonsen commented Jan 22, 2021

Describe the bug
An endpoint which takes a body which has enum fields is being incorrectly generated with the type Union[Unset, None]. Of not here is that the enum itself is correctly generated in its own file, but are not used (or even imported) in the model for the wrapping type.

To Reproduce
This is a minimal specification I came up with, which reproduces the problem:

We generate the client using simply openapi-python-client generate and the specification below, with no additional configuration.

The result of this is

@attr.s(auto_attribs=True)
class EnumWrapper:
    """  """

    myenum: Union[Unset, None] = UNSET
    additional_properties: Dict[str, Any] = attr.ib(init=False, factory=dict)

    def to_dict(self) -> Dict[str, Any]:
        myenum = None

        field_dict: Dict[str, Any] = {}
        field_dict.update(self.additional_properties)
        field_dict.update({})
        if myenum is not UNSET:
            field_dict["myenum"] = myenum

        return field_dict

    @staticmethod
    def from_dict(src_dict: Dict[str, Any]) -> "EnumWrapper":
        d = src_dict.copy()
        myenum = None

        enum_wrapper = EnumWrapper(
            myenum=myenum,
        )

        enum_wrapper.additional_properties = d
        return enum_wrapper

   # Rest of the file omitted for brevity

Expected behavior
We only just upgraded to the version which uses Unset, so I am not quite sure what exactly the type should be, but I assume something along the lines of either Optional[MyEnum], Union[MyEnum, Unset] or Union[Optional[MyEnum], Unset]

OpenAPI Spec File

openapi: 3.0.2
info:
  title: bar
  version: unknown
paths:
  /foo:
    post:
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/EnumWrapper'
        required: true
      responses:
        '204':
          description: Successful Response
components:
  schemas:
    MyEnum:
      title: MyEnum
      enum:
        - TypeA
        - TypeB
      type: string
      description: An enumeration.
    EnumWrapper:
      title: EnumWrapper
      type: object
      properties:
        myenum:
          allOf:
            - $ref: '#/components/schemas/MyEnum'

Desktop (please complete the following information):

  • OS: [Ubuntu 20.04]
  • Python Version: 3.7.9
  • openapi-python-client version: 0.7.3
@frjonsen frjonsen added the 🐞bug Something isn't working label Jan 22, 2021
@frjonsen
Copy link
Author

frjonsen commented Jan 25, 2021

After further testing, I've realized what makes this break is the allOf in

properties:
  myenum:
    allOf:
      - $ref: '#/components/schemas/MyEnum'

This works fine, however

properties:
  myenum:
      $ref: '#/components/schemas/MyEnum'

This also lead me to notice that this problem is not limited to enums, but any nested models where the $ref is inside an allOf. The first version is what FastAPI/Pydantic generates, and as far as I can tell it is a perfectly valid specification.

Although it's not directly related to this library, I noticed this repo is tagged with "FastAPI", so I will also post the workaround I came up with until this is fixed. I added the following to the Config of my Pydantic models and described by their documentation:

@staticmethod
def schema_extra(schema: Dict[str, Any]) -> None:
  for field_name, field_values in schema.get("properties", {}).items():
      if "allOf" not in field_values:
          continue
      # $ref fiels can't have siblings, so only keeping the $ref
      schema["properties"][field_name] = {"$ref": field_values["allOf"][0]["$ref"]}

This works for us because we don't have very complex nesting, but assuming "allOf" only has a single field could probably cause issues for more complex cases.

@dbanty
Copy link
Collaborator

dbanty commented Jan 25, 2021

@frjonsen thanks for following up! If the problem is related to allOf, it's actually because there's no support for it at all yet in this library. There's an open issue for this (#98) and a PR which will add support whenever I get around to reviewing it 😅(#312).

@frjonsen
Copy link
Author

It does indeed seem to be related to #98. Excellent to hear it's already being worked on. I will close this as a duplicate then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants