Fix #1215

Author: cowtowncoder

release-notes/VERSION

@@ -6,6 +6,10 @@ Project: jackson-databind
 
 2.7.6 (not yet released)
 
+#1215: Problem with type specialization for Maps with `@JsonDeserialize(as=subtype)`
+ (reported by brentryan@github)
+#1277: Inefficient resolution of parametrized types
+ (reported by Andriy P, plokhotnyuk@github)
 #1279: Ensure DOM parsing defaults to not expanding external entities
 
 2.7.5 (11-Jun-2016)

src/main/java/com/fasterxml/jackson/databind/type/TypeBindings.java

@@ -209,7 +209,7 @@ public TypeBindings withUnboundVariable(String name)
     /* Accessors
     /**********************************************************************
      */
-    
+
     /**
      * Find type bound to specified name, if there is one; returns bound type if so, null if not.
      */
@@ -293,6 +293,18 @@ public boolean hasUnbound(String name) {
         return false;
     }
 
+    /**
+     * Factory method that will create an object that can be used as a key for
+     * caching purposes by {@link TypeFactory}
+     *
+     * @since 2.8
+     */
+    public Object asKey(Class<?> rawBase) {
+        // safe to pass _types array without copy since it is not exposed via
+        // any access, nor modified by this class
+        return new AsKey(rawBase, _types, _hashCode);
+    }
+
     /*
     /**********************************************************************
     /* Standard methods
@@ -302,7 +314,7 @@ public boolean hasUnbound(String name) {
     @Override public String toString()
     {
         if (_types.length == 0) {
-            return "";
+            return "<>";
         }
         StringBuilder sb = new StringBuilder();
         sb.append('<');
@@ -409,5 +421,53 @@ public static TypeVariable<?>[] paramsFor2(Class<?> erasedType)
             }
             return erasedType.getTypeParameters();
         }    
-    }    
+    }
+
+    /**
+     * Helper type used to allow caching of generic types
+     *
+     * @since 2.8
+     */
+    final static class AsKey {
+        private final Class<?> _raw;
+        private final JavaType[] _params;
+        private final int _hash;
+
+        public AsKey(Class<?> raw, JavaType[] params, int hash) {
+            _raw = raw ;
+            _params = params;
+            _hash = hash;
+        }
+
+        @Override
+        public int hashCode() { return _hash; }
+
+        @Override
+        public boolean equals(Object o) {
+            if (o == this) return true;
+            if (o == null) return false;
+            if (o.getClass() != getClass()) return false;
+            AsKey other = (AsKey) o;
+
+            if ((_hash == other._hash) && (_raw == other._raw)) {
+                final JavaType[] otherParams = other._params;
+                final int len = _params.length;
+
+                if (len == otherParams.length) {
+                    for (int i = 0; i < len; ++i) {
+                        if (!_params[i].equals(otherParams[i])) {
+                            return false;
+                        }
+                    }
+                    return true;
+                }
+            }
+            return false;
+        }
+
+        @Override
+        public String toString() {
+            return _raw.getName()+"<>";
+        }
+    }
 }

src/main/java/com/fasterxml/jackson/databind/type/TypeFactory.java

@@ -114,7 +114,7 @@ public final class TypeFactory
      * actual generic types), we will use small cache to avoid repetitive
      * resolution of core types
      */
-    protected final LRUMap<Class<?>, JavaType> _typeCache = new LRUMap<Class<?>, JavaType>(16, 100);
+    protected final LRUMap<Object,JavaType> _typeCache = new LRUMap<Object,JavaType>(16, 100);
 
     /*
     /**********************************************************
@@ -380,31 +380,17 @@ public JavaType constructSpecializedType(JavaType baseType, Class<?> subclass)
             //    our specific neeeds.
             // 29-Mar-2016, tatu: See [databind#1173]  (and test `TypeResolverTest`)
             //  for a case where this code does get invoked: not ideal
+            // 29-Jun-2016, tatu: As to bindings, this works for [databind#1215], but
+            //  not certain it would reliably work... but let's hope for best for now
+            TypeBindings tb = _bindingsForSubtype(baseType, typeParamCount, subclass);
             if (baseType.isInterface()) {
-                newType = baseType.refine(subclass, TypeBindings.emptyBindings(), null,
-                        new JavaType[] { baseType });
+                newType = baseType.refine(subclass, tb, null, new JavaType[] { baseType });
             } else {
-                newType = baseType.refine(subclass, TypeBindings.emptyBindings(), baseType,
-                        NO_TYPES);
+                newType = baseType.refine(subclass, tb, baseType, NO_TYPES);
             }
             // Only SimpleType returns null, but if so just resolve regularly
             if (newType == null) {
-                // But otherwise gets bit tricky, as we need to partially resolve the type hierarchy
-                // (hopefully passing null Class for root is ok)
-                TypeBindings tb = null;
-
-                // 14-Apr-2016, tatu: One possible short-cut; if type parameter counts
-                //   match, chances are they ought to match. Let's take our chances...
-                if (baseType.containedTypeCount() == typeParamCount) {
-                    if (typeParamCount == 1) {
-                        tb = TypeBindings.create(subclass, baseType.containedType(0));
-                    } else if (typeParamCount == 2) {
-                        tb = TypeBindings.create(subclass, baseType.containedType(0),
-                                baseType.containedType(1));
-                    }
-                }
-                newType = _fromClass(null, subclass,
-                        (tb == null) ? TypeBindings.emptyBindings() : tb);
+                newType = _fromClass(null, subclass, tb);
             }
         } while (false);
 
@@ -458,6 +444,29 @@ public JavaType constructSpecializedType(JavaType baseType, Class<?> subclass)
         */
     }
 
+    private TypeBindings _bindingsForSubtype(JavaType baseType, int typeParamCount, Class<?> subclass)
+    {
+        // But otherwise gets bit tricky, as we need to partially resolve the type hierarchy
+        // (hopefully passing null Class for root is ok)
+        int baseCount = baseType.containedTypeCount();
+        if (baseCount == typeParamCount) {
+            if (typeParamCount == 1) {
+                return TypeBindings.create(subclass, baseType.containedType(0));
+            }
+            if (typeParamCount == 2) {
+                return TypeBindings.create(subclass, baseType.containedType(0),
+                        baseType.containedType(1));
+            }
+            List<JavaType> types = new ArrayList<JavaType>(baseCount);
+            for (int i = 0; i < baseCount; ++i) {
+                types.add(baseType.containedType(i));
+            }
+            return TypeBindings.create(subclass, types);
+        }
+        // Otherwise, two choices: match N first, or empty. Do latter, for now
+        return TypeBindings.emptyBindings();
+    }
+
     /**
      * Method similar to {@link #constructSpecializedType}, but that creates a
      * less-specific type of given type. Usually this is as simple as simply
@@ -1157,15 +1166,17 @@ protected JavaType _fromClass(ClassStack context, Class<?> rawType, TypeBindings
         if (result != null) {
             return result;
         }
-        // Barring that, we may have recently constructed an instance:
-        // !!! TODO 16-Oct-2015, tatu: For now let's only cached non-parameterized; otherwise
-        //     need better cache key
-        boolean cachable = (bindings == null) || bindings.isEmpty();
-        if (cachable) {
-            result = _typeCache.get(rawType); // ok, cache object is synced
-            if (result != null) {
-                return result;
-            }
+        // Barring that, we may have recently constructed an instance
+        final Object key;
+        if ((bindings == null) || bindings.isEmpty()) {
+            key = rawType;
+            result = _typeCache.get(key); // ok, cache object is synced
+        } else {
+            key = bindings.asKey(rawType);
+        }
+        result = _typeCache.get(key); // ok, cache object is synced
+        if (result != null) {
+            return result;
         }
 
         // 15-Oct-2015, tatu: recursive reference?
@@ -1225,10 +1236,7 @@ else if (superClass != null) {
             }
         }
         context.resolveSelfReferences(result);
-
-        if (cachable) {
-            _typeCache.putIfAbsent(rawType, result); // cache object syncs
-        }
+        _typeCache.putIfAbsent(key, result); // cache object syncs
         return result;
     }
 

src/test/java/com/fasterxml/jackson/failing/TypeRefinementForMap1215Test.java renamed to src/test/java/com/fasterxml/jackson/databind/jsontype/TypeRefinementForMap1215Test.java

@@ -1,4 +1,4 @@
-package com.fasterxml.jackson.failing;
+package com.fasterxml.jackson.databind.jsontype;
 
 import java.util.LinkedHashMap;
 import java.util.Map;