-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
[Spring][server] oneOf Interface for Spring Boot #10392
Conversation
…rator into spring_fix_5381
…rator into spring_fix_5381
…rator into spring_fix_5381 � Conflicts: � modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java
# Conflicts: # modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java
# Conflicts: # modules/openapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java
# Conflicts: # modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java # modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/SpringCodegen.java
…api-generator into jburgess-spring_fix_5381_jb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be a check for JACKSON library. Otherwise, code with GSON will be not compilable.
See PR #10286
|
||
@Override | ||
public void addImportsToOneOfInterface(List<Map<String, String>> imports) { | ||
for (String i : Arrays.asList("JsonSubTypes", "JsonTypeInfo")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review. Can you please submit a PR against this branch with the suggested fix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for (Map<String, Object> mo : models) { | ||
CodegenModel cm = (CodegenModel) mo.get("model"); | ||
if (this.serializableModel) { | ||
cm.getVendorExtensions().putIfAbsent("x-implements", new ArrayList<String>()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use new ArrayList<String>(1)
as you know the size upfront
if (p.defaultValue == null) { | ||
return; | ||
} | ||
Boolean fixLong = (p.isLong && "l".equals(p.defaultValue.substring(p.defaultValue.length()-1))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use boolean
lowercase as values can't be null
@Override | ||
public void addImportsToOneOfInterface(List<Map<String, String>> imports) { | ||
for (String i : Arrays.asList("JsonSubTypes", "JsonTypeInfo")) { | ||
Map<String, String> oneImport = new HashMap<String, String>() {{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
final Map<String, String> oneImport = new HashMap<>(1);
oneImport.put("import", importMapping.get(i));
it's faster and more readable. using {{ }}
is normally considered antipattern unless in static {}
block
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unless in static {} block
A lot of people would complain on this too
} | ||
|
||
@Override | ||
public void postProcessParameter(CodegenParameter p) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the meantime, postProcessParameter() moved to AbstractJavaCodeGen with [Java][Jaxrs-Resteasy] Fixes generator devaultValues for int64/float/… (#8988)
CodegenModel cm = (CodegenModel) mo.get("model"); | ||
if (this.serializableModel) { | ||
cm.getVendorExtensions().putIfAbsent("x-implements", new ArrayList<String>()); | ||
((ArrayList<String>) cm.getVendorExtensions().get("x-implements")).add("Serializable"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
x-implements Serializable is at least duplicated in JavaClientCodeGen.
@wing328 I really would like to get this done. Should I support / bring this PR up to date? |
Hi, I'm also following this PR with great interested. Thanks for the contributor's work on this. I have a question about this. Taking the committed example ByRefOrValue document as an example: Why is it that the generated Apologies if this is explained elsewhere. I didn't find it. |
@cachescrubber @pedrodovale Have you checked this PR #10463 ? |
@Orachigami Thx for the heads up. I'll have a look at #10463. @wing328 This PR is still tagged with the 5.4.0 milestone and not closed. Why is that? |
@wing328 , anyone. Which of the Java generators (if any) has the most complete and stable support for polymorphism (oneOf/anyOf)? EDIT: The java generator (libraries okhttp-gson-nextgen and jersey2) include an implementation where a wrapper class holding the oneOfs is generated, including a custom Deserializer. |
Yes @Orachigami, I did. Thanks to the oneOf unit tests in the fork I was able to model a type and its subtypes and obtain the classes with the desired JsonSubTypes annotations. I've also got the expected sources using the code in this PR. Here is the class that contains the unit tests (hope it helps someone): SpringCodegenTest Cheers! |
@wing328 The branch has merge conflicts. Could you please merge in the current master? If you need any help merging pojo.java feel free to contact me. |
@pedrodovale The classes enumerated in the oneOf might allready extend other classes. Thus the marker interface as an implemenation strategy. Your suggestion is also quite reasonable and reminds me on the XSD way to model extensions - which works quite well indeed. For example the following XSD would generate code as you describe, ie
|
@wing328 @Orachigami, There will be a clash if the class denoted by the oneOf contains a type property, which is an Enum. The Interface will generate
The OAS in question could be downloaded here. Disclaimer: I'am neither the Author nor Api Owner. |
Closed via #11650 |
Resolved merge conflicts for #8091
Credits: @jburgess
PR checklist
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*
.For Windows users, please run the script in Git BASH.
master
(5.3.0),6.0.x
cc @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) @nmuesch (2021/01)