If the property is required, the absentValue is ignored and a MismatchedInputException is thrown.

[k163377]

Search before asking

• I searched in the issues and found nothing similar.

Describe the bug

If the property is required, the absentValue is ignored and a MismatchedInputException is thrown.

Version Information

2.15.3

Reproduction

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.JacksonException;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.deser.std.StdDeserializer;
import org.junit.jupiter.api.Test;

import java.io.IOException;

import static org.junit.jupiter.api.Assertions.assertEquals;

public class GitHubXXX {
    static class MyDeser extends StdDeserializer<String> {
        public MyDeser() {
            super(String.class);
        }

        @Override
        public String deserialize(JsonParser p, DeserializationContext ctxt) throws IOException, JacksonException {
            return p.getValueAsString();
        }

        @Override
        public Object getAbsentValue(DeserializationContext ctxt) {
            return "absent";
        }
    }

    static class Dto {
        private final String foo;
        private final String bar;

        @JsonCreator
        Dto(
                @JsonProperty(value = "foo", required = false)
                @JsonDeserialize(using = MyDeser.class)
                String foo,
                @JsonDeserialize(using = MyDeser.class)
                @JsonProperty(value = "bar", required = true)
                String bar
        ) {
            this.foo = foo;
            this.bar = bar;
        }
    }

    @Test
    void test() throws JsonProcessingException {
        ObjectMapper mapper = new ObjectMapper();

        Dto r1 = mapper.readValue("{\"bar\":\"value\"}", Dto.class);
        assertEquals("absent", r1.foo);
        assertEquals("value", r1.bar);

        // -> throws MismatchedInputException: Missing required creator property 'bar' (index 1)
        Dto r2 = mapper.readValue("{}", Dto.class);
        assertEquals("absent", r2.foo);
        assertEquals("absent", r2.bar);
    }
}

Expected behavior

If absentValue returns a non-null value, then processing should continue.

Additional context

By organizing these priorities, we believe that the deserialization process in kotlin-module can be written easily and without differences in behavior.
https://github.com/FasterXML/jackson-module-kotlin/blob/fc47a94e81586990dd19d05087580652c14b7f7a/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt#L75-L89

[JooHyukKim]

Makes sense

[JooHyukKim]

What do you think #4202, @k163377 ?

[k163377]

@JooHyukKim
Thank you very much for your quick response.

By the way, why is the required decision made before the Fourth: default value?
I am not familiar with this process, but I think that if the default value is non-null, the process should continue as well as the NullValueProvider.
Sorry if I do not understand the process.

[JooHyukKim]

By the way, why is the required decision made before the Fourth: default value?

I agree, that in common logical sense required validation should come only "after" absent value and default value accesses are attempted. Just trying to keep things to work as before while addressing issue here.

[k163377]

I see, LGTM.
Personally, I feel that it would be even better if you had a comment to add to that.

[JooHyukKim]

I see, LGTM. Personally, I feel that it would be even better if you had a comment to add to that.

True, added such comment accordingly. 🙏🏼

[k163377]

Thanks a lot!

[cowtowncoder]

Hmmh. Apologies if I am missing something obvious but required specifically means that value MUST come from input, and not from other sources like defaulting logic.
If defaulting is used then required should be false; the two are mutually exclusive.

If I understood this correctly, then resolution would be "works as designed".
(but I probably did miss something)

[JooHyukKim]

then resolution would be "works as designed".

@k163377 FYI, will close PR #4202 as per comment above. So required means from input source, not absent logic

[cowtowncoder]

Will close, although am willing to hear counter-arguments if there are some.

[k163377]

Sorry for the late reply.
Well, I think there is a bit of a difference in interpretation of kotlin-module, since nullability is strongly related to the determination of the value of required.

Personally, I think it would be more natural for kotlin-module to continue deserializing as long as the value is provided by Jackson.
It would be nice if there was a way to read them out without generating errors.

[cowtowncoder]

@k163377 It sounds like perhaps Kotlin module is considering/using required to indicate nullability?
That would be different from intended semantics unfortunately: I am not sure what the best way to resolve this discrepancy would be.

Does Kotlin module's AnnotationIntrospector return True for hasRequiredMarker()?
If so, that is the problem. If so, would be good to figure out how to change that.

[k163377]

@cowtowncoder
First, the KotlinModule reports non-required for parameters if nullable or default arguments are available, otherwise it reports required (actually there is a more complex decision).
On the other hand, a call to Creator in Kotlin is possible if nullability is satisfied, regardless of whether a value is entered on the JSON.
https://github.com/FasterXML/jackson-module-kotlin/blob/2.17/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinAnnotationIntrospector.kt

As you say, there is a difference in semantics here.

If so, would be good to figure out how to change that.

A strict application of the policy would be that KotlinModule should not override hasRequiredMarker for deserialization.
Because, if the abscent value is provided by Jackson, KotlinModule can continue to deserialize.

On the other hand, this implementation has a very long history, and the extent of the impact of a destructive change is not known.
At least the following use cases have been reported.
FasterXML/jackson-module-kotlin#609

Also, since most of the time no absent value is provided by Jackson, we can consider it required if it is non-null in the primary use case.

If the impact on existing is to be kept to a minimum, it would be preferable to relax the databind restriction to allow more flexible deserialization in the ValueInstantiator, without changing the KotlinModule side of the specification.

[cowtowncoder]

At this point I do not want to change semantics of required. I think it can be used to infer "non-nullable", but it really cannot be used to mean "if not passed, use Absent value"... unless a new MapperFeature is introduced.

So adding such MapperFeature (USE_ABSENT_FOR_MISSING_REQUIRED or whatever) would be a possible way forward. Kotlin module would need to see this feature during registration.

I am open to other ways to allow Kotlin module to change behavior for just types it handles; but do not want changes for "regular" non-Kotlin POJO handling.

[k163377]

First, changing the implementation of KotlinModule regarding required can be a big disruptive change.
The KotlinModule has treated required like nullability for a very long time, and in fact I have found code in my own workplace that depends on it.

Secondly, such a change in behavior basically only affects deserialization related to classes defined in Kotlin, so it has no effect on POJO.
It is KotlinModule's own behavior to try to use absentValue if not passed.
https://github.com/FasterXML/jackson-module-kotlin/blob/cf890c64371072ba5138f1223c0de71cbb2a2542/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt#L82

For these reasons, I personally do not think this behavior should be changed.

[cowtowncoder]

Sigh. Ok, too bad this wasn't caught earlier. Required was never meant to convey anything other than what it suggests: value is required from input.

So I guess behavior of Kotlin module will and need to remain different in this regard.

Still, as I said, I am open to MapperFeature for changing behavior wrt defaulting.