Conversation
There was a problem hiding this comment.
All reported issues were addressed across 212 files
Note: This PR contains a large number of files. cubic only reviews up to 100 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
On a pro plan you can use ultrareview for larger PRs.
Re-trigger cubic
|
Hi @wing328 This PR has been waiting to be reviewed for over a month. I know there are a lot of PRs to review and that it's complicated, but it's pretty important to me. Thank you very much |
There was a problem hiding this comment.
All reported issues were addressed across 51 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 24 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 176 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 17 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…ditexTech/fork-openapi-generator into feature/OpenAPIToolsGH-24002-optional
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 272 files
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
Requires human review: Auto-approval blocked because this review re-detected 3 unresolved issues already reported by Cubic.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="samples/client/petstore/java/webclient-springBoot4-jackson3-jspecify-optional-getters/src/main/java/org/openapitools/client/ApiClient.java">
<violation number="1" location="samples/client/petstore/java/webclient-springBoot4-jackson3-jspecify-optional-getters/src/main/java/org/openapitools/client/ApiClient.java:140">
P2: The mapper supplied to these constructors is discarded, so WebClient codecs and client-side JSON parameter serialization use different mappers. Thread the supplied mapper through the delegating constructor and store it in `this.mapper`.</violation>
</file>
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
Re-trigger cubic
…-jspecify-optional-getters/.travis.yml Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
- JavaSpring: annotate raw getters of non-required nullable fields with @nullable when optionalGettersForNullableFieldsOnly is enabled (JSpecify contract) - JavaSpring: disable Lombok Getter/Data when the option is enabled so Lombok does not generate raw getters bypassing Optional<T> - Java restclient/resttemplate/webclient: register Jackson Jdk8Module and add jackson-datatype-jdk8 dependency when the option is used with Jackson 2, so Optional getters serialize the field value instead of the empty/present shape - Remove stale generated DefaultApi/test files out of sync with the FILES manifest in the three new client samples and regenerate all samples so FILES and disk contents match
There was a problem hiding this comment.
1 issue found across 25 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/main/resources/Java/libraries/resttemplate/pom.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/Java/libraries/resttemplate/pom.mustache:320">
P3: The new dependency block is only rendered for Jackson 2 ({{^useJackson3}}), but every optional-getters sample config uses springBoot4-jackson3, so CI never generates this branch of the template. Generate one Jackson 2 resttemplate (or restclient) sample with optionalGettersForNullableFieldsOnly=true so the added dependency is exercised.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…enerators The option was registered in AbstractJavaCodegen and therefore advertised by every Java-based generator, but only the restclient/resttemplate/webclient libraries and the spring generator implement it in their model templates. Gate the registration behind supportsOptionalGettersForNullableFieldsOnly() (default false), enable it in JavaClientCodegen and SpringCodegen, and opt out in JavaCamelServerCodegen and JavaMicroprofileServerCodegen which inherit from supporting superclasses. Regenerate the affected docs/generators/*.md pages.
The shared Java travis.mustache hardcodes openjdk8-12 regardless of the generator's Java target. For useSpringBoot4 samples (Java 17 requirement) the Travis matrix must list openjdk17 and openjdk21 instead. Emit the JDK list conditionally on useSpringBoot4 and regenerate the affected samples.
There was a problem hiding this comment.
All reported issues were addressed across 45 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
- Add jackson-datatype-jdk8 to the Gradle build of restclient/resttemplate/ webclient when optionalGettersForNullableFieldsOnly is enabled (mirrors pom) - resttemplate: build the JSON converter from a fully configured mapper (JavaTimeModule, Jdk8Module for Jackson 2; JsonMapper for Jackson 3) and register Jdk8Module on the XML mapper as well, so withXml clients serialize Optional getters correctly and the Jackson 3 combination no longer renders a template with a missing JSON converter - restclient: register Jdk8Module on the XML mapper for withXml clients - webclient: apply Jdk8Module to a copy of a caller-supplied mapper so the ApiClient(mapper, dateFormat) constructor honors Optional getters - AbstractJavaCodegen: relocate the Lombok getter-suppression after the lombok annotation parsing (it was being overwritten) and only drop the Getter flag: Lombok skips a getter when an explicit one exists, so lombok.Data-generated accessors no longer shadow the explicit Optional getters - Add unit test covering the Jackson 2/withXml resttemplate generation
There was a problem hiding this comment.
All reported issues were addressed across 12 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Per review discussion: the option is only meaningful with Jackson 3, which
supports java.util.Optional natively.
- Remove the Jdk8Module registration and the jackson-datatype-jdk8
dependency (pom and gradle) added previously for Jackson 2
- Revert the resttemplate buildRestTemplate JSON converter to the plain
Jackson 2 setup; the Jackson 3 branch keeps a fully configured JsonMapper
- Validate the combination: JavaClientCodegen (restclient/resttemplate/
webclient) and SpringCodegen now fail with a clear message when the
option is enabled without useJackson3 (which requires useSpringBoot4)
- Update the option description and regenerate docs/generators/{java,spring}.md
- Update unit tests: happy paths add useJackson3/useSpringBoot4, and a new
test asserts the jackson-2 rejection
There was a problem hiding this comment.
1 issue found across 20 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/SpringCodegen.java">
<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/SpringCodegen.java:690">
P3: This message reads as if enabling either flag should work, but setting only `useSpringBoot4` never turns on Jackson 3: nothing in `processOpts` auto-enables `useJackson3`, and it must be set explicitly (it in turn requires `useSpringBoot4`, enforced on line 685). A user who sets only `useSpringBoot4` still hits this error and gets told to enable something they already enabled. Use the same wording as the sibling generator `JavaClientCodegen` (error at line 437): "enable useJackson3 and useSpringBoot4".</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Closes: #24002
PR checklist
Commit all changed files.
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*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
@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) @martin-mfg (2023/08) @wing328
Summary by cubic
Adds the opt-in
optionalGettersForNullableFieldsOnlyoption, which makes getters returnOptional<T>for non-required, non-nullable fields inJavaandJavaSpringgenerators (closes #24002). The option requires Jackson 3 (useJackson3, which requiresuseSpringBoot4) and is off by default; fields and setters keep their raw types, and nullable and required fields are unaffected.New Features
AbstractJavaCodegenand enables it in the Javarestclient,resttemplate, andwebclientandJavaSpringPOJO templates; other Java-based generators opt out so the option is not advertised.resttemplateJackson 3 branch builds its JSON converter from a fully configuredJsonMapper.Bug Fixes
Optionalwhen the option is enabled.@Getterflag when the option is on so Lombok does not emit raw getters that bypassOptional<T>, and annotates raw nullable getters with@NullableinJavaSpring.Written for commit 04b08b5. Summary will update on new commits.