[BUG][JAVA] OneOf and inline creates a selfreference in case of naming matching

danielalmqvist commented 2 weeks ago

Bug Report Checklist

[x] Have you provided a full/minimal spec to reproduce the issue?
[x] Have you validated the input using an OpenAPI validator (example)?
[x] Have you tested with the latest master to confirm the issue still exists?
[x] Have you searched for related issues/PRs?
[x] What's the actual output vs expected output?
[ ] [Optional] Sponsorship to speed up the bug fix or feature request (example)

Description

If I have a a component that references another component in a oneOf the naming of the classes becomes the same and it creates an object with a reference to itself, with other words when generating the classes one of them gets replaced by the other one.

We are not the owners of the schema and can modify it locally, but it is a big file and a lot to modify everytime we get a new version of the definition.

For now we are using manually adding modelNameMapping to handle this, but would prefer if the naming would make sure it does not replace another already defined class.

Expected is that 2 classes that get generated, one for the oneof and one for the actual component. Actual is that only the oneof is generated.

openapi-generator version

7.9.0

OpenAPI declaration file content or url

A simplified json verified in https://editor.swagger.io/ from the 12k line json we use:

{
  "openapi": "3.0.0",
  "info": {
    "title": "API title",
    "version": "1.0.0"
  },
  "paths": {
    "/api/test": {
      "get": {
        "operationId": "Test",
        "responses": {
          "200": {
            "description": "",
            "content": {
              "application/json": {
                "schema": {
                  "type": "array",
                  "items": {
                    "$ref": "#/components/schemas/Margin"
                  }
                }
              }
            }
          }
        }
      }
    }
  },
  "components": {
    "schemas": {
      "Margin": {
        "type": "object",
        "additionalProperties": false,
        "properties": {
          "price": {
            "nullable": true,
            "oneOf": [
              {
                "$ref": "#/components/schemas/MarginPrice"
              }
            ]
          },
          "percent": {
            "nullable": true,
            "oneOf": [
              {
                "$ref": "#/components/schemas/Percent"
              }
            ]
          }
        }
      },
      "MarginPrice": {
        "type": "object",
        "additionalProperties": false,
        "properties": {
          "value": {
            "type": "number",
            "format": "decimal",
            "nullable": true
          },
          "currency": {
            "nullable": true,
            "oneOf": [
              {
                "$ref": "#/components/schemas/Currency"
              }
            ]
          }
        }
      },
      "Percent": {
        "type": "object",
        "additionalProperties": false,
        "properties": {
          "value": {
            "type": "number",
            "format": "decimal",
            "nullable": true
          }
        }
      },
      "Currency": {
        "type": "object",
        "description": "Currency identifier.",
        "additionalProperties": false,
        "required": [
          "code"
        ],
        "properties": {
          "code": {
            "type": "string",
            "description": "Three-letter code identifying the currency according to ISO 4217, e.g. \"SEK\", \"EUR\".",
            "maxLength": 3,
            "minLength": 3,
            "pattern": "AUD|CAD|CHF|CNY|CZK|DKK|EUR|GBP|HKD|HUF|IDR|ILS|ISK|JPY|KWD|MAD|MXN|MYR|NOK|NZD|PLN|RUB|SAR|SEK|SGD|THB|TRY|USD|ZAR",
            "nullable": false
          }
        }
      }
    }
  }
}

Related issues/PRs

https://github.com/OpenAPITools/openapi-generator/issues/18039

Suggest a fix

In org.openapitools.codegen.InlineModelResolver.gatherInlineModels it has a code that looks like the following: String schemaName = resolveModelName(prop.getTitle(), modelPrefix + "_" + propName); Resulting in a schemaName that is parentName_propName. In org.openapitools.codegen.AbstractJavaCodegen.toModelName it has the following code:

        // phone_number => PhoneNumber
        final String camelizedName = camelize(nameWithPrefixSuffix);

that is used to define the classname for the class. Resulting in a classname of parentNamePropName

With this example above we would get two schemas MarginPrice and Margin_price. When converting them to classes both will be named MarginPrice and it results in the oneOf-class being the winner and the original MarginPrice gets replaced.

The suggested fix would be to have similar to what uniqueName does for InlineModelResolver, but on classname level.

wing328 commented 3 days ago

With this example above we would get two schemas MarginPrice and Margin_price.

what about using the model name mapping option modelNameMappings to map one of these names to something else?

https://github.com/openapitools/openapi-generator/blob/master/docs/customization.md#name-mapping

danielalmqvist commented 3 days ago

As mentioned in the issue we are using modelNameMappings for these. But we had to write some additional code to find the ones that had the issue. The api is still under development from our provider and they can do some adjustments (but we are not the only consumer) and these are not wrong persay in the definition.

The json-file we are using now is 33k rows and going through it manually would take a while, but we are fixing it with some code to identify it and add it to modelNameMappings.

At the moment we have only 22 occurrences.

So the issue is not major, but rather something that could be improved, since there is features around it. But I would still consider it a bug since it is replacing one implementation with another since there is a clash in the name.

OpenAPITools / openapi-generator