-
-
Notifications
You must be signed in to change notification settings - Fork 7k
Add option to set container's default to empty container #21269
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
Conversation
7a71f9d
to
4974932
Compare
To test this enhancement, please execute the following and replace petstore.yaml with your own spec
For windows users using DOS prompt, please replace ./mvnw command with
|
Hi, I’ve tested the PR, specifically focusing on the case where we have a nullable, non-required array property. The current implementation would work if the default value were already null, but unfortunately that’s not the case. Due to a regression introduced in version 7.5.0, such properties are now defaulting to empty arrays ([]) even when not required and marked as nullable. As a result, the behavior remains unchanged with this PR. It would be very helpful if the new defaultToEmptyContainer option allowed us to explicitly opt out of setting a default altogether, preserving null for optional containers. That way, we could fully restore the expected behavior for nullable. |
thanks for testing it. i'll try to take another look this weekend |
@KristiAt can you please pull the latest to give it another try? I've updated the PR with some fixes and tests. |
I think that's the current behavior when the option is enabled - optional array is default to null (as its default value in the schema is null) |
for nullable optional properties, please use
but the default value is null by default so looks like there's a bug with nullable optional. I'll add a test to confirm the behaviour. |
i think that's the expected behavior. whatever you provided in the defaultToEmptyContainer option will be default to an empty list/container/set etc. enabling this option will respect the default value provided in the spec. |
just so we're on the same page. I try to generate an array (optional nullable) of integer with spring generator (library: spring-cloud) and this is what I got
and you want to initalize it with null instead, i.e.
is my understanding correct? |
fyi. I tested with
|
using your spec with I got the following in
looks like this meet your requirement
|
Did you perform a |
Yes, I’ve installed version 7.14.0-SNAPSHOT with your changes. Could you please confirm if the usage of in my POM file is correct? |
should it be |
It does not work. I tried it previously. |
can you change you should get the following in the log:
that should confirm you're using the latest snapshot version built from this branch/pr |
Just to make sure. You did pull the latest changes (commits) to your local branch before rebuilding the JARs, right? |
Yes, I can see your code changes from this draft. I am unable to figure out the correct way of using this new parameter in my pom file. |
are you able to repeat the error message in your end? |
I tested the pr branch with our spec by setting the new property to the empty string and it works. No default values are generated anymore. |
@bodograumann thanks for testing the change to confirm it works for your use cases |
No, I'm having trouble finding the correct syntax to add a parameter to my pom.xml file. The command works as expected when passed via the CLI, but I haven’t been able to get it working through the POM configuration. I've tried the following approaches, but none have worked:
|
@KristiAt I'll take another look later this week. looks like a maven plugin usage issue For the time being, I'll merge this PR so that more users can try it out. |
Add option to set container's default to empty container using the generator's option
defaultToEmptyContainer
(this new option is supported by generators that respect the default value in the spec)e.g.
set optional array and map default value to empty container
set nullable array (required) default value to empty container
set nullable array (optional) default value to empty container
to simply enable this option to respect default values in the spec (basically null if not specified):
to close #18735
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.
master
(upcoming7.x.0
minor release - breaking changes with fallbacks),8.0.x
(breaking changes without fallbacks)