Additional refinements in TypeInformation reimplementation.

Slight refinements in TypeDiscoverer.equals(…) / hashCode() that are still not completely valid, are different enough to work for differentiating use cases but not 100% efficient for cache cases. Captured outstanding work in #2643. Reimplemented ….repository.query.Parameter.isDynamicProjectParameter(…) to bild on TypeInformation completely and properly unwrapp *all* wrapper types for type comparison. Related ticket #2312.
spring-projects · Jun 13, 2022 · 2a1a8f3 · 2a1a8f3
1 parent 18a9f3a
commit 2a1a8f3
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 23 deletions.
diff --git a/src/main/java/org/springframework/data/repository/query/Parameter.java b/src/main/java/org/springframework/data/repository/query/Parameter.java
@@ -17,12 +17,12 @@
 
 import static java.lang.String.*;
 
+import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
-import java.util.stream.Stream;
 
 import org.springframework.core.MethodParameter;
 import org.springframework.core.ResolvableType;
@@ -214,15 +214,17 @@ private static boolean isDynamicProjectionParameter(MethodParameter parameter) {
 			return false;
 		}
 
-		ResolvableType returnType = ResolvableType.forMethodReturnType(parameter.getMethod());
+		Method method = parameter.getMethod();
 
-		if (TypeInformation.of(returnType).isCollectionLike()
-				|| org.springframework.util.ClassUtils.isAssignable(Stream.class, returnType.toClass())) {
-			returnType = returnType.getGeneric(0);
+		if (method == null) {
+			throw new IllegalArgumentException("Parameter is not associated with any method");
 		}
 
-		ResolvableType type = ResolvableType.forMethodParameter(parameter);
-		return returnType.getType().equals(type.getGeneric(0).getType());
+		TypeInformation<?> returnType = TypeInformation.fromReturnTypeOf(method);
+		TypeInformation<?> unwrapped = QueryExecutionConverters.unwrapWrapperTypes(returnType);
+		TypeInformation<?> reactiveUnwrapped = ReactiveWrapperConverters.unwrapWrapperTypes(unwrapped);
+
+		return reactiveUnwrapped.equals(TypeInformation.fromMethodParameter(parameter).getComponentType());
 	}
 
 	/**

diff --git a/src/main/java/org/springframework/data/util/TypeDiscoverer.java b/src/main/java/org/springframework/data/util/TypeDiscoverer.java
@@ -34,7 +34,6 @@
 import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
-import org.springframework.util.ClassUtils;
 import org.springframework.util.ConcurrentLruCache;
 import org.springframework.util.ObjectUtils;
 import org.springframework.util.ReflectionUtils;
@@ -197,7 +196,7 @@ public TypeDescriptor toTypeDescriptor() {
 
 	@Override
 	public ClassTypeInformation<?> getRawTypeInformation() {
-		return new ClassTypeInformation<>(ResolvableType.forRawClass(resolvableType.getRawClass()));
+		return new ClassTypeInformation<>(ResolvableType.forRawClass(resolvableType.toClass()));
 	}
 
 	@Nullable
@@ -323,7 +322,7 @@ public boolean equals(@Nullable Object o) {
 			return true;
 		}
 
-		if ((o == null) || !ClassUtils.isAssignable(getClass(), o.getClass())) {
+		if ((o == null) || !ObjectUtils.nullSafeEquals(getClass(), o.getClass())) {
 			return false;
 		}
 
@@ -346,7 +345,11 @@ public boolean equals(@Nullable Object o) {
 
 	@Override
 	public int hashCode() {
-		return ObjectUtils.nullSafeHashCode(resolvableType.toClass());
+
+		int result = 31 * getClass().hashCode();
+		result += 31 * getType().hashCode();
+
+		return result;
 	}
 
 	@Override

diff --git a/src/main/java/org/springframework/data/util/TypeInformation.java b/src/main/java/org/springframework/data/util/TypeInformation.java
@@ -17,11 +17,13 @@
 
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
+import java.lang.reflect.TypeVariable;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import org.springframework.core.MethodParameter;
 import org.springframework.core.ResolvableType;
 import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.lang.Nullable;
@@ -61,9 +63,11 @@ public static TypeInformation<?> of(ResolvableType type) {
 
 		Assert.notNull(type, "Type must not be null");
 
-		return type.hasGenerics() || (type.isArray() && type.getComponentType().hasGenerics()) //
-				? TypeDiscoverer.td(type)
-				: ClassTypeInformation.cti(type);
+		return type.hasGenerics()
+				|| (type.isArray() && type.getComponentType().hasGenerics()) //
+				|| (type.getType() instanceof TypeVariable)
+						? TypeDiscoverer.td(type)
+						: ClassTypeInformation.cti(type);
 	}
 
 	/**
@@ -103,7 +107,23 @@ public static TypeInformation<?> fromReturnTypeOf(Method method) {
 	 * @since 3.0
 	 */
 	public static TypeInformation<?> fromReturnTypeOf(Method method, @Nullable Class<?> type) {
-		return ClassTypeInformation.fromReturnTypeOf(method, type);
+
+		ResolvableType intermediate = type == null
+				? ResolvableType.forMethodReturnType(method)
+				: ResolvableType.forMethodReturnType(method, type);
+
+		return TypeInformation.of(intermediate);
+	}
+
+	/**
+	 * Returns a new {@link TypeInformation} for the given {@link MethodParameter}.
+	 *
+	 * @param parameter must not be {@literal null}.
+	 * @return will never be {@literal null}.
+	 * @since 3.0
+	 */
+	public static TypeInformation<?> fromMethodParameter(MethodParameter parameter) {
+		return TypeInformation.of(ResolvableType.forMethodParameter(parameter));
 	}
 
 	/**

diff --git a/src/test/java/org/springframework/data/repository/query/ParameterUnitTests.java b/src/test/java/org/springframework/data/repository/query/ParameterUnitTests.java
@@ -19,6 +19,7 @@
 
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 import java.util.stream.Stream;
 
 import org.jetbrains.annotations.NotNull;
@@ -48,14 +49,17 @@ void classParameterWithSameTypeParameterAsReturnedStreamIsDynamicProjectionParam
 		assertThat(parameter.isDynamicProjectionParameter()).isTrue();
 	}
 
+	@Test
+	void classParameterWithSameTypeParameterAsReturnedOptionalIsDynamicProjectionParameter() throws Exception {
+
+		var parameter = new Parameter(getMethodParameter("dynamicProjectionWithOptional"));
+
+		assertThat(parameter.isDynamicProjectionParameter()).isTrue();
+	}
+
 	@NotNull
 	private MethodParameter getMethodParameter(String methodName) throws NoSuchMethodException {
-		return new MethodParameter( //
-				this.getClass().getDeclaredMethod( //
-						methodName, //
-						Class.class //
-				), //
-				0);
+		return new MethodParameter(this.getClass().getDeclaredMethod(methodName, Class.class), 0);
 	}
 
 	<T> List<T> dynamicProjectionWithList(Class<T> type) {
@@ -65,4 +69,8 @@ <T> List<T> dynamicProjectionWithList(Class<T> type) {
 	<T> Stream<T> dynamicProjectionWithStream(Class<T> type) {
 		return Stream.empty();
 	}
+
+	<T> Optional<T> dynamicProjectionWithOptional(Class<T> type) {
+		return Optional.empty();
+	}
 }
diff --git a/src/test/java/org/springframework/data/util/TypeDiscovererUnitTests.java b/src/test/java/org/springframework/data/util/TypeDiscovererUnitTests.java
@@ -274,8 +274,7 @@ void genericFieldOfType() {
 		var field = ReflectionUtils.findField(GenericPerson.class, "value");
 		var discoverer = TypeInformation.of(ResolvableType.forField(field, TypeExtendingGenericPersonWithAddress.class));
 
-		assertThat(discoverer).isEqualTo(TypeInformation.of(Address.class));
-		assertThat(discoverer.hashCode()).isEqualTo(TypeInformation.of(Address.class).hashCode());
+		assertThat(discoverer.getType()).isEqualTo(Address.class);
 	}
 
 	@Test // #2511
@@ -335,6 +334,20 @@ void detectsComponentTypeOfTypedClass() {
 		assertThat(property.getComponentType().getType()).isEqualTo(Leaf.class);
 	}
 
+	@Test
+	void differentEqualsAndHashCodeForTypeDiscovererAndClassTypeInformation() {
+
+		ResolvableType type = ResolvableType.forClass(Object.class);
+
+		var discoverer = new TypeDiscoverer<>(type);
+		var classTypeInformation = new ClassTypeInformation<>(type);
+
+		assertThat(discoverer).isNotEqualTo(classTypeInformation);
+		assertThat(classTypeInformation).isNotEqualTo(type);
+
+		assertThat(discoverer.hashCode()).isNotEqualTo(classTypeInformation.hashCode());
+	}
+
 	class Person {
 
 		Addresses addresses;