[2.10.0] Regression deserializing Optional<JsonNode>

[MatthewPhinney]

See the following code example.

@Immutable
@JsonSerialize(as = ImmutableSimpleTest.class)
@JsonDeserialize(as = ImmutableSimpleTest.class)
public interface SimpleTest {
    @Default
    default Optional<String> getString() {
        return Optional.empty();
    }

    @Default
    default Optional<JsonNode> getJsonNode() {
        return Optional.empty();
    }

    static void main(String[] args) throws IOException {
        final ObjectMapper objectMapper = new ObjectMapper().registerModule(new Jdk8Module());

        SimpleTest simpleTest = ImmutableSimpleTest.builder().build();

        final SimpleTest deserialized = objectMapper.readValue(objectMapper.writeValueAsBytes(simpleTest), SimpleTest.class);
        System.out.println(simpleTest);
        System.out.println(deserialized);
    }
}

When using jackson-databind 2.10.0 and jackson-datatype-jdk8 2.9.10 with immutables value 2.8.1 the code works as expected: deserialized equals simpleTest.

When using jackson-datatype-jdk8 2.10.0, the two values are no longer equal. The deserialized JsonNode is Optional[null] rather than Optional.empty.

[cowtowncoder]

I think I would need a test case that does not use Immutables -- that is, post-processed classes generated. This just to make sure what I test is exactly as reported: test cases should not add new dependencies to frameworks.

[MatthewPhinney]

The following code also exhibits this difference in behavior. Try running the following with jackson-datatype-jdk8 v2.9.10, and then again with v2.10.0.

@JsonSerialize(as = SimpleTest.class)
@JsonDeserialize(as = SimpleTest.class)
public class SimpleTest {
    private Optional<String> name = Optional.empty();
    private Optional<JsonNode> node = Optional.empty();

    public Optional<String> getName() {
        return name;
    }

    public Optional<JsonNode> getNode() {
        return node;
    }

    @Override
    public String toString() {
        return "Name: " + name + "\nNode: " + node;
    }

    @Override
    public boolean equals(Object o) {
        if (o == this) return true;
        if (!(o instanceof SimpleTest)) {
            return false;
        }
        SimpleTest simpleTest = (SimpleTest) o;
        return simpleTest.name.equals(name) && simpleTest.node.equals(node);
    }

    public static void main(String[] args) throws IOException {
        final ObjectMapper objectMapper = new ObjectMapper().registerModule(new Jdk8Module());

        SimpleTest simpleTest = new SimpleTest();

        final SimpleTest deserialized = objectMapper.readValue(objectMapper.writeValueAsBytes(simpleTest), SimpleTest.class);

        System.out.println(deserialized.equals(simpleTest));
        System.out.println(simpleTest);
        System.out.println(deserialized);
    }
}

[cowtowncoder]

Thank you.

[cowtowncoder]

Ok. I can reproduce the test, but I am actually not sure whether this is a bug or feature.

The problem is this: serializing empty Optional needs to become null (since the way Jackson handles Optionals is as thin wrapper that does not include any decoration like JSON Array). But so would Optional.of(NullNode), too. So that in itself would produce ambiguous content. Reading back, similar problem occurs: null could imply Optional.empty() or Optional.of(NullNode).

Behavior wrt deserialization could be changed by one of 2 things:

1. Change JsonNodeDeserializer.getNullValue() to return plain null, instead of NullNode OR
2. Change OptionalDeserializer.referenceValue() to create empty, instead of asking deserializer.getEmptyValue()

but the challenge there is whether this would break some other handling. Doing (1) will break one test in jackson-databind, wrt JDK serializability (although fundamentally just due to NullNode <-> null in json ambiguity). Doing (2) might be safer, although once again it is not certain it would not break something else. But at least I don't think I could do either for 2.10.x. I'll have to think about this wrt 2.11.

[cowtowncoder]

FWTW, if you want to change behavior to work around the issue, you can sub-class (or just create alternative version) of OptionalDeserializer, register via Module (or SimpleModule, but have a look at how Jdk8Module handles it since it's for ReferenceType).

[cowtowncoder]

Was trying to remember why the change was made (on May 7th, 2019), and finally found it:

https://github.com/FasterXML/jackson-databind/issues/2303

which is... legit, I suppose. But I am not sure there is a way to change behavior to make null become "Optional.empty()" without breaking nested reference values.

One possibility I suppose would be adding a configuration setting.

[flobar25]

I think this change is also part of the new behavior : https://github.com/FasterXML/jackson-databind/issues/2430

[proshin-roman]

Just my 5 cents. I was looking around for a way how to differentiate between a field with null value and a missing field by having only the final Java object. And my assumption was that Jackson already handles it by using Optional<?>.

Just a short example:

Assuming I have a Java DTO with a field of Optional<String> type, then I would expect the following behavior of the deserialization process:

• JSON contains the field with null value -> results in an empty Optional in the target java field;
• JSON doesn't contain the field -> results in the null value of the target field.

In that way I could easily differentiate between these two cases and implement the logic accordingly. It might be especially important for implementing PATCH methods in CRUD APIs (no field - no changes; field is null - update the value).

But now I see that the logic is different: Optional is set to empty() in both cases mentioned above. As I can see, it all goes to this method com.fasterxml.jackson.datatype.jdk8.OptionalDeserializer#getNullValue that always wraps the null value into an instance of Optional. In my opinion it's a bug, as it lowers the flexibility of the mapping.

What do you think, guys? Maybe there is a workaround for my problem? If so, I will appreciate any link or idea.

[cowtowncoder]

@proshin-roman Could you please file a new issue, outlining specific request. With 2.13 (or maybe rather 2.14) I think we actually could resolve/improve this -- 2.13 added a new method

JsonDeserializer.getAbsentValue() 

that would be the key here. I was tempted to work on Optional for 2.13.0 but ran out of time.

The reason for new issue would be to distinguish question of 2.10 behavior change (which by now is... defined to be intended as 2.11, 2.12 and 2.13 do it) from the question of how things should work.

You can refer to this issue from the new one as background.

[cowtowncoder]

Closing this issue in favor of new ones.