[java][client] oneOf support for jackson clients #5120
Conversation
|
👍 Thanks for opening this issue! The team will review the labels and make any necessary changes. |
There was a problem hiding this comment.
LGTM. Thanks for re-targeting against 4.3.x to be on the safe side.
Potential (but not expected) breaking change: @JsonTypeInfo Annotations have changed the include type. Any code which might be using reflection against these should be updated.
Note that this change may overwrite existing generated documentation to include a new "Implemented Interfaces" section. If unwanted, a user will need to override the doc template.
instead of PROPERTY. This extends OpenAPITools#5120 to JavaSpring.
This fixes issue OpenAPITools#3796 for JavaSpring. It's a very straightfoward extension of OpenAPITools#5120 for the JavaSpring generator (that PR was just for the Java generator).
This fixes issue OpenAPITools#3796 for JavaSpring. It's a very straightfoward extension of OpenAPITools#5120 for the JavaSpring generator (that PR was just for the Java generator).
|
Hi @bkabrda, many thanks for addressing this long outstanding issue! I tried generating code for a model using oneOf with master version acf8592 today, and ran into the issue that a number of classes OneOfXxxxYyyyy are imported and used in the generated code, but there are no Java files defining these types. I looked at your changes, and was expecting those to be generated by the oneof_interface.mustache template, but it seems this template is never called. Is this a known issue with this patch, or am I doing something wrong? |
|
@rkoehn hey 👋 the |
|
As @rkoehn, I tried generating code for a model using oneOf with master version and On the other hand, does this issue also solve the Spring code generator? Thank you |
|
So this change only affects Java clients. I don't use server codegens, so I usually don't touch them (however the approach that I used should be easily transferrable for server codegens IMO). Could one of you folks share the full commandline used for generating? |
|
I think I understand the problem I ran into a bit better now. The command I used is: Everything works fine if I use oneOf at the top level of a schema with no additional propeties, effectively making this an interface. However, when I build an object with multiple properties, and one of them is a A complete (broken) example is here: The error occurring during maven build of the generated models is: If I redefine Response in the following way it works, without generating any additional classes: |
|
@bkabrda +1 would like to see this for servers too. I'm using Maven plugin to generate SpringBoot server side code with: <plugin>
<groupId>org.openapitools</groupId>
<artifactId>openapi-generator-maven-plugin</artifactId>
<version>4.2.3</version>
<executions>
<execution>
<id>generate</id>
<goals>
<goal>generate</goal>
</goals>
<configuration>
<inputSpec>${project.basedir}/src/main/resources/api.yaml</inputSpec>
<generatorName>spring</generatorName>
<generateSupportingFiles>true</generateSupportingFiles>
<configOptions>
<sourceFolder>src/gen/java/main</sourceFolder>
<library>spring-boot</library>
<java8>true</java8>
<interfaceOnly>true</interfaceOnly>
<skipDefaultInterface>false</skipDefaultInterface>
<delegatePattern>true</delegatePattern>
</configOptions>
</configuration>
</execution>
</executions>
</plugin> |
|
@rkoehn ah, I see. I thought I did cover that case, but apparently I didn't. Could you please open a new issue to get that tracked, including the reproducer you provided? I'll try to fix that, if I manage to find some time. Thanks! @dkirrane yeah, so my PR didn't affect server generation at all, so that still has the old beavior. That said, it should be possible to extend Java server codegen to also use my approach. Could you please also open a new issue to get that tracked? I'm not sure I'll be able to find time to work on that, but I can definitely provide some pointers on how to do this for anyone who would want to work on it. |
|
openapi-generator version 4.3.0-SNAPSHOT, resttemplate with jackson. Generated model codes: NativeApp.java (WebApp.java, ServiceApp.java, and other related files all have the same problem, skipped pasting here)
Is this related to implementations of this PR? |
|
@lyuanlai yeah, I think the code now assumes that the discriminator property is always of type string, but in your PR it's an enum, which generates a new class in Java. Feel free to open a new issue report for that, I'll try to fix it if time permits. |
|
Understood. I think your reply answers 2), what about 1)? Is that the same root cause too? |
|
@lyuanlai I think these two issues might be related. Feel free to put them in the same issue report and I'll try to investigate. |
copy methods from OpenAPITools#5120 (oneOf support for jackson clients) to implement same in spring
copy methods from OpenAPITools#5120 (oneOf support for jackson clients) to implement same in spring generate oneOf Class that has all properties from inherited classes fix property name of inherited model for oneOf fix imports for oneOf create oneOf stuff only if useOneOfInterfaces is setted
This fixes issue OpenAPITools#3796 for JavaSpring. It's a very straightfoward extension of OpenAPITools#5120 for the JavaSpring generator (that PR was just for the Java generator).
|
Hi all, I've come up with a different implementation to support oneOf in the Java client (jersey2-experimental). Please give it a try and let me know if you've any feedback. Example: I did run some tests locally and the result is good so far. UPDATE: added |
|
@wing328 This seems to be working. Thanks! |
|
Since it's an experimental implementation, we should be able to get it into 4.3.1 release. As discussed with @bkabrda , we'll officially switch to this implementation in the 5.0.0 release (upcoming major release with breaking changes). |
|
dhanyawaad |
Awesome, what is the ETA for the 4.3.1 and 5.0.0? |
|
In 4.3.1 I still have the issue. I tried building 5.0.0-SNAPSHOT, still the same: OneOfx classes are not generated. |
|
Please try the latest master with |
Can you give more detail I
I followed this example, but it seems this branch no longer exists. but it creates lots of useless file, which mess up my dev environment. |
This is exactly the same code as #4785 but submitted for 4.3.x branch (all the commits from that referenced PR were squashed to a single commit here).
PR checklist
./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.shif updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).master,4.3.x,5.0.x. Default:master.@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)