Polymorphic deserialization causes IllegalArgumentException in Jackson 2.8.1

[cshannon]

There seems to be an issue trying to deserialize an object after serialization. The error message is odd because it says that the parent is not a subtype of the child, which is correct because that is of course backwards to what the relationship actually is.

Here is a very simple test case that works fine with Jackson 2.7.5 but is broken with Jackson 2.8.1.

package jackson.test;

import org.junit.Test;
import com.fasterxml.jackson.databind.ObjectMapper;

public class BrokenJackson {

    @Test
    public void test() throws Exception {
        ObjectMapper objectMapper = new ObjectMapper();
        final JacksonChild childObject = new JacksonChild();

        String value = objectMapper.writeValueAsString(childObject);
        objectMapper.readValue(value, JacksonChild.class);
    }
}

package jackson.test;

import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonInclude;

@JsonIgnoreProperties(ignoreUnknown = true)
@JsonInclude(JsonInclude.Include.NON_EMPTY)
public class JacksonChild extends JacksonParent {

}

package jackson.test;

import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonSubTypes;
import com.fasterxml.jackson.annotation.JsonTypeInfo;

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME,
    include = JsonTypeInfo.As.PROPERTY, property = "type",
    defaultImpl = JacksonParent.class)
@JsonSubTypes({ @JsonSubTypes.Type(value = JacksonChild.class, name = "child") })
@JsonIgnoreProperties(ignoreUnknown = true)
@JsonInclude(JsonInclude.Include.NON_EMPTY)
public class JacksonParent {

}

If you run the test case with these 3 classes, the resulting stacktrace is:

java.lang.IllegalArgumentException: Class jackson.test.JacksonParent not subtype of [simple type, class jackson.test.JacksonChild]
    at com.fasterxml.jackson.databind.type.TypeFactory.constructSpecializedType(TypeFactory.java:359)
    at com.fasterxml.jackson.databind.jsontype.impl.StdTypeResolverBuilder.buildTypeDeserializer(StdTypeResolverBuilder.java:118)
    at com.fasterxml.jackson.databind.deser.BasicDeserializerFactory.findTypeDeserializer(BasicDeserializerFactory.java:1363)
    at com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:481)
    at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:3890)
    at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:3785)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2833)
    at jackson.test.BrokenJackson.test(BrokenJackson.java:16)
.....

[cowtowncoder]

This is by design: as exception says, JacksonParent is not a subtype of JacksonChild. Problem being that although you ask for JacksonChild, it is possible that due to defaultImpl, JacksonParent would be returned; and that is not assignment-compatible.
Earlier versions did not verify this, although they would have resulted in ClassCastException if this happened.
In this case, fix would be to use:

  (JacksonChild) objectMapper.readValue(value, JacksonParent.class);

if you know for sure that defaultImpl is never used.

Now: it may be that ideally exception should be postponed and only thrown if problem occurs, not just because it might occur. I think there may already be an issue for this; but I'll leave this open if not (if there is, I'll close one as dup).

[cshannon]

I understand what you are saying but the main problem is that the behavior here has changed. I have existing code written (that has worked for a while) like my example and no longer works even though I know there will not be an error when reading the value.

I think that you are correct with the statement that postponing the exception to only occur if there is an actual problem would be the best solution because in many cases (such as my simple example here) code can be written so that it can be guaranteed that it won't fail. Also, it could be configurable (ie fail fast versus dynamically failing later) if you want.

[cowtowncoder]

@cshannon I understand that change of behavior is unfortunate. It was not intentional, but side product of rewrite and cleanup of type handling code, resulting in more correct handling -- however, in this case omission of check had useful side. Sometimes catching potential problems early is not win-win.

Anyway; the challenge in this particular case has to do with nominal type, passing that down the stack.
The explicit issue for possible fix is #1358, so I may close this issue as a dup; for now it makes sense to keep open I think.

[nsawadsky]

I'm also running into this issue. We're using Jackson with Jersey, so the workaround of changing the requested valueType to the parent class is not available to us.

[cowtowncoder]

@nsawadsky then you need to fix defaultImpl to be compatible with type declarations endpoints use.

[nsawadsky]

We have two endpoints, one that accepts ChildA and one that accepts ChildB. The behavior that we're trying to preserve is that on the first endpoint, ChildA will be the defaultImpl, even if the type identifier is omitted. This won't work on the second endpoint (a class cast exception would be fine if clients omit the type identifier on that endpoint) -- it's the behaviour on the first endpoint that we need to preserve (to avoid breaking existing clients). Maybe a weird case, but I just wanted to flag it as another scenario to consider.

[cowtowncoder]

@nsawadsky Ok thank you for additional information. Ideally this could be improved to defer the exception throwing, as I indicated earlier: so that as long as the original declaration is self-consistent things would work until given JSON that can not work.

[cowtowncoder]

Closing, same as #1488 and there is I think an issue for improving handling for this particular case, to defer throwing exception.