From 37b072f22602bcdc707226861f7109e1c3d98b4e 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. $ Conflicts: $ src/main/java/org/springframework/data/util/TypeDiscoverer.java --- .../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 | 28 +++++------ .../data/util/TypeInformation.java | 25 ++++------ .../util/TypeVariableTypeInformation.java | 15 ++++-- .../support/RepositoriesUnitTests.java | 5 +- 12 files changed, 116 insertions(+), 106 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 5c04f1ff0..b21e1aae6 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); } /* @@ -74,25 +76,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 ac67f7411..4e8d26190 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 bbc23e61b..f6e56a45f 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; @@ -120,12 +118,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. @@ -181,7 +173,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); @@ -209,7 +201,7 @@ public class DomainClassConverter { - TypeDescriptor idTypeDescriptor = getIdTypeDescriptor(it); + TypeDescriptor idTypeDescriptor = it.getIdTypeInformation().toTypeDescriptor(); return sourceType.equals(idTypeDescriptor) || conversionService.canConvert(sourceType, idTypeDescriptor); @@ -289,7 +281,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 a5cb17a15..fae1234e2 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; } @@ -289,7 +288,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; @@ -299,7 +298,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 8f782f177..5dd0c1337 100644 --- a/src/main/java/org/springframework/data/util/ClassTypeInformation.java +++ b/src/main/java/org/springframework/data/util/ClassTypeInformation.java @@ -29,6 +29,7 @@ import java.util.Map.Entry; 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; @@ -56,6 +57,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}. @@ -90,8 +92,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)); } /** @@ -173,6 +178,11 @@ public class ClassTypeInformation extends TypeDiscoverer { * (non-Javadoc) * @see java.lang.Object#toString() */ + @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 e0e824272..503685461 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(); } /* @@ -65,6 +75,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()); } /** @@ -293,22 +295,14 @@ class TypeDiscoverer implements TypeInformation { return parameterTypes.length == 0 ? null : parameterTypes[0]; } - /* - * (non-Javadoc) - * @see org.springframework.data.util.TypeInformation#getType() - */ + @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); } /* @@ -552,10 +546,10 @@ class TypeDiscoverer implements TypeInformation { return getSuperTypeWithin(MAP_TYPES); } - /* - * (non-Javadoc) - * @see java.lang.Object#equals(java.lang.Object) - */ + protected ResolvableType toResolvableType() { + return ResolvableType.forType(type); + } + @Override public boolean equals(@Nullable Object obj) { @@ -739,7 +733,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 ec0952963..55351a8a4 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(); } /* 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 198633486..d2c7981a5 100755 --- a/src/test/java/org/springframework/data/repository/support/RepositoriesUnitTests.java +++ b/src/test/java/org/springframework/data/repository/support/RepositoriesUnitTests.java @@ -305,7 +305,8 @@ class RepositoriesUnitTests { String 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); } @@ -317,7 +318,7 @@ class RepositoriesUnitTests { */ @Override public TypeInformation getDomainTypeInformation() { - return this.domainType; + return domainType == null ? super.getDomainTypeInformation() : domainType; } /*