Skip to content

Commit 4f5924e

Browse files
vbabaninvbabanin
authored
Resolve forwarded type arguments across POJO hierarchy edges (#2020)
JAVA-6065 JAVA-5110 JAVA-6138 --------- Co-authored-by: Ross Lawley <ross.lawley@gmail.com> (cherry picked from commit 0956b9f)
1 parent 2baa2c9 commit 4f5924e

24 files changed

Lines changed: 963 additions & 16 deletions

‎bson/src/main/org/bson/codecs/pojo/PojoBuilderHelper.java‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -264,11 +264,23 @@ private static <T> Set<ClassWithParentTypeData<? super T>> getClassHierarchy(fin
264264
TypeData<?> parentClassTypeData = classTypeData;
265265
while (currentClass != null && !currentClass.isEnum() && !currentClass.equals(Object.class)) {
266266
classesToScan.add(new ClassWithParentTypeData<>(currentClass, parentClassTypeData));
267-
parentClassTypeData = TypeData.newInstance(currentClass.getGenericSuperclass(), currentClass);
268-
for (Class<?> interfaceClass : currentClass.getInterfaces()) {
269-
classesToScan.addAll(getClassHierarchy((Class<? super T>) interfaceClass, parentClassTypeData));
267+
268+
List<TypeVariable<?>> currentTypeParams = asList(currentClass.getTypeParameters());
269+
Type[] genericInterfaces = currentClass.getGenericInterfaces();
270+
Class<?>[] interfaces = currentClass.getInterfaces();
271+
for (int i = 0; i < interfaces.length; i++) {
272+
TypeData<?> ifaceResolved = TypeData.newInstance(
273+
genericInterfaces[i], interfaces[i], currentTypeParams, parentClassTypeData);
274+
classesToScan.addAll(getClassHierarchy((Class<? super T>) interfaces[i], ifaceResolved));
275+
}
276+
277+
Class<? super T> superClass = currentClass.getSuperclass();
278+
if (superClass != null) {
279+
parentClassTypeData = TypeData.newInstance(
280+
currentClass.getGenericSuperclass(), superClass,
281+
currentTypeParams, parentClassTypeData);
270282
}
271-
currentClass = currentClass.getSuperclass();
283+
currentClass = superClass;
272284
}
273285
return classesToScan;
274286
}

‎bson/src/main/org/bson/codecs/pojo/TypeData.java‎

Lines changed: 50 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package org.bson.codecs.pojo;
1818

19+
import javax.annotation.Nullable;
1920
import java.lang.reflect.Field;
2021
import java.lang.reflect.Method;
2122
import java.lang.reflect.ParameterizedType;
@@ -61,32 +62,70 @@ public static TypeData<?> newInstance(final Field field) {
6162
}
6263

6364
public static <T> TypeData<T> newInstance(final Type genericType, final Class<T> clazz) {
64-
TypeData.Builder<T> builder = TypeData.builder(clazz);
65-
if (genericType instanceof ParameterizedType) {
66-
ParameterizedType pType = (ParameterizedType) genericType;
65+
// No enclosing class context: type variables have nothing to resolve against and erase to Object.
66+
return newInstance(genericType, clazz, Collections.<TypeVariable<?>>emptyList(), null);
67+
}
68+
69+
static <T> TypeData<T> newInstance(final Type genericParentType, final Class<T> parentClass,
70+
final List<TypeVariable<?>> currentClassTypeParameters,
71+
@Nullable final TypeData<?> currentClassTypeData) {
72+
TypeData.Builder<T> builder = TypeData.builder(parentClass);
73+
if (genericParentType instanceof ParameterizedType) {
74+
ParameterizedType pType = (ParameterizedType) genericParentType;
6775
for (Type argType : pType.getActualTypeArguments()) {
68-
getNestedTypeData(builder, argType);
76+
builder.addTypeParameter(resolveTypeArgument(argType, currentClassTypeParameters, currentClassTypeData));
6977
}
7078
}
7179
return builder.build();
7280
}
7381

7482
@SuppressWarnings({"unchecked", "rawtypes"})
75-
private static <T> void getNestedTypeData(final TypeData.Builder<T> builder, final Type type) {
83+
private static TypeData<?> resolveTypeArgument(final Type type,
84+
final List<TypeVariable<?>> currentClassTypeParameters,
85+
@Nullable final TypeData<?> currentClassTypeData) {
7686
if (type instanceof ParameterizedType) {
7787
ParameterizedType pType = (ParameterizedType) type;
7888
TypeData.Builder paramBuilder = TypeData.builder((Class) pType.getRawType());
7989
for (Type argType : pType.getActualTypeArguments()) {
80-
getNestedTypeData(paramBuilder, argType);
90+
paramBuilder.addTypeParameter(resolveTypeArgument(argType, currentClassTypeParameters, currentClassTypeData));
8191
}
82-
builder.addTypeParameter(paramBuilder.build());
83-
} else if (type instanceof WildcardType) {
84-
builder.addTypeParameter(TypeData.builder((Class) ((WildcardType) type).getUpperBounds()[0]).build());
92+
return paramBuilder.build();
8593
} else if (type instanceof TypeVariable) {
86-
builder.addTypeParameter(TypeData.builder(Object.class).build());
94+
return resolveTypeVariable((TypeVariable<?>) type, currentClassTypeParameters, currentClassTypeData);
8795
} else if (type instanceof Class) {
88-
builder.addTypeParameter(TypeData.builder((Class) type).build());
96+
return TypeData.builder((Class) type).build();
97+
} else if (type instanceof WildcardType) {
98+
// A wildcard cannot be the top-level type argument of an extends/implements clause (JLS §8.1.4,
99+
// §8.1.5), but it can appear nested inside one (e.g. extends Base<List<? extends Number>>) or in a
100+
// field/method generic type. Resolve it to its upper bound so any type variable inside the bound is
101+
// still substituted against the current context.
102+
return resolveTypeArgument(((WildcardType) type).getUpperBounds()[0], currentClassTypeParameters,
103+
currentClassTypeData);
104+
} else {
105+
// Any other Type (e.g. GenericArrayType) is erased to Object.
106+
return TypeData.builder(Object.class).build();
107+
}
108+
}
109+
110+
private static TypeData<?> resolveTypeVariable(final TypeVariable<?> type, final List<TypeVariable<?>> currentClassTypeParameters,
111+
@Nullable final TypeData<?> currentClassTypeData) {
112+
if (currentClassTypeData != null) {
113+
for (int i = 0; i < currentClassTypeParameters.size(); i++) {
114+
// Given 'class B<T> extends A<T> {}':
115+
// - JLS §6.3: the scope of B's type parameter T includes the superclass clause.
116+
// - JLS §6.5.5.1: T in "extends A<T>" therefore denotes B's T, not A's.
117+
// Both reflection paths represent the same declaration, and the TypeVariable
118+
// contract states "all instances representing a type variable must be equal() to
119+
// each other".
120+
if (currentClassTypeParameters.get(i).equals(type)) {
121+
if (i < currentClassTypeData.getTypeParameters().size()) {
122+
return currentClassTypeData.getTypeParameters().get(i);
123+
}
124+
break;
125+
}
126+
}
89127
}
128+
return TypeData.builder(Object.class).build();
90129
}
91130

92131
/**

‎bson/src/test/unit/org/bson/codecs/pojo/ClassModelTest.java‎

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,12 @@
2020

2121
import org.bson.codecs.pojo.entities.CollectionNestedPojoModel;
2222
import org.bson.codecs.pojo.entities.ConcreteAndNestedAbstractInterfaceModel;
23+
import org.bson.codecs.pojo.entities.ForwardingInterfaceModel;
24+
import org.bson.codecs.pojo.entities.ForwardingDualInterfaceModel;
25+
import org.bson.codecs.pojo.entities.ForwardingMixedModel;
26+
import org.bson.codecs.pojo.entities.ForwardingModel;
27+
import org.bson.codecs.pojo.entities.ForwardingArrayModel;
28+
import org.bson.codecs.pojo.entities.ForwardingNestedModel;
2329
import org.bson.codecs.pojo.entities.GenericHolderModel;
2430
import org.bson.codecs.pojo.entities.InterfaceBasedModel;
2531
import org.bson.codecs.pojo.entities.ListGenericExtendedModel;
@@ -266,7 +272,60 @@ public void testSimpleWithStaticModel() {
266272

267273
}
268274

269-
<T> TypeData.Builder<T> createBuilder(final Class<T> clazz, final Class<?>... types) {
275+
@Test
276+
public void testForwardingClassChain() {
277+
ClassModel<?> classModel = ClassModel.builder(ForwardingModel.class).build();
278+
279+
assertEquals(1, classModel.getPropertyModels().size());
280+
assertEquals(createTypeData(String.class), classModel.getPropertyModel("value").getTypeData());
281+
}
282+
283+
@Test
284+
public void testForwardingInterfaceChain() {
285+
ClassModel<?> classModel = ClassModel.builder(ForwardingInterfaceModel.class).build();
286+
287+
assertEquals(1, classModel.getPropertyModels().size());
288+
assertEquals(createTypeData(Integer.class), classModel.getPropertyModel("value").getTypeData());
289+
}
290+
291+
@Test
292+
public void testForwardingNested() {
293+
ClassModel<?> classModel = ClassModel.builder(ForwardingNestedModel.class).build();
294+
295+
assertEquals(1, classModel.getPropertyModels().size());
296+
assertEquals(createTypeData(List.class, String.class), classModel.getPropertyModel("value").getTypeData());
297+
}
298+
299+
@Test
300+
public void testForwardingArrayTypeVariableErasedToObject() {
301+
// The type argument `T[]` in `extends ForwardingArrayLevel2<T[]>` is a GenericArrayType;
302+
// getTypeParameterMap does not handle GenericArrayType, so the `value` property erases to
303+
// Object regardless of the concrete binding at the leaf subclass.
304+
ClassModel<?> classModel = ClassModel.builder(ForwardingArrayModel.class).build();
305+
306+
assertEquals(1, classModel.getPropertyModels().size());
307+
assertEquals(createTypeData(Object.class), classModel.getPropertyModel("value").getTypeData());
308+
}
309+
310+
@Test
311+
public void testForwardingMixedClassAndInterface() {
312+
ClassModel<?> classModel = ClassModel.builder(ForwardingMixedModel.class).build();
313+
314+
assertEquals(2, classModel.getPropertyModels().size());
315+
assertEquals(createTypeData(String.class), classModel.getPropertyModel("field1").getTypeData());
316+
assertEquals(createTypeData(Integer.class), classModel.getPropertyModel("field2").getTypeData());
317+
}
318+
319+
@Test
320+
public void testForwardingDualInterface() {
321+
ClassModel<?> classModel = ClassModel.builder(ForwardingDualInterfaceModel.class).build();
322+
323+
assertEquals(2, classModel.getPropertyModels().size());
324+
assertEquals(createTypeData(String.class), classModel.getPropertyModel("field2").getTypeData());
325+
assertEquals(createTypeData(Integer.class), classModel.getPropertyModel("field1").getTypeData());
326+
}
327+
328+
<T> TypeData.Builder<T> createBuilder(final Class<T> clazz, final Class<?>... types) {
270329
TypeData.Builder<T> builder = TypeData.builder(clazz);
271330
List<TypeData<?>> subTypes = new ArrayList<>();
272331
for (final Class<?> type : types) {

‎bson/src/test/unit/org/bson/codecs/pojo/PojoRoundTripTest.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@
3030
import org.bson.codecs.pojo.entities.ConventionModel;
3131
import org.bson.codecs.pojo.entities.DuplicateAnnotationAllowedModel;
3232
import org.bson.codecs.pojo.entities.FieldAndPropertyTypeMismatchModel;
33+
import org.bson.codecs.pojo.entities.ForwardingInterfaceModel;
34+
import org.bson.codecs.pojo.entities.ForwardingModel;
35+
import org.bson.codecs.pojo.entities.ForwardingNestedModel;
36+
import org.bson.codecs.pojo.entities.ForwardingWildcardModel;
3337
import org.bson.codecs.pojo.entities.GenericHolderModel;
3438
import org.bson.codecs.pojo.entities.GenericTreeModel;
3539
import org.bson.codecs.pojo.entities.InterfaceBasedModel;
@@ -526,6 +530,28 @@ private static List<TestData> testCases() {
526530
getPojoCodecProviderBuilder(BsonExtraElementsMapModel.class),
527531
"{'integerField': 42, 'stringField': 'myString', 'a': 'a', 'b': 'b'}"));
528532

533+
data.add(new TestData("Forwarding class chain resolves to String",
534+
new ForwardingModel("hello"),
535+
getPojoCodecProviderBuilder(ForwardingModel.class),
536+
"{'value': 'hello'}"));
537+
538+
data.add(new TestData("Forwarding interface chain resolves to Integer",
539+
new ForwardingInterfaceModel(7),
540+
getPojoCodecProviderBuilder(ForwardingInterfaceModel.class),
541+
"{'value': 7}"));
542+
543+
data.add(new TestData("Forwarding nested generic chain resolves to List<String>",
544+
new ForwardingNestedModel(asList("a", "b", "c")),
545+
getPojoCodecProviderBuilder(ForwardingNestedModel.class),
546+
"{'value': ['a', 'b', 'c']}"));
547+
data.add(new TestData("Forwarding nested wildcard chain resolves to List<? extends ShapeModelAbstract>",
548+
new ForwardingWildcardModel(asList(getShapeModelCircle(), getShapeModelRectangle())),
549+
getPojoCodecProviderBuilder(ForwardingWildcardModel.class, ShapeModelAbstract.class,
550+
ShapeModelCircle.class, ShapeModelRectangle.class),
551+
"{'value': [{'_t': 'org.bson.codecs.pojo.entities.ShapeModelCircle', 'color': 'orange', 'radius': 4.2}, "
552+
+ "{'_t': 'org.bson.codecs.pojo.entities.ShapeModelRectangle', 'color': 'green', 'width': 22.1, "
553+
+ "'height': 105.0}]}"));
554+
529555
return data;
530556
}
531557

0 commit comments

Comments
 (0)