Disabling FAIL_ON_INVALID_SUBTYPE breaks deserialization in an edge case with generics + custom (de)serializers

[stevestorey]

Describe the bug
Disabling FAIL_ON_INVALID_SUBTYPE can cause deserialization (involving generic types) to fail, in such a way that values that were previously deserialized correctly when the feature is enabled get converted to null when the feature is disabled.

My reading is that the feature should control whether an exception is thrown, or whether null is returned?

     * Feature that determines what happens when type of a polymorphic
     * value (indicated for example by {@link com.fasterxml.jackson.annotation.JsonTypeInfo})
     * cannot be found (missing) or resolved (invalid class name, unmappable id);
     * if enabled, an exception ir thrown; if false, null value is used instead.

but in this case, the type resolution is working fine with the feature enabled, but returns nulls when it's disabled which seems to break the docs (and seems likely that it's a bug)

Version information
Tests fail with all of 2.10.2, 2.11.3, 2.11.4-SNAPSHOT

To Reproduce
Standalone test case:

package bugs;

import static com.fasterxml.jackson.annotation.JsonTypeInfo.As.PROPERTY;
import static com.fasterxml.jackson.annotation.JsonTypeInfo.Id.CLASS;

import java.io.IOException;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.TreeNode;
import com.fasterxml.jackson.databind.BeanProperty;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.JsonDeserializer;
import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializerProvider;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import com.fasterxml.jackson.databind.deser.ContextualDeserializer;
import com.fasterxml.jackson.databind.deser.std.StdScalarDeserializer;
import com.fasterxml.jackson.databind.node.NumericNode;
import com.fasterxml.jackson.databind.ser.std.StdScalarSerializer;

public class DisablingSubTypeFailureBreaksDeserialization {

    private ObjectMapper objectMapper = new ObjectMapper();

    @Test // PASSES
    public void testWithSubTypeFailuresEnabled() throws IOException {
        objectMapper.enable(DeserializationFeature.FAIL_ON_INVALID_SUBTYPE);
        runTest();
    }

    @Test // FAILS as the ID is `null`
    public void testWithSubTypeFailuresDisabled() throws IOException {
        objectMapper.disable(DeserializationFeature.FAIL_ON_INVALID_SUBTYPE);
        runTest();
    }
    
    private void runTest() throws IOException {
        // Not the prettiest JSON, but functional ... The array is because of the custom serializer
        String json = "{\"id\":[\"" + WrappedId.class.getName() + "\",1]}";
        Assertions.assertEquals(json, objectMapper.writeValueAsString(new IdHolderWithTypeInfo<>(new WrappedId(1L))));
        IdHolderWithTypeInfo<?> readBack = objectMapper.readValue(json, IdHolderWithTypeInfo.class);
        Assertions.assertTrue(readBack.getId() instanceof WrappedId); // FAILS when FAIL_ON_INVALID_SUBTYPE is disabled
        Assertions.assertEquals(1L, readBack.getId().getId());
    }

    interface HasId {
        Long getId();
    }

    @JsonSerialize(using = TestSerializer.class)
    @JsonDeserialize(using = TestDeserializer.class)
    static class WrappedId implements HasId {
        private final Long id;

        WrappedId(Long id) {
            this.id = id;
        }

        @Override
        public Long getId() {
            return id;
        }
    }

    /**
     * @param <I> this generic type appears to be part of the problem. If you change this
     * to extend WrappedId instead of HasId then the tests pass again
     */
    private static class IdHolderWithTypeInfo<I extends HasId /* WrappedId instead of HasId works */> {
        private final I id;

        @JsonCreator
        IdHolderWithTypeInfo(@JsonProperty("id") I id) {
            this.id = id;
        }

        @JsonTypeInfo(include = PROPERTY, property = "class", use = CLASS)
        public I getId() {
            return id;
        }
    }

    static class TestSerializer extends StdScalarSerializer<WrappedId> {
        private static final long serialVersionUID = 1L;

        TestSerializer() {
            super(WrappedId.class, true);
        }

        @Override
        public void serialize(WrappedId value, JsonGenerator gen, SerializerProvider provider) throws IOException {
            gen.writeNumber(value.getId());
        }
    }

    static class TestDeserializer extends JsonDeserializer<WrappedId> implements ContextualDeserializer {

        @Override
        public JsonDeserializer<WrappedId> createContextual(DeserializationContext ctxt, BeanProperty property)
                throws JsonMappingException {
            return new StdScalarDeserializer<>(WrappedId.class) {
                private static final long serialVersionUID = 1L;

                @Override
                public WrappedId deserialize(JsonParser p, DeserializationContext ctxt)
                        throws IOException, JsonProcessingException {
                    TreeNode node = p.getCodec().readTree(p);
                    long value = ((NumericNode) node).asLong();
                    return new WrappedId(value);
                }
            };
        }

        @Override
        public WrappedId deserialize(JsonParser p, DeserializationContext ctxt)
                throws IOException, JsonProcessingException {
            throw new UnsupportedOperationException("Should have used the delegate");
        }
    }
}

Expected behavior
Both tests pass

Additional context
I expect that there are several things that are cotributing to the issue, not least the use of the custom serializers. Our 'real' code example is trying to (de)serialize ID types as scalar types based on string prefixes, and we occasionally want to use such WrappedId types in generic types some of which require generic type information to be included in the JSON.

[cowtowncoder]

Quick note: there is also 2.12.0-rc1 available for testing. I assume case would fail with it too but thought worth mentioning as there are many fixes since 2.11.

[stevestorey]

I assume case would fail with it too

Your assumption is indeed correct - I confirm that the test also fails with 2.12.0-rc1 version too.

I'm happy to have a poke about to try and investigate a fix if you could suggest where in the code the bug is likely to be (and then I can start debugging where the logic diverges between the 2 cases) ?

[cowtowncoder]

Just realized that #2775 may be related: fix was added since 2.12.0-rc1.
Other than that, I will just note that combination of polymorphic type handling on generic types can be tricky combination to work right and I would recommend trying to avoid it (that is, for having type-parameterized type and using that as polymorphic type). Still, I don't know that is problematic here.

Otherwise assuming things are not working, maybe looking at commits for fix in #2775 may help.

[cowtowncoder]

Passes for me, with latest 2.12 snapshot (that is, cannot reproduce).

[stevestorey]

I updated my test case to use 2.12.0-SNAPSHOT from the Sonatype repo (https://oss.sonatype.org/content/repositories/snapshots), whose MANIFEST.MF says ..

Implementation-Build-Date: 2020-10-11 20:33:28+0000

but I still get the same failure? Do I need to wait for a more recent snapshot build?

[cowtowncoder]

Latest snapshot is not 2.12.0-SNAPSHOT but 2.12.0-rc2-SNAPSHOT, so that is the difference here.

[stevestorey]

Right - yes, I confirm it's fixed with the 2.12.0-rc2-SNAPSHOT version - closing - thanks again!

[cowtowncoder]

@stevestorey thank you for verifying this!