Polishing.

Delegate annotation attribute resolution to configured BeanNameGenerator resolving method and test for enabling reactive repositories.

Original Pull Request: #3083

Author: christophstrobl

src/main/java/org/springframework/data/repository/config/AnnotationRepositoryConfigurationSource.java

@@ -53,6 +53,7 @@
  * @author Mark Paluch
  * @author Johannes Englmeier
  * @author Florian Cramer
+ * @author Christoph Strobl
  */
 public class AnnotationRepositoryConfigurationSource extends RepositoryConfigurationSourceSupport {
 
@@ -314,15 +315,10 @@ private static BeanNameGenerator configuredOrDefaultBeanNameGenerator(Annotation
 			Class<? extends Annotation> annotation, ClassLoader beanClassLoader,
 			@Nullable BeanNameGenerator importBeanNameGenerator) {
 
-		Map<String, Object> annotationAttributes = metadata.getAnnotationAttributes(annotation.getName());
-
-		if (annotationAttributes != null) {
-
-			BeanNameGenerator beanNameGenerator = getBeanNameGenerator(annotationAttributes, beanClassLoader);
+		BeanNameGenerator beanNameGenerator = getConfiguredBeanNameGenerator(metadata, annotation, beanClassLoader);
 
-			if (beanNameGenerator != null) {
-				return beanNameGenerator;
-			}
+		if (beanNameGenerator != null) {
+			return beanNameGenerator;
 		}
 
 		return defaultBeanNameGenerator(importBeanNameGenerator);
@@ -348,26 +344,30 @@ private static BeanNameGenerator defaultBeanNameGenerator(@Nullable BeanNameGene
 	}
 
 	/**
-	 * Obtain a configured {@link BeanNameGenerator}.
+	 * Obtain a configured {@link BeanNameGenerator} if present.
 	 *
 	 * @param beanClassLoader a class loader to load the configured {@link BeanNameGenerator} class in case it was
 	 *          configured as String instead of a Class instance.
-	 * @return the bean name generator.
+	 * @return the bean name generator or {@literal null} if not configured.
 	 */
 	@Nullable
 	@SuppressWarnings("unchecked")
-	private static BeanNameGenerator getBeanNameGenerator(Map<String, Object> annotationAttributes,
-			ClassLoader beanClassLoader) {
+	private static BeanNameGenerator getConfiguredBeanNameGenerator(AnnotationMetadata metadata,
+		Class<? extends Annotation> annotation, ClassLoader beanClassLoader) {
 
-		Object configuredBeanNameGenerator = annotationAttributes.get(BEAN_NAME_GENERATOR);
+		Map<String, Object> annotationAttributes = metadata.getAnnotationAttributes(annotation.getName());
+		if(annotationAttributes == null || !annotationAttributes.containsKey(BEAN_NAME_GENERATOR)) {
+			return null;
+		}
 
+		Object configuredBeanNameGenerator = annotationAttributes.get(BEAN_NAME_GENERATOR);
 		if (configuredBeanNameGenerator == null) {
 			return null;
 		}
 
-		if (configuredBeanNameGenerator instanceof String cbng) {
+		if (configuredBeanNameGenerator instanceof String beanNameGeneratorTypeName) {
 			try {
-				configuredBeanNameGenerator = ClassUtils.forName(cbng, beanClassLoader);
+				configuredBeanNameGenerator = ClassUtils.forName(beanNameGeneratorTypeName, beanClassLoader);
 			} catch (Exception o_O) {
 				throw new RuntimeException(o_O);
 			}

src/test/java/org/springframework/data/repository/config/AnnotationRepositoryConfigurationSourceUnitTests.java

@@ -15,8 +15,9 @@
  */
 package org.springframework.data.repository.config;
 
-import static org.assertj.core.api.Assertions.*;
-import static org.mockito.Mockito.*;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
+import static org.mockito.Mockito.mock;
 
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
@@ -36,14 +37,17 @@
 import org.springframework.core.type.StandardAnnotationMetadata;
 import org.springframework.data.repository.PagingAndSortingRepository;
 import org.springframework.data.repository.config.basepackage.repo.PersonRepository;
+import org.springframework.data.repository.core.support.DummyReactiveRepositoryFactory;
 import org.springframework.data.repository.core.support.DummyRepositoryFactory;
+import org.springframework.data.repository.reactive.ReactiveCrudRepository;
 
 /**
  * Unit tests for {@link AnnotationRepositoryConfigurationSource}.
  *
  * @author Oliver Gierke
  * @author Thomas Darimont
  * @author Mark Paluch
+ * @author Christoph Strobl
  */
 class AnnotationRepositoryConfigurationSourceUnitTests {
 
@@ -180,6 +184,17 @@ void considerBeanNameGenerator() {
 		assertThat(getConfigSource(DefaultConfiguration.class).generateBeanName(bd)).isEqualTo("personRepository");
 	}
 
+	@Test // GH-3082
+	void considerBeanNameGeneratorForReactiveRepos() {
+
+		RootBeanDefinition bd = new RootBeanDefinition(DummyReactiveRepositoryFactory.class);
+		bd.getConstructorArgumentValues().addGenericArgumentValue(ReactivePersonRepository.class);
+
+		assertThat(getConfigSource(ConfigurationWithBeanNameGenerator.class).generateBeanName(bd))
+			.isEqualTo(ReactivePersonRepository.class.getName());
+		assertThat(getConfigSource(DefaultConfiguration.class).generateBeanName(bd)).isEqualTo("annotationRepositoryConfigurationSourceUnitTests.ReactivePersonRepository");
+	}
+
 	private AnnotationRepositoryConfigurationSource getConfigSource(Class<?> type) {
 
 		AnnotationMetadata metadata = new StandardAnnotationMetadata(type, true);
@@ -204,6 +219,9 @@ static class ConfigurationWithExplicitFilter {}
 	@EnableRepositories(nameGenerator = FullyQualifiedAnnotationBeanNameGenerator.class)
 	static class ConfigurationWithBeanNameGenerator {}
 
+	@EnableReactiveRepositories(nameGenerator = FullyQualifiedAnnotationBeanNameGenerator.class)
+	static class ReactiveConfigurationWithBeanNameGenerator {}
+
 	@Retention(RetentionPolicy.RUNTIME)
 	@interface SampleAnnotation {
 
@@ -214,4 +232,6 @@ static class ConfigurationWithBeanNameGenerator {}
 
 	@SampleAnnotation
 	static class ConfigWithSampleAnnotation {}
+
+	interface ReactivePersonRepository extends ReactiveCrudRepository<Person, String> {}
 }

src/test/java/org/springframework/data/repository/config/EnableReactiveRepositories.java

@@ -19,6 +19,7 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 
+import org.springframework.beans.factory.support.BeanNameGenerator;
 import org.springframework.context.annotation.ComponentScan.Filter;
 import org.springframework.context.annotation.Import;
 import org.springframework.data.repository.core.support.ReactiveDummyRepositoryFactoryBean;
@@ -43,6 +44,8 @@
 
 	Class<?> repositoryBaseClass() default ReactiveSortingRepository.class;
 
+	Class<? extends BeanNameGenerator> nameGenerator() default BeanNameGenerator.class;
+
 	String namedQueriesLocation() default "";
 
 	String repositoryImplementationPostfix() default "Impl";

src/test/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupportUnitTests.java

@@ -77,7 +77,6 @@ void registersBeanDefinitionForFoundBean() {
 		assertNoBeanDefinitionRegisteredFor("profileRepository");
 	}
 
-
 	@Test // GH-2584
 	void shouldExposeFragmentsAsBean() {
 
@@ -202,7 +201,8 @@ static class LimitsImplementationBasePackages {}
 	@EnableRepositories(basePackageClasses = MyNestedRepository.class, considerNestedRepositories = true,
 			excludeFilters = {
 					@Filter(type = FilterType.ASSIGNABLE_TYPE,
-							value = RepositoryConfigurationExtensionSupportUnitTests.ReactiveRepository.class),
+							value = { RepositoryConfigurationExtensionSupportUnitTests.ReactiveRepository.class,
+									AnnotationRepositoryConfigurationSourceUnitTests.ReactivePersonRepository.class }),
 					@Filter(type = FilterType.ASSIGNABLE_TYPE, value = MyOtherRepository.class) })
 	static class NestedRepositoriesConfiguration {}