FasterXML / jackson-modules-java8

Set of support modules for Java 8 datatypes (Optionals, date/time) and features (parameter names)
Apache License 2.0
401 stars 117 forks source link

readerForUpdating(objectToUpdate).readValue(json) behaves unexpectedly on Optional<List> #214

Closed jc84-dev closed 3 years ago

jc84-dev commented 3 years ago

assume we have


class A {
//@JsonMerge  --> if this is added here, it will throw exception in the runtime
public Optional<List<String>> list = Optional.empty();
}

A obj = objectMapper.readValue("{list:['a']", A.class)   --> obj.list = ["a"] as expected
readerForUpdating(obj).readValue({list:['b'])    --> obj.list = ["b"]  which is not as expected.  I expected obj.list = ["a", "b"] here 
cowtowncoder commented 3 years ago

Hmmh. I can see why this is sub-optimal. I think the root cause may be due to Optional itself being immutable (you cannot change value one has, only create new instance), which may be why merge is not attempted.

Quick question: just for reproduction, would behavior be identical if you used AtomicReference? I ask because they both use same base deserializer, ReferenceTypeDeserializer, and should for the most part exhibit same behavior. If so, I could add reproduction using AtomicReference in jackson-databind and same fix would likely apply. If that did not fail the problem is more likely in this package.

jc84-dev commented 3 years ago

AtomicReference works well. looks like it is specific to Optional

cowtowncoder commented 3 years ago

Was about to claim I cannot reproduce this, until I realized I had not initialized Optional field to Optional.empty(). Once I add that, I see the exception.

Note that @JsonMerge is absolutely needed there (or global configuration to default to merge): otherwise that property won't be merged no matter what.

I think I can fix this for 2.12.4.

cowtowncoder commented 3 years ago

As per notes, now fixed in 2.12 branch, to be included in 2.12.4.

jc84-dev commented 3 years ago

thanks