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

[jaxrs-cxf-cdi] Generate better fluent api in case of inheritance #5766

Closed
wants to merge 1 commit into from

Conversation

selliera
Copy link
Contributor

@selliera selliera commented Mar 31, 2020

When there is a use of "allOf" in a model definition, since openapi-generator 4.3.2 the generated model class uses inheritance, and only defines fluent interface methods for local fields.
This is rather inconvenient when creating objects, because some fluent setters return an object of the super type, and not the actual type. This also breaks client code that used that feature from 4.3.x.

I added overridden definitions for the fluent setters when needed. I also included a fix proposed in another pull request #5756, to fix the definition of "equals" and "hashCode". I'm proposing this into a new PR instead of appending to that one because while the "equals" and "hashCode" change is really just a bugfix change, this one may raise more debate.

PR checklist

  • Read the contribution guidelines.
  • If contributing template-only or documentation-only changes which will change sample output, build the project before.
  • Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
  • File the PR against the correct branch: master, 4.3.x, 5.0.x. Default: master.
  • Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

@bbdouglas (2017/07) @sreeshas (2017/08) @jfiala (2017/08) @lukoyanov (2017/09) @cbornet (2017/09) @jeff9finger (2018/01) @karismann (2019/03) @Zomzog (2019/04) @lwlee2608 (2019/10) @bkabrda (2020/01)
JAX-RS CXF (CDI): @nickcmaynard

@selliera selliera force-pushed the cxf-cdi-allof-fluent branch 3 times, most recently from e4504cf to 5852034 Compare April 8, 2020 18:48
@selliera
Copy link
Contributor Author

selliera commented Apr 8, 2020

In the case of inheritance on model classes, I'm considering adding a constructor, to be able to write things like:

Parent p = new Parent().someParentField().someOtherParentField();
Model m = new Model(p).someModelField();

That can be useful when one reuses the definition of Parent in some places. If you have a function to generate and fill a Parent instance, then you can get a Model from it and add Model specifis values.

When using allOf in the model, it is convenient to generated overridden
fluent setters, in order to return the super type instead of the base
type.
Otherwise, one has to use the setters/getters, and this makes the users
code more complex and harder to read.

I added values to the parentVars model field, using the same technique
as used in the csharp model.
@wing328
Copy link
Member

wing328 commented Apr 24, 2020

When there is a use of "allOf" in a model definition, since openapi-generator 4.3.2 the generated model class uses inheritance

It's a bug and has been fixed. Can you please give the latest master another try?

@selliera selliera closed this Apr 24, 2020
@selliera
Copy link
Contributor Author

Yes, with the current master, the allOf behavior is back to what it was with before 4.2.3. This PR is useless now

@selliera selliera deleted the cxf-cdi-allof-fluent branch January 6, 2021 11:44
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