DATACMNS-1172 - Limit repository custom implementation scanning by default to repository interface packages.

Custom repository implementation scan defaults to the repository interface package and its subpackages and no longer scans all configured base packages. Scan for fragment implementations defaults to the fragment interface package. Using the interface package for scanning aligns the behavior with the documentation.

This default can be changed with @Enable…Repositories annotations that support the limitImplementationBasePackages parameter to scan in repository base packages for custom implementations:

@EnableJpaRepositories(limitImplementationBasePackages = false)
@Configuration
class AppConfiguration {
  // …
}

Declaring the implementation along with the interface in the same package is an established design pattern allowing to limit the scanning scope. A limited scope improves scanning performance as it can skip elements on the classpath, that do not provide that particular package.

Previously, we scanned for implementations using the configured base packages that were also used to discover repository interfaces. These base packages can be broader for applications that spread repository interfaces across multiple packages.

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

@@ -53,6 +53,7 @@ import org.springframework.util.StringUtils;
  * @author Thomas Darimont
  * @author Peter Rietzler
  * @author Jens Schauder
+ * @author Mark Paluch
  */
 public class AnnotationRepositoryConfigurationSource extends RepositoryConfigurationSourceSupport {
 
@@ -64,6 +65,7 @@ public class AnnotationRepositoryConfigurationSource extends RepositoryConfigura
 	private static final String REPOSITORY_FACTORY_BEAN_CLASS = "repositoryFactoryBeanClass";
 	private static final String REPOSITORY_BASE_CLASS = "repositoryBaseClass";
 	private static final String CONSIDER_NESTED_REPOSITORIES = "considerNestedRepositories";
+	private static final String LIMIT_IMPLEMENTATION_BASE_PACKAGES = "limitImplementationBasePackages";
 
 	private final AnnotationMetadata configMetadata;
 	private final AnnotationMetadata enableAnnotationMetadata;
@@ -320,6 +322,20 @@ public class AnnotationRepositoryConfigurationSource extends RepositoryConfigura
 		return attributes.containsKey(CONSIDER_NESTED_REPOSITORIES) && attributes.getBoolean(CONSIDER_NESTED_REPOSITORIES);
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.repository.config.RepositoryConfigurationSourceSupport#shouldLimitRepositoryImplementationBasePackages()
+	 */
+	@Override
+	public boolean shouldLimitRepositoryImplementationBasePackages() {
+
+		if (!attributes.containsKey(LIMIT_IMPLEMENTATION_BASE_PACKAGES)) {
+			return true;
+		}
+
+		return attributes.getBoolean(LIMIT_IMPLEMENTATION_BASE_PACKAGES);
+	}
+
 	/* 
 	 * (non-Javadoc)
 	 * @see org.springframework.data.repository.config.RepositoryConfigurationSource#getAttribute(java.lang.String)

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

@@ -46,6 +46,7 @@ import org.springframework.util.Assert;
  * @author Christoph Strobl
  * @author Peter Rietzler
  * @author Jens Schauder
+ * @author Mark Paluch
  */
 @RequiredArgsConstructor
 public class CustomRepositoryImplementationDetector {
@@ -71,7 +72,7 @@ public class CustomRepositoryImplementationDetector {
 		return detectCustomImplementation( //
 				configuration.getImplementationClassName(), //
 				configuration.getImplementationBeanName(), //
-				configuration.getBasePackages(), //
+				configuration.getImplementationBasePackages(configuration.getImplementationClassName()), //
 				configuration.getExcludeFilters(), //
 				bd -> configuration.getConfigurationSource().generateBeanName(bd));
 	}

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

@@ -34,6 +34,7 @@ import org.springframework.util.StringUtils;
  *
  * @author Oliver Gierke
  * @author Jens Schauder
+ * @author Mark Paluch
  */
 @RequiredArgsConstructor
 public class DefaultRepositoryConfiguration<T extends RepositoryConfigurationSource>
@@ -71,6 +72,18 @@ public class DefaultRepositoryConfiguration<T extends RepositoryConfigurationSou
 		return configurationSource.getBasePackages();
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.repository.config.RepositoryConfiguration#getBasePackages(String)
+	 */
+	@Override
+	public Streamable<String> getImplementationBasePackages(String interfaceClassName) {
+
+		return configurationSource.shouldLimitRepositoryImplementationBasePackages()
+				? Streamable.of(ClassUtils.getPackageName(interfaceClassName))
+				: getBasePackages();
+	}
+
 	/* 
 	 * (non-Javadoc)
 	 * @see org.springframework.data.repository.config.RepositoryConfiguration#getRepositoryInterface()

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

@@ -202,7 +202,8 @@ class RepositoryBeanDefinitionBuilder {
 				.concat(configuration.getConfigurationSource().getRepositoryImplementationPostfix().orElse("Impl"));
 
 		Optional<AbstractBeanDefinition> beanDefinition = implementationDetector.detectCustomImplementation(className, null,
-				configuration.getBasePackages(), exclusions, bd -> configuration.getConfigurationSource().generateBeanName(bd));
+				configuration.getImplementationBasePackages(fragmentInterfaceName), exclusions,
+				bd -> configuration.getConfigurationSource().generateBeanName(bd));
 
 		return beanDefinition.map(bd -> new RepositoryFragmentConfiguration(fragmentInterfaceName, bd));
 	}

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

@@ -37,6 +37,15 @@ public interface RepositoryConfiguration<T extends RepositoryConfigurationSource
 	 */
 	Streamable<String> getBasePackages();
 
+	/**
+	 * Returns the base packages to scan for repository implementations.
+	 *
+	 * @param interfaceClassName class name of the interface.
+	 * @return
+	 * @since 2.0
+	 */
+	Streamable<String> getImplementationBasePackages(String interfaceClassName);
+
 	/**
 	 * Returns the interface name of the repository.
 	 *

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

@@ -31,6 +31,7 @@ import org.springframework.lang.Nullable;
  * @author Thomas Darimont
  * @author Peter Rietzler
  * @author Jens Schauder
+ * @author Mark Paluch
  */
 public interface RepositoryConfigurationSource {
 
@@ -62,6 +63,17 @@ public interface RepositoryConfigurationSource {
 	 */
 	Optional<String> getRepositoryImplementationPostfix();
 
+	/**
+	 * Returns whether to limit repository implementation base packages for custom implementation scanning. If
+	 * {@literal true}, then custom implementation scanning considers only the package of the repository/fragment
+	 * interface and its subpackages for a scan. Otherwise, all {@link #getBasePackages()} are scanned for repository
+	 * implementations
+	 * 
+	 * @return {@literal true} if base packages are limited to the actual repository package.
+	 * @since 2.0
+	 */
+	boolean shouldLimitRepositoryImplementationBasePackages();
+
 	/**
 	 * @return
 	 */

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

@@ -117,4 +117,13 @@ public abstract class RepositoryConfigurationSourceSupport implements Repository
 	public boolean shouldConsiderNestedRepositories() {
 		return false;
 	}
+
+	/* 
+	 * (non-Javadoc)
+	 * @see org.springframework.data.repository.config.RepositoryConfigurationSource#isLimitRepositoryImplementationBasePackages()
+	 */
+	@Override
+	public boolean shouldLimitRepositoryImplementationBasePackages() {
+		return true;
+	}
 }

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

@@ -73,8 +73,9 @@ public class AnnotationRepositoryConfigurationSourceUnitTests {
 
 		Streamable<BeanDefinition> candidates = source.getCandidates(new DefaultResourceLoader());
 
-		assertThat(candidates).hasSize(2).extracting("beanClassName").containsOnly(MyRepository.class.getName(),
-				ComposedRepository.class.getName());
+		assertThat(candidates).extracting("beanClassName")
+				.contains(MyRepository.class.getName(), ComposedRepository.class.getName())
+				.doesNotContain(MyOtherRepository.class.getName(), ExcludedRepository.class.getName());
 	}
 
 	@Test // DATACMNS-47
@@ -102,6 +103,14 @@ public class AnnotationRepositoryConfigurationSourceUnitTests {
 		assertThat(source.shouldConsiderNestedRepositories()).isTrue();
 	}
 
+	@Test // DATACMNS-1172
+	public void returnsLimitImplementationBasePackages() {
+
+		assertThat(getConfigSource(DefaultConfiguration.class).shouldLimitRepositoryImplementationBasePackages()).isTrue();
+		assertThat(getConfigSource(DefaultConfigurationWithoutBasePackageLimit.class)
+				.shouldLimitRepositoryImplementationBasePackages()).isFalse();
+	}
+
 	@Test // DATACMNS-456
 	public void findsStringAttributeByName() {
 
@@ -155,6 +164,9 @@ public class AnnotationRepositoryConfigurationSourceUnitTests {
 	@EnableRepositories(considerNestedRepositories = true)
 	static class DefaultConfigurationWithNestedRepositories {}
 
+	@EnableRepositories(limitImplementationBasePackages = false)
+	static class DefaultConfigurationWithoutBasePackageLimit {}
+
 	@EnableRepositories(excludeFilters = { @Filter(Primary.class) })
 	static class ConfigurationWithExplicitFilter {}
 

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

@@ -34,19 +34,20 @@ import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.config.ConstructorArgumentValues;
 import org.springframework.beans.factory.support.RootBeanDefinition;
 import org.springframework.data.repository.query.QueryLookupStrategy.Key;
+import org.springframework.data.util.Streamable;
 
 /**
  * Unit tests for {@link DefaultRepositoryConfiguration}.
  * 
  * @author Oliver Gierke
  * @author Jens Schauder
+ * @author Mark Paluch
  */
 @RunWith(MockitoJUnitRunner.class)
 public class DefaultRepositoryConfigurationUnitTests {
 
 	@Mock RepositoryConfigurationSource source;
 
-	BeanDefinition definition = new RootBeanDefinition("com.acme.MyRepository");
 	RepositoryConfigurationExtension extension = new SimplerRepositoryConfigurationExtension("factory", "module");
 
 	@Before
@@ -83,6 +84,33 @@ public class DefaultRepositoryConfigurationUnitTests {
 		assertThat(getConfiguration(source).getRepositoryFactoryBeanClassName()).isEqualTo("custom");
 	}
 
+	@Test // DATACMNS-1172
+	public void limitsImplementationBasePackages() {
+
+		when(source.shouldLimitRepositoryImplementationBasePackages()).thenReturn(true);
+
+		assertThat(getConfiguration(source).getImplementationBasePackages("com.acme.MyRepository"))
+				.containsOnly("com.acme");
+	}
+
+	@Test // DATACMNS-1172
+	public void limitsImplementationBasePackagesOfNestedClass() {
+
+		when(source.shouldLimitRepositoryImplementationBasePackages()).thenReturn(true);
+
+		assertThat(getConfiguration(source).getImplementationBasePackages(NestedInterface.class.getName()))
+				.containsOnly("org.springframework.data.repository.config");
+	}
+
+	@Test // DATACMNS-1172
+	public void shouldNotLimitImplementationBasePackages() {
+
+		when(source.getBasePackages()).thenReturn(Streamable.of("com", "org.coyote"));
+
+		assertThat(getConfiguration(source).getImplementationBasePackages("com.acme.MyRepository")).contains("com",
+				"org.coyote");
+	}
+
 	private DefaultRepositoryConfiguration<RepositoryConfigurationSource> getConfiguration(
 			RepositoryConfigurationSource source) {
 		RootBeanDefinition beanDefinition = createBeanDefinition();
@@ -105,4 +133,6 @@ public class DefaultRepositoryConfigurationUnitTests {
 
 		return beanDefinition;
 	}
+
+	private interface NestedInterface {}
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012 the original author or authors.
+ * Copyright 2012-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -49,4 +49,6 @@ public @interface EnableRepositories {
 	String repositoryImplementationPostfix() default "Impl";
 
 	boolean considerNestedRepositories() default false;
+
+	boolean limitImplementationBasePackages() default true;
 }

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

@@ -34,6 +34,7 @@ import org.springframework.core.env.StandardEnvironment;
 import org.springframework.core.io.DefaultResourceLoader;
 import org.springframework.core.type.AnnotationMetadata;
 import org.springframework.core.type.StandardAnnotationMetadata;
+import org.springframework.data.repository.config.basepackage.FragmentImpl;
 import org.springframework.data.repository.core.support.DummyRepositoryFactoryBean;
 
 /**
@@ -84,6 +85,28 @@ public class RepositoryBeanDefinitionRegistrarSupportUnitTests {
 		assertNoBeanDefinitionRegisteredFor("excludedRepositoryImpl");
 	}
 
+	@Test // DATACMNS-1172
+	public void shouldLimitImplementationBasePackages() {
+
+		AnnotationMetadata metadata = new StandardAnnotationMetadata(LimitsImplementationBasePackages.class, true);
+
+		registrar.registerBeanDefinitions(metadata, registry);
+
+		assertBeanDefinitionRegisteredFor("personRepository");
+		assertNoBeanDefinitionRegisteredFor("fragmentImpl");
+	}
+
+	@Test // DATACMNS-1172
+	public void shouldNotLimitImplementationBasePackages() {
+
+		AnnotationMetadata metadata = new StandardAnnotationMetadata(UnlimitedImplementationBasePackages.class, true);
+
+		registrar.registerBeanDefinitions(metadata, registry);
+
+		assertBeanDefinitionRegisteredFor("personRepository");
+		assertBeanDefinitionRegisteredFor("fragmentImpl");
+	}
+
 	@Test // DATACMNS-360
 	public void registeredProfileRepositoriesIfProfileActivated() {
 
@@ -152,7 +175,11 @@ public class RepositoryBeanDefinitionRegistrarSupportUnitTests {
 	@EnableRepositories(
 			includeFilters = @Filter(type = FilterType.ASSIGNABLE_TYPE, value = RepositoryWithFragmentExclusion.class),
 			basePackageClasses = RepositoryWithFragmentExclusion.class)
-	static class FragmentExclusionConfiguration {
+	static class FragmentExclusionConfiguration {}
 
-	}
+	@EnableRepositories(basePackageClasses = FragmentImpl.class)
+	static class LimitsImplementationBasePackages {}
+
+	@EnableRepositories(basePackageClasses = FragmentImpl.class, limitImplementationBasePackages = false)
+	static class UnlimitedImplementationBasePackages {}
 }

src/test/java/org/springframework/data/repository/config/basepackage/FragmentImpl.java

@@ -0,0 +1,23 @@
+/*
+ * Copyright 2017 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.repository.config.basepackage;
+
+import org.springframework.data.repository.config.basepackage.repo.Fragment;
+
+/**
+ * @author Mark Paluch
+ */
+public class FragmentImpl implements Fragment {}

src/test/java/org/springframework/data/repository/config/basepackage/repo/Fragment.java

@@ -0,0 +1,21 @@
+/*
+ * Copyright 2017 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.repository.config.basepackage.repo;
+
+/**
+ * @author Mark Paluch
+ */
+public interface Fragment {}

src/test/java/org/springframework/data/repository/config/basepackage/repo/PersonRepository.java

@@ -0,0 +1,24 @@
+/*
+ * Copyright 2017 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.repository.config.basepackage.repo;
+
+import org.springframework.data.mapping.Person;
+import org.springframework.data.repository.Repository;
+
+/**
+ * @author Mark Paluch
+ */
+public interface PersonRepository extends Repository<Person, String>, Fragment {}