openapi-generator icon indicating copy to clipboard operation
openapi-generator copied to clipboard

[Python] Fix the post processing of enums

Open 0xMattijs opened this issue 1 year ago • 4 comments

description

This fixes the issue described in #16560 of a problem with the Python family of converters that when processing enums, none of them substitutes the enum var extensions.

When processing the following schema:

openapi: 3.0.0
info:
  version: 1.0.0
  title: Weather API
paths: {}

components:
  schemas:
    WeatherType:
      type: integer
      format: int32
      enum:
        - 42
        - 18
        - 56
      x-enum-descriptions:
        - 'Blue sky'
        - 'Slightly overcast'
        - 'Take an umbrella with you'
      x-enum-varnames:
        - Sunny
        - Cloudy
        - Rainy

The generator would yield:

from datetime import date, datetime  # noqa: F401

from typing import List, Dict  # noqa: F401

from openapi_server.models.base_model import Model
from openapi_server import util


class WeatherType(Model):
    """NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).

    Do not edit the class manually.
    """

    """
    allowed enum values
    """
    NUMBER_42 = 42
    NUMBER_18 = 18
    NUMBER_56 = 56
    def __init__(self):  # noqa: E501
        """WeatherType - a model defined in OpenAPI

        """
        self.openapi_types = {
        }

        self.attribute_map = {
        }

    @classmethod
    def from_dict(cls, dikt) -> 'WeatherType':
        """Returns the dict as a model

        :param dikt: A dict.
        :type: dict
        :return: The WeatherType of this WeatherType.  # noqa: E501
        :rtype: WeatherType
        """
        return util.deserialize_model(dikt, cls)

The variables have the default NUMBER_x substitutions.

problem

The problem is caused by the fact that the enum var extension substitutions done bypostProcessModelsEnum at the beginning of postProcessModelsMap are overwritten at a later point in that same method.

https://github.com/OpenAPITools/openapi-generator/blob/296a6ac5163b226015511b4bea42b78f0d2966f3/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractPythonCodegen.java#L868

The overwriting of the variable names occurs here:

https://github.com/OpenAPITools/openapi-generator/blob/296a6ac5163b226015511b4bea42b78f0d2966f3/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractPythonCodegen.java#L986C10-L998C14

Note: the proposed solution mentioned in #16560 of calling postProcessModelsEnum as part of postProcessModels seems ineffective, as the override postProcessModels is never called by the generator.

proposed solution

Add a condition to test if x-enum-varnames exist before performing the default NUMBER_x substition.

A previous attempt to fix the issue by moving postProcessModelsEnum to the end of the method lead to a complication with string enum types, ending up with variable names ending up enclosed in double quotes.

With the proposed modification the generated is as expected:

class WeatherType(Model):
    """NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech).

    Do not edit the class manually.
    """

    """
    allowed enum values
    """
    Sunny = 42
    Cloudy = 18
    Rainy = 56
    def __init__(self):  # noqa: E501
        """WeatherType - a model defined in OpenAPI

        """
        self.openapi_types = {
        }

        self.attribute_map = {
        }

checklist

  • [x] Read the contribution guidelines.
  • [x] Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • [x] Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh ./bin/configs/*.yaml
    ./bin/utils/export_docs_generators.sh
    
    (For Windows users, please run the script in Git BASH) Commit all changed files. This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master. These must match the expectations made by your contribution. You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*. IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
  • [x] File the PR against the correct branch: master (upcoming 7.1.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
  • [x] If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

0xMattijs avatar May 03 '24 19:05 0xMattijs

It turns out there are still some complications with this PR, as moving the postProcessModelsEnum will cause the generator to emit faulty variable names for string enums:

E.g.:

 class OuterEnum(str, Enum):
     """
     allowed enum values
     """

    'placed' = 'placed'
    'approved' = 'approved'
    'delivered' = 'delivered'

Expected:

class OuterEnum(str, Enum):
     """
     allowed enum values
     """
     PLACED = 'placed'
     APPROVED = 'approved'
     DELIVERED = 'delivered'

0xMattijs avatar May 03 '24 19:05 0xMattijs

Since the desired enum post processor is run before all other transformations in the Python generator, a working approach is to prevent the enum transformation for int enums to run if the x-enum-varnames vendorExtensions is present.

0xMattijs avatar May 03 '24 21:05 0xMattijs

thanks for the PR. can you please add a test spec to modules/openapi-generator/src/test/resources/3_0/python/petstore-with-fake-endpoints-models-for-testing.yaml ?

wing328 avatar May 06 '24 12:05 wing328

@wing328 I have added the definitions below to petstore-with-fake-endpoints-models-for-testing.yaml and I have executed bin/generate-samples.sh.

Is there anything else I need to do, like defining the expected results somewhere?

        enum_number_vendor_ext:
          type: integer
          format: int32
          enum:
            - 42
            - 18
            - 56
          x-enum-descriptions:
            - 'Description for 42'
            - 'Description for 18'
            - 'Description for 56'
          x-enum-varnames:
            - FortyTwo
            - Eigtheen
            - FiftySix
        enum_string_vendor_ext:
          type: string
          enum:
            - FOO
            - Bar
            - baz
          x-enum-descriptions:
            - 'Description for FOO'
            - 'Description for Bar'
            - 'Description for baz'
          x-enum-varnames:
            - FOOVar
            - BarVar
            - bazVar

0xMattijs avatar May 06 '24 13:05 0xMattijs

thanks for the PR, which has been merged.

have a nice weekend

wing328 avatar May 11 '24 16:05 wing328