[Java][Spring] Implement JsonNullable in openapiNullable correctly
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
- [x] Read the contribution guidelines.
- [x] 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:
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./mvnw clean package ./bin/generate-samples.sh ./bin/utils/export_docs_generators.sh./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 :
Commit all changed files../bin/utils/ensure-up-to-date.sh - [x] File the PR against the correct branch:
master(6.3.0) (minor release - breaking changes with fallbacks),7.0.x(breaking changes without fallbacks) - [x] 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)
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?
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 https://github.com/OpenAPITools/openapi-generator/issues/14765 - might be worth discussing there?
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.
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".
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
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.
https://github.com/OpenAPITools/openapi-generator/issues/17538