projectlombok / lombok

Very spicy additions to the Java programming language.
https://projectlombok.org/
Other
12.91k stars 2.39k forks source link

@Singular fields in @Builder are always non-null after build() #1071

Open Zteve opened 8 years ago

Zteve commented 8 years ago

Re-opening issue number: 968, as it has become a serious problem for us.

.build() on a builder with a @Singular collection field always constructs a (non-null) collection even if the field isn't ever set (added to).

So, a field like private final List<String> things; with a @Builder Cons(@Singular List<String> things) { this.things = things; } constructor, means that Cons.builder().build() will produce a Cons object with an empty non-null list.

We serialize most of our Lombok-generated structures into Json, and if a field is null we don't want it to appear in the generated Json. However, these @Singular fields are never null so they would always be put in the Json. This is incorrect for us, so we have taken to annotating them with @JsonInclude(NON_EMPTY) which explicitly avoids putting them in if they are empty.

Problem solved? No: because now we do not generate any Json when the field is set to an empty collection. There is no solution in the Json annotation, because Lombok conflates "not set" and "set to empty".

We would like fields to remain null (its initial value) if we do not set it on the builder. For all fields. Even @Singular ones. null is not the same as empty. Presently there is no way to set a @Singular field to null.

The fix to this is quite simple: do not generate the empty list(s) backing a @Singular field until one of it's setters is called. When build() is called, only set the @Singular field if the backing list(s) are non-null.

This anomaly prevents setting a @Singular field to null, and should be regarded as a bug.

Zteve commented 8 years ago

PS: Note that changing this semantics doesn't affect any of the guarantees that the @Singular documentation records:

Zteve commented 8 years ago

PPS: A friend of mine has pointed out that this behaviour could be made optional by the inclusion of an "initialization" parameter on @Singular. initEmpty (meaning initially an empty collection) would be the default (current) behaviour, and initNull (meaning the new behaviour) would achieve the result that I desire. This of course would be entirely backward compatible. The modifications to the generating code are fairly trivial.

glyn commented 8 years ago

A use case for this feature is a rich Java API ([1]) driving a correspondingly rich REST API. In the REST API, there are certain "update" operations where an empty collection means "update the collection to empty" and a null collection means "do not update the collection". Essentially, parameters which are not set via the builder need to be left "unset" so the update can be applied only to the parameters which were explicitly set. So there is a semantic distinction between null and an empty collection.

[1] https://github.com/cloudfoundry/cf-java-client

glyn commented 8 years ago

Further background: I am discussing with Immutables.org ([1]) whether they can step up to this requirement. Clearly, we would prefer to stick with Lombok, but the requirement seems quite general and should apply to both projects.

[1] https://groups.google.com/d/msg/immutables/nv0VKt95iUE/US0IEg79IgAJ

Zteve commented 8 years ago

It is a shame, but as this issue is a show-stopper for us, and we have no response here since the issue was raised (and a negative response the first time the issue was aired) it looks as though we are migrating away from Lombok entirely.

mathroule commented 6 years ago

Same issue using Lombok on JSON mapping class. As proposed by @Zteve, having an init empty or null option would be great!

Zteve commented 6 years ago

@mathroule Since there has been no response to this issue (raised nearly two years ago) I wouldn't hold your breath waiting for a solution.

Incidentally, we moved our (large) implementation to Immutables (with some effort but not much pain) and didn't look back.

slorber commented 6 years ago

+1 this is confusing, I'd like to differentiate from null to empty collections in my app. Would be ok for a support involving Optional

krzyk commented 6 years ago

If this will be implemented, then Singular should allow placing a Builder.Default annotation on a collection.

jearton commented 5 years ago

+1

jearton commented 5 years ago

@rzwitserloot Please have a look at a glance.

grv87 commented 5 years ago

PPS: A friend of mine has pointed out that this behaviour could be made optional by the inclusion of an "initialization" parameter on @Singular

Better to reuse existing @NonNull annotation.