diff --git a/src/main/java/com/fasterxml/jackson/databind/JavaType.java b/src/main/java/com/fasterxml/jackson/databind/JavaType.java index 01407051d7..aae3285a3d 100644 --- a/src/main/java/com/fasterxml/jackson/databind/JavaType.java +++ b/src/main/java/com/fasterxml/jackson/databind/JavaType.java @@ -86,7 +86,7 @@ protected JavaType(Class raw, int additionalHash, Object valueHandler, Object typeHandler, boolean asStatic) { _class = raw; - _hash = raw.getName().hashCode() + additionalHash; + _hash = 31 * additionalHash + raw.hashCode(); _valueHandler = valueHandler; _typeHandler = typeHandler; _asStatic = asStatic; @@ -645,5 +645,5 @@ public String getErasedSignature() { public abstract boolean equals(Object o); @Override - public final int hashCode() { return _hash; } + public int hashCode() { return _hash; } } diff --git a/src/main/java/com/fasterxml/jackson/databind/type/IdentityEqualityType.java b/src/main/java/com/fasterxml/jackson/databind/type/IdentityEqualityType.java new file mode 100644 index 0000000000..e25f8e8e7e --- /dev/null +++ b/src/main/java/com/fasterxml/jackson/databind/type/IdentityEqualityType.java @@ -0,0 +1,31 @@ +package com.fasterxml.jackson.databind.type; + +import com.fasterxml.jackson.databind.JavaType; + +/** + * Internal abstract type representing {@link TypeBase} implementations which use reference equality. + */ +abstract class IdentityEqualityType extends TypeBase { + protected IdentityEqualityType( + Class raw, + TypeBindings bindings, + JavaType superClass, + JavaType[] superInts, + int hash, + Object valueHandler, + Object typeHandler, + boolean asStatic) { + super(raw, bindings, superClass, superInts, hash, valueHandler, typeHandler, asStatic); + } + + @Override + public final boolean equals(Object o) { + return o == this; + } + + @Override + public final int hashCode() { + // The identity hashCode must be used otherwise all instances will have colliding hashCodes. + return System.identityHashCode(this); + } +} diff --git a/src/main/java/com/fasterxml/jackson/databind/type/MapLikeType.java b/src/main/java/com/fasterxml/jackson/databind/type/MapLikeType.java index 8f0ee909ee..d745e981e4 100644 --- a/src/main/java/com/fasterxml/jackson/databind/type/MapLikeType.java +++ b/src/main/java/com/fasterxml/jackson/databind/type/MapLikeType.java @@ -36,8 +36,9 @@ protected MapLikeType(Class mapType, TypeBindings bindings, JavaType superClass, JavaType[] superInts, JavaType keyT, JavaType valueT, Object valueHandler, Object typeHandler, boolean asStatic) { - super(mapType, bindings, superClass, superInts, keyT.hashCode() - ^ valueT.hashCode(), valueHandler, typeHandler, asStatic); + super(mapType, bindings, superClass, superInts, + 31 * keyT.hashCode() + valueT.hashCode(), + valueHandler, typeHandler, asStatic); _keyType = keyT; _valueType = valueT; } diff --git a/src/main/java/com/fasterxml/jackson/databind/type/PlaceholderForType.java b/src/main/java/com/fasterxml/jackson/databind/type/PlaceholderForType.java index a2c66b8f90..31b17de881 100644 --- a/src/main/java/com/fasterxml/jackson/databind/type/PlaceholderForType.java +++ b/src/main/java/com/fasterxml/jackson/databind/type/PlaceholderForType.java @@ -8,7 +8,7 @@ * * @since 2.8.11 */ -public class PlaceholderForType extends TypeBase +public class PlaceholderForType extends IdentityEqualityType { private static final long serialVersionUID = 1L; @@ -99,11 +99,6 @@ public String toString() { return getErasedSignature(new StringBuilder()).toString(); } - @Override - public boolean equals(Object o) { - return (o == this); - } - private T _unsupported() { throw new UnsupportedOperationException("Operation should not be attempted on "+getClass().getName()); } diff --git a/src/main/java/com/fasterxml/jackson/databind/type/ResolvedRecursiveType.java b/src/main/java/com/fasterxml/jackson/databind/type/ResolvedRecursiveType.java index 859f050d25..b6867d347c 100644 --- a/src/main/java/com/fasterxml/jackson/databind/type/ResolvedRecursiveType.java +++ b/src/main/java/com/fasterxml/jackson/databind/type/ResolvedRecursiveType.java @@ -7,7 +7,7 @@ * * @since 2.7 */ -public class ResolvedRecursiveType extends TypeBase +public class ResolvedRecursiveType extends IdentityEqualityType { private static final long serialVersionUID = 1L; @@ -126,16 +126,16 @@ public String toString() { return sb.toString(); } - @Override - public boolean equals(Object o) { - if (o == this) return true; - if (o == null) return false; - if (o.getClass() == getClass()) { + //@Override + //public boolean equals(Object o) { + //if (o == this) return true; + //if (o == null) return false; + //if (o.getClass() == getClass()) { // 16-Jun-2017, tatu: as per [databind#1658], cannot do recursive call since // there is likely to be a cycle... // but... true or false? - return false; + //return false; /* // Do NOT ever match unresolved references @@ -145,7 +145,7 @@ public boolean equals(Object o) { return (o.getClass() == getClass() && _referencedType.equals(((ResolvedRecursiveType) o).getSelfReferencedType())); */ - } - return false; - } + //} + //return false; + //} } diff --git a/src/main/java/com/fasterxml/jackson/databind/type/SimpleType.java b/src/main/java/com/fasterxml/jackson/databind/type/SimpleType.java index 03be121076..e878043f31 100644 --- a/src/main/java/com/fasterxml/jackson/databind/type/SimpleType.java +++ b/src/main/java/com/fasterxml/jackson/databind/type/SimpleType.java @@ -52,8 +52,17 @@ protected SimpleType(Class cls, TypeBindings bindings, JavaType superClass, JavaType[] superInts, Object valueHandler, Object typeHandler, boolean asStatic) { - super(cls, bindings, superClass, superInts, - 0, valueHandler, typeHandler, asStatic); + super( + cls, + bindings, + superClass, + superInts, + // TypeBase normalizes null bindings to the singleton returned by TypeBindings.emptyBindings() + // so we must compute the same hashCode in both cases. + (bindings == null ? TypeBindings.emptyBindings() : bindings).hashCode(), + valueHandler, + typeHandler, + asStatic); } /** diff --git a/src/main/java/com/fasterxml/jackson/databind/type/TypeBindings.java b/src/main/java/com/fasterxml/jackson/databind/type/TypeBindings.java index c8c4d3588d..aa4f8cd7de 100644 --- a/src/main/java/com/fasterxml/jackson/databind/type/TypeBindings.java +++ b/src/main/java/com/fasterxml/jackson/databind/type/TypeBindings.java @@ -56,12 +56,9 @@ private TypeBindings(String[] names, JavaType[] types, String[] uvars) if (_names.length != _types.length) { throw new IllegalArgumentException("Mismatching names ("+_names.length+"), types ("+_types.length+")"); } - int h = 1; - for (int i = 0, len = _types.length; i < len; ++i) { - h += _types[i].hashCode(); - } _unboundVariables = uvars; - _hashCode = h; + // hashCode and equality are based solely on _types. + _hashCode = Arrays.hashCode(_types); } public static TypeBindings emptyBindings() { @@ -253,6 +250,24 @@ public JavaType findBoundType(String name) return null; } + /** + * Returns true if a shallow search of the type bindings includes a placeholder + * type which uses reference equality, thus cannot produce cache hits. This + * is an optimization to avoid churning memory in the cache unnecessarily. + * Note that it is still possible for nested type information to contain such + * placeholder types (see NestedTypes1604Test for an example) so it's vital + * that they produce a distribution of hashCode values, even if they may push + * reusable data out of the cache. + */ + private boolean invalidCacheKey() { + for (JavaType type : _types) { + if (type instanceof IdentityEqualityType) { + return true; + } + } + return false; + } + public boolean isEmpty() { return (_types.length == 0); } @@ -309,11 +324,17 @@ public boolean hasUnbound(String name) { * Factory method that will create an object that can be used as a key for * caching purposes by {@link TypeFactory} * + * @return An object which can be used as a key in TypeFactory, or {@code null} if no key can be created. + * * @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 + if (invalidCacheKey()) { + // If placeholders are present, no key may be returned because the key is unhelpful without context. + return null; + } return new AsKey(rawBase, _types, _hashCode); } @@ -351,17 +372,8 @@ public Object asKey(Class rawBase) { return false; } TypeBindings other = (TypeBindings) o; - int len = _types.length; - if (len != other.size()) { - return false; - } - JavaType[] otherTypes = other._types; - for (int i = 0; i < len; ++i) { - if (!otherTypes[i].equals(_types[i])) { - return false; - } - } - return true; + // hashCode and equality are based solely on _types. + return _hashCode == other._hashCode && Arrays.equals(_types, other._types); } /* @@ -450,7 +462,7 @@ final static class AsKey { public AsKey(Class raw, JavaType[] params, int hash) { _raw = raw ; _params = params; - _hash = hash; + _hash = 31 * raw.hashCode() + hash; } @Override diff --git a/src/main/java/com/fasterxml/jackson/databind/type/TypeFactory.java b/src/main/java/com/fasterxml/jackson/databind/type/TypeFactory.java index 80d86669de..0324482986 100644 --- a/src/main/java/com/fasterxml/jackson/databind/type/TypeFactory.java +++ b/src/main/java/com/fasterxml/jackson/databind/type/TypeFactory.java @@ -1465,7 +1465,7 @@ protected JavaType _fromClass(ClassStack context, Class rawType, TypeBindings } else { key = bindings.asKey(rawType); } - result = _typeCache.get(key); // ok, cache object is synced + result = key == null ? null : _typeCache.get(key); // ok, cache object is synced if (result != null) { return result; } @@ -1529,7 +1529,7 @@ else if (superClass != null) { context.resolveSelfReferences(result); // 16-Jul-2016, tatu: [databind#1302] is solved different way, but ideally we shouldn't // cache anything with partially resolved `ResolvedRecursiveType`... so maybe improve - if (!result.hasHandlers()) { + if (key != null && !result.hasHandlers()) { _typeCache.putIfAbsent(key, result); // cache object syncs } return result; diff --git a/src/test/java/com/fasterxml/jackson/databind/type/TestTypeBindings.java b/src/test/java/com/fasterxml/jackson/databind/type/TestTypeBindings.java index a7bb3e0dda..b1b975ccd0 100644 --- a/src/test/java/com/fasterxml/jackson/databind/type/TestTypeBindings.java +++ b/src/test/java/com/fasterxml/jackson/databind/type/TestTypeBindings.java @@ -5,6 +5,8 @@ import com.fasterxml.jackson.databind.BaseMapTest; import com.fasterxml.jackson.databind.JavaType; +import static org.junit.Assert.assertNotEquals; + /** * Simple tests to verify for generic type binding functionality * implemented by {@link TypeBindings} class. @@ -87,4 +89,40 @@ public void testInvalidBindings() verifyException(e, "class expects 2"); } } + + // for [databind#3876] + public void testEqualityAndHashCode() + { + JavaType stringType = DEFAULT_TF.constructType(String.class); + JavaType integerType = DEFAULT_TF.constructType(Integer.class); + TypeBindings listStringBindings = TypeBindings.create(List.class, stringType); + TypeBindings listStringBindingsWithUnbound = listStringBindings.withUnboundVariable("X"); + TypeBindings iterableStringBindings = TypeBindings.create(Iterable.class, stringType); + TypeBindings mapStringInt = TypeBindings.create(Map.class, stringType, integerType); + TypeBindings mapIntString = TypeBindings.create(Map.class, integerType, stringType); + // Ensure that type variable names used by List and Iterable do not change in future java versions + assertEquals("E", listStringBindings.getBoundName(0)); + assertEquals("T", iterableStringBindings.getBoundName(0)); + // These TypeBindings bind the same types in the same order + assertEquals(listStringBindings, iterableStringBindings); + assertEquals(listStringBindings.hashCode(), iterableStringBindings.hashCode()); + // Type bindings which differ by an unbound variable still evaluate to equal + assertEquals(listStringBindingsWithUnbound, listStringBindings); + assertEquals(listStringBindingsWithUnbound.hashCode(), listStringBindings.hashCode()); + // However type bindings for the same types in different order must differ: + assertNotEquals(mapStringInt, mapIntString); + assertNotEquals(mapStringInt.hashCode(), mapIntString.hashCode()); + + Object iterableStringBaseList = iterableStringBindings.asKey(List.class); + Object listStringBaseList = listStringBindings.asKey(List.class); + Object listStringBindingsWithUnboundBaseList = listStringBindingsWithUnbound.asKey(List.class); + Object mapStringIntBaseMap = mapStringInt.asKey(Map.class); + Object mapIntStringBaseMap = mapIntString.asKey(Map.class); + assertEquals(iterableStringBaseList, listStringBaseList); + assertEquals(iterableStringBaseList.hashCode(), listStringBaseList.hashCode()); + assertEquals(listStringBindingsWithUnboundBaseList, listStringBaseList); + assertEquals(listStringBindingsWithUnboundBaseList.hashCode(), listStringBaseList.hashCode()); + assertNotEquals(mapStringIntBaseMap, mapIntStringBaseMap); + assertNotEquals(mapStringIntBaseMap.hashCode(), mapIntStringBaseMap.hashCode()); + } } diff --git a/src/test/java/com/fasterxml/jackson/databind/type/TestTypeFactory.java b/src/test/java/com/fasterxml/jackson/databind/type/TestTypeFactory.java index 9f0b6f2b69..4c9fd943e2 100644 --- a/src/test/java/com/fasterxml/jackson/databind/type/TestTypeFactory.java +++ b/src/test/java/com/fasterxml/jackson/databind/type/TestTypeFactory.java @@ -7,6 +7,8 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.*; +import static org.junit.Assert.assertNotEquals; + /** * Simple tests to verify that the {@link TypeFactory} constructs * type information as expected. @@ -355,6 +357,17 @@ public void testCollectionTypesRefined() assertEquals(AbstractList.class, superType.getRawClass()); } + // for [databind#3876] + @SuppressWarnings("rawtypes") + public void testCollectionsHashCode() + { + TypeFactory tf = newTypeFactory(); + JavaType listOfCollection = tf.constructType(new TypeReference>() { }); + JavaType collectionOfList = tf.constructType(new TypeReference>() { }); + assertNotEquals(listOfCollection, collectionOfList); + assertNotEquals(listOfCollection.hashCode(), collectionOfList.hashCode()); + } + /* /********************************************************** /* Unit tests: map type parameter resolution @@ -397,6 +410,24 @@ public void testMaps() assertEquals(tf.constructType(Boolean.class), mapType.getContentType()); } + // for [databind#3876] + public void testMapsHashCode() + { + TypeFactory tf = newTypeFactory(); + JavaType mapStringInt = tf.constructType(new TypeReference>() {}); + JavaType mapIntString = tf.constructType(new TypeReference>() {}); + assertNotEquals(mapStringInt, mapIntString); + assertNotEquals( + "hashCode should depend on parameter order", + mapStringInt.hashCode(), + mapIntString.hashCode()); + + JavaType mapStringString = tf.constructType(new TypeReference>() {}); + JavaType mapIntInt = tf.constructType(new TypeReference>() {}); + assertNotEquals(mapStringString, mapIntInt); + assertNotEquals(mapStringString.hashCode(), mapIntInt.hashCode()); + } + // since 2.7 public void testMapTypesRefined() { @@ -671,4 +702,20 @@ public void testParameterizedClassType() { assertEquals(1, t.containedTypeCount()); assertEquals(CharSequence.class, t.containedType(0).getRawClass()); } + + // for [databind#3876] + public void testParameterizedSimpleType() { + TypeFactory tf = TypeFactory.defaultInstance(); + + JavaType charSequenceClass = tf.constructType(new TypeReference>() { }); + JavaType numberClass = tf.constructType(new TypeReference>() { }); + + assertEquals(SimpleType.class, charSequenceClass.getClass()); + assertEquals(SimpleType.class, numberClass.getClass()); + + assertNotEquals(charSequenceClass, numberClass); + assertNotEquals( + "hash values should be distributed", + charSequenceClass.hashCode(), numberClass.hashCode()); + } }