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

[JAVA] Generate and use variable name for setting discriminator and fix #9205 #9207

Merged
merged 1 commit into from
Mar 6, 2019

Conversation

DaveChamberlinKidd
Copy link
Contributor

@DaveChamberlinKidd DaveChamberlinKidd commented Feb 22, 2019

PR checklist

  • Read the contribution guidelines.
  • Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Doesn't seem to be a name on the technical committee for Java.
The original change was made by @SergeyLyakhov and the person who authorized it @wing328

Description of the PR

This fixes a bug with discriminators that are specified with underscores.

In v2.4.0 a change was made which automatically populated the discriminator variable
with the class name. However whilst the variable name was converted from the model to
a java friendly variable, for example object_type to objectType in the parent class
the set variable didn't translate accordingly and so leaves uncompilable code.

As a result the generated code produced was...

  public ParentObject() {
    this.object_type = this.getClass().getSimpleName();
  }
  public ParentObject objectType(String objectType) {
    this.objectType = objectType;
    return this;
  }

and not

    this.objectType = this.getClass().getSimpleName();

I created new discriminatorClassVarName variable in the CodegenModel to store the converted discriminator name and populated it with the discriminator value converted to the correct style by calling the existing toParamName method.

I'm happy to refactor that code before merging in as the nested ifs are getting a bit nasty but didn't want to do that until someone had signed off on the approach as I don't know the code base and might be missing a better solution.

Also change the pojo mustache template to use the new variable rather than the plain discriminator value currently used.

If the PR is missing info you need apologies it's my first.

This fixes a bug with discriminators that are specified with underscores.

In v2.4.0 a change was made which automatically populated the discriminator variable
with the class name. However whilst the variable name was converted from the model to
a java friendly variable, for example object_type to objectType in the parent class
the set variable didn't translate accordingly and so leaves uncompilable code.

As a result the generated code produced was...

```
  public ParentObject() {
    this.object_type = this.getClass().getSimpleName();
  }
  public ParentObject objectType(String objectType) {
    this.objectType = objectType;
    return this;
  }
```
and not

```
    this.objectType = this.getClass().getSimpleName();
```
@DaveChamberlinKidd DaveChamberlinKidd changed the title Generate and use variable name for setting discriminator and fix #9205 [JAVA] Generate and use variable name for setting discriminator and fix #9205 Feb 22, 2019
@DaveChamberlinKidd
Copy link
Contributor Author

Also feeling bad that I haven't added a test though you can see the example swagger.yaml in the issue I raised #9205.

@HugoMario HugoMario merged commit c67ffd5 into swagger-api:master Mar 6, 2019
@HugoMario
Copy link
Contributor

hey @flamangoes, thanks for this PR, changes looks pretty neat for me, it will only affect the files where new property is used, so i think it's ok to merge. i'm going to check this issue to see if it's present on v3 and port it .

@DaveChamberlinKidd
Copy link
Contributor Author

DaveChamberlinKidd commented Mar 7, 2019 via email

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

Successfully merging this pull request may close these issues.

2 participants