Consider component names for custom implementation and fragment bean registration.

We now consider the the actual repository & fragment bean name when checking for existing bean definitions of default custom implementation beans. Previously, we used the repository interface name without considering the repository bean name.

Closes spring-projects#2487.
Original Pull Request: spring-projects#2488

Author: mp911de

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

@@ -18,6 +18,7 @@
 import java.util.Collection;
 import java.util.Optional;
 import java.util.Set;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
 import org.springframework.beans.factory.config.BeanDefinition;
@@ -107,8 +108,22 @@ public Optional<AbstractBeanDefinition> detectCustomImplementation(Implementatio
 				.filter(lookup::matches) //
 				.collect(StreamUtils.toUnmodifiableSet());
 
+		return selectImplementationCandidate(lookup, definitions, () -> {
+
+			if (definitions.isEmpty()) {
+				return Optional.empty();
+			}
+
+			return Optional.of(definitions.iterator().next());
+		});
+	}
+
+	private static Optional<AbstractBeanDefinition> selectImplementationCandidate(
+			ImplementationLookupConfiguration lookup, Set<BeanDefinition> definitions,
+			Supplier<Optional<BeanDefinition>> fallback) {
+
 		return SelectionSet //
-				.of(definitions, c -> c.isEmpty() ? Optional.empty() : throwAmbiguousCustomImplementationException(c)) //
+				.of(definitions, c -> c.isEmpty() ? fallback.get() : throwAmbiguousCustomImplementationException(c)) //
 				.filterIfNecessary(lookup::hasMatchingBeanName) //
 				.uniqueResult() //
 				.map(AbstractBeanDefinition.class::cast);

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

@@ -15,7 +15,6 @@
  */
 package org.springframework.data.repository.config;
 
-import java.beans.Introspector;
 import java.io.IOException;
 
 import org.springframework.beans.factory.config.BeanDefinition;
@@ -42,22 +41,16 @@ class DefaultImplementationLookupConfiguration implements ImplementationLookupCo
 	private final String interfaceName;
 	private final String beanName;
 
-	/**
-	 * Creates a new {@link DefaultImplementationLookupConfiguration} for the given
-	 * {@link ImplementationDetectionConfiguration} and interface name.
-	 *
-	 * @param config must not be {@literal null}.
-	 * @param interfaceName must not be {@literal null} or empty.
-	 */
-	DefaultImplementationLookupConfiguration(ImplementationDetectionConfiguration config, String interfaceName) {
+	DefaultImplementationLookupConfiguration(ImplementationDetectionConfiguration config, String interfaceName,
+			String beanName) {
 
 		Assert.notNull(config, "ImplementationDetectionConfiguration must not be null");
 		Assert.hasText(interfaceName, "Interface name must not be null or empty");
+		Assert.hasText(beanName, "Bean name must not be null or empty!");
 
 		this.config = config;
 		this.interfaceName = interfaceName;
-		this.beanName = Introspector
-				.decapitalize(ClassUtils.getShortName(interfaceName).concat(config.getImplementationPostfix()));
+		this.beanName = beanName;
 	}
 
 	@Override

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

@@ -22,6 +22,7 @@
 import org.springframework.core.type.filter.TypeFilter;
 import org.springframework.data.config.ConfigurationUtils;
 import org.springframework.data.repository.query.QueryLookupStrategy.Key;
+import org.springframework.data.util.Lazy;
 import org.springframework.data.util.Streamable;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
@@ -45,13 +46,15 @@ public class DefaultRepositoryConfiguration<T extends RepositoryConfigurationSou
 	private final T configurationSource;
 	private final BeanDefinition definition;
 	private final RepositoryConfigurationExtension extension;
+	private final Lazy<String> beanName;
 
 	public DefaultRepositoryConfiguration(T configurationSource, BeanDefinition definition,
 			RepositoryConfigurationExtension extension) {
 
 		this.configurationSource = configurationSource;
 		this.definition = definition;
 		this.extension = extension;
+		this.beanName = Lazy.of(() -> configurationSource.generateBeanName(definition));
 	}
 
 	public String getBeanId() {
@@ -90,8 +93,7 @@ public String getImplementationClassName() {
 	}
 
 	public String getImplementationBeanName() {
-		return configurationSource.generateBeanName(definition)
-				+ configurationSource.getRepositoryImplementationPostfix().orElse("Impl");
+		return beanName.get() + configurationSource.getRepositoryImplementationPostfix().orElse("Impl");
 	}
 
 	@Nullable
@@ -117,6 +119,11 @@ public String getRepositoryFactoryBeanClassName() {
 				.orElseGet(extension::getRepositoryFactoryBeanClassName);
 	}
 
+	@Override
+	public String getRepositoryBeanName() {
+		return beanName.get();
+	}
+
 	@Override
 	public boolean isLazyInit() {
 		return definition.isLazyInit() || !configurationSource.getBootstrapMode().equals(BootstrapMode.DEFAULT);

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

@@ -90,7 +90,8 @@ default ImplementationLookupConfiguration forFragment(String fragmentInterfaceNa
 
 		Assert.hasText(fragmentInterfaceName, "Fragment interface name must not be null or empty");
 
-		return new DefaultImplementationLookupConfiguration(this, fragmentInterfaceName);
+		return new DefaultImplementationLookupConfiguration(this, fragmentInterfaceName,
+				Introspector.decapitalize(ClassUtils.getShortName(fragmentInterfaceName).concat(getImplementationPostfix())));
 	}
 
 	/**
@@ -103,7 +104,8 @@ default ImplementationLookupConfiguration forRepositoryConfiguration(RepositoryC
 
 		Assert.notNull(config, "RepositoryConfiguration must not be null");
 
-		return new DefaultImplementationLookupConfiguration(this, config.getRepositoryInterface()) {
+		return new DefaultImplementationLookupConfiguration(this, config.getRepositoryInterface(),
+				config.getImplementationBeanName()) {
 
 			@Override
 			public Streamable<String> getBasePackages() {

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

@@ -147,7 +147,7 @@ RepositoryConfigurationAdapter<?> buildMetadata(RepositoryConfiguration<?> confi
 
 		List<RepositoryFragmentConfiguration> repositoryFragmentConfigurationStream = fragmentMetadata
 				.getFragmentInterfaces(configuration.getRepositoryInterface()) //
-				.map(it -> detectRepositoryFragmentConfiguration(it, config)) //
+				.map(it -> detectRepositoryFragmentConfiguration(it, config, configuration)) //
 				.flatMap(Optionals::toStream).toList();
 
 		if (repositoryFragmentConfigurationStream.isEmpty()) {
@@ -183,26 +183,42 @@ private Optional<String> registerCustomImplementation(RepositoryConfiguration<?>
 
 		ImplementationLookupConfiguration lookup = configuration.toLookupConfiguration(metadataReaderFactory);
 
-		String beanName = lookup.getImplementationBeanName();
+		String configurationBeanName = lookup.getImplementationBeanName();
 
 		// Already a bean configured?
-		if (registry.containsBeanDefinition(beanName)) {
-			return Optional.of(beanName);
+		if (registry.containsBeanDefinition(configurationBeanName)) {
+
+			if (logger.isDebugEnabled()) {
+				logger.debug(String.format("Custom repository implementation already registered: %s", configurationBeanName));
+			}
+
+			return Optional.of(configurationBeanName);
 		}
 
 		Optional<AbstractBeanDefinition> beanDefinition = implementationDetector.detectCustomImplementation(lookup);
 
 		return beanDefinition.map(it -> {
 
-			if (logger.isDebugEnabled()) {
-				logger.debug("Registering custom repository implementation: " + lookup.getImplementationBeanName() + " "
-						+ it.getBeanClassName());
-			}
-
+			String scannedBeanName = configuration.getConfigurationSource().generateBeanName(it);
 			it.setSource(configuration.getSource());
-			registry.registerBeanDefinition(beanName, it);
 
-			return beanName;
+			if (registry.containsBeanDefinition(scannedBeanName)) {
+
+				if (logger.isDebugEnabled()) {
+					logger.debug(String.format("Custom repository implementation already registered: %s %s", scannedBeanName,
+							it.getBeanClassName()));
+				}
+			} else {
+
+				if (logger.isDebugEnabled()) {
+					logger.debug(String.format("Registering custom repository implementation: %s %s", scannedBeanName,
+							it.getBeanClassName()));
+				}
+
+				registry.registerBeanDefinition(scannedBeanName, it);
+			}
+
+			return scannedBeanName;
 		});
 	}
 
@@ -232,19 +248,20 @@ private Stream<RepositoryFragmentConfiguration> registerRepositoryFragmentsImple
 				.toImplementationDetectionConfiguration(metadataReaderFactory);
 
 		return fragmentMetadata.getFragmentInterfaces(configuration.getRepositoryInterface()) //
-				.map(it -> detectRepositoryFragmentConfiguration(it, config)) //
+				.map(it -> detectRepositoryFragmentConfiguration(it, config, configuration)) //
 				.flatMap(Optionals::toStream) //
 				.peek(it -> potentiallyRegisterFragmentImplementation(configuration, it)) //
 				.peek(it -> potentiallyRegisterRepositoryFragment(configuration, it));
 	}
 
 	private Optional<RepositoryFragmentConfiguration> detectRepositoryFragmentConfiguration(String fragmentInterface,
-			ImplementationDetectionConfiguration config) {
+			ImplementationDetectionConfiguration config, RepositoryConfiguration<?> configuration) {
 
 		ImplementationLookupConfiguration lookup = config.forFragment(fragmentInterface);
 		Optional<AbstractBeanDefinition> beanDefinition = implementationDetector.detectCustomImplementation(lookup);
 
-		return beanDefinition.map(bd -> new RepositoryFragmentConfiguration(fragmentInterface, bd));
+		return beanDefinition.map(bd -> new RepositoryFragmentConfiguration(fragmentInterface, bd,
+				configuration.getConfigurationSource().generateBeanName(bd)));
 	}
 
 	private void potentiallyRegisterFragmentImplementation(RepositoryConfiguration<?> repositoryConfiguration,
@@ -254,16 +271,21 @@ private void potentiallyRegisterFragmentImplementation(RepositoryConfiguration<?
 
 		// Already a bean configured?
 		if (registry.containsBeanDefinition(beanName)) {
-			return;
-		}
 
-		if (logger.isDebugEnabled()) {
-			logger.debug(String.format("Registering repository fragment implementation: %s %s", beanName,
-					fragmentConfiguration.getClassName()));
+			if (logger.isDebugEnabled()) {
+				logger.debug(String.format("Repository fragment implementation already registered: %s", beanName));
+			}
+
+			return;
 		}
 
 		fragmentConfiguration.getBeanDefinition().ifPresent(bd -> {
 
+			if (logger.isDebugEnabled()) {
+				logger.debug(String.format("Registering repository fragment implementation: %s %s", beanName,
+						fragmentConfiguration.getClassName()));
+			}
+
 			bd.setSource(repositoryConfiguration.getSource());
 			registry.registerBeanDefinition(beanName, bd);
 		});
@@ -276,11 +298,16 @@ private void potentiallyRegisterRepositoryFragment(RepositoryConfiguration<?> co
 
 		// Already a bean configured?
 		if (registry.containsBeanDefinition(beanName)) {
+
+			if (logger.isDebugEnabled()) {
+				logger.debug(String.format("RepositoryFragment already registered: %s", beanName));
+			}
+
 			return;
 		}
 
 		if (logger.isDebugEnabled()) {
-			logger.debug("Registering repository fragment: " + beanName);
+			logger.debug(String.format("Registering RepositoryFragment: %s", beanName));
 		}
 
 		BeanDefinitionBuilder fragmentBuilder = BeanDefinitionBuilder.rootBeanDefinition(RepositoryFragment.class,

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

@@ -84,6 +84,22 @@ public interface RepositoryConfiguration<T extends RepositoryConfigurationSource
 	 */
 	String getRepositoryFactoryBeanClassName();
 
+	/**
+	 * Returns the custom implementation bean name to be used.
+	 *
+	 * @return
+	 * @since 3.0
+	 */
+	String getImplementationBeanName();
+
+	/**
+	 * Returns the bean name of the repository to be used.
+	 *
+	 * @return
+	 * @since 3.0
+	 */
+	String getRepositoryBeanName();
+
 	/**
 	 * Returns the source of the {@link RepositoryConfiguration}.
 	 *

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

@@ -75,6 +75,16 @@ public String getRepositoryFactoryBeanClassName() {
 		return repositoryConfiguration.getRepositoryFactoryBeanClassName();
 	}
 
+	@Override
+	public String getImplementationBeanName() {
+		return repositoryConfiguration.getImplementationBeanName();
+	}
+
+	@Override
+	public String getRepositoryBeanName() {
+		return repositoryConfiguration.getRepositoryBeanName();
+	}
+
 	@Override
 	@Nullable
 	public Object getSource() {

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

@@ -36,6 +36,7 @@ public final class RepositoryFragmentConfiguration {
 	private final String interfaceName;
 	private final String className;
 	private final Optional<AbstractBeanDefinition> beanDefinition;
+	private final String beanName;
 
 	/**
 	 * Creates a {@link RepositoryFragmentConfiguration} given {@code interfaceName} and {@code className} of the
@@ -45,13 +46,7 @@ public final class RepositoryFragmentConfiguration {
 	 * @param className must not be {@literal null} or empty.
 	 */
 	public RepositoryFragmentConfiguration(String interfaceName, String className) {
-
-		Assert.hasText(interfaceName, "Interface name must not be null or empty");
-		Assert.hasText(className, "Class name must not be null or empty");
-
-		this.interfaceName = interfaceName;
-		this.className = className;
-		this.beanDefinition = Optional.empty();
+		this(interfaceName, className, Optional.empty(), generateBeanName(className));
 	}
 
 	/**
@@ -69,20 +64,40 @@ public RepositoryFragmentConfiguration(String interfaceName, AbstractBeanDefinit
 		this.interfaceName = interfaceName;
 		this.className = ConfigurationUtils.getRequiredBeanClassName(beanDefinition);
 		this.beanDefinition = Optional.of(beanDefinition);
+		this.beanName = generateBeanName();
+	}
+
+	RepositoryFragmentConfiguration(String interfaceName, AbstractBeanDefinition beanDefinition, String beanName) {
+		this(interfaceName, ConfigurationUtils.getRequiredBeanClassName(beanDefinition), Optional.of(beanDefinition),
+				beanName);
 	}
 
-	public RepositoryFragmentConfiguration(String interfaceName, String className,
-			Optional<AbstractBeanDefinition> beanDefinition) {
+	private RepositoryFragmentConfiguration(String interfaceName, String className,
+			Optional<AbstractBeanDefinition> beanDefinition, String beanName) {
+
+		Assert.hasText(interfaceName, "Interface name must not be null or empty!");
+		Assert.notNull(beanDefinition, "Bean definition must not be null!");
+		Assert.notNull(beanName, "Bean name must not be null!");
+
 		this.interfaceName = interfaceName;
 		this.className = className;
 		this.beanDefinition = beanDefinition;
+		this.beanName = beanName;
+	}
+
+	private String generateBeanName() {
+		return generateBeanName(getClassName());
+	}
+
+	private static String generateBeanName(String className) {
+		return Introspector.decapitalize(ClassUtils.getShortName(className));
 	}
 
 	/**
 	 * @return name of the implementation bean.
 	 */
 	public String getImplementationBeanName() {
-		return Introspector.decapitalize(ClassUtils.getShortName(getClassName()));
+		return this.beanName;
 	}
 
 	/**