Fix #921 (backported from master)

FasterXML · Apr 29, 2020 · 02894ee · 02894ee
1 parent 8eb15fd
commit 02894ee
Show file tree

Hide file tree

Showing 7 changed files with 116 additions and 17 deletions.
diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x
@@ -1058,6 +1058,8 @@ Máté Rédecsi (rmatesz@github)
 Ville Koskela (vjkoskela@github)
   * Contributed #2487: BeanDeserializerBuilder Protected Factory Method for Extension
   (2.11.0)
+  * Contributed fix for #792: Deserialization Not Working Right with Generic Types and Builders
+  (2.12.0)
 
 Fitz (Joongsoo.Park) (joongsoo@github)
   * Contributed #2511: Add `SerializationFeature.WRITE_SELF_REFERENCES_AS_NULL`
@@ -1102,3 +1104,7 @@ Simone D'Avico (simonedavico@github)
 Robin Roos (robinroos@github)
   * Contributed #2636: ObjectReader readValue lacks Class<T> argument
  (2.11.0)
+
+Mike Gilbode (gilbode@github)
+  * Reported #792: Deserialization Not Working Right with Generic Types and Builders
+  (2.12.0)
diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x
@@ -4,6 +4,15 @@ Project: jackson-databind
 === Releases === 
 ------------------------------------------------------------------------
 
+2.12.0 (not yet released)
+
+#792: Deserialization Not Working Right with Generic Types and Builders
+ (reported by Mike G; fix contributed by Ville K)
+
+2.11.1 (not yet released)
+
+-
+
 2.11.0 (26-Apr-2020)
 
 #953: i-I case conversion problem in Turkish locale with case-insensitive deserialization

diff --git a/src/main/java/com/fasterxml/jackson/databind/MapperFeature.java b/src/main/java/com/fasterxml/jackson/databind/MapperFeature.java
@@ -300,6 +300,21 @@ public enum MapperFeature implements ConfigFeature
      */
     USE_BASE_TYPE_AS_DEFAULT_IMPL(false),
 
+    /**
+     * Feature that enables inferring builder type bindings from the value type
+     * being deserialized. This requires that the generic type declaration on
+     * the value type match that on the builder exactly: mismatched type declarations
+     * are not necessarily detected by databind.
+     *<p>
+     * Feature is enabled by default which means that deserialization does
+     * support deserializing types via builders with type parameters (generic types).
+     *<p>
+     * See: https://github.com/FasterXML/jackson-databind/issues/921
+     *
+     * @since 2.12
+     */
+    INFER_BUILDER_TYPE_BINDINGS(true),
+
     /*
     /******************************************************
     /* View-related features

diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java
@@ -144,12 +144,18 @@ public JsonDeserializer<Object> createBeanDeserializer(DeserializationContext ct
     }
 
     @Override
-    public JsonDeserializer<Object> createBuilderBasedDeserializer(DeserializationContext ctxt,
-            JavaType valueType, BeanDescription beanDesc, Class<?> builderClass)
-                    throws JsonMappingException
+    public JsonDeserializer<Object> createBuilderBasedDeserializer(
+            DeserializationContext ctxt, JavaType valueType, BeanDescription beanDesc,
+            Class<?> builderClass)
+        throws JsonMappingException
     {
         // First: need a BeanDescription for builder class
-        JavaType builderType = ctxt.constructType(builderClass);
+        JavaType builderType;
+        if (ctxt.isEnabled(MapperFeature.INFER_BUILDER_TYPE_BINDINGS)) {
+            builderType = ctxt.getTypeFactory().constructParametricType(builderClass, valueType.getBindings());
+        } else {
+            builderType = ctxt.constructType(builderClass);
+        }
         BeanDescription builderDesc = ctxt.getConfig().introspectForBuilder(builderType);
         return buildBuilderBasedDeserializer(ctxt, valueType, builderDesc);
     }

diff --git a/src/main/java/com/fasterxml/jackson/databind/type/TypeFactory.java b/src/main/java/com/fasterxml/jackson/databind/type/TypeFactory.java
@@ -1043,7 +1043,30 @@ public JavaType constructParametricType(Class<?> parametrized, Class<?>... param
      */
     public JavaType constructParametricType(Class<?> rawType, JavaType... parameterTypes)
     {
-        return _fromClass(null, rawType, TypeBindings.create(rawType, parameterTypes));
+        return constructParametricType(rawType, TypeBindings.create(rawType, parameterTypes));
+    }
+
+    /**
+     * Factory method for constructing {@link JavaType} that
+     * represents a parameterized type. The type's parameters are
+     * specified as an instance of {@link TypeBindings}. This
+     * is useful if you already have the type's parameters such
+     * as those found on {@link JavaType}. For example, you could
+     * call
+     * <pre>
+     *   return TypeFactory.constructParametricType(ArrayList.class, javaType.getBindings());
+     * </pre>
+     * This effectively applies the parameterized types from one
+     * {@link JavaType} to another class.
+     *
+     * @param rawType Actual type-erased type
+     * @param parameterTypes Type bindings for the raw type
+     *
+     * @since 2.12
+     */
+    public JavaType constructParametricType(Class<?> rawType, TypeBindings parameterTypes)
+    {
+        return _fromClass(null, rawType, parameterTypes);
     }
 
     /**

diff --git a/...ailing/BuilderDeserializationTest921.java → ...uilder/BuilderWithTypeParametersTest.java b/...ailing/BuilderDeserializationTest921.java → ...uilder/BuilderWithTypeParametersTest.java
@@ -1,13 +1,17 @@
-package com.fasterxml.jackson.failing;
+package com.fasterxml.jackson.databind.deser.builder;
 
-import java.util.List;
-
-import com.fasterxml.jackson.annotation.*;
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.core.type.TypeReference;
-import com.fasterxml.jackson.databind.*;
+import com.fasterxml.jackson.databind.BaseMapTest;
+import com.fasterxml.jackson.databind.MapperFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import java.util.LinkedHashMap;
+import java.util.List;
 
-public class BuilderDeserializationTest921
+// [databind#921]: support infering type parameters from Builder
+public class BuilderWithTypeParametersTest
     extends BaseMapTest
 {
     public static class MyPOJO {
@@ -77,15 +81,30 @@ public MyGenericPOJOWithCreator<T> build() {
       }
     }
 
-    public void testWithBuilder() throws Exception {
-      final ObjectMapper mapper = new ObjectMapper();
+    public void testWithBuilderInferringBindings() throws Exception {
+        final ObjectMapper mapper = jsonMapperBuilder()
+                .enable(MapperFeature.INFER_BUILDER_TYPE_BINDINGS)
+                .build();
+        final String json = aposToQuotes("{ 'data': [ { 'x': 'x', 'y': 'y' } ] }");
+        final MyGenericPOJO<MyPOJO> deserialized =
+                mapper.readValue(json, new TypeReference<MyGenericPOJO<MyPOJO>>() {});
+        assertEquals(1, deserialized.data.size());
+        Object ob = deserialized.data.get(0);
+        assertNotNull(ob);
+        assertEquals(MyPOJO.class, ob.getClass());
+    }
+
+    public void testWithBuilderWithoutInferringBindings() throws Exception {
+        final ObjectMapper mapper = jsonMapperBuilder()
+                .disable(MapperFeature.INFER_BUILDER_TYPE_BINDINGS)
+                .build();
       final String json = aposToQuotes("{ 'data': [ { 'x': 'x', 'y': 'y' } ] }");
       final MyGenericPOJO<MyPOJO> deserialized =
           mapper.readValue(json, new TypeReference<MyGenericPOJO<MyPOJO>>() {});
       assertEquals(1, deserialized.data.size());
       Object ob = deserialized.data.get(0);
       assertNotNull(ob);
-      assertEquals(MyPOJO.class, ob.getClass());
+      assertEquals(LinkedHashMap.class, ob.getClass());
     }
 
     public void testWithCreator() throws Exception {

diff --git a/src/test/java/com/fasterxml/jackson/databind/type/TestTypeFactory.java b/src/test/java/com/fasterxml/jackson/databind/type/TestTypeFactory.java
@@ -160,7 +160,7 @@ public void testParametricTypes()
     {
         TypeFactory tf = TypeFactory.defaultInstance();
         // first, simple class based
-        JavaType t = tf.constructParametrizedType(ArrayList.class, Collection.class, String.class); // ArrayList<String>
+        final JavaType t = tf.constructParametrizedType(ArrayList.class, Collection.class, String.class); // ArrayList<String>
         assertEquals(CollectionType.class, t.getClass());
         JavaType strC = tf.constructType(String.class);
         assertEquals(1, t.containedTypeCount());
@@ -176,6 +176,13 @@ public void testParametricTypes()
         assertEquals(t, t2.containedType(1));
         assertNull(t2.containedType(2));
 
+        // [databind#921]: using type bindings
+        JavaType t3 = tf.constructParametricType(HashSet.class, t.getBindings()); // HashSet<String>
+        assertEquals(CollectionType.class, t3.getClass());
+        assertEquals(1, t3.containedTypeCount());
+        assertEquals(strC, t3.containedType(0));
+        assertNull(t3.containedType(1));
+
         // and then custom generic type as well
         JavaType custom = tf.constructParametrizedType(SingleArgGeneric.class, SingleArgGeneric.class,
                 String.class);
@@ -184,10 +191,24 @@ public void testParametricTypes()
         assertEquals(strC, custom.containedType(0));
         assertNull(custom.containedType(1));
 
+        // and then custom generic type from TypeBindings ([databind#921])
+        JavaType custom2 = tf.constructParametricType(SingleArgGeneric.class, t.getBindings());
+        assertEquals(SimpleType.class, custom2.getClass());
+        assertEquals(1, custom2.containedTypeCount());
+        assertEquals(strC, custom2.containedType(0));
+        assertNull(custom2.containedType(1));
+
         // should also be able to access variable name:
         assertEquals("X", custom.containedTypeName(0));
+    }
+
+    @SuppressWarnings("deprecation")
+    public void testInvalidParametricTypes()
+    {
+        final TypeFactory tf = TypeFactory.defaultInstance();
+        final JavaType strC = tf.constructType(String.class);
 
-        // And finally, ensure that we can't create invalid combinations
+        // ensure that we can't create invalid combinations
         try {
             // Maps must take 2 type parameters, not just one
             tf.constructParametrizedType(Map.class, Map.class, strC);
@@ -202,7 +223,7 @@ public void testParametricTypes()
             verifyException(e, "Cannot create TypeBindings for class ");
         }
     }
-
+    
     /**
      * Test for checking that canonical name handling works ok
      */