-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fix #1604 for 2.8.11 #1800
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #1604 for 2.8.11 #1800
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -480,23 +480,82 @@ private TypeBindings _bindingsForSubtype(JavaType baseType, int typeParamCount, | |
// (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); | ||
// 2017-10-17 epollan [databind#1604] | ||
// Given "stacks" of type bindings contained in `baseType`; e.g. {@code baseType : Foo<A<B>, X>}, | ||
// with bindings A[B] (indicating a type binding to the JavaType for `A` with a nested | ||
// type binding to `B`) and X; pop off all type bindings that might be implied in the | ||
// `subclass`'s definition relative to the `baseType`. | ||
// E.g. {@code subclass : Bar<B, X>} where {@code Bar<T, U> extends Foo<A<T>, U>}, would | ||
// inherit type bindings B and X. It would _not_ inherit `baseType`'s type binding to A[B] | ||
// -- it would only inherit the inner type binding to B. | ||
// This is because {@code Bar<?, ?>} already _is a_ {@code Foo<A<?>, ?>}, and serialization config | ||
// for the class itself already comprehends the types that the inheritance specification | ||
// has structurally locked into the base type's bindings. | ||
JavaType[] bindings = new JavaType[baseCount]; | ||
for (int i = 0; i < baseCount; ++i) { | ||
types.add(baseType.containedType(i)); | ||
bindings[i] = _bindingForSubtypeAtBindingPosition(baseType, i, subclass); | ||
} | ||
return TypeBindings.create(subclass, types); | ||
return TypeBindings.create(subclass, bindings); | ||
} | ||
// Otherwise, two choices: match N first, or empty. Do latter, for now | ||
return TypeBindings.emptyBindings(); | ||
} | ||
|
||
private JavaType _bindingForSubtypeAtBindingPosition(JavaType baseType, int bindingPosition, Class<?> subclass) { | ||
JavaType binding = baseType.containedType(bindingPosition); | ||
// While the type binding is nested with exactly one inner binding, AND the subtype's definition | ||
// itself already implies the outermost binding, then... | ||
while (binding.containedTypeCount() == 1 && _subtypeImpliesBinding(baseType, binding, bindingPosition, subclass)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This new code path is only activated when a top-level binding type is, itself, bound in a nested fashion to exactly one "inner" type There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It just so happens that this can be done for each type binding in sequence -- no combinatorics involved. |
||
// ...peel the outer binding off | ||
binding = binding.containedType(0); | ||
} | ||
return binding; | ||
} | ||
|
||
/** | ||
* True if the subclass already implies the base type's binding at {@code bindingPosition} | ||
* (see {@link JavaType#containedType}). An example of an "implied" binding would be where | ||
* the subclass is the erased class `C` (defined as {@code C<T> extends A<B<T>>}). In this scenario, `A` | ||
* would be the baseType and would be bound to `B` with a _nested_ inner binding to some type | ||
* for `T`. `C` structurally implies the `B` binding without it being present as a type parameter -- | ||
* it's part of the class definition, itself. | ||
*/ | ||
private boolean _subtypeImpliesBinding(JavaType baseType, JavaType baseTypeBinding, int bindingPosition, Class<?> subclass) { | ||
// Should take a A<B<C>, D>, e.g., and make it A<B<?>, D> when bindingPosition == 0, e.g. | ||
JavaType wildcardedBase = _fromClass( | ||
null, | ||
baseType.getRawClass(), | ||
_wildcardedBindings(baseType, baseTypeBinding, bindingPosition) | ||
); | ||
|
||
JavaType[] wildcards = new JavaType[subclass.getTypeParameters().length]; | ||
Arrays.fill(wildcards, CORE_TYPE_OBJECT); | ||
JavaType wildcardedSubclass = _fromClass( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe |
||
null, | ||
subclass, | ||
TypeBindings.create( | ||
subclass, | ||
wildcards | ||
) | ||
); | ||
return wildcardedSubclass.findSuperType(baseType.getRawClass()).equals(wildcardedBase); | ||
} | ||
|
||
/** | ||
* Should take a {@code A<B<C>, D>}, e.g., and make it {@code A<B<?>, D>} when {@code bindingPosition == 0}, e.g. | ||
* Basically, reconstruct the base type's bindings by "wildcarding" the binding at the given position. All | ||
* other bindings are used as-is. | ||
*/ | ||
private TypeBindings _wildcardedBindings(JavaType baseType, JavaType baseTypeBinding, int bindingPosition) { | ||
JavaType[] bindingTypes = new JavaType[baseType.containedTypeCount()]; | ||
for (int i = 0; i < bindingTypes.length; i++) { | ||
bindingTypes[i] = (i == bindingPosition) ? | ||
_fromClass(null, baseTypeBinding.getRawClass(), TypeBindings.create(baseTypeBinding.getRawClass(), CORE_TYPE_OBJECT)) : | ||
baseType.containedType(i); | ||
} | ||
return TypeBindings.create(baseType.getRawClass(), bindingTypes); | ||
} | ||
|
||
/** | ||
* Method similar to {@link #constructSpecializedType}, but that creates a | ||
* less-specific type of given type. Usually this is as simple as simply | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
package com.fasterxml.jackson.databind.type; | ||
|
||
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
import com.fasterxml.jackson.databind.*; | ||
|
||
// for [databind#1604] | ||
public class NestedTypes1604Test extends BaseMapTest { | ||
public static class Data<T> { | ||
private T data; | ||
|
||
public Data(T data) { | ||
this.data = data; | ||
} | ||
|
||
public T getData() { | ||
return data; | ||
} | ||
|
||
public static <T> Data<List<T>> of(List<T> data) { | ||
return new DataList<>(data); | ||
} | ||
} | ||
|
||
public static class DataList<T> extends Data<List<T>> { | ||
public DataList(List<T> data) { | ||
super(data); | ||
} | ||
} | ||
|
||
public static class Inner { | ||
private int index; | ||
|
||
public Inner(int index) { | ||
this.index = index; | ||
} | ||
|
||
public int getIndex() { | ||
return index; | ||
} | ||
} | ||
|
||
public static class BadOuter { | ||
private Data<List<Inner>> inner; | ||
|
||
public BadOuter(Data<List<Inner>> inner) { | ||
this.inner = inner; | ||
} | ||
|
||
public Data<List<Inner>> getInner() { | ||
return inner; | ||
} | ||
} | ||
|
||
public static class GoodOuter { | ||
private DataList<Inner> inner; | ||
|
||
public GoodOuter(DataList<Inner> inner) { | ||
this.inner = inner; | ||
} | ||
|
||
public DataList<Inner> getInner() { | ||
return inner; | ||
} | ||
} | ||
|
||
public void testIssue1604() throws Exception { | ||
final ObjectMapper objectMapper = objectMapper(); | ||
List<Inner> inners = new ArrayList<>(); | ||
for (int i = 0; i < 2; i++) { | ||
inners.add(new Inner(i)); | ||
} | ||
String expectedJson = aposToQuotes("{'inner':{'data':[{'index':0},{'index':1}]}}"); | ||
assertEquals( | ||
expectedJson, | ||
objectMapper.writeValueAsString(new GoodOuter(new DataList<>(inners))) | ||
); | ||
assertEquals( | ||
expectedJson, | ||
objectMapper.writeValueAsString(new BadOuter(Data.of(inners))) | ||
); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I split the difference by removing the two inlined cases and using the more optimal array-based invocation for the general case. Just did this for code clarity.