From 2f43bd593e99a813e9197fa1e2902c3318131cde Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 30 Apr 2024 15:20:25 +0200 Subject: [PATCH] Limit generics length to declared length. Also, rename method to reflect what it actually returns. Document generic usage constraints in RepositoryFactoryBeanSupport subclasses. Closes #3089 --- .../RepositoryConfigurationDelegate.java | 18 ++++----- .../support/RepositoryFactoryBeanSupport.java | 11 +++++- ...ositoryConfigurationDelegateUnitTests.java | 38 ++++++++++++++++--- 3 files changed, 49 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationDelegate.java b/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationDelegate.java index 435e12bbbd..aad4756add 100644 --- a/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationDelegate.java +++ b/src/main/java/org/springframework/data/repository/config/RepositoryConfigurationDelegate.java @@ -186,7 +186,7 @@ public List registerRepositoriesIn(BeanDefinitionRegist } RootBeanDefinition beanDefinition = (RootBeanDefinition) definitionBuilder.getBeanDefinition(); - beanDefinition.setTargetType(getRepositoryInterface(configuration)); + beanDefinition.setTargetType(getRepositoryFactoryBeanType(configuration)); beanDefinition.setResourceDescription(configuration.getResourceDescription()); String beanName = configurationSource.generateBeanName(beanDefinition); @@ -312,17 +312,16 @@ private static ApplicationStartup getStartup(BeanDefinitionRegistry registry) { } /** - * Returns the repository interface of the given {@link RepositoryConfiguration} as loaded {@link Class}. + * Returns the repository factory bean type from the given {@link RepositoryConfiguration} as loaded {@link Class}. * * @param configuration must not be {@literal null}. * @return can be {@literal null}. */ @Nullable - private ResolvableType getRepositoryInterface(RepositoryConfiguration configuration) { + private ResolvableType getRepositoryFactoryBeanType(RepositoryConfiguration configuration) { String interfaceName = configuration.getRepositoryInterface(); - ClassLoader classLoader = resourceLoader.getClassLoader() == null - ? ClassUtils.getDefaultClassLoader() + ClassLoader classLoader = resourceLoader.getClassLoader() == null ? ClassUtils.getDefaultClassLoader() : resourceLoader.getClassLoader(); classLoader = classLoader != null ? classLoader : getClass().getClassLoader(); @@ -346,7 +345,7 @@ private ResolvableType getRepositoryInterface(RepositoryConfiguration configu ResolvableType[] declaredGenerics = ResolvableType.forClass(factoryBean).getGenerics(); ResolvableType[] parentGenerics = ResolvableType.forClass(RepositoryFactoryBeanSupport.class, factoryBean) .getGenerics(); - List resolvedGenerics = new ArrayList(factoryBean.getTypeParameters().length); + List resolvedGenerics = new ArrayList<>(factoryBean.getTypeParameters().length); for (int i = 0; i < parentGenerics.length; i++) { @@ -358,12 +357,11 @@ private ResolvableType getRepositoryInterface(RepositoryConfiguration configu } if (resolvedGenerics.size() < declaredGenerics.length) { - for (int j = parentGenerics.length; j < declaredGenerics.length; j++) { - resolvedGenerics.add(declaredGenerics[j]); - } + resolvedGenerics.addAll(Arrays.asList(declaredGenerics).subList(parentGenerics.length, declaredGenerics.length)); } - return ResolvableType.forClassWithGenerics(factoryBean, resolvedGenerics.toArray(ResolvableType[]::new)); + return ResolvableType.forClassWithGenerics(factoryBean, + resolvedGenerics.subList(0, declaredGenerics.length).toArray(ResolvableType[]::new)); } /** diff --git a/src/main/java/org/springframework/data/repository/core/support/RepositoryFactoryBeanSupport.java b/src/main/java/org/springframework/data/repository/core/support/RepositoryFactoryBeanSupport.java index 98278af327..f3e6c13dae 100644 --- a/src/main/java/org/springframework/data/repository/core/support/RepositoryFactoryBeanSupport.java +++ b/src/main/java/org/springframework/data/repository/core/support/RepositoryFactoryBeanSupport.java @@ -46,10 +46,17 @@ import org.springframework.util.Assert; /** - * Adapter for Springs {@link FactoryBean} interface to allow easy setup of repository factories via Spring + * Adapter for Spring's {@link FactoryBean} interface to allow easy setup of repository factories via Spring * configuration. + *

+ * Subclasses may pass-thru generics, provide a fixed domain, provide a fixed identifier type, or provide additional + * generic type parameters. Type parameters must appear in the same order the ones from this class (repository type, + * entity type, identifier type, additional type parameters). Using a different ordering will result in invalid type + * definitions. * - * @param the type of the repository + * @param the type of the repository. + * @param the entity type. + * @param the entity identifier type. * @author Oliver Gierke * @author Thomas Darimont * @author Mark Paluch diff --git a/src/test/java/org/springframework/data/repository/config/RepositoryConfigurationDelegateUnitTests.java b/src/test/java/org/springframework/data/repository/config/RepositoryConfigurationDelegateUnitTests.java index 71a13568d7..6ba7582203 100644 --- a/src/test/java/org/springframework/data/repository/config/RepositoryConfigurationDelegateUnitTests.java +++ b/src/test/java/org/springframework/data/repository/config/RepositoryConfigurationDelegateUnitTests.java @@ -28,6 +28,7 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.quality.Strictness; + import org.springframework.aop.framework.Advised; import org.springframework.aot.hint.RuntimeHints; import org.springframework.beans.factory.ListableBeanFactory; @@ -266,6 +267,16 @@ void registersGenericsForAdditionalGenericsRepositoryFactoryBean() { assertThat(it.getGeneric(3).getType()).isInstanceOf(TypeVariable.class); } + @Test // GH-3074 + void considersGenericLength() { + + ResolvableType it = registerBeanDefinition(IdAndEntityConstrainingFactoryBean.class); + + assertThat(it.getGenerics()).hasSize(2); + assertThat(it.getGeneric(0).resolve()).isEqualTo(MyAnnotatedRepository.class); + assertThat(it.getGeneric(1).resolve()).isEqualTo(Person.class); + } + private static ListableBeanFactory assertLazyRepositoryBeanSetup(Class configClass) { var context = new AnnotationConfigApplicationContext(configClass); @@ -329,8 +340,8 @@ private ResolvableType registerBeanDefinition(Class repositoryFactoryType) { AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); RepositoryConfigurationSource source = new AnnotationRepositoryConfigurationSource(metadata, - EnableRepositories.class, context, context.getEnvironment(), - context.getDefaultListableBeanFactory(), new AnnotationBeanNameGenerator()) { + EnableRepositories.class, context, context.getEnvironment(), context.getDefaultListableBeanFactory(), + new AnnotationBeanNameGenerator()) { @Override public Optional getRepositoryFactoryBeanClassName() { @@ -343,10 +354,8 @@ public Optional getRepositoryFactoryBeanClassName() { List repositories = delegate.registerRepositoriesIn(context, extension); - assertThat(repositories).hasSize(1).element(0) - .extracting(BeanComponentDefinition::getBeanDefinition) - .extracting(BeanDefinition::getResolvableType) - .isNotNull(); + assertThat(repositories).hasSize(1).element(0).extracting(BeanComponentDefinition::getBeanDefinition) + .extracting(BeanDefinition::getResolvableType).isNotNull(); return repositories.get(0).getBeanDefinition().getResolvableType(); } @@ -374,4 +383,21 @@ protected AdditionalGenericsRepositoryFactoryBean(Class repositoryI super(repositoryInterface); } } + + static abstract class ModuleRepositoryFactoryBean, S, ID> + extends RepositoryFactoryBeanSupport { + + protected ModuleRepositoryFactoryBean(Class repositoryInterface) { + super(repositoryInterface); + } + } + + static abstract class IdAndEntityConstrainingFactoryBean, T extends Person> + extends RepositoryFactoryBeanSupport { + protected IdAndEntityConstrainingFactoryBean(Class repositoryInterface) { + super(repositoryInterface); + } + + } + }