From 3e9451c93987d2decb1e8589f8ffc736287a6185 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Tue, 29 Mar 2022 18:09:43 +0200 Subject: [PATCH] Polishing for full type information on RepositoryMetadata. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Avoid leaking internals from TypeInformation by exposing a ….toTypeDescriptor() method in TypeInformation directly. This causes the ResolvableType handling to become an implementation detail within ClassTypeInformation and avoid client code to manually deal with both ResolvableType and TypeDescriptor creation. Changed MethodLookups back to use the resolved domain types as the advanced generics resolutions seems not to be needed here and we can avoid Fixes #2518. --- .../repository/core/RepositoryMetadata.java | 48 ++++++++++--------- .../support/AnnotationRepositoryMetadata.java | 24 ++++------ .../support/DefaultRepositoryMetadata.java | 24 +++++----- .../core/support/MethodLookups.java | 4 +- .../support/DomainClassConverter.java | 14 ++---- .../support/ReflectionRepositoryInvoker.java | 8 ++-- .../data/util/ClassTypeInformation.java | 10 ++++ .../util/ParentTypeAwareTypeInformation.java | 17 ++++++- .../data/util/TypeDiscoverer.java | 31 +++++------- .../data/util/TypeInformation.java | 25 ++++------ .../util/TypeVariableTypeInformation.java | 15 ++++-- .../support/RepositoriesUnitTests.java | 6 +-- 12 files changed, 117 insertions(+), 109 deletions(-) diff --git a/src/main/java/org/springframework/data/repository/core/RepositoryMetadata.java b/src/main/java/org/springframework/data/repository/core/RepositoryMetadata.java index 136630e81..8786ec1e1 100644 --- a/src/main/java/org/springframework/data/repository/core/RepositoryMetadata.java +++ b/src/main/java/org/springframework/data/repository/core/RepositoryMetadata.java @@ -31,27 +31,6 @@ import org.springframework.data.util.TypeInformation; */ public interface RepositoryMetadata { - /** - * Returns the id {@link TypeInformation} the given class is declared for. - * - * @return the {@link TypeInformation} class of the entity managed by the repository. - */ - TypeInformation getIdTypeInformation(); - - /** - * Returns the domain {@link TypeInformation} the repository is declared for. - * - * @return the domain class the repository is handling. - */ - TypeInformation getDomainTypeInformation(); - - /** - * Returns the repository interface. - * - * @return - */ - Class getRepositoryInterface(); - /** * Returns the raw id class the given class is declared for. * @@ -60,7 +39,7 @@ public interface RepositoryMetadata { default Class getIdType() { return getIdTypeInformation().getType(); } - + /** * Returns the raw domain class the repository is declared for. * @@ -70,6 +49,31 @@ public interface RepositoryMetadata { return getDomainTypeInformation().getType(); } + /** + * Returns the {@link TypeInformation} of the id type of the repository. + * + * @return the {@link TypeInformation} class of the identifier of the entity managed by the repository. Will never be + * {@literal null}. + * @since 2.7 + */ + TypeInformation getIdTypeInformation(); + + /** + * Returns the {@link TypeInformation}of the domain type the repository is declared to manage. Will never be + * {@literal null}. + * + * @return the domain class the repository is handling. + * @since 2.7 + */ + TypeInformation getDomainTypeInformation(); + + /** + * Returns the repository interface. + * + * @return + */ + Class getRepositoryInterface(); + /** * Returns the type {@link Method} return type as it is declared in the repository. Considers suspended methods and * does not unwrap component types but leaves those for further inspection. diff --git a/src/main/java/org/springframework/data/repository/core/support/AnnotationRepositoryMetadata.java b/src/main/java/org/springframework/data/repository/core/support/AnnotationRepositoryMetadata.java index 8bb9d4607..12ba5fb7c 100644 --- a/src/main/java/org/springframework/data/repository/core/support/AnnotationRepositoryMetadata.java +++ b/src/main/java/org/springframework/data/repository/core/support/AnnotationRepositoryMetadata.java @@ -15,6 +15,8 @@ */ package org.springframework.data.repository.core.support; +import java.util.function.Function; + import org.springframework.core.annotation.AnnotationUtils; import org.springframework.data.repository.RepositoryDefinition; import org.springframework.data.repository.core.RepositoryMetadata; @@ -52,8 +54,8 @@ public class AnnotationRepositoryMetadata extends AbstractRepositoryMetadata { Assert.isTrue(AnnotationUtils.findAnnotation(repositoryInterface, RepositoryDefinition.class) != null, () -> String.format(NO_ANNOTATION_FOUND, repositoryInterface.getName())); - this.idType = resolveIdType(repositoryInterface); - this.domainType = resolveDomainType(repositoryInterface); + this.idType = resolveType(repositoryInterface, RepositoryDefinition::idClass); + this.domainType = resolveType(repositoryInterface, RepositoryDefinition::domainClass); } @Override @@ -66,25 +68,15 @@ public class AnnotationRepositoryMetadata extends AbstractRepositoryMetadata { return this.domainType; } - private TypeInformation resolveIdType(Class repositoryInterface) { + private static TypeInformation resolveType(Class repositoryInterface, + Function> extractor) { RepositoryDefinition annotation = AnnotationUtils.findAnnotation(repositoryInterface, RepositoryDefinition.class); - if (annotation == null || annotation.idClass() == null) { - throw new IllegalArgumentException(String.format("Could not resolve id type of %s!", repositoryInterface)); - } - - return ClassTypeInformation.from(annotation.idClass()); - } - - private TypeInformation resolveDomainType(Class repositoryInterface) { - - RepositoryDefinition annotation = AnnotationUtils.findAnnotation(repositoryInterface, RepositoryDefinition.class); - - if (annotation == null || annotation.domainClass() == null) { + if ((annotation == null) || (extractor.apply(annotation) == null)) { throw new IllegalArgumentException(String.format("Could not resolve domain type of %s!", repositoryInterface)); } - return ClassTypeInformation.from(annotation.domainClass()); + return ClassTypeInformation.from(extractor.apply(annotation)); } } diff --git a/src/main/java/org/springframework/data/repository/core/support/DefaultRepositoryMetadata.java b/src/main/java/org/springframework/data/repository/core/support/DefaultRepositoryMetadata.java index 48ee763f1..92f7c4d64 100644 --- a/src/main/java/org/springframework/data/repository/core/support/DefaultRepositoryMetadata.java +++ b/src/main/java/org/springframework/data/repository/core/support/DefaultRepositoryMetadata.java @@ -56,26 +56,28 @@ public class DefaultRepositoryMetadata extends AbstractRepositoryMetadata { this.domainType = resolveTypeParameter(arguments, 0, () -> String.format("Could not resolve domain type of %s!", repositoryInterface)); - + this.idType = resolveTypeParameter(arguments, 1, () -> String.format("Could not resolve id type of %s!", repositoryInterface)); } + @Override + public TypeInformation getIdTypeInformation() { + return this.idType; + } + + @Override + public TypeInformation getDomainTypeInformation() { + return this.domainType; + } + private static TypeInformation resolveTypeParameter(List> arguments, int index, Supplier exceptionMessage) { - if (arguments.size() <= index || arguments.get(index) == null) { + if ((arguments.size() <= index) || (arguments.get(index) == null)) { throw new IllegalArgumentException(exceptionMessage.get()); } - return arguments.get(index).getGenericTypeInformation(); - } - - public TypeInformation getIdTypeInformation() { - return this.idType; - } - - public TypeInformation getDomainTypeInformation() { - return this.domainType; + return arguments.get(index); } } diff --git a/src/main/java/org/springframework/data/repository/core/support/MethodLookups.java b/src/main/java/org/springframework/data/repository/core/support/MethodLookups.java index e9812ccf2..dc9ed7c82 100644 --- a/src/main/java/org/springframework/data/repository/core/support/MethodLookups.java +++ b/src/main/java/org/springframework/data/repository/core/support/MethodLookups.java @@ -121,8 +121,8 @@ interface MethodLookups { Assert.notNull(repositoryMetadata, "Repository metadata must not be null!"); - this.entityType = ResolvableType.forType(repositoryMetadata.getDomainTypeInformation().getGenericType()); - this.idType = ResolvableType.forType(repositoryMetadata.getIdTypeInformation().getGenericType()); + this.entityType = repositoryMetadata.getDomainTypeInformation().toTypeDescriptor().getResolvableType(); + this.idType = repositoryMetadata.getIdTypeInformation().toTypeDescriptor().getResolvableType(); this.repositoryInterface = repositoryMetadata.getRepositoryInterface(); } diff --git a/src/main/java/org/springframework/data/repository/support/DomainClassConverter.java b/src/main/java/org/springframework/data/repository/support/DomainClassConverter.java index 2471ca52c..0a6f6a3e6 100644 --- a/src/main/java/org/springframework/data/repository/support/DomainClassConverter.java +++ b/src/main/java/org/springframework/data/repository/support/DomainClassConverter.java @@ -21,7 +21,6 @@ import java.util.Set; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; -import org.springframework.core.ResolvableType; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.TypeDescriptor; import org.springframework.core.convert.converter.ConditionalGenericConverter; @@ -30,7 +29,6 @@ import org.springframework.data.repository.CrudRepository; import org.springframework.data.repository.core.EntityInformation; import org.springframework.data.repository.core.RepositoryInformation; import org.springframework.data.util.Lazy; -import org.springframework.data.util.TypeInformation; import org.springframework.lang.NonNull; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -104,12 +102,6 @@ public class DomainClassConverter idType = information.getIdTypeInformation(); - return new TypeDescriptor(ResolvableType.forType(idType.getGenericType()), null, idType.getType().getAnnotations()); - } /** * Converter to create domain types from any source that can be converted into the domain types identifier type. @@ -157,7 +149,7 @@ public class DomainClassConverter domainType = targetType.getType(); RepositoryInvoker invoker = repositoryInvokerFactory.getInvokerFor(domainType); RepositoryInformation information = repositories.getRequiredRepositoryInformation(domainType); - TypeDescriptor idTypeDescriptor = getIdTypeDescriptor(information); + TypeDescriptor idTypeDescriptor = information.getIdTypeInformation().toTypeDescriptor(); Object id = conversionService.convert(source, sourceType, idTypeDescriptor); @@ -181,7 +173,7 @@ public class DomainClassConverter { - TypeDescriptor idTypeDescriptor = getIdTypeDescriptor(it); + TypeDescriptor idTypeDescriptor = it.getIdTypeInformation().toTypeDescriptor(); return sourceType.equals(idTypeDescriptor) || conversionService.canConvert(sourceType, idTypeDescriptor); @@ -249,7 +241,7 @@ public class DomainClassConverter { - TypeDescriptor idTypeDescriptor = getIdTypeDescriptor(it); + TypeDescriptor idTypeDescriptor = it.getIdTypeInformation().toTypeDescriptor(); return targetType.equals(idTypeDescriptor) || conversionService.canConvert(idTypeDescriptor, targetType); diff --git a/src/main/java/org/springframework/data/repository/support/ReflectionRepositoryInvoker.java b/src/main/java/org/springframework/data/repository/support/ReflectionRepositoryInvoker.java index 1e7a187d9..c9c8abd7c 100644 --- a/src/main/java/org/springframework/data/repository/support/ReflectionRepositoryInvoker.java +++ b/src/main/java/org/springframework/data/repository/support/ReflectionRepositoryInvoker.java @@ -21,7 +21,6 @@ import java.util.List; import java.util.Optional; import org.springframework.core.MethodParameter; -import org.springframework.core.ResolvableType; import org.springframework.core.convert.ConversionException; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.TypeDescriptor; @@ -74,7 +73,7 @@ class ReflectionRepositoryInvoker implements RepositoryInvoker { this.repository = repository; this.methods = metadata.getCrudMethods(); TypeInformation idType = metadata.getIdTypeInformation(); - this.idTypeDescriptor = new TypeDescriptor(ResolvableType.forType(idType.getGenericType()), null, idType.getType().getAnnotations()); + this.idTypeDescriptor = idType.toTypeDescriptor(); this.conversionService = conversionService; } @@ -249,7 +248,7 @@ class ReflectionRepositoryInvoker implements RepositoryInvoker { protected Object convertId(Object id) { Assert.notNull(id, "Id must not be null!"); - TypeDescriptor idDescriptor = TypeDescriptor.forObject(id); + TypeDescriptor idDescriptor = TypeDescriptor.forObject(id); if (idDescriptor.isAssignableTo(idTypeDescriptor)) { return id; @@ -259,7 +258,8 @@ class ReflectionRepositoryInvoker implements RepositoryInvoker { if (result == null) { throw new IllegalStateException( - String.format("Identifier conversion of %s to %s unexpectedly returned null!", id, idTypeDescriptor.getType())); + String.format("Identifier conversion of %s to %s unexpectedly returned null!", id, + idTypeDescriptor.getType())); } return result; diff --git a/src/main/java/org/springframework/data/util/ClassTypeInformation.java b/src/main/java/org/springframework/data/util/ClassTypeInformation.java index 404b9be04..84a460358 100644 --- a/src/main/java/org/springframework/data/util/ClassTypeInformation.java +++ b/src/main/java/org/springframework/data/util/ClassTypeInformation.java @@ -28,6 +28,7 @@ import java.util.Map; import java.util.Set; import org.springframework.core.GenericTypeResolver; +import org.springframework.core.convert.TypeDescriptor; import org.springframework.util.Assert; import org.springframework.util.ConcurrentReferenceHashMap; import org.springframework.util.ConcurrentReferenceHashMap.ReferenceType; @@ -55,6 +56,7 @@ public class ClassTypeInformation extends TypeDiscoverer { } private final Class type; + private final Lazy descriptor; /** * Simple factory method to easily create new instances of {@link ClassTypeInformation}. @@ -89,8 +91,11 @@ public class ClassTypeInformation extends TypeDiscoverer { * @param type */ ClassTypeInformation(Class type) { + super(type, getTypeVariableMap(type)); + this.type = type; + this.descriptor = Lazy.of(() -> TypeDescriptor.valueOf(type)); } /** @@ -152,6 +157,11 @@ public class ClassTypeInformation extends TypeDiscoverer { return (TypeInformation) type; } + @Override + public TypeDescriptor toTypeDescriptor() { + return descriptor.get(); + } + @Override public String toString() { return type.getName(); diff --git a/src/main/java/org/springframework/data/util/ParentTypeAwareTypeInformation.java b/src/main/java/org/springframework/data/util/ParentTypeAwareTypeInformation.java index 61d0b9d19..4bc6e104c 100644 --- a/src/main/java/org/springframework/data/util/ParentTypeAwareTypeInformation.java +++ b/src/main/java/org/springframework/data/util/ParentTypeAwareTypeInformation.java @@ -19,6 +19,8 @@ import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; import java.util.Map; +import org.springframework.core.ResolvableType; +import org.springframework.core.convert.TypeDescriptor; import org.springframework.lang.Nullable; /** @@ -29,6 +31,7 @@ import org.springframework.lang.Nullable; public abstract class ParentTypeAwareTypeInformation extends TypeDiscoverer { private final TypeDiscoverer parent; + private final Lazy descriptor; private int hashCode; /** @@ -44,7 +47,14 @@ public abstract class ParentTypeAwareTypeInformation extends TypeDiscoverer parent, Map, Type> map) { super(type, map); + this.parent = parent; + this.descriptor = Lazy.of(() -> new TypeDescriptor(toResolvableType(), null, null)); + } + + @Override + public TypeDescriptor toTypeDescriptor() { + return descriptor.get(); } @Override @@ -57,6 +67,11 @@ public abstract class ParentTypeAwareTypeInformation extends TypeDiscoverer extends TypeDiscoverer implements TypeInformation { this.componentType = Lazy.of(this::doGetComponentType); this.valueType = Lazy.of(this::doGetMapValueType); this.typeVariableMap = typeVariableMap; - this.hashCode = 17 + 31 * type.hashCode() + 31 * typeVariableMap.hashCode(); + this.hashCode = 17 + (31 * type.hashCode()) + (31 * typeVariableMap.hashCode()); } /** @@ -291,18 +284,14 @@ class TypeDiscoverer implements TypeInformation { return parameterTypes.length == 0 ? null : parameterTypes[0]; } + @Override public Class getType() { return resolvedType.get(); } - - @Override - public Type getGenericType() { - return type; - } @Override - public TypeInformation getGenericTypeInformation() { - return createInfo(type); + public TypeDescriptor toTypeDescriptor() { + return new TypeDescriptor(toResolvableType(), getType(), null); } @Override @@ -499,6 +488,10 @@ class TypeDiscoverer implements TypeInformation { return getSuperTypeWithin(MAP_TYPES); } + protected ResolvableType toResolvableType() { + return ResolvableType.forType(type); + } + @Override public boolean equals(@Nullable Object obj) { @@ -656,7 +649,7 @@ class TypeDiscoverer implements TypeInformation { @Override public int hashCode() { int result = ObjectUtils.nullSafeHashCode(typeInformation); - result = 31 * result + ObjectUtils.nullSafeHashCode(typeParameters); + result = (31 * result) + ObjectUtils.nullSafeHashCode(typeParameters); return result; } } diff --git a/src/main/java/org/springframework/data/util/TypeInformation.java b/src/main/java/org/springframework/data/util/TypeInformation.java index 6df89ff3c..d822db888 100644 --- a/src/main/java/org/springframework/data/util/TypeInformation.java +++ b/src/main/java/org/springframework/data/util/TypeInformation.java @@ -17,9 +17,9 @@ package org.springframework.data.util; import java.lang.reflect.Constructor; import java.lang.reflect.Method; -import java.lang.reflect.Type; import java.util.List; +import org.springframework.core.convert.TypeDescriptor; import org.springframework.lang.Nullable; /** @@ -150,15 +150,6 @@ public interface TypeInformation { * @return */ Class getType(); - - /** - * Returns the type of the property with all resolvable generics applied - * - * @return - */ - default Type getGenericType() { - return getType(); - } /** * Returns the user type of the property if proxied. @@ -181,13 +172,6 @@ public interface TypeInformation { */ ClassTypeInformation getRawTypeInformation(); - /** - * Returns a {@link TypeInformation} to represent the {@link TypeInformation} of the type of the current instance with all the generics parameters resolved. - * - * @return - */ - TypeInformation getGenericTypeInformation(); - /** * Transparently returns the {@link java.util.Map} value type if the type is a {@link java.util.Map}, returns the * component type if the type {@link #isCollectionLike()} or the simple type if none of this applies. @@ -305,4 +289,11 @@ public interface TypeInformation { return !type.equals(getType()) && type.isAssignableFrom(getType()); } + /** + * Returns the {@link TypeDescriptor} equivalent of this {@link TypeInformation}. + * + * @return will never be {@literal null}. + * @since 2.7 + */ + TypeDescriptor toTypeDescriptor(); } diff --git a/src/main/java/org/springframework/data/util/TypeVariableTypeInformation.java b/src/main/java/org/springframework/data/util/TypeVariableTypeInformation.java index d1fc1935c..3423ff414 100644 --- a/src/main/java/org/springframework/data/util/TypeVariableTypeInformation.java +++ b/src/main/java/org/springframework/data/util/TypeVariableTypeInformation.java @@ -19,6 +19,7 @@ import static org.springframework.util.ObjectUtils.*; import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; +import java.util.List; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -33,6 +34,7 @@ import org.springframework.util.Assert; class TypeVariableTypeInformation extends ParentTypeAwareTypeInformation { private final TypeVariable variable; + private final Lazy>> parameters; /** * Creates a new {@link TypeVariableTypeInformation} for the given {@link TypeVariable} owning {@link Type} and parent @@ -49,11 +51,18 @@ class TypeVariableTypeInformation extends ParentTypeAwareTypeInformation { Assert.notNull(variable, "TypeVariable must not be null!"); this.variable = variable; + this.parameters = Lazy.of(() -> { + return createInfo(getTypeVariableMap().getOrDefault(variable, Object.class)).getTypeArguments(); + }); } - + + /* + * (non-Javadoc) + * @see org.springframework.data.util.TypeDiscoverer#getTypeArguments() + */ @Override - public TypeInformation getGenericTypeInformation() { - return createInfo(getTypeVariableMap().getOrDefault(variable, Object.class)); + public List> getTypeArguments() { + return parameters.get(); } @Override diff --git a/src/test/java/org/springframework/data/repository/support/RepositoriesUnitTests.java b/src/test/java/org/springframework/data/repository/support/RepositoriesUnitTests.java index 0ef34cf02..9741e2834 100755 --- a/src/test/java/org/springframework/data/repository/support/RepositoriesUnitTests.java +++ b/src/test/java/org/springframework/data/repository/support/RepositoriesUnitTests.java @@ -28,7 +28,6 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.quality.Strictness; - import org.springframework.aop.framework.ProxyFactory; import org.springframework.beans.factory.support.AbstractBeanDefinition; import org.springframework.beans.factory.support.BeanDefinitionBuilder; @@ -305,7 +304,8 @@ class RepositoriesUnitTests { var domainType = super.getDomainType().getName().concat("Entity"); try { - this.domainType = ClassTypeInformation.from(ClassUtils.forName(domainType, CustomRepositoryMetadata.class.getClassLoader())); + this.domainType = ClassTypeInformation + .from(ClassUtils.forName(domainType, CustomRepositoryMetadata.class.getClassLoader())); } catch (Exception e) { throw new RuntimeException(e); } @@ -313,7 +313,7 @@ class RepositoriesUnitTests { @Override public TypeInformation getDomainTypeInformation() { - return this.domainType; + return domainType == null ? super.getDomainTypeInformation() : domainType; } @Override