FasterXML / jackson-modules-java8

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

Absent nested optional shennanigans #310

Open Tillerino opened 4 months ago

Tillerino commented 4 months ago

Uh... so this is partially for science, I guess. It's obscure enough to not matter, but I'd like to understand the reasoning. I stumbled upon this because I am working on a smaller databind library myself and of course I'm always comparing to Jackson for sensible defaults. Observe the following:

record R(Optional<Optional<String>> s) { }

R r = new ObjectMapper()
    .registerModule(new Jdk8Module())
    .readValue("{}", new TypeReference<>() {});

assertFalse(r.s.isPresent());

Here, an Optional<Optional<String>> property is absent from the JSON to deserialize. Naively, I expected the property to be set to an empty optional, but actually it's Optional.of(Optional.empty()), so the snippet above crashes.

I didn't find any tests which explicitly cover this here, so I guess it wasn't really considered a worthwhile edge case. But I think following through with these weird cases helps in figuring out consistent behaviour in general. Do you think that the current behaviour (Optional.of(Optional.empty())) is correct or do you think Optional.empty() would be correct?

cowtowncoder commented 4 months ago

Tough question; TBH I am not 100% sure. But I think this is more an artifact of how content type, and empty values are defined than anything else -- so it may be the way it is because implementing different behavior is difficult.

Put another way: resolving type definition works and is not problematic; but the way empty values are resolved leads to recursive calls.

But fwtw, my initial intuition is that you are right and the ideal case would be that the "outermost" Optional would be empty.

So, if you (or anyone else) has time to dig into this, I would consider PR for changing behavior.

Handling is implemented partly in OptionalDeserializer of this module; and partly in its base class from jackson-databind, src/main/java/com/fasterxml/jackson/databind/deser/std/ReferenceTypeDeserializer.java