[Java][Spring] Implement JsonNullable in openapiNullable correctly

[daberni]

fixes #14765

JsonNullable was implemented incorrectly, checking for nullable property instead of required property as originally defined by OpenAPI Spec

This is a major breaking change and this PR should primarily be used for discussion. Implementation is only done for JavaSpring first, but other Java CodeGenerators would be affected too.

I first skipped the generation of other samples for the moment, but can be done when further discussion did happen.

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package 
  ./bin/generate-samples.sh
  ./bin/utils/export_docs_generators.sh
  

  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*.
  For Windows users, please run the script in Git BASH.
• [-] In case you are adding a new generator, run the following additional script :

  ./bin/utils/ensure-up-to-date.sh
  

  Commit all changed files.
• File the PR against the correct branch: master (6.3.0) (minor release - breaking changes with fallbacks), 7.0.x (breaking changes without fallbacks)
• If your PR is targeting a particular programming language: @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)

[borsch]

does notNullable == required?

[daberni]

no, these are 2 different descriptions for a property.

But I am currently not 100% if @NotNull JsonNullable<String> would work. The idea is, that if the value is defined, it must not be null. But it is valid that it's undefined.

[daberni]

according to the documentation in jackson-databind-nullable this should work:

The module comes with an integrated ValueExtractor that automatically unwraps the contained value of the JsonNullable if used together with javax.validation Bean validation (JSR 380).

[borsch]

JsonNullable was implemented incorrectly, checking for nullable property instead of required property as originally defined by OpenAPI Spec

Hi @daberni
Could you please point to part of spec which you are referring to?

[welshm]

JsonNullable was implemented incorrectly, checking for nullable property instead of required property as originally defined by OpenAPI Spec

Hi @daberni Could you please point to part of spec which you are referring to?

@borsch - I added my feedback to the issue #14765 - might be worth discussing there?

[jspetrak]

JsonNullable was implemented incorrectly, checking for nullable property instead of required property as originally defined by OpenAPI Spec

Hi @daberni Could you please point to part of spec which you are referring to?

I see it in the second paragram of the README.md file.

[daberni]

JsonNullable was implemented incorrectly, checking for nullable property instead of required property as originally defined by OpenAPI Spec

Hi @daberni Could you please point to part of spec which you are referring to?

I highlighted it in the related issue, it starts with the second sentence of the jackson-databind-nullable Library:

The JsonNullable wrapper shall be used to wrap Java bean fields for which it is important to distinguish between an explicit "null" and the field not being present. A typical usage is when implementing Json Merge Patch where an explicit "null"has the meaning "set this field to null / remove this field" whereas a non-present field has the meaning "don't change the value of this field".

[vershnik]

JsonNullable is needed for Java API server to be able to distinguish between an explicit "null" and the field not being present in the request from the client.

If the field is defined as nullable: false, then Java server has no problem, null in that field should be treated as "not present, do not update the field".

Therefore, using x-is-jackson-optional-nullable only makes sence for fields defined as nullable: true and the current implementation is ok

[jspetrak]

Hello @daberni does this patch also fixes inheritence in combination with nullability? We are hitting this issue #15640 so I would be glad if your fix covered it as well.

[Kavan72]

#17538

[jspetrak]

I know I initially resisted such change in logic, but on the second thought it sounds more logical and we should definitely make this change! (I will try early against JavaSpring+MapStruct code I work on when a RC build is available.)

The maintainers of the project may comment on the actaul changes in the generator templates.