[BUG][JAVA] optional collection fields are initialized since 7.5.0

[mosesonline]

Bug Report Checklist

• Have you provided a full/minimal spec to reproduce the issue?
• Have you validated the input using an OpenAPI validator (example)?
• Have you tested with the latest master to confirm the issue still exists?
• Have you searched for related issues/PRs?
• What's the actual output vs expected output?
• [Optional] Sponsorship to speed up the bug fix or feature request (example)

Hello,

first: thank you for the great tool.

Description

Our classes look not the same anymore since generator version 7.5.0. All the collections are initialized. I could use the 'containerDefaultToNull' property, but than all collections are null, also the required. Before I change our code I would like to know if this is an expected behavior?

Regards

openapi-generator version

7.5.0+

OpenAPI declaration file content or url

f360c69#diff-4b50afe9f2863efdbde293ab01bf3775b7e99a32d330260569cb2c63db4d7f59

Generation Details

As the test SpringCodegenTest#testCollectionTypesWithDefaults_issue_collection requires I expect the nullable fields to be null and the required fields to be initialised, but both are.

Steps to reproduce

Run the test in the fork and branch https://github.com/mosesonline/openapi-generator/tree/required-collection-issue.
You can see the behavior before in the other branch https://github.com/mosesonline/openapi-generator/tree/required-collection-issue-on74 I implemented the same test there and it is green.

Related issues/PRs

Maybe the new behavior was introduced with #18104?

PR: #18738

Suggest a fix

If it's an issue and not expected behavior, I can work on a fix.

[jpfinne]

@mosesonline
The changes was done in #15891 to "fix" #18080 (23 comments!)
So you ask to revert the behaviour. IMHO you're right.

The use of !required was intially introduced by @wing328 in #14961 to fix #14875 and #14833.

Maybe the main issue is with the handling of containerDefaultToNull that could have a wrong implementation for required collections.
As you say: "I could use the 'containerDefaultToNull' property, but than all collections are null, also the required"
See #14310

I have difficulties finding the "correct" implementation for required and nullable yes/no (not to mention openapiNullable and useOptional for the spring generator). Look for example at the crazy discussion in #14765 (47 comments!)
Finally it was implemented and merged in #17202. That produces unmaintanable mustache templates, multiple bugs and regressions (for example there are NullPointerExceptions when using optional in spring!)

It seems people agree to disagree.
To end the debate we could define a matrix defining the behaviour of nullable/required in the contracts + configOptions in the generator. And potentially adding new configOptions to satisfy everyone's opinion.

[rjeberhard]

@mosesonline, what's the status of this fix? I saw that you created a PR, but that it had at least one failing suite. Is there a way that I or my team can help? This same issue is causing a downstream problem in the Java client. Thanks.

[jpfinne]

@mosesonline I understand your frustation.
I could work on that issue. But currently I have the feeling that several PR are not reviewed nor merged.
It's quite discouraging.

[rjeberhard]

Thanks @jpfinne. I see that the project has a huge number of open PR's. Is there a way that I can help shepherd this one or encourage it to get attention? At least for the Java client, it is making current releases of the client unusable because the client generates empty content "{}" for fields that should be unset.

[wing328]

I could work on that issue. But currently I have the feeling that several PR are not reviewed nor merged.
It's quite discouraging.

@jpfinne sorry to hear that. if you can keep the PR small, definitely it will be much easier to review.

i know contributors like to have one (big) PRs with several enhancements/bug fixes to make their life easier but a PR changing several hundred files (including samples update) is just not easy to review/get it merged.

@rjeberhard if you can help review other PRs and ideally test these locally, that would be great (you can mark the PR as "approved" to draw our attentions).

[mosesonline]

Sorry, for being silent. I would be happy for any help to fix theis issue.

[jorgerod]

Hello @wing328 @mosesonline @jpfinne

This is again a breaking change. We cannot absorb these changes in minor versions.

Also, in my opinion it seems wrong. By default, nullable is false, so if you don't set that property, collections must be initialised.

[mosesonline]

The initial change to initialize the collections was also a breaking change. It broke our code.

[jorgerod]

The initial change to initialize the collections was also a breaking change. It broke our code.

The same thing happened to me...

But the solution I think is to add to the API nullable: true (remember that by default nullable is false).

What do you think?