Deserialization failure with Polymorphism using JsonTypeInfo defaultImpl, subtype as target

[j1234d2003]

Issue verified on version jackson-databind 2.8.7 on Ubuntu 14.04, Java 1.8.0_25. A sample code
CTestTypeId.java.txt
demonstrates this bug.

A class hierarchy that implements an interface annotated with a @JsonTypeInfo(use=Id.NAME, include=As.PROPERTY, property="typeInfo", defaultImpl=CBaseClass.class) and a @JsonTypeIdResolve fails to deserialize a derived class that extends the defaultImpl CBaseClass with exception IllegalArgumentException stating that the defaultImpl class is not a subtype of the extending derived class thrown at com.fasterxml.jackson.databind.type.TypeFactory.constructSpecializedType(TypeFactory.java:359)

Consider interface CTestInterface annotated with
@JsonTypeInfo(use=Id.NAME, include=As.PROPERTY, property="typeInfo", defaultImpl=CBaseClass.class) and
@JsonTypeIdResolver(CTypeMapper.class)

Consider two classes:

1. CBaseClass that implements CTestInterface and
2. CDerivedClass extends CBaseClass.
   On attempt to deserialize data for a CDerivedClass using an ObjectMapper, the type-deserializer failed to be built as demonstrated by the following stack trace. Note that an instance of CBaseClass gets deserialized correctly. For my target usecase, data does not always include the "typeInfo" property and hence defaultImpl use becomes necessary.

Exception in thread "main" java.lang.IllegalArgumentException: Class com.example.CTestTypeId$CBaseClass not subtype of [simple type, class com.example.CTestTypeId$CDerivedClass]
at com.fasterxml.jackson.databind.type.TypeFactory.constructSpecializedType(TypeFactory.java:359)
at com.fasterxml.jackson.databind.jsontype.impl.StdTypeResolverBuilder.buildTypeDeserializer(StdTypeResolverBuilder.java:128)
at com.fasterxml.jackson.databind.deser.BasicDeserializerFactory.findTypeDeserializer(BasicDeserializerFactory.java:1373)
at com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:482)
at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:3899)
at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:3794)
at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2842)
at com.example.CTestTypeId.main(CTestTypeId.java:177)

I am trying to migrate from jackson-databind 2.1.2 to a more recent production version. Is it possible to know a version that does not suffer from this bug as I need to migrate to a stable version very quickly.

[cowtowncoder]

@j1234d2003 It is not possible to know which version would behave differently before understand specific issue. And although explanation may be helpful, what really helps is a reproduction, ideally simple unit test.
As an educated guess, however, I would suspect that version that changes behavior is either 2.7 or 2.8, so on short term trying 2.6.7 or 2.7.9 makes sense.
(and in general changes like this are very unlikely within patches so it's good to find minor version match rirst).

But also make sure you follow the logic of defaultImpl resolution: it absolutely has to be a sub-type of base type that is being annotated; and must not be overridden by subtypes. That is, it is not possibly to specify more specific default implementations for subtypes: there can only be exactly one default implementation type, and one that is compatible with the nominal base type (usually type with @JsonTypeInfo annotation).
That is, it is possible exception you get is result of more thorough validation of annotations, and something that was formerly not caught.

I suspect this could be related to #1358.

[j1234d2003]

Thank you for your response. I agree unit testing on older version is vital and would start by 2.7.9.

For longer term solution, I would like to ensure I understand the logic of defaultImpl resolution explained in your response and reflect further based on the polymorphic usage I have seen over the years. I also studied issues 1358 and 1339 to improve my understanding of the problem.

In my example:

• @JsonTypeInfo is annotated on interface CTestInterface and defaultImpl = CBaseClass - the top-most concrete implemetation of CTestInterface. So it obeys "it absolutely has to be a sub-type of base type that is being annotated;"

• CDerivedClass extends CBaseClass and new ObjectMapper().readValue(value, CDerivedClass.class); fails whereas new ObjectMapper().readValue(value, CBaseClass.class); succeeds - the latter correctly deserializes an instance of CDerivedClass if encoded value includes the id property and an instance of CBaseClass if encoded value does not include the id property

• "That is, it is not possible to specify more specific default implementations for subtypes: there can only be exactly one default implementation type, and one that is compatible with the nominal base type". I am assuming that since @JsonTypeInfo annotations may be present on any member of a class hierarchy, multiple annotations with different defaultImpl cannot be allowed. We don't have that here in our test example.

So based on your description, it seems like you are stating new ObjectMapper().readValue(value, CDerivedClass.class); fails because:

• CDerivedClass extends the defaultImpl=CBaseClass: "and must not be overridden by subtypes."

It should be okay to fail if the encoded value DID NOT contain the id property and you try to decode the value as a CDerivedClass. However, if the encoded value contains the id property, there should be no reason to depend on defaultImpl and therefore it should not fail.

If a defaultImpl class can never be extended, this is truly troublesome in many real-world usecases. Having a Java interface and a class hierarchy that implements this hierarchy is fairly common. For many usecases such as in messaging protocols, it is necessary to have a default format if format is not explicitly specified. Now the selection of default format is typically based on the domain semantics and not class hierarchy structure. Therefore, a requirement that the defaultImpl class must always be at bottom (away from root - as a leaf) of class hierarchy is impractical in many cases. Even if it is designed with such constraint, a future refactoring that requires extending that defaultImpl class can break the software.

It many cases, the software has knowledge of actual target type using means outside the decoders (e.g. message headers) and therefore, it is reasonable to expect that a decoding attempt would be performed with the exact expected type - not the defaultImpl type - we don't want a leakage of an undesirable restriction into software due to usage of Jackson decoding library.

E.g. Consider a Shape interface annotated using @JsonTypeInfo with defaultImpl of Rectangle class. Now Rectangle cannot be extended by a Square class because that would cause decoding of all Square objects to fail - even if each of these encoded Square object data contained an id property. Moreover, I may have to refactor Rectangle class in future to allow RoundedRectangle class in future. Just because I need a few RoundedRectangle instances, I cannot change that to be the default shape - my default shape is dictated by semantics of use - not my class hierarchy.

It would seem that the correct behavior should be that you cannot expect to get a Square instance when the encoded object does not have any id - you would get a Rectangle instance as that is the defaultImpl. However, if the encoded object does include an id property, you should be able to get a Square instance. As a matter of fact, with id property in the encoded object, I should be able to deserialize any member of class hierarchy irrespective of if/what is the defaultImpl.

Please share your thoughts.

[j1234d2003]

Sorry, I accidentally closed it.

[cowtowncoder]

I wish I had more to spend here, but from glance I think #1358 would be along lines you are thinking.
Ideally I think that pushing failure detection to a later stage would make sense here, as per discussion in that issue (and another that prompted it, not unlike this one I think).

There is also the question of whether attempt to direct deserialization into subtype (Rectangle) is valid use case at all for base type Shape, since even regardless of default implementation that is not a Rectangle, type id could also give incompatible type Triangle.
While it is of course possible to allow that assuming caller knows better to guarantee there shouldn't be a problem, this is not the only way to go about it.

In the meantime, in practical terms, I do not think behavior here will be changed in patch releases: so the first version where usage would work as desired is 2.9.
The way to make code work for now is to use nominal target type of Shape (or whatever class @JsonTypeInfo is used, for root values; or whatever is the declared type of property), and use explicit cast on result. This is guaranteed to work and does not really change binding aspects; but it does prevent validation from failing for defaultImpl. It should also work with earlier versions so change itself does not require upgrade.

[j1234d2003]

Thank you for your detailed response.

In order to perform a thorough analysis of current behavior and find an older version that would allow me to proceed in the short term, here is a sample test program using the Shape class hierarchy:
RectangleTest.java.txt

For version 2.8.7, here are the results:
2.8.7.txt

As is apparent from this result, a new problem has been detected that is related to defaultImpl:
When used as a member attribute inside another instance, it is not possible to use the concrete types other than the annotated interface or the defaultImpl class. Here, Shape and Rectangle members work correctly but decoding fails when there is a Square or Circle member nested inside another object.

This problem seems more troubling as I cannot comprehend a workaround like the explicit case you proposed. It is would be unreasonable to restrict use of concrete types in other domain-specific classes as only members with those concrete types may make sense for those domain-specific classes.

As before, the problem with top-most object type persists as follows:
When deserializing as top-most object, it is not possible to use concrete types other than the annotated interface or the defaultImpl class. i.e. new ObjectMapper().readValue(encodedValue, Square.class) and new ObjectMapper().readValue(encodedValue, Circle.class) fail to deserialize as expected.
As per your suggestion, there may be a temporary workaround using explicit casting for this problem.

Using the actual concrete type for deserialization is a valid usecase as in most of our target usecases, it is necessary to know the type of content you are expecting - like when you are processing a message, you already have message type from a previously decoded message header (may be other format) and now you are decoding the message payload to get additional information to process this message. It would be best if we did not have to use the Interface type or some super-type due to deserialization library constraints as you already know what you need. Getting something other than that may be an error.

One can also look at this problem in another way: The type id resolver is able to completely resolve the type from the encoded data and therefore it should be allowed to do so. It seems reasonable to depend on defaultImpl when type could not be detected from the encoded data and therefore there is no further recourse to determine how to proceed w.r.t. type detection. If defaultImpl is not meant to work like this, it would be desirable to provide something which does offer this functionality. It seems overly restrictive for a deserialization library to constraint the type of expected data or data-type of domain-specific classes. It would help if a deserializer throws an exception only when the underlying encoded data cannot be decoded to what the user requested. Right now it throws an exception even when the encoded data is correct.

The same tests for 2.7.9 (with few mods for TypeIdResolver impl) did not fail for any of the above usecases. Here is the result for 2.7.9 testing:
2.7.9.txt

So I plan to move to 2.7.9 in short term but hope you can help fix this issue so that I can proceed to the latest version in the near future. Hope you will be able to address both of these problem usecases. You hinted on 2.9 version. Do you have an estimated release time-frame for stable version for 2.9?