OpenAPITools / openapi-generator

OpenAPI Generator allows generation of API client libraries (SDK generation), server stubs, documentation and configuration automatically given an OpenAPI Spec (v2, v3)
https://openapi-generator.tech
Apache License 2.0
21.77k stars 6.57k forks source link

[BUG][C#] Should use fully-qualified class name for models #16123

Open stevelknievel76 opened 1 year ago

stevelknievel76 commented 1 year ago
Description

I have exactly the same issue as this: https://stackoverflow.com/questions/59692538/ambiguous-reference-error-in-openapi-generated-c-sharp-code

I have a model class called Environment and using the csharp-netcore generator this causes compile errors due to the ambiguous reference.

I could not see this listed as a bug for OpenAPI Generator.

There is an open issue for Swagger codegen https://github.com/swagger-api/swagger-codegen/issues/2630 as mentioned.

Error is:

Api\EnvironmentApi.cs(1053,173): error CS0104: 'Environment' is an ambiguous reference between 'Org.OpenAPITools.Model.Environment' and 'System.Environment'

openapi-generator version

6.6.0

OpenAPI declaration file content or url
{
  "openapi": "3.0.1",
  "info": {
    "title": "CloudData",
    "version": "1.0"
  },
  "paths": {
    "/api/v1/environments": {
      "get": {
        "tags": [
          "Environment"
        ],
        "responses": {
          "200": {
            "description": "Success",
            "content": {
              "text/plain": {
                "schema": {
                  "type": "array",
                  "items": {
                    "$ref": "#/components/schemas/Environment"
                  }
                }
              },
              "application/json": {
                "schema": {
                  "type": "array",
                  "items": {
                    "$ref": "#/components/schemas/Environment"
                  }
                }
              },
              "text/json": {
                "schema": {
                  "type": "array",
                  "items": {
                    "$ref": "#/components/schemas/Environment"
                  }
                }
              }
            }
          }
        }
      },
      "post": {
        "tags": [
          "Environment"
        ],
        "requestBody": {
          "content": {
            "application/json": {
              "schema": {
                "$ref": "#/components/schemas/Environment"
              }
            },
            "text/json": {
              "schema": {
                "$ref": "#/components/schemas/Environment"
              }
            },
            "application/*+json": {
              "schema": {
                "$ref": "#/components/schemas/Environment"
              }
            }
          }
        },
        "responses": {
          "200": {
            "description": "Success",
            "content": {
              "text/plain": {
                "schema": {
                  "$ref": "#/components/schemas/Environment"
                }
              },
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/Environment"
                }
              },
              "text/json": {
                "schema": {
                  "$ref": "#/components/schemas/Environment"
                }
              }
            }
          }
        }
      }
    },
    "/api/v1/environments/{id}": {
      "get": {
        "tags": [
          "Environment"
        ],
        "parameters": [
          {
            "name": "id",
            "in": "path",
            "required": true,
            "schema": {
              "type": "string",
              "format": "uuid"
            }
          }
        ],
        "responses": {
          "200": {
            "description": "Success",
            "content": {
              "text/plain": {
                "schema": {
                  "$ref": "#/components/schemas/Environment"
                }
              },
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/Environment"
                }
              },
              "text/json": {
                "schema": {
                  "$ref": "#/components/schemas/Environment"
                }
              }
            }
          }
        }
      },
      "put": {
        "tags": [
          "Environment"
        ],
        "parameters": [
          {
            "name": "id",
            "in": "path",
            "required": true,
            "schema": {
              "type": "string",
              "format": "uuid"
            }
          }
        ],
        "requestBody": {
          "content": {
            "application/json": {
              "schema": {
                "$ref": "#/components/schemas/Environment"
              }
            },
            "text/json": {
              "schema": {
                "$ref": "#/components/schemas/Environment"
              }
            },
            "application/*+json": {
              "schema": {
                "$ref": "#/components/schemas/Environment"
              }
            }
          }
        },
        "responses": {
          "200": {
            "description": "Success"
          }
        }
      },
      "delete": {
        "tags": [
          "Environment"
        ],
        "parameters": [
          {
            "name": "id",
            "in": "path",
            "required": true,
            "schema": {
              "type": "string",
              "format": "uuid"
            }
          }
        ],
        "responses": {
          "200": {
            "description": "Success"
          }
        }
      }
    }
  },
  "components": {
    "schemas": {
      "Environment": {
        "type": "object",
        "properties": {
          "id": {
            "type": "string",
            "format": "uuid"
          },
          "name": {
            "type": "string",
            "nullable": true
          }
        },
        "additionalProperties": false
      }
    },
    "securitySchemes": {
      "Bearer": {
        "type": "apiKey",
        "description": "JWT Token issued for your account using Bearer scheme.",
        "name": "Authorization",
        "in": "header"
      }
    }
  },
  "security": [
    {
      "Bearer": [ ]
    }
  ]
}
Generation Details

npm i @openapitools/openapi-generator-cli npx openapi-generator-cli generate -i swagger.json -g csharp-netcore -o output

Steps to reproduce

cd output dotnet build

Related issues/PRs

https://stackoverflow.com/questions/59692538/ambiguous-reference-error-in-openapi-generated-c-sharp-code https://github.com/swagger-api/swagger-codegen/issues/2630

Suggest a fix

C# generator should be changed to use fully qualified class names.

gturri commented 1 year ago

In case other people with the same need arrive here too: it appears that the option --model-name-prefix was added in openapi-generator as well and that it can fix this build issue. (some more details on stackoverflow) So, sure, I would still appreciate having the ability to have fully-qualified-name instead, but at least there is this workaround.

wing328 commented 1 year ago

In case other people with the same need arrive here too: it appears that the option --model-name-prefix

Another way is to use the model name mapping option. https://github.com/OpenAPITools/openapi-generator/blob/master/docs/customization.md#name-mapping

e.g. java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g csharp -i modules/openapi-generator/src/test/resources/3_0/petstore.yaml -o /tmp/csharp/ --model-name-mappings Tag=Label

gturri commented 1 year ago

Thanks for this pointer @wing328 👍 . It's interesting indeed. But, if I understand correctly, it requires to have some "ad hoc" command line for each Open Api specification file. For instance it makes it tedious to have a CI component that can take the Open Api specifications from any team and generate the C# client as some pre-build step (not unless each team has to manually specify the mappin ghtey need).

On the other hand, something that is not clear to me is: why was PR #16178 closed without being merged? @devhl-labs says it generates the full qualidied name everywhere it's needed. Would there be a drawback in merging it?

wing328 commented 1 year ago

full qualidied name everywhere

Agreed it's an elegant solution to to avoid name conflicts and we tried it with the java client generator a while ago. The output doesn't look pretty and it's not what the users want (popular SDKs/frameworks/production code do not use fully qualified names either)

wing328 commented 1 year ago

UPDATE: filed https://github.com/OpenAPITools/openapi-generator/pull/16996 to add "Environment" to the reserved word list. Tested with the spec you provided and no longer see the issue.

gturri commented 12 months ago

Thanks for this feedback @wing328 👍 . I was not aware users don't like having fully qualified class names (in my CI we generate C# and Java clients for all my internal users and they seem to not care about it... but TBH I'm in a context where sources are generated on the fly so users never see those. So I was not aware that in a more general case users don't like it).

Thanks for #16996 ! 👍 Could I selfishly ask that you also include TimeZone and OperatingSystem? (I have users who are using those names in their APIs (which conflicts with classes in System). Sure, I could consider the model name mapping option, but the more it works out of the box, the better :)). Or perhaps you'd rather I propose another PR myself, similar to you one, which would add those 2 names I'm interested in?

wing328 commented 12 months ago

but TBH I'm in a context where sources are generated on the fly so users never see those.

We made similar assumption before but it turns out some users like customising the output a lot to meet their unique requirement.

Could I selfishly ask that you also include TimeZone and OperatingSystem?

I'll add those.

wing328 commented 12 months ago

Just merged #16996.

Please give it a try with the latest master.

gturri commented 12 months ago

thanks!

venuselite commented 11 months ago

Hello @wing328 and @gturri ,

it took me a while to find this issue, but we actually do have an issue with the changes in #16996 We have a field called TimeZone which worked greatly until now.

I tried multiple things to not have this field renamed to varTimeZone but have not succeeded so far.

This is how I call the script:

docker run \
    --rm \
    -v $(pwd):/opt/swagger \
    $commonVirtualRegistry/$openApiDockerImage generate \
    --input-spec /opt/swagger/$SWAGGER_FILE \
    --generator-name csharp \
    --library httpclient \
    --output /opt/swagger/$PROJECT_NAME \
    --model-name-prefix ${MODEL_NAME_PREFIX} \
    --reserved-words-mappings TimeZone=TimeZone \
    --reserved-words-mappings timeZone=timeZone \
    --model-name-mappings timeZone=timeZoneString \
    --model-name-mappings timeZoneString=timeZoneString \
    --additional-properties=packageVersion=$VERSION \
    --additional-properties=packageName=$PROJECT_NAME \
    --additional-properties=targetFramework=$TARGET_FRAMEWORK_DOTNET \
    --additional-properties=useDateTimeOffset=true \
    --additional-properties=validatable=false \
    --additional-properties=useOneOfDiscriminatorLookup=true \
    --additional-properties=optionalEmitDefaultValues=true \
    --global-property skipFormModel=false \
    --verbose

and this is the part from my api definition:

        timeZone:
          type: string
          pattern: '^.*\S.*$'

Any idea how I can resolve this?

Thank you!