Jackson Databind How to correctly configure JsonMapper to deserialize Optional.empty() instead of null in case of missing property

Search before asking

• [x] I searched in the issues and found nothing similar.

Describe the bug

Optional handling differs between Jackson 2.x and 3.0 despite the information that Jdk8 module was pulled in to core.

Version Information

3.0

Reproduction

I have a record class with an Optional field. When I use JsonMapper from Jackson 3.0, I'm unable to configure it in a way that will inject Optional.empty if either value is null or absent in the JSON.

Works in Jackson 2.x:

public class TestOptional
{
    public static void main(String[] args) throws JsonProcessingException {
        JsonMapper.Builder builder = JsonMapper.builder();
        builder.addModule(new Jdk8Module());
        builder.defaultPropertyInclusion(JsonInclude.Value.construct(JsonInclude.Include.NON_ABSENT, JsonInclude.Include.ALWAYS));

        Person person = new Person("Alice", Optional.empty());
        ObjectMapper mapper = builder.build();

        System.out.println(mapper.writeValueAsString(person));
        System.out.println(mapper.readValue(mapper.writeValueAsString(person), Person.class));
        System.out.println(mapper.readValue("{\"name\": \"Alice\"}}", Person.class));
        System.out.println(mapper.readValue("{\"name\": \"Alice\", \"nickname\": null}}", Person.class));
    }

    record Person(String name, Optional<String> nickname)
    {
        public Person {
            requireNonNull(name, "name is null");
            requireNonNull(nickname, "nickname is null");
        }
    }
}

Doesn't work in Jackson 3:

public class TestOptional
{
    public static void main(String[] args)
    {
        JsonMapper.Builder builder = JsonMapper.builder();
        builder.changeDefaultPropertyInclusion(_ -> JsonInclude.Value.construct(JsonInclude.Include.NON_ABSENT, JsonInclude.Include.ALWAYS));

        Person person = new Person("Alice", Optional.empty());
        ObjectMapper mapper = builder.build();

        System.out.println(mapper.writeValueAsString(person));
        System.out.println(mapper.readValue(mapper.writeValueAsString(person), Person.class));
        System.out.println(mapper.readValue("{\"name\": \"Alice\"}}", Person.class));
        System.out.println(mapper.readValue("{\"name\": \"Alice\", \"nickname\": null}}", Person.class));
    }

    record Person(String name, Optional<String> nickname)
    {
        public Person {
            requireNonNull(name, "name is null");
            requireNonNull(nickname, "nickname is null");
        }
    }
}

Result:

{"name":"Alice"}
Exception in thread "main" tools.jackson.databind.exc.ValueInstantiationException: Cannot construct instance of `io.airlift.json.TestOptional$Person`, problem: nickname is null
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); byte offset: #UNKNOWN]
    at tools.jackson.databind.exc.ValueInstantiationException.from(ValueInstantiationException.java:44)
    at tools.jackson.databind.DeserializationContext.instantiationException(DeserializationContext.java:2076)
    at tools.jackson.databind.deser.std.StdValueInstantiator.wrapAsDatabindException(StdValueInstantiator.java:581)
    at tools.jackson.databind.deser.std.StdValueInstantiator.rewrapCtorProblem(StdValueInstantiator.java:602)
    at tools.jackson.databind.deser.std.StdValueInstantiator.createFromObjectWith(StdValueInstantiator.java:289)
    at tools.jackson.databind.deser.ValueInstantiator.createFromObjectWith(ValueInstantiator.java:270)
    at tools.jackson.databind.deser.bean.PropertyBasedCreator.build(PropertyBasedCreator.java:252)
    at tools.jackson.databind.deser.bean.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:697)
    at tools.jackson.databind.deser.bean.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1417)
    at tools.jackson.databind.deser.bean.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:480)
    at tools.jackson.databind.deser.bean.BeanDeserializer.deserialize(BeanDeserializer.java:200)
    at tools.jackson.databind.deser.DeserializationContextExt.readRootValue(DeserializationContextExt.java:265)
    at tools.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:2610)
    at tools.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:1522)
    at io.airlift.json.TestOptional.main(TestOptional.java:22)
Caused by: java.lang.NullPointerException: nickname is null
    at java.base/java.util.Objects.requireNonNull(Objects.java:246)
    at io.airlift.json.TestOptional$Person.<init>(TestOptional.java:31)
    at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:735)
    at tools.jackson.databind.introspect.AnnotatedConstructor.call(AnnotatedConstructor.java:113)
    at tools.jackson.databind.deser.std.StdValueInstantiator.createFromObjectWith(StdValueInstantiator.java:287)
    ... 10 more

Process finished with exit code 1

Expected behavior

Optionals should work :)

Additional context

No response

Comment From: pjfanning

@wendigo I created a unit test but it seems to pass - #5336 - could you review it?

Comment From: JooHyukKim

Some findings...

1. Seems like Jdk8OptionalDeserializer is returning absent value as null.
2. In our case is actually "absent".
3. Our shared configuration indeed should only include NON_ABSENT values? builder.defaultPropertyInclusion(JsonInclude.Value.construct(JsonInclude.Include.NON_ABSENT, JsonInclude.Include.ALWAYS));

I am trying hard to understand,

1. Should NON_ABSENT be ALWAYS? -- To adhere to documentation
2. Or should the behavior be preserved?

Comment From: wendigo

@JooHyukKim I've added some test cases to #5336 which works under 2.x but fails on 3.0. The existing behavior doesn't make sense at all to me (this is common for a POJO that has an Optional field to be deserialized from a JSON that doesn't contain that optional field at all - because what's the point in sending "field": null

Comment From: JooHyukKim

Okay so it turns out this check in 2.x is removed in 3.x

Comment From: JooHyukKim

because what's the point in sending "field": null

Yes, I agree with you, what's the point sending null value? But contradictoriliy, your configuration says only "ONLY include sending null value" value (as far as I understand).

        builder.defaultPropertyInclusion(JsonInclude.Value.construct(JsonInclude.Include.NON_ABSENT, JsonInclude.Include.ALWAYS));

This is why I am re-reading through my findings in above comment

Comment From: wendigo

        /**
         * Value that indicates that properties are included unless their value
         * is:
         *<ul>
         *  <li>null</li>
         *  <li>"absent" value of a referential type (like Java 8 `Optional`, or
         *     {@link java.util.concurrent.atomic.AtomicReference}); that is, something
         *     that would not deference to a non-null value.
         * </ul>
         * This option is mostly used to work with "Optional"s (Java 8, Guava).
         *
         * @since 2.6
         */
        NON_ABSENT,

Comment From: wendigo

Which I read as "emit a field if the Optional has a value"

Comment From: JooHyukKim

Yes, but it doesn't here "{\"name\": \"Alice\"}}? Some examples in the JavaDoc would've been useful here actually.

Comment From: JooHyukKim

Seems like this something has to do with #3601. Heading there.

Comment From: wendigo

    record RecordWithOptionalsOnly(Optional<String> optional)
    {
        public RecordWithOptionalsOnly
        {
            requireNonNull(optional, "optional is null");
        }
    }

    record RecordWithOptionalField(RecordWithOptionalsOnly record)
    {
        public RecordWithOptionalField
        {
            requireNonNull(record, "record is null");
        }
    }

    @Test
    void deserializeNestedOptional() throws Exception
    {
        JsonMapper mapper = JsonMapper.builder()
                .changeDefaultPropertyInclusion(_ -> JsonInclude.Value.construct(JsonInclude.Include.NON_ABSENT, JsonInclude.Include.ALWAYS))
                .build();

        assertEquals(mapper.writeValueAsString(new RecordWithOptionalField(new RecordWithOptionalsOnly(Optional.empty()))), a2q("{'record':{}}"));
        mapper.readValue(mapper.writeValueAsString(new RecordWithOptionalField(new RecordWithOptionalsOnly(Optional.empty()))), RecordWithOptionalField.class);
        mapper.readValue(a2q("{'record':{}}"), RecordWithOptionalField.class);
    }

That doesn't even round trip. It correctly serializes this POJO to {"record": {}} but it fails to deserialize it back:

tools.jackson.databind.exc.ValueInstantiationException: Cannot construct instance of `io.airlift.log.RecordWithOptionalTest$RecordWithOptionalsOnly`, problem: optional is null
 at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); byte offset: #UNKNOWN] (through reference chain: io.airlift.log.RecordWithOptionalTest$RecordWithOptionalField["record"])

Comment From: JooHyukKim

I understand that your previous behavior is failing, but we should be working towards what right and designed behavior. If I understand correctly, I see two complications here....

First, configuration issue. I am not 100% sure if your configuration is correct?

JsonMapper mapper = JsonMapper.builder()
                .changeDefaultPropertyInclusion(_ -> JsonInclude.Value.construct(JsonInclude.Include.NON_ABSENT, JsonInclude.Include.ALWAYS))
                .build();

.. says include only NON_ABSENT values.

Second, regardless of confiuration, even using ALWAYS, OptionalDeserializer would return null because it is hard-coded, like below.

Comment From: wendigo

Yes, it is correct. We've used that in Jackson 2.x and we don't want Optional.empty() fields to be serialized (Optional.empty() was treated as absent)

Comment From: JooHyukKim

So JsonInclude is used correctly, then./

Re-reading through #3601, seems like we COULD change behavior if needed. But need to solve the complexity of handling explicit-null vs absent value.

Comment From: cowtowncoder

Quick note: this issue seems to combine serialization and deserialization aspects: I understand that from end user perspective these are related; but from implementation perspective we need to try to separate them.

So it'd be good to limit this issue into one, or split into 2 issues (keeping this for one side, create another for the other side).

For example, @JooHyukKim 's comment mentions @JsonInclude which only affects serialization (which Optional valued properties are included in serialized JSON -- but then refers to OptionalDeserializer which is part of deserialization. Two are not directly related: and specifically getAbsentValue() has slightly different meaning for serialization and deserialization (in ser, it's whether Java value marks "absent" value; only relevant for Optional (and similar Ref types); in deser, it means "absence [missing] from (incoming) JSON content".

Comment From: wendigo

@cowtowncoder I understand that but as a user of any encoder I'd expect it to round-trip. If I can serialize POJO and get some JSON, I should be able to get this POJO back when deserializing.

Right now I can serialize POJO with Optional<T> field to a JSON that omits field but I won't be able to read it back.

Comment From: cowtowncoder

@wendigo That is fine (user POV), but my point is different: issue/bug NEEDS to be changed to separate the aspects, address just one at a time. I am confused by conflation here. Change(s), if any, will be merged separately as well.

Comment From: wendigo

@cowtowncoder what should I do then?

Comment From: JooHyukKim

@wendigo Would below style-ed usage solve your problem? It works in 3.x and they both round-trip

public class HiHiTest {
    record RecordWithOptionalParam(String name, Optional<String> optional) { }

    JsonMapper mapper = JsonMapper.builder()
            .changeDefaultPropertyInclusion( hi -> JsonInclude.Value.construct(JsonInclude.Include.ALWAYS, JsonInclude.Include.ALWAYS))
            .build();

    @Test
    void deserializeIssue5335Null() throws Exception
    {

        String ser = mapper.writeValueAsString(new RecordWithOptionalParam("SomeName", null));

        RecordWithOptionalParam output = mapper.readValue(ser, RecordWithOptionalParam.class);

        assertEquals("SomeName", output.name());
        assertTrue(output.optional().isEmpty());
    }

    @Test
    void deserializeIssue5335Empty() throws Exception
    {

        String ser = mapper.writeValueAsString(new RecordWithOptionalParam("SomeName", Optional.empty()));

        RecordWithOptionalParam output = mapper.readValue(ser, RecordWithOptionalParam.class);

        assertEquals("SomeName", output.name());
        assertTrue(output.optional().isEmpty());
    }
}

Comment From: wendigo

@JooHyukKim no, it doesn't

@Test
    void deserializeIssue5335Empty() throws Exception
    {
        String ser = "{\"name\":\"SomeName\"}";

        RecordWithOptionalParam output = mapper.readValue(ser, RecordWithOptionalParam.class);

        assertEquals("SomeName", output.name());
        assertTrue(output.optional().isEmpty());
    }

You can round trip if you always emit a optional field but that's not an issue I'm reporting here.

java.lang.NullPointerException: Cannot invoke "java.util.Optional.isEmpty()" because the return value of "io.airlift.mcp.TestMcp$RecordWithOptionalParam.optional()" is null

Comment From: JooHyukKim

And my suggestion was to always emit optional field by configuring to use ALWAYS. Because it seemed you wanted a roundtrip.

Comment From: wendigo

@JooHyukKim we have existing client libraries that are using old jackson and will not emit optional fields and the server that needs to consume it. There is no way to provide a configuration that matches Jackson 2.x semantics around Optionals.

Comment From: JooHyukKim

Hmmmm... then how about not upgrading the mapper to Jackson 3 and keep everything Jackson 2 until anythiing is suggested and resolved? That way there will be no problem.

Comment From: wendigo

@JooHyukKim that's a blocker for us so we cannot upgrade until it's fixed in jackson 3.x line or we have a workaround. That's the only blocker that we caught as we were running Jackson 3.0 tests last week and got it working except for this one.

Comment From: JooHyukKim

Hmmm..... how about overwrite OptionalDeserializer behavior and just return Optional.empty()?

Comment From: wendigo

@JooHyukKim how can I do that?

Comment From: JooHyukKim

I thought it would be ValueDeserializerModifier.modifyReferenceDeserializer() but when constructing Optional values, it bypasses 🤔.

Comment From: cowtowncoder

@wendigo We need to focus on solving one side first, if both ser and deser have issues.

So, I suggest starting by outlining exactly how 3.x changes behavior from 2.x in a way that does not work for your usage. Or, alternatively, doing this for deserialization. And either way avoiding discussing "the other side" except if/as needed to understand requirement for the main part.

Second of all, let's NOT use "System.out.println()" as validation; only assertions. Otherwise there's always confusion on what exactly is expected (and various other issues wrt printing of Java Objects in general).

Comment From: cowtowncoder

Ok: after re-reading, I think what this comes down is the desire to deserialize Optional.empty() in case of MISSING property value (as well as explicit null). And serialization aspect only matters as context (some clients are leaving out serialization, hence missing vs null).

So far so good.

One minor question: test is for Record type -- but is this Record-specific? Would POJO work? (my guess: no, POJO would have the same issue).

But looking at Jdk8OptionalDeserializer.java, it has this which seems like the cause:

    /**
     * Let's actually NOT coerce missing Creator parameters into empty value.
     */
    @Override
    public Object getAbsentValue(DeserializationContext ctxt) {
        // 21-Sep-2022, tatu: [databind#3601] Let's make absent become `null`,
        //   NOT "null value" (Empty)
        return null;
    }

So the reason for behavior is #3601... explicit change.

What we might need is a new DeserializationFeature to allow changing logic here.

Comment From: JooHyukKim

Would this be Optional-specific DeserializationFeature? I'm thinking there could be similar type, but I can't think of one now.