JsonMapper to deserialize `Optional.empty()` instead of `null` in case of missing property (2.x compatibility)

[wendigo]

Search before asking

• 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

[pjfanning]

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

[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?

[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

[JooHyukKim]

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

[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

[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,

[wendigo]

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

[JooHyukKim]

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

[JooHyukKim]

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

[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"])

[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.

[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)

[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.