Skip to content
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

[BUG] Python generator incorrectly renames valid model names #3428

Open
rizwansaeed opened this issue Jul 23, 2019 · 6 comments
Open

[BUG] Python generator incorrectly renames valid model names #3428

rizwansaeed opened this issue Jul 23, 2019 · 6 comments

Comments

@rizwansaeed
Copy link
Contributor

rizwansaeed commented Jul 23, 2019

Description

The generator is renaming valid model object names. Looking at https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PythonClientCodegen.java#L119 it seems that it is ignoring casing which means that valid names such as Property are being renamed to ModelProperty

openapi-generator version

4.1.0-SNAPSHOT

OpenAPI declaration file content or url
swagger: "2.0"
info:
  description: "API description"
  version: "1.0.0"
  title: "Demo API"
  license:
    name: "Apache 2.0"
    url: "http://www.apache.org/licenses/LICENSE-2.0.html"
host: "host.com"
basePath: "/v2"
tags:
- name: "myapi"
  description: "do something"

schemes:
- "https"
- "http"
paths:
  /myroute:
    post:
      tags:
      - "myapi"
      summary: "add something"
      description: ""
      operationId: "addSomthing"
      consumes:
      - "application/json"
      produces:
      - "application/json"
      parameters:
      - in: "body"
        name: "body"
        description: "Do Something"
        required: true
        schema:
          $ref: "#/definitions/Property"
      responses:
        405:
          description: "Invalid input"

definitions:
  Property:
    type: "object"
    properties:
      id:
        type: "integer"
        format: "int64"
Command line used for generation
$ docker run --rm -v ${PWD}:/local openapitools/openapi-generator-cli generate -i /local/example.yml -g python -o /local/out/sdk
Suggest a fix

Preserve casing when checking for keywords

@auto-labeler
Copy link

auto-labeler bot commented Jul 23, 2019

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

@Fjolnir-Dvorak
Copy link
Contributor

I will look into it and create a pull request <- Memo to myself so I find the issue again :D

@Fjolnir-Dvorak
Copy link
Contributor

Fjolnir-Dvorak commented Jul 26, 2019

This is actually a bigger issue concerning every generator. The function is implemented in the abstract generator which can not easily be changed since I do not know if casing is done before or after the function call isReservedWord. A change here would require to validate every single generator (with automated tests and checking whether a change would be possible or not.) before the function isReservedWord could be made case sensitive. A short minded fix would be to add another parameter whether to ignore case or not which would be a good idea but presumably would lead to not fixing the bug for every language.

An idea would be, since it changes the output of the generated code in every language, to add this fix as an opt in flag and to change it to default on the next breaking change release.

@rizwansaeed
Copy link
Contributor Author

Looking at the code I suspected that might be the case. For our current use case an optional flag on the Python generator would be helpful 👍

@Fjolnir-Dvorak
Copy link
Contributor

While refactoring I saw that half of the generators cheated and made their own solution instead of fixing this bug. Java for example did it in an most amusing way They are comparing a lower camelCased word with a lowercased one. All reserved words are lowercased. If they would have a camelCased word in the reserved list (which they do) that would not me marked as reserved. Well, guess I have to fix that now, too...

Fjolnir-Dvorak added a commit to Fjolnir-Dvorak/openapi-generator that referenced this issue Jul 26, 2019
…ase keywords as well as case sensitive keywords. Java, PHP and Python had changed, 65 unversioned sample files and 6 changed files. I also added a lot of TODOs to check whether the keywords of a gnerator should be registered case sensitive or not. DefaultCodegen got a new test where I demonstrated that the feature should work as intended. There could be a few bugs fixed in here where the input was lower cased but the reserved word not when comparing them I removed the old reservedWord and replaced it with two new version with case and without. They are also private so that they can not be used directly to assure consistent usage across the different generators
@Fjolnir-Dvorak
Copy link
Contributor

sorry that this issue is not yet patched in live. I still don't have all responses from the other code generator maintainers. I updated the project and will ask if it will be OK to potentially break the other generators since there are no maintainers listed or they didn't respond.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants