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 fix: Kotlin generator doesn't support inheritance #1026

Merged
merged 1 commit into from
Sep 19, 2018

Conversation

irishshagua
Copy link
Contributor

@irishshagua irishshagua commented Sep 12, 2018

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: master, 3.3.x, 4.0.x. Default: master.
    • Can't find a 3.3.x branch, so just using default 🤷‍♂️
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.
    @jimschubert @dr4ke616

Description of the PR

Kinda fix for Issue 1025

  • allVars is duplicating child preoperties when models are inherited. Filter out these duplicates in the KotlinSpringServerCodegen
  • isInherited property was not being populated in the CodegenModel, re-parse the models in the KotlinSpringServerCodegen class and populate the property here
  • Add template support for Kotlin models which require inheritance from a base class to support oneOf declarations in the api yaml

@dr4ke616
Copy link
Contributor

LGTM: @jimschubert any thoughts?

@@ -498,4 +502,16 @@ public void execute(Template.Fragment fragment, Writer writer) throws IOExceptio
writer.write(fragment.execute().replaceAll(from, to));
}
}

// Can't figure out the logic in DefaultCodegen but optional vars are getting duplicated when there's
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with you it should be done in DefautlCodegen. I'll try to figure out a fix later in DefaultCodegen so as to fix the issue for other generators as well.

@wing328
Copy link
Member

wing328 commented Sep 18, 2018

Thanks for the PR but your commit (as shown in the Commits tab) is not linked to your Github account, which means this PR won't count as your contribution in https://github.com/OpenAPITools/openapi-generator/graphs/contributors.

Let me know if you need help fixing it.

Ref: https://github.com/OpenAPITools/openapi-generator/wiki/FAQ#how-can-i-update-commits-that-are-not-linked-to-my-github-account

@wing328
Copy link
Member

wing328 commented Sep 18, 2018

Can't find a 3.3.x branch, so just using default

Current master is 3.3.0 as we merged 3.3.x into master previously to prepare for the release.

Copy link
Member

@wing328 wing328 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but would recommend fixing the commit authorship before merging.

  * allVars is duplicating child preoperties when models are inherited. Filter out these duplicates in the KotlinSpringServerCodegen
  * isInherited property was not being populated in the CodegenModel, re-parse the models in the KotlinSpringServerCodegen class and populate the property here
  * Add template support for Kotlin models which require inheritance from a base class to support oneOf declarations in the api yaml
  * Change optional for parameters to use Kotlins nullable
  * Update petstore api with Optional -> Nullable
@irishshagua
Copy link
Contributor Author

Cheers @wing328, changed the git author now...

@dr4ke616 dr4ke616 merged commit b7edad5 into OpenAPITools:master Sep 19, 2018
jaumard pushed a commit to jaumard/openapi-generator that referenced this pull request Sep 21, 2018
…s#1026)

* allVars is duplicating child preoperties when models are inherited. Filter out these duplicates in the KotlinSpringServerCodegen
  * isInherited property was not being populated in the CodegenModel, re-parse the models in the KotlinSpringServerCodegen class and populate the property here
  * Add template support for Kotlin models which require inheritance from a base class to support oneOf declarations in the api yaml
  * Change optional for parameters to use Kotlins nullable
  * Update petstore api with Optional -> Nullable
@wing328 wing328 changed the title Issue 1025: Kotlin generator doesn't support inheritance Bug fix: Kotlin generator doesn't support inheritance Oct 2, 2018
@wing328
Copy link
Member

wing328 commented Oct 2, 2018

@irishshagua thanks again for the PR, which is included in the v3.3.0 minor release: https://twitter.com/oas_generator/status/1046941449609068544

A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
…s#1026)

* allVars is duplicating child preoperties when models are inherited. Filter out these duplicates in the KotlinSpringServerCodegen
  * isInherited property was not being populated in the CodegenModel, re-parse the models in the KotlinSpringServerCodegen class and populate the property here
  * Add template support for Kotlin models which require inheritance from a base class to support oneOf declarations in the api yaml
  * Change optional for parameters to use Kotlins nullable
  * Update petstore api with Optional -> Nullable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants