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

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-commons</artifactId>
-	<version>2.0.0.BUILD-SNAPSHOT</version>
+	<version>2.0.0.DATACMNS-1172-SNAPSHOT</version>
 
 	<name>Spring Data Core</name>
 

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

@@ -53,6 +53,7 @@
  * @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 boolean shouldConsiderNestedRepositories() {
 		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 @@
  * @author Christoph Strobl
  * @author Peter Rietzler
  * @author Jens Schauder
+ * @author Mark Paluch
  */
 @RequiredArgsConstructor
 public class CustomRepositoryImplementationDetector {
@@ -71,7 +72,7 @@ public Optional<AbstractBeanDefinition> detectCustomImplementation(RepositoryCon
 		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 @@
  *
  * @author Oliver Gierke
  * @author Jens Schauder
+ * @author Mark Paluch
  */
 @RequiredArgsConstructor
 public class DefaultRepositoryConfiguration<T extends RepositoryConfigurationSource>
@@ -71,6 +72,18 @@ public Streamable<String> getBasePackages() {
 		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 @@ private Optional<RepositoryFragmentConfiguration> detectRepositoryFragmentConfig
 				.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 @@
  * @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 @@ protected Iterable<TypeFilter> getIncludeFilters() {
 	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 void evaluatesExcludeFiltersCorrectly() {
 
 		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 void returnsConsiderNestedRepositories() {
 		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 @@ static class DefaultConfigurationWithBasePackage {}
 	@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.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 void prefersSourcesRepositoryFactoryBeanClass() {
 		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 @@ private static RootBeanDefinition createBeanDefinition() {
 
 		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 @@
 	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.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 void registersBeanDefinitionWithoutFragmentImplementations() {
 		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 @@ protected String getModulePrefix() {
 	@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 {}