-
-
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] fix default value for nullable containers #14959
Conversation
if (this.{{name}} == null) { | ||
this.{{name}} = {{{defaultValue}}}; | ||
} | ||
{{/defaultValue}} |
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.
what if openApiNullable=false
& no default value?
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.
good catch, i just spotted the same issue with #14961.
i think we will need to create an empty container (array or set) if it's null.
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.
pushed a fix. please have another look when you've time. thanks.
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.
Code change looks fine. I think you have unrelated changes here
Just one question about error cases.
@@ -17,7 +17,7 @@ | |||
* SpecialModelName | |||
*/ | |||
|
|||
@JsonTypeName("$special[model.name]") | |||
@JsonTypeName("_special_model.name_") |
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.
I wouldn't expect some of these changes from your mustache changes - bad branch?
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 3.x spec, the schema name must conform to
-components.schemas.Schema name $special[model.name] doesn't adhere to regular expression ^[a-zA-Z0-9\.\-_]+$
(this PR also migrates test spec used by spring from 2.0 to 3.0)
{{/openApiNullable}} | ||
{{^openApiNullable}} | ||
private {{>nullableDataType}} {{name}} = {{#required}}{{{defaultValue}}}{{/required}}{{^required}}null{{/required}}; |
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.
If the value is removed when required
is true - does Jackson or something else validate it is set at some point?
Otherwise, will responses to clients will fail because a required field is missing?
I do realize that required means that a default value shouldn't be used. Just trying to think of how the error scenario is detected.
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.
looks like @NotNull
should be used instead per https://stackoverflow.com/a/45877203/677735
(I think it's out of scope of this PR which aims to fix the default value)
FYI. Filed #14981 to migrate remaining config files related to spring generators to use 3.x spec. |
Something in the 7.5.0 release toggled the behavior for me. On 7.4.0 all collections where default to null now with 7.5.0 all collections are initialized empty. Since you seem to be familiar with this topic @wing328 any idea what changed in the last release? |
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
(6.3.0) (minor release - breaking changes with fallbacks),7.0.x
(breaking changes without fallbacks)cc @cachescrubber (2022/02) @welshm (2022/02) @MelleD (2022/02) @atextor (2022/02) @manedev79 (2022/02) @javisst (2022/02) @borsch (2022/02) @banlevente (2022/02) @Zomzog (2022/09)