From 467589680add00ab49108bbbe7db375d81a75837 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 18 Oct 2017 17:14:15 -0700 Subject: [PATCH] Fix #1604 --- release-notes/VERSION | 2 + .../databind/type/PlaceholderForType.java | 109 +++++++++++++ .../jackson/databind/type/ReferenceType.java | 2 +- .../jackson/databind/type/SimpleType.java | 2 +- .../jackson/databind/type/TypeFactory.java | 153 +++++++++--------- .../type}/NestedTypes1604Test.java | 2 +- .../type}/TestTypeFactory1604.java | 59 ++++--- 7 files changed, 226 insertions(+), 103 deletions(-) create mode 100644 src/main/java/com/fasterxml/jackson/databind/type/PlaceholderForType.java rename src/test/java/com/fasterxml/jackson/{failing => databind/type}/NestedTypes1604Test.java (98%) rename src/test/java/com/fasterxml/jackson/{failing => databind/type}/TestTypeFactory1604.java (67%) diff --git a/release-notes/VERSION b/release-notes/VERSION index 90f62d9378..a2f59c9c30 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -5,11 +5,13 @@ Project: jackson-databind 2.8.11 (not yet released) +#1604: Nested type arguments doesn't work with polymorphic types #1767: Allow `DeserializationProblemHandler` to respond to primitive types (reported by nhtzr@github) #1768: Improve `TypeFactory.constructFromCanonical()` to work with `java.lang.reflect.Type.getTypeName()` format + 2.8.10 (24-Aug-2017) #1657: `StdDateFormat` deserializes dates with no tz/offset as UTC instead of diff --git a/src/main/java/com/fasterxml/jackson/databind/type/PlaceholderForType.java b/src/main/java/com/fasterxml/jackson/databind/type/PlaceholderForType.java new file mode 100644 index 0000000000..54ed40275a --- /dev/null +++ b/src/main/java/com/fasterxml/jackson/databind/type/PlaceholderForType.java @@ -0,0 +1,109 @@ +package com.fasterxml.jackson.databind.type; + +import com.fasterxml.jackson.databind.JavaType; + +/** + * Helper type used when introspecting bindings for already resolved types, + * needed for specialization. + * + * @since 2.8.11 + */ +public class PlaceholderForType extends TypeBase +{ + private static final long serialVersionUID = 1L; + + protected final int _ordinal; + + /** + * Type assigned during wildcard resolution (which follows type + * structure resolution) + */ + protected JavaType _actualType; + + public PlaceholderForType(int ordinal) + { + super(Object.class, TypeBindings.emptyBindings(), + TypeFactory.unknownType(), null, 1, // super-class, super-interfaces, hashCode + null, null, false); // value/type handler, as-static + _ordinal = ordinal; + } + + public JavaType actualType() { return _actualType; } + public void actualType(JavaType t) { _actualType = t; } + + // Override to get better diagnostics + @Override + protected String buildCanonicalName() { + return toString(); + } + + @Override + public StringBuilder getGenericSignature(StringBuilder sb) { + return getErasedSignature(sb); + } + + @Override + public StringBuilder getErasedSignature(StringBuilder sb) { + sb.append('$').append(_ordinal+1); + return sb; + } + + @Override + public JavaType withTypeHandler(Object h) { + return _unsupported(); + } + + @Override + public JavaType withContentTypeHandler(Object h) { + return _unsupported(); + } + + @Override + public JavaType withValueHandler(Object h) { + return _unsupported(); + } + + @Override + public JavaType withContentValueHandler(Object h) { + return _unsupported(); + } + + @Override + public JavaType withContentType(JavaType contentType) { + return _unsupported(); + } + + @Override + public JavaType withStaticTyping() { + return _unsupported(); + } + + @Override + public JavaType refine(Class rawType, TypeBindings bindings, JavaType superClass, JavaType[] superInterfaces) { + return _unsupported(); + } + + @Override + protected JavaType _narrow(Class subclass) { + return _unsupported(); + } + + @Override + public boolean isContainerType() { + return false; + } + + @Override + 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/ReferenceType.java b/src/main/java/com/fasterxml/jackson/databind/type/ReferenceType.java index 5779e83e51..4add34ee0f 100644 --- a/src/main/java/com/fasterxml/jackson/databind/type/ReferenceType.java +++ b/src/main/java/com/fasterxml/jackson/databind/type/ReferenceType.java @@ -25,7 +25,7 @@ public class ReferenceType extends SimpleType * @since 2.8 */ protected final JavaType _anchorType; - + protected ReferenceType(Class cls, TypeBindings bindings, JavaType superClass, JavaType[] superInts, JavaType refType, JavaType anchorType, 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 c48c0f3398..1fb2433003 100644 --- a/src/main/java/com/fasterxml/jackson/databind/type/SimpleType.java +++ b/src/main/java/com/fasterxml/jackson/databind/type/SimpleType.java @@ -211,7 +211,7 @@ public JavaType refine(Class rawType, TypeBindings bindings, // SimpleType means something not-specialized, so: return null; } - + @Override protected String buildCanonicalName() { 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 08455bf5d3..41ffafb2b0 100644 --- a/src/main/java/com/fasterxml/jackson/databind/type/TypeFactory.java +++ b/src/main/java/com/fasterxml/jackson/databind/type/TypeFactory.java @@ -399,102 +399,94 @@ public JavaType constructSpecializedType(JavaType baseType, Class subclass) newType = _fromClass(null, subclass, TypeBindings.emptyBindings()); break; } - - // If not, we'll need to do more thorough forward+backwards resolution. Sigh. - - // 20-Oct-2015, tatu: Container, Map-types somewhat special. There is - // a way to fully resolve and merge hierarchies; but that gets expensive - // so let's, for now, try to create close-enough approximation that - // is not 100% same, structurally, but has equivalent information for - // 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 + // (4) If all else fails, do the full traversal using placeholders TypeBindings tb = _bindingsForSubtype(baseType, typeParamCount, subclass); - if (baseType.isInterface()) { - newType = baseType.refine(subclass, tb, null, new JavaType[] { baseType }); - } else { - newType = baseType.refine(subclass, tb, baseType, NO_TYPES); - } - // Only SimpleType returns null, but if so just resolve regularly - if (newType == null) { - newType = _fromClass(null, subclass, tb); - } + newType = _fromClass(null, subclass, tb); + } while (false); // 25-Sep-2016, tatu: As per [databind#1384] also need to ensure handlers get // copied as well newType = newType.withHandlersFrom(baseType); return newType; + } - // 20-Oct-2015, tatu: Old simplistic approach - - /* - // Currently mostly SimpleType instances can become something else - if (baseType instanceof SimpleType) { - // and only if subclass is an array, Collection or Map - if (subclass.isArray() - || Map.class.isAssignableFrom(subclass) - || Collection.class.isAssignableFrom(subclass)) { - // need to assert type compatibility... - if (!baseType.getRawClass().isAssignableFrom(subclass)) { - throw new IllegalArgumentException("Class "+subclass.getClass().getName()+" not subtype of "+baseType); - } - // this _should_ work, right? - JavaType subtype = _fromClass(null, subclass, TypeBindings.emptyBindings()); - // one more thing: handlers to copy? - Object h = baseType.getValueHandler(); - if (h != null) { - subtype = subtype.withValueHandler(h); - } - h = baseType.getTypeHandler(); - if (h != null) { - subtype = subtype.withTypeHandler(h); - } - return subtype; - } + private TypeBindings _bindingsForSubtype(JavaType baseType, int typeParamCount, Class subclass) + { + PlaceholderForType[] placeholders = new PlaceholderForType[typeParamCount]; + for (int i = 0; i < typeParamCount; ++i) { + placeholders[i] = new PlaceholderForType(i); } - // But there is the need for special case for arrays too, it seems - if (baseType instanceof ArrayType) { - if (subclass.isArray()) { - // actually see if it might be a no-op first: - ArrayType at = (ArrayType) baseType; - Class rawComp = subclass.getComponentType(); - if (at.getContentType().getRawClass() == rawComp) { - return baseType; - } - JavaType componentType = _fromAny(null, rawComp, null); - return ((ArrayType) baseType).withComponentType(componentType); - } + TypeBindings b = TypeBindings.create(subclass, placeholders); + // First: pseudo-resolve to get placeholders in place: + JavaType tmpSub = _fromClass(null, subclass, b); + // Then find super-type + JavaType baseWithPlaceholders = tmpSub.findSuperType(baseType.getRawClass()); + if (baseWithPlaceholders == null) { // should be found but... + throw new IllegalArgumentException(String.format( + "Internal error: unable to locate supertype (%s) from resolved subtype %s", baseType.getRawClass().getName(), + subclass.getName())); + } + // and traverse type hierarchies to both verify and to resolve placeholders + String error = _resolveTypePlaceholders(baseType, baseWithPlaceholders); + if (error != null) { + throw new IllegalArgumentException("Failed to specialize base type "+baseType.toCanonical()+" as " + +subclass.getName()+", problem: "+error); } - // otherwise regular narrowing should work just fine - return baseType.narrowBy(subclass); - */ + final JavaType[] typeParams = new JavaType[typeParamCount]; + for (int i = 0; i < typeParamCount; ++i) { + JavaType t = placeholders[i].actualType(); + // 18-Oct-2017, tatu: Looks like sometimes we have incomplete bindings (even if not + // common, it is possible if subtype is type-erased class with added type + // variable -- see test(s) with "bogus" type(s)). + if (t == null) { + t = unknownType(); + } + typeParams[i] = t; + } + return TypeBindings.create(subclass, typeParams); } - private TypeBindings _bindingsForSubtype(JavaType baseType, int typeParamCount, Class subclass) + private String _resolveTypePlaceholders(JavaType sourceType, JavaType actualType) + throws IllegalArgumentException { - // 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 expectedTypes = sourceType.getBindings().getTypeParameters(); + List actualTypes = actualType.getBindings().getTypeParameters(); + for (int i = 0, len = expectedTypes.size(); i < len; ++i) { + JavaType exp = expectedTypes.get(i); + JavaType act = actualTypes.get(i); + if (!_verifyAndResolvePlaceholders(exp, act)) { + return String.format("Type parameter #%d/%d differs; can not specialize %s with %s", + (i+1), len, exp.toCanonical(), act.toCanonical()); } - List types = new ArrayList(baseCount); - for (int i = 0; i < baseCount; ++i) { - types.add(baseType.containedType(i)); + } + return null; + } + + private boolean _verifyAndResolvePlaceholders(JavaType exp, JavaType act) + { + // See if we have an actual type placeholder to resolve; if yes, replace + if (act instanceof PlaceholderForType) { + ((PlaceholderForType) act).actualType(exp); + return true; + } + // if not, try to verify compatibility. But note that we can not + // use simple equality as we need to resolve recursively + if (exp.getRawClass() != act.getRawClass()) { + return false; + } + // But we can check type parameters "blindly" + List expectedTypes = exp.getBindings().getTypeParameters(); + List actualTypes = act.getBindings().getTypeParameters(); + for (int i = 0, len = expectedTypes.size(); i < len; ++i) { + JavaType exp2 = expectedTypes.get(i); + JavaType act2 = actualTypes.get(i); + if (!_verifyAndResolvePlaceholders(exp2, act2)) { + return false; } - return TypeBindings.create(subclass, types); } - // Otherwise, two choices: match N first, or empty. Do latter, for now - return TypeBindings.emptyBindings(); + return true; } /** @@ -1394,13 +1386,12 @@ protected JavaType _fromParamType(ClassStack context, ParameterizedType ptype, // always of type Class: if not, need to add more code to resolve it to Class. Type[] args = ptype.getActualTypeArguments(); int paramCount = (args == null) ? 0 : args.length; - JavaType[] pt; TypeBindings newBindings; if (paramCount == 0) { newBindings = EMPTY_BINDINGS; } else { - pt = new JavaType[paramCount]; + JavaType[] pt = new JavaType[paramCount]; for (int i = 0; i < paramCount; ++i) { pt[i] = _fromAny(context, args[i], parentBindings); } diff --git a/src/test/java/com/fasterxml/jackson/failing/NestedTypes1604Test.java b/src/test/java/com/fasterxml/jackson/databind/type/NestedTypes1604Test.java similarity index 98% rename from src/test/java/com/fasterxml/jackson/failing/NestedTypes1604Test.java rename to src/test/java/com/fasterxml/jackson/databind/type/NestedTypes1604Test.java index b77d3f89d6..753f0172dc 100644 --- a/src/test/java/com/fasterxml/jackson/failing/NestedTypes1604Test.java +++ b/src/test/java/com/fasterxml/jackson/databind/type/NestedTypes1604Test.java @@ -1,4 +1,4 @@ -package com.fasterxml.jackson.failing; +package com.fasterxml.jackson.databind.type; import java.util.ArrayList; import java.util.List; diff --git a/src/test/java/com/fasterxml/jackson/failing/TestTypeFactory1604.java b/src/test/java/com/fasterxml/jackson/databind/type/TestTypeFactory1604.java similarity index 67% rename from src/test/java/com/fasterxml/jackson/failing/TestTypeFactory1604.java rename to src/test/java/com/fasterxml/jackson/databind/type/TestTypeFactory1604.java index ad07c0c2e7..6bfec7c908 100644 --- a/src/test/java/com/fasterxml/jackson/failing/TestTypeFactory1604.java +++ b/src/test/java/com/fasterxml/jackson/databind/type/TestTypeFactory1604.java @@ -1,4 +1,4 @@ -package com.fasterxml.jackson.failing; +package com.fasterxml.jackson.databind.type; import java.util.*; @@ -12,15 +12,11 @@ public class TestTypeFactory1604 extends BaseMapTest { static class Data1604 { } - static class DataList1604 extends Data1604> { - } + static class DataList1604 extends Data1604> { } - static class RefinedDataList1604 extends DataList1604 { - } + static class RefinedDataList1604 extends DataList1604 { } - public static class SneakyDataList1604 extends Data1604> { - - } + public static class SneakyDataList1604 extends Data1604> { } static class TwoParam1604 { } @@ -64,34 +60,59 @@ public void testCustomTypesRefinedSneaky() JavaType subtype = tf.constructSpecializedType(base, SneakyDataList1604.class); assertEquals(SneakyDataList1604.class, subtype.getRawClass()); - assertEquals(1, subtype.containedTypeCount()); - JavaType paramType = subtype.containedType(0); - assertEquals(Long.class, paramType.getRawClass()); + assertEquals(2, subtype.containedTypeCount()); + assertEquals(Long.class, subtype.containedType(1).getRawClass()); + // first one, "bogus", has to be essentially "unknown" + assertEquals(Object.class, subtype.containedType(0).getRawClass()); // and have correct parent too - assertEquals(DataList1604.class, subtype.getSuperClass().getRawClass()); + assertEquals(Data1604.class, subtype.getSuperClass().getRawClass()); } public void testTwoParamSneakyCustom() { TypeFactory tf = newTypeFactory(); - JavaType type = tf.constructType(new TypeReference>() { }); + JavaType type = tf.constructType(new TypeReference>>() { }); assertEquals(TwoParam1604.class, type.getRawClass()); assertEquals(String.class, type.containedType(0).getRawClass()); - assertEquals(Long.class, type.containedType(1).getRawClass()); + JavaType ct = type.containedType(1); + assertEquals(List.class, ct.getRawClass()); + assertEquals(Long.class, ct.getContentType().getRawClass()); JavaType subtype = tf.constructSpecializedType(type, SneakyTwoParam1604.class); assertEquals(SneakyTwoParam1604.class, subtype.getRawClass()); assertEquals(TwoParam1604.class, subtype.getSuperClass().getRawClass()); assertEquals(2, subtype.containedTypeCount()); - // should properly resolve type parameters despite sneaky switching + // should properly resolve type parameters despite sneaky switching, including "unwounding" + // `List` wrapper JavaType first = subtype.containedType(0); - assertEquals(List.class, first.getRawClass()); - assertEquals(1, first.containedTypeCount()); - assertEquals(Long.class, first.containedType(0).getRawClass()); - + assertEquals(Long.class, first.getRawClass()); JavaType second = subtype.containedType(1); assertEquals(String.class, second.getRawClass()); } + + // Also: let's not allow mismatching binding + public void testErrorForMismatch() + { + TypeFactory tf = newTypeFactory(); + + // NOTE: plain `String` NOT `List` + JavaType base = tf.constructType(new TypeReference>() { }); + + try { + tf.constructSpecializedType(base, DataList1604.class); + fail("Should not pass"); + } catch (IllegalArgumentException e) { + verifyException(e, "Failed to specialize"); + verifyException(e, "Data1604"); + verifyException(e, "DataList1604"); + } + } + + /* + static class TwoParam1604 { } + + static class SneakyTwoParam1604 extends TwoParam1604> { } + */ }