How to bypass MrBean for types with existing deserialization logic

[robbytx]

This is more of a question, possibly with a suggested change for greater compatibility.

I'm trying to better understand MrBean's incompatibility with polymorphic types, since I'd really like to use both MrBean and @JsonTypeInfo.

From my limited testing, it appears that I can override the _suitableType method (not sure if that's officially supported), so that I can bypass MrBean's materialization for specific abstract types. When I do this with a polymorphic interface annotated by @JsonTypeInfo, everything seems to work -- Jackson's polymorphic type resolution kicks in, and MrBean can still be used to materialize the specific subclasses/interfaces as desired.

Since I've got that working, I'm exploring how to implement the _suitableType method generically, and I've found that the following override seems to do the job:

    @Override
    protected boolean _suitableType(JavaType type) {
        return !type.hasHandlers() && super._suitableType(type);
    }

As far as I can tell from my limited testing (and review of Jackson's code in this area), JavaType.hasHandlers will return true if the type has a dedicated TypeDeserializer or other deserializer instance. This seems to always be the case for classes that are annotated with @JsonTypeInfo, as well as any other cases where a specific deserializer has been registered.

So, my question is:

1. Is there some downside/pitfall to this generic implementation that I'm missing?
2. If not, could this implementation be added as an enhancement to MrBean itself?

I should note that I'm interested exclusively in the deserialization use case, so perhaps I'm overlooking some complexities in the serialization use case.

[cowtowncoder]

Hmmh. Interesting question. One concern I have is that of JavaType.hasHandlers(), which is sort of legacy mechanism, leftover from fairly early versions. It is used, and if it appears to work I suspect it is fine to use, but I do consider it more of an internal implementation detail with respect to version compatibility.
But there are no plans to remove it, especially with Jackson 2.x.
One additional concern is probably that handlers referenced only refer to operation being consider currently: that is, when reading, value and type deserializers; when writing, value and type serializers. Not sure if that could lead to actual problems or not.

As to whether it could/should be part of standard Mr Bean, that is a good question. If there's enough test coverage for some scenarios (addition of custom (de)serializer(s), it seems like potentially useful addition.
It should be something configurable, at any rate, which leads to API question: should there be a set of simple MrBeanFeatures (or similar) to enable/disable, or perhaps an enum that defines standard set of choices for types to include/exclude.

But I am open to something like this being added as a standard feature; it could go in 2.12.0 (api additions need to go in new minor versions).

[robbytx]

One additional concern is probably that handlers referenced only refer to operation being consider currently: that is, when reading, value and type deserializers; when writing, value and type serializers. Not sure if that could lead to actual problems or not.

The method in question is called by AbstractTypeResolver.resolveAbstractType, which is exclusively used during deserialization. I understand that MrBean is designed to support the serialization use case as well, but I think that only occurs on MrBean-generated types after they have been materialized during deserialization.

Additional Context

What I'm seeing is that MrBean is materializing interfaces/abstract classes that contain @JsonTypeInfo annotation; however, Jackson data binding later consults the JsonTypeInfo annotation and chooses the suitable subtype, so that the result is that the JSON is parsed according to the proper type.

Therefore the scope of this issue is confined to:

1. The case when FAIL_ON_UNMATERIALIZED_METHOD is enabled, and there is some non-property method declared in the abstract type that is implemented by each of the subclasses. In this case, the materialization of the abstract type is problematic because it throws an exception (Unrecognized abstract method).
2. The README's description is incorrect:

Because of this, Mr Bean will ''not materialize any types annotated with @JsonTypeInfo annotation''.

Refining the Workaround

After looking into this further, I think the hasHandlers check is too broad in that it includes the case when valueHandler is present, but I'm unable to construct any scenario where that is the case AND where the resolveAbstractType method gets invoked. I'd be interested to know whether there is some case that I'm overlooking.

If the above paragraph holds, then I think we can "simplify" the workaround to:

    @Override
    protected boolean _suitableType(JavaType type) {
        return type.getTypeHandler() == null && super._suitableType(type);
    }

Problems generalizing the solution

In testing this logic further (against 2.10 fwiw), I've observed that getTypeHandler() is only set when the type in question is referenced as a property of some other type. It seems like this information comes from the logic in BasicDeserializerFactory.resolveMemberAndTypeAnnotations.

If the type containing JsonTypeInfo is instead the root value to deserialize, or even if it is the content of some container type that is the root, then getTypeHandler() returns null, leaving the AbstractTypeMaterializer with no awareness of the fact that the class has polymorphic type annotations.

What's more, the decision made by AbstractTypeMaterializer based on the state of the JavaType it receives is effectively cached in the shape of the resulting JsonDeserializer, and the deserializer cache is ignorant of the fact that the deserializer for this type was dependent on the presence of the type handler information. This implies that the sequence of types deserialized will have an effect on whether a particular type is properly deserialized, which is obviously undesirable.

In general, I'm not sure how best to proceed on a patch for this issue, because I have the following questions:

1. Should the JavaType passed to AbstractTypeMaterializer always contain a type (or value) handler, if one exists?
2. Should jackson-databind instead avoid attempting to materialize types for which it has JsonTypeInfo or a custom deserializer, unless that information itself (e.g. via JsonTypeInfo.defaultImpl, JsonSubTypes.Type.value, or a delegating deserializer) necessitates deserializing the type in question?
3. Should the deserializer cache key (currently controlled by SimpleType.equals) incorporate additional details like the type handler? Or should a deserializer created for a type with a type handler be uncacheable, as is done for container types in DeserializerCache._hasCustomHandlers?

In general, it seems like these questions/limitations prevent MrBean from safely determining for a particular type whether Jackson databinding is already configured to handle the specified type, so I'm starting to think that this issue may be better addressed in the databinding project.

Hopefully, my analysis here is reasonably coherent. Let me know if anything is unclear, and of course, let me know what you think about these concerns.

[cowtowncoder]

@robbytx One quick note: yes, "type handler" only gets assigned when @JsonTypeInfo is applied to property (accessor: field, method of constructor parameter), but not when @JsonTypeInfo is declared on class itself, I think.
And I think your analysis correct overall.

As I mentioned earlier, handlers that are (in some cases) attached to JavaType are not, in my opinion, good indicators.
I would like to get rid of that usage, ideally, to make JavaType instances immutable -- this is not easy to do, but I would like to separate handler use case from typing use case.

I also suspect that some more support may be needed from jackson-databind.

Some challenges here are:

• Although all annotation access should ideally go through AnnotationIntrospector, it has one big drawback for this use case -- annotations are flattened, so ALL super-type annotations are considered inheritable. So we could not really distinguish case of base type from that of leaf type (leaf inherits annotations from base).
• Direct lookup of annotation is technically easy, of course, but hard-codes logic to that one annotation (as opposed to being flexible to consider, f.ex, JAXB equivalents (which exist))
• Looking at AnnotationIntrospector, handling of @JsonTypeInfo (and equivalents) is messy, predating better modularization (of keeping most of actual usage logic out of it): findTypeResolver() is what would be called for type, which builds handlers which is not only overhead but could potentially cause problems
• ... and when only looking at type (class), we could possibly miss other problem cases where @JsonTypeInfo is applied via property. As well as, well, Default Typing...

While I am not sure how to address these, it might be useful to have a setting to at least configure logic for types not to materialize -- perhaps we can start more reliably avoiding specific set of types.

Also: adding some failing tests (under src/test/.../failing) could help, starting with the case you mention where abstract @JsonTypeInfo annotated base type has unimplemented abstract methods (but use case serializes/deserializers leaf types which implement them, i.e. never need to deserialize the base type).

[robbytx]

@cowtowncoder thanks for your response. I'm glad that it seems like I'm not too far off-track. Thanks for clarifying the current challenges and considerations.

As I mentioned earlier, handlers that are (in some cases) attached to JavaType are not, in my opinion, good indicators.

Yeah, that's what I was thinking, even if it's not what I was hoping.

... it might be useful to have a setting to at least configure logic for types not to materialize.

This is currently possible, although awkward, by overriding _suitableType and returning false for types to exclude. Obviously there's room for making this easier, but I'm not convinced this is all that valuable.

I'd rather try to provide some sort of configuration that would automatically handle certain use cases. Some options I can see:

1. Adding a MrBean-specific annotation that prevents it from being materialized
2. Adding some option to exclude types that have a JsonTypeInfo or equivalent annotation.
3. Combining the above: adding some option to exclude types that have an annotation of the user's choice, optionally with an option to include annotations on supertypes.

I'm not very familiar with default typing, so it's a bit hard for me to imagine how these should interact. Clearly the JAVA_LANG_OBJECT can be safely used in conjunction with MrBean. I think OBJECT_AND_NON_CONCRETE (or a more expansive setting) is where you run into conflicts with MrBean's functionalty. In an ideal scenario, I would think that when both are enabled, then the type information embedded in JSON should take precedence:

• If the type in JSON specifies a concrete type, then it should be used rather than the type that was declared/provided statically.
• If the type in JSON specifies an abstract type, only then should MrBean get involved to try to generate a concrete class.

This matches how I think Jackson should handle JsonTypeInfo, JsonTypeResolver, and similar mechanisms -- Jackson should exhaust all of its other type resolution facilities, and it should only consult MrBean when those facilities leave it with an abstract class/interface.

So... the more I think about this, the more I'm left with the impression that MrBean is simply getting involved too early in the type resolution process. I'd be interested whether you agree with this, or whether there are use cases that I'm failing to consider, where MrBean should preempt other annotations/default typing.

[cowtowncoder]

Yes, it is possible that the effort would be better spent on jackson-databind side and see if materialization would only be called after the hassle with polymorphic types is resolved; this would cover default typing case as well as explicit type/property annotation. Challenge is that of whether this is easy to pull off reliably...

I think that AbstractDeserializer may be created to properly delegate TypeDeserializer action, to locate real subtype. But I don't remember exactly how that would work... materialization itself is called just for POJOs (that is, not for Collection(Like), Map(Like), Enum or reference types); this from BeanDeserializerFactory.createBeanDeserializer().

Problem really is that (value) deserializer handling is quite isolated from type deserializers (type deserializer is what handles type<->id conversion, inclusion/extraction), especially during construction.

[robbytx]

In further testing, I've discovered a subtle issue where the solution I proposed above may fail.

As I noted above, the issue depends on the context in which the type is first encountered and therefore materialized:

What's more, the decision made by AbstractTypeMaterializer based on the state of the JavaType it receives is effectively cached in the shape of the resulting JsonDeserializer, and the deserializer cache is ignorant of the fact that the deserializer for this type was dependent on the presence of the type handler information. This implies that the sequence of types deserialized will have an effect on whether a particular type is properly deserialized, which is obviously undesirable.

The scenario where I'm seeing the failure is if a sub-type (to be materialized by MrBean) is referenced directly (as opposed to indirectly via its parent type) as the property (or the element/key/value type of some property) of another type. In this scenario, it appears that the JavaType will be assigned a type handler (which is some TypeDeserializer) prior to abstract type materialization.

Here is some example code:

package example;

import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.module.mrbean.AbstractTypeMaterializer;
import com.fasterxml.jackson.module.mrbean.MrBeanModule;

public class Test {

    @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "id", defaultImpl = SubType.class)
    public interface PolymorphicType {
    }

    public interface SubType extends PolymorphicType {
    }

    public interface PolymorphicTypeContainer {

        PolymorphicType getValue();
    }

    public interface SubTypeContainer {

        SubType getValue();
    }

    public static void main(String[] args) throws JsonProcessingException {
        ObjectMapper mapper = new ObjectMapper()
                .registerModule(new MrBeanModule(new AbstractTypeMaterializer() {
                    @Override
                    protected boolean _suitableType(JavaType type) {
                        return type.getTypeHandler() == null && super._suitableType(type);
                    }
                }));

        // This line breaks because type.getTypeHandler() will return an AsPropertyTypeDeserializer due to inherited @JsonTypeInfo
        System.out.println(mapper.readValue("{\"value\":{}}", SubTypeContainer.class).getValue());
        System.out.println(mapper.readValue("{\"value\":{}}", PolymorphicTypeContainer.class).getValue());
    }
}

that fails (in Jackson 2.11.3 / 2.12.1) with:

Exception in thread "main" com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot construct instance of `example.Test$SubType` (no Creators, like default constructor, exist): abstract types either need to be mapped to concrete types, have custom deserializer, or contain additional type information
 at [Source: (String)"{"value":{}}"; line: 1, column: 11] (through reference chain: com.fasterxml.jackson.module.mrbean.generated.example.Test$PolymorphicTypeContainer["value"])
	at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:67)
	at com.fasterxml.jackson.databind.DeserializationContext.reportBadDefinition(DeserializationContext.java:1764)
	at com.fasterxml.jackson.databind.DatabindContext.reportBadDefinition(DatabindContext.java:400)
	at com.fasterxml.jackson.databind.DeserializationContext.handleMissingInstantiator(DeserializationContext.java:1209)
	at com.fasterxml.jackson.databind.deser.AbstractDeserializer.deserialize(AbstractDeserializer.java:274)
	at com.fasterxml.jackson.databind.jsontype.impl.AsPropertyTypeDeserializer._deserializeTypedUsingDefaultImpl(AsPropertyTypeDeserializer.java:188)
	at com.fasterxml.jackson.databind.jsontype.impl.AsPropertyTypeDeserializer.deserializeTypedFromObject(AsPropertyTypeDeserializer.java:113)
	at com.fasterxml.jackson.databind.deser.AbstractDeserializer.deserializeWithType(AbstractDeserializer.java:263)
	at com.fasterxml.jackson.databind.deser.impl.MethodProperty.deserializeAndSet(MethodProperty.java:138)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.vanillaDeserialize(BeanDeserializer.java:324)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:187)
	at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:322)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4593)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3548)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3516)
	at example.Test.main(Test.java:40)

If you switch the order such that PolymorphicTypeContainer is deserialized first (or if you simply do mapper.readerFor(PolymorphicTypeContainer.class) or mapper.readerFor(SubType.class) first), then the JavaType instance passed to _suitableType for the SubType interface will have no type handler assigned, and the SubType will be materialized, and everything will work just fine.

I'm noting this primarily to caution anyone who attempts use the workaround that this issue could arise if you have direct references to subtypes. It appears the simplest workaround to this issue is to force early materialization of such types via mapper.readerFor(SubType.class).