Refactoring before trying to fix #3235

Author: cowtowncoder

src/main/java/com/fasterxml/jackson/databind/jsontype/TypeResolverBuilder.java

@@ -51,7 +51,7 @@ public interface TypeResolverBuilder<T extends TypeResolverBuilder<T>>
      * available during type resolution
      */
     public Class<?> getDefaultImpl();
-    
+
     /*
     /**********************************************************
     /* Actual builder methods

src/main/java/com/fasterxml/jackson/databind/jsontype/impl/StdTypeResolverBuilder.java

@@ -168,47 +168,39 @@ public TypeDeserializer buildTypeDeserializer(DeserializationConfig config,
         throw new IllegalStateException("Do not know how to construct standard type serializer for inclusion type: "+_includeAs);
     }
 
-    protected JavaType defineDefaultImpl(DeserializationConfig config, JavaType baseType) {
-        JavaType defaultImpl;
-        if (_defaultImpl == null) {
-            if (config.isEnabled(MapperFeature.USE_BASE_TYPE_AS_DEFAULT_IMPL) && !baseType.isAbstract()) {
-                defaultImpl = baseType;
-            } else {
-                defaultImpl = null;
-            }
-        } else {
+    protected JavaType defineDefaultImpl(DeserializationConfig config, JavaType baseType)
+    {
+        if (_defaultImpl != null) {
             // 20-Mar-2016, tatu: It is important to do specialization go through
             //   TypeFactory to ensure proper resolution; with 2.7 and before, direct
             //   call to JavaType was used, but that cannot work reliably with 2.7
             // 20-Mar-2016, tatu: Can finally add a check for type compatibility BUT
             //   if so, need to add explicit checks for marker types. Not ideal, but
             //   seems like a reasonable compromise.
-            if ((_defaultImpl == Void.class)
-                    || (_defaultImpl == NoClass.class)) {
-                defaultImpl = config.getTypeFactory().constructType(_defaultImpl);
-            } else {
-                if (baseType.hasRawClass(_defaultImpl)) { // common enough to check
-                    defaultImpl = baseType;
-                } else if (baseType.isTypeOrSuperTypeOf(_defaultImpl)) {
-                    // most common case with proper base type...
-                    defaultImpl = config.getTypeFactory()
-                            .constructSpecializedType(baseType, _defaultImpl);
-                } else {
-                    // 05-Apr-2018, tatu: As [databind#1565] and [databind#1861] need to allow
-                    //    some cases of seemingly incompatible `defaultImpl`. Easiest to just clear
-                    //    the setting.
-
-                    /*
-                    throw new IllegalArgumentException(
-                            String.format("Invalid \"defaultImpl\" (%s): not a subtype of basetype (%s)",
-                                    ClassUtil.nameOf(_defaultImpl), ClassUtil.nameOf(baseType.getRawClass()))
-                            );
-                            */
-                    defaultImpl = null;
-                }
+            if ((_defaultImpl == Void.class) || (_defaultImpl == NoClass.class)) {
+                // 18-Sep-2021, tatu: This has specific meaning: these two markers will
+                //    be used to conjure `null` value out of invalid type ids
+                return config.getTypeFactory().constructType(_defaultImpl);
+            }
+            if (baseType.hasRawClass(_defaultImpl)) { // tiny optimization
+                return baseType;
             }
+            if (baseType.isTypeOrSuperTypeOf(_defaultImpl)) {
+                // most common case with proper base type...
+                return config.getTypeFactory()
+                        .constructSpecializedType(baseType, _defaultImpl);
+            }
+            if (baseType.hasRawClass(_defaultImpl)) {
+                return baseType;
+            }
+        }
+        // use base type as default should always be used as the last choice.
+        if (config.isEnabled(MapperFeature.USE_BASE_TYPE_AS_DEFAULT_IMPL)
+               && !baseType.isAbstract()) {
+            // still can not resolve by default impl, fall back to use base type as default impl
+            return baseType;
         }
-        return defaultImpl;
+        return null;
     }
 
     /*

src/test/java/com/fasterxml/jackson/databind/jsontype/TestBaseTypeAsDefault.java

@@ -7,15 +7,12 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.exc.InvalidTypeIdException;
 
-import java.io.IOException;
-
 public class TestBaseTypeAsDefault extends BaseMapTest
 {
     @JsonTypeInfo(use = JsonTypeInfo.Id.CLASS, property = "@class")
     static class Parent {
     }
 
-
     static class Child extends Parent {
     }
 
@@ -44,18 +41,18 @@ static class ChildOfChild extends ChildOfAbstract {
     protected ObjectMapper MAPPER_WITHOUT_BASE = jsonMapperBuilder()
             .disable(MapperFeature.USE_BASE_TYPE_AS_DEFAULT_IMPL)
             .build();
-    
-    public void testPositiveForParent() throws IOException {
+
+    public void testPositiveForParent() throws Exception {
         Object o = MAPPER_WITH_BASE.readerFor(Parent.class).readValue("{}");
         assertEquals(o.getClass(), Parent.class);
     }
 
-    public void testPositiveForChild() throws IOException {
+    public void testPositiveForChild() throws Exception {
         Object o = MAPPER_WITH_BASE.readerFor(Child.class).readValue("{}");
         assertEquals(o.getClass(), Child.class);
     }
 
-    public void testNegativeForParent() throws IOException {
+    public void testNegativeForParent() throws Exception {
         try {
             /*Object o =*/ MAPPER_WITHOUT_BASE.readerFor(Parent.class).readValue("{}");
             fail("Should not pass");
@@ -64,7 +61,7 @@ public void testNegativeForParent() throws IOException {
         }
     }
 
-    public void testNegativeForChild() throws IOException {
+    public void testNegativeForChild() throws Exception {
         try {
             /*Object o =*/ MAPPER_WITHOUT_BASE.readerFor(Child.class).readValue("{}");
             fail("Should not pass");
@@ -73,19 +70,19 @@ public void testNegativeForChild() throws IOException {
         }
     }
 
-    public void testConversionForAbstractWithDefault() throws IOException {
+    public void testConversionForAbstractWithDefault() throws Exception {
         // should pass shouldn't it?
         Object o = MAPPER_WITH_BASE.readerFor(AbstractParentWithDefault.class).readValue("{}");
         assertEquals(o.getClass(), ChildOfChild.class);
     }
 
-    public void testPositiveWithTypeSpecification() throws IOException {
+    public void testPositiveWithTypeSpecification() throws Exception {
         Object o = MAPPER_WITH_BASE.readerFor(Parent.class)
                 .readValue("{\"@class\":\""+Child.class.getName()+"\"}");
         assertEquals(o.getClass(), Child.class);
     }
 
-    public void testPositiveWithManualDefault() throws IOException {
+    public void testPositiveWithManualDefault() throws Exception {
         Object o = MAPPER_WITH_BASE.readerFor(ChildOfAbstract.class).readValue("{}");
 
         assertEquals(o.getClass(), ChildOfChild.class);