Do not include null value for non required fields

[fjtirado]

When a not required field is not set, a null value for that field is included in the request. In order to make the request smaller, we can skip that by adding an annotation in the generated code

[michalsomora]

Event though i partially agree with @fjtirado imho i think this feature should be configurable and we shouldnt force the users to skip serialization of null fields since there is a standard way how to do it in quarkus: https://quarkus.io/guides/all-config#quarkus-jackson_quarkus-jackson-serialization-inclusion @hbelmiro could you please share your thoughts on this

[michalsomora]

@ricardozanini maybe you have a time to think about my comment hm?

[mcruzdev]

Leaving my opinion here, I think it should be configurable

[fjtirado]

This affects the payload that is sent by the generated code to the REST server. Since we are using POJOs to generate the request and most optional fields are never set by users, the request was essentially a lot of unneeded properties set to null, which kills the purpose of the optional flag (they are optional, they are not set, but we were always including them in the request, that seems pretty weird, in my opinion) So not sending them seems the right thing to do Is there any use case where the payload should include an optional field set to null? I can only think of some servers where setting a value to null has semantic value (for example deleting an array when the request include a null for that array property). For those ones, I think that, rather than configuration, we should look at if the field is described as nullable in the Open API file. See nullable section here. If it is nullable, then null should not be skipped for that property.

[ricardozanini]

@fjtirado I agree that optional fields should not be forced to null. Otherwise, it smells anti-pattern.

On the other hand, I remember someone in this community raising a concern that a server won't accept a request if an optional field wasn't sent as part of the body (of course, again, a case where the server was badly implemented).

That being said, what should be optional, as in a configuration option, is the ability to force set to null non-required fields to address these edge cases. So this PR can be as is, but we should add a property to revert to the original behavior.

[fjtirado]

For those cases, they added nullable to the open api spec. I think we need another PR to handle nullable. And if the openapi spec file does not match the server behaviour (does not have the nullable and the server behave as it was there), then, as it is probably done in other cases, the file should be edited locally to adjust it to the server behaviour (which is kind of equivalent to add a local property to enforce sending optional fields set to null, with the difference that the null will be sent only for the nullable fields)

[ricardozanini]

Nevertheless, a new PR can address this case. @michalsomora have you faced such problems on your end since you first raised this concern?

[michalsomora]

Yes, I encountered the issue on the client side (post-backend upgrade), where it did not handle missing null fields correctly. This issue definitely needs to be resolved on the client side. However, I wanted to discuss whether it is necessary to enforce non-null serialization at the DTO level when users can configure it globally using:

quarkus.jackson.serialization-inclusion=non-null

In my opinion, the debate about why someone might need to serialize nulls is not relevant. Why does the library prevent configuring serialization as needed? Should we introduce a new option to change this? I would argue against it, as we already have a global configuration that perfectly addresses this requirement.

[fjtirado]

@michalsomora quarkus.jackson.serialization-inclusion will affect all json payloads, however the idea was to remove nulls only for optional fields. Thats why annotations are included for individual properties, becuase not all properties follows the same policy. Null for mandatory fields are still included as part of the payload. As explained before, I think we need a new issue to handle nullable. If nullable is present for an optional field, optional field set to null should be sent.