Repository navigation
[kotlin-spring] Implement skipDefaultValues in mustache templates - #23077
mikhailsnnt wants to merge 3 commits into
Conversation
|
thanks for the PR. what about doing it in the openapi normalizer instead by adding a rule to remove the default value from the schema so that all generators can support this enhancement as well? |
|
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. |
There was a problem hiding this comment.
2 issues found across 4 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/KotlinSpringServerCodegen.java">
<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/KotlinSpringServerCodegen.java:80">
P2: Public generator option key was changed from `skipDefaultValues` to misspelled `ommitDefaultNullValues`, breaking backward compatibility and silently ignoring existing caller configs.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/kotlin-spring/dataClassReqVar.mustache">
<violation number="1">
P1: dataClassReqVar.mustache removed the skipDefaultValues guard instead of replacing it with ommitDefaultNullValues, causing unconditional default rendering for required vars.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @Deprecated(message = ""){{/deprecated}}{{#vendorExtensions.x-field-extra-annotation}} | ||
| {{{.}}}{{/vendorExtensions.x-field-extra-annotation}} | ||
| @get:JsonProperty("{{{baseName}}}"){{#isInherited}} override{{/isInherited}} {{>modelMutable}} {{{name}}}: {{#isEnum}}{{#isArray}}{{baseType}}<{{/isArray}}{{classname}}.{{{nameInPascalCase}}}{{#isArray}}>{{/isArray}}{{/isEnum}}{{^isEnum}}{{{dataType}}}{{/isEnum}}? = {{^defaultValue}}null{{/defaultValue}}{{#defaultValue}}{{^isNumber}}{{{defaultValue}}}{{/isNumber}}{{#isNumber}}{{{dataType}}}("{{{defaultValue}}}"){{/isNumber}}{{/defaultValue}} No newline at end of file | ||
| @get:JsonProperty("{{{baseName}}}"){{#isInherited}} override{{/isInherited}} {{>modelMutable}} {{{name}}}: {{#isEnum}}{{#isArray}}{{baseType}}<{{/isArray}}{{classname}}.{{{nameInPascalCase}}}{{#isArray}}>{{/isArray}}{{/isEnum}}{{^isEnum}}{{{dataType}}}{{/isEnum}}?{{^defaultValue}}{{^ommitDefaultNullValues}} = null{{/ommitDefaultNullValues}}{{/defaultValue}}{{#defaultValue}} = {{^isNumber}}{{{defaultValue}}}{{/isNumber}}{{#isNumber}}{{{dataType}}}("{{{defaultValue}}}"){{/isNumber}}{{/defaultValue}} No newline at end of file |
There was a problem hiding this comment.
why not simply remove = null part without an option?
cc @karismann (2019/03) @Zomzog (2019/04) @andrewemery (2019/10) @4brunu (2019/11) @yutaka0m (2020/03) @stefankoppier (2022/06) @e5l (2024/10) @dennisameling (2026/02)
There was a problem hiding this comment.
@wing328
Likely in many use cases = null part is fine.
And also this change would be not backwards compatible
There was a problem hiding this comment.
1 issue found across 3 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/KotlinSpringServerCodegen.java">
<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/KotlinSpringServerCodegen.java:592">
P2: Public option key rename removed backward compatibility: legacy `ommitDefaultNullValues` is no longer accepted, breaking existing generator configurations.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if (additionalProperties.containsKey(OMIT_DEFAULT_NULL_VALUES)) { | ||
| this.setOmitDefaultNullValues(convertPropertyToBoolean(OMIT_DEFAULT_NULL_VALUES)); | ||
| } |
There was a problem hiding this comment.
P2: Public option key rename removed backward compatibility: legacy ommitDefaultNullValues is no longer accepted, breaking existing generator configurations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/KotlinSpringServerCodegen.java, line 592:
<comment>Public option key rename removed backward compatibility: legacy `ommitDefaultNullValues` is no longer accepted, breaking existing generator configurations.</comment>
<file context>
@@ -589,10 +589,10 @@ public void processOpts() {
- if (additionalProperties.containsKey(OMMIT_DEFAULT_NULL_VALUES)) {
- this.setOmmitDefaultNullValues(convertPropertyToBoolean(OMMIT_DEFAULT_NULL_VALUES));
+ if (additionalProperties.containsKey(OMIT_DEFAULT_NULL_VALUES)) {
+ this.setOmitDefaultNullValues(convertPropertyToBoolean(OMIT_DEFAULT_NULL_VALUES));
}
</file context>
| if (additionalProperties.containsKey(OMIT_DEFAULT_NULL_VALUES)) { | |
| this.setOmitDefaultNullValues(convertPropertyToBoolean(OMIT_DEFAULT_NULL_VALUES)); | |
| } | |
| if (additionalProperties.containsKey(OMIT_DEFAULT_NULL_VALUES) || additionalProperties.containsKey("ommitDefaultNullValues")) { | |
| String omitDefaultNullValuesKey = additionalProperties.containsKey(OMIT_DEFAULT_NULL_VALUES) | |
| ? OMIT_DEFAULT_NULL_VALUES | |
| : "ommitDefaultNullValues"; | |
| this.setOmitDefaultNullValues(convertPropertyToBoolean(omitDefaultNullValuesKey)); | |
| } |
|
Hi, this is something that I also need. |
|
Hello @mikhailsnnt @wing328
The latest unresolved issue by cubic-dev-ai seems to be unnecessary, since the misspelled version was never used. |
|
would it be better to implement a rule in openapi normalizer to remove the default values (parameters, properties) so that we don't need to repeat the same feature in all generators ? https://openapi-generator.tech/docs/customization/#openapi-normalizer |
|
Personally, I would prefer the current solution rather than implementing a new rule. |
Summary
skipDefaultValuesoption toKotlinSpringServerCodegenand wire it into mustache templatesdataClassOptVar.mustacheanddataClassReqVar.mustachewith{{^skipDefaultValues}}...{{/skipDefaultValues}}guardsskipDefaultValues=true, generated Kotlin data class properties no longer include= nullor other default value assignmentsTest plan
testSkipDefaultValuestest inKotlinSpringServerCodegenTestthat generates petstore models with the flag enabled and asserts= nullis absentmvn test -pl modules/openapi-generator -Dtest=KotlinSpringServerCodegenTest#testSkipDefaultValuesSummary by cubic
Adds
OMIT_DEFAULT_NULL_VALUEStoKotlinSpringServerCodegento skip default null assignments in generated Kotlin data classes. When enabled, optional properties no longer include "= null".New Features
OMIT_DEFAULT_NULL_VALUESswitch and property to omit default nulls.kotlin-spring/dataClassOptVar.mustacheto guard defaults withomitDefaultNullValues.Migration
OMIT_DEFAULT_NULL_VALUES=trueviaadditionalPropertiesor the generator switch.skipDefaultValues; default behavior is unchanged when not set.Written for commit 24a7a73. Summary will update on new commits.