Custom TypeResolverBuilder and registering subtypes in the ObjectMapper's configuration

[Bluexin]

Hello,
I am using a custom TypeResolverBuilder to enable custom polymorphic deserialization for classes to which I can't add Jackson annotations. Here's a sample piece of (Kotlin) code, followed by some explanations :

private val delegates: MutableMap<Class<*>, TypeResolverBuilder<*>> = Reference2ObjectOpenHashMap()

init {
    // register delegates -- I have only included their configuration for brevity
    /* first delegate, using a property value -- CustomTypeIdResolver looks up IDs registered at runtime
        init(JsonTypeInfo.Id.NAME, CustomTypeIdResolver(TypeFactory.defaultInstance()))
        inclusion(JsonTypeInfo.As.EXISTING_PROPERTY)
        typeProperty(MyClass::type.name)
    */
    /* second delegate, using deduction
        init(JsonTypeInfo.Id.DEDUCTION, null)
    */
}

override fun useForType(t: JavaType) = t.rawClass in delegates

override fun buildTypeSerializer(
    config: SerializationConfig,
    baseType: JavaType,
    subtypes: Collection<NamedType>?
): TypeSerializer? {
    val delegate = delegates[baseType.rawClass]
    return if (delegate != null) delegate.buildTypeSerializer(config, baseType, subtypes)
    else super.buildTypeSerializer(config, baseType, subtypes)
}

override fun buildTypeDeserializer(
    config: DeserializationConfig,
    baseType: JavaType,
    subtypes: Collection<NamedType>?
): TypeDeserializer? {
    val delegate = delegates[baseType.rawClass]
    return if (delegate != null) delegate.buildTypeDeserializer(config, baseType, subtypes)
    else super.buildTypeDeserializer(config, baseType, subtypes)
}

This custom TypeResolverBuilder delegates to instances of ObjectMapper.DefaultTypeResolverBuilder to enable specific deserialization for each handled base type (it would be too easy if they were all the same). One of them uses JsonTypeInfo.Id.NAME with an existing property (this one seems to work fine), and the other uses JsonTypeInfo.Id.DEDUCTION.
The latter fails because it expects subtypes to be non null. I would like to avoid having to hardcode them in the custom TypeResolverBuilder, and instead rely on the configuration's SubtypeResolver (aka JsonMapper.Builder::registerSubtypes) for reusability.

It could be that I'm going a completely wrong way about this. I also thought of using a custom AnnotationIntrospector instead, which might be the next thing I look into if this seems too bad.
The reason I did not go for full custom (de)serializers is I hoped to get Afterburner/Blackbird to be able to kick in. It is not clear to me if it does for polymorphic deserialization, but if it doesn't I guess that would be a reasonable approach.
If my way seems like something that should work, I found a reason as to why it currently doesn't, with a potential fix (in Jackson Databind) :

https://github.com/FasterXML/jackson-databind/blob/f0af53d085eb2aa9f7f6199846cc526068e09977/src/main/java/com/fasterxml/jackson/databind/deser/BasicDeserializerFactory.java#L1782-L1795
Here I see the config's SubtypeResolver is only used when the TypeResolver used is the one from the AnnotationIntrospector.
Is there a specific reason for this ?
From my testing it seemed safe to always query the subtypes from the config's SubtypeResolver, as it would result in an empty collection when none are found (which from my pov is nicer to work with than a nullable collection).
Would there be anything obviously wrong with the following piece of code instead of the original ?

if (b == null) {
    b = config.getDefaultTyper(baseType);
    if (b == null) {
        return null;
    }
}
final Collection<NamedType> subtypes = config.getSubtypeResolver().collectAndResolveSubtypesByTypeId(config, ac);

If this looks like a fix that could be made, I'll happily create a PR for this so I check no tests break etc.
Of course, if there is a proper easy way to implement what I need, I'll happily throw away these few hours of pulling hair trying to understand how all these internals are tied together :)

[cowtowncoder]

I can't say for sure whether this would work with Kotlin module, but if you could do the same for plain old jackson-databind, that'd be something I could get merged -- and should then work with Kotlin module too.
From the look of it, I don't think this is Kotlin-specific, but I mention this just in case as module does override some features.

Specifically, if all the tests in jackson-databind still work with the change, it seems like a safe change and I'd be happy to get it merged in 2.14 -- especially if there is a test that would fail before change, pass after it.

And in fact I can transfer this issue to jackson-databind if we can verify it is non-Kotlin-specific (databind can not have a dependency to Kotlin core).

[Bluexin]

Sounds good to me !
Sorry I meant to open the issue in databind but opened it after a few hours of looking into the inner workings of jackson and I guess my brain couldn't keep up. But this issue is indeed not kotlin-specific. Feel free to move it to the correct repo :)

[Bluexin]

Just to be clear, which branch should I target ? The change is small so it should be easy to swap branches, but you specifically mentioned 2.14 while the contributing doc state

Most bug-fix Pull Requests should be made against the current stable branch, 2.13

[cowtowncoder]

For sake of completeness: yes, 2.14 is to be used in this particular case, just to minimize chance of regression for 2.13.

[Bluexin]

Fixed in FasterXML/jackson-databind#3505