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 spring-projects/spring-data-commons#2643.

Reimplemented ….repository.query.Parameter.isDynamicProjectParameter(…) to bild on TypeInformation completely and properly unwrapp *all* wrapper types for type comparison.

Related ticket #2312.
This commit is contained in:
Oliver Drotbohm
2022-06-13 15:45:00 +02:00
parent 18a9f3aa13
commit 2a1a8f387f
5 changed files with 69 additions and 23 deletions

View File

@@ -17,12 +17,12 @@ package org.springframework.data.repository.query;
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 @@ public class 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());
}
/**

View File

@@ -34,7 +34,6 @@ import org.springframework.core.ResolvableType;
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 @@ class TypeDiscoverer<S> implements TypeInformation<S> {
@Override
public ClassTypeInformation<?> getRawTypeInformation() {
return new ClassTypeInformation<>(ResolvableType.forRawClass(resolvableType.getRawClass()));
return new ClassTypeInformation<>(ResolvableType.forRawClass(resolvableType.toClass()));
}
@Nullable
@@ -323,7 +322,7 @@ class TypeDiscoverer<S> implements TypeInformation<S> {
return true;
}
if ((o == null) || !ClassUtils.isAssignable(getClass(), o.getClass())) {
if ((o == null) || !ObjectUtils.nullSafeEquals(getClass(), o.getClass())) {
return false;
}
@@ -346,7 +345,11 @@ class TypeDiscoverer<S> implements TypeInformation<S> {
@Override
public int hashCode() {
return ObjectUtils.nullSafeHashCode(resolvableType.toClass());
int result = 31 * getClass().hashCode();
result += 31 * getType().hashCode();
return result;
}
@Override

View File

@@ -17,11 +17,13 @@ package org.springframework.data.util;
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 interface TypeInformation<S> {
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 interface TypeInformation<S> {
* @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));
}
/**

View File

@@ -19,6 +19,7 @@ import static org.assertj.core.api.Assertions.*;
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 @@ class ParameterUnitTests {
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 @@ class ParameterUnitTests {
<T> Stream<T> dynamicProjectionWithStream(Class<T> type) {
return Stream.empty();
}
<T> Optional<T> dynamicProjectionWithOptional(Class<T> type) {
return Optional.empty();
}
}

View File

@@ -274,8 +274,7 @@ public class TypeDiscovererUnitTests {
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 @@ public class TypeDiscovererUnitTests {
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;