From da0d964d2efec206a06602f78cc8dc5435eb339a Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 27 Sep 2017 09:43:45 +0200 Subject: [PATCH] DATACMNS-1172 - Limit repository custom implementation scanning to repository interface packages. Custom repository implementation scan uses 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. 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. Original pull request: #248. --- ...ustomRepositoryImplementationDetector.java | 3 +- .../DefaultRepositoryConfiguration.java | 12 ++++++ .../config/RepositoryConfiguration.java | 9 +++++ ...faultRepositoryConfigurationUnitTests.java | 39 ++++++++++++++++++- ...anDefinitionRegistrarSupportUnitTests.java | 19 ++++++++- .../config/SampleConfiguration.java | 8 +++- .../basepackage/PersonRepositoryImpl.java | 23 +++++++++++ .../basepackage/repo/PersonRepository.java | 24 ++++++++++++ .../repo/PersonRepositoryCustom.java | 21 ++++++++++ 9 files changed, 153 insertions(+), 5 deletions(-) create mode 100644 src/test/java/org/springframework/data/repository/config/basepackage/PersonRepositoryImpl.java create mode 100644 src/test/java/org/springframework/data/repository/config/basepackage/repo/PersonRepository.java create mode 100644 src/test/java/org/springframework/data/repository/config/basepackage/repo/PersonRepositoryCustom.java diff --git a/src/main/java/org/springframework/data/repository/config/CustomRepositoryImplementationDetector.java b/src/main/java/org/springframework/data/repository/config/CustomRepositoryImplementationDetector.java index dfcd4a60b..0f84c2378 100644 --- a/src/main/java/org/springframework/data/repository/config/CustomRepositoryImplementationDetector.java +++ b/src/main/java/org/springframework/data/repository/config/CustomRepositoryImplementationDetector.java @@ -39,6 +39,7 @@ import org.springframework.util.StringUtils; * @author Oliver Gierke * @author Mark Paluch * @author Peter Rietzler + * @author Mark Paluch */ public class CustomRepositoryImplementationDetector { @@ -80,7 +81,7 @@ public class CustomRepositoryImplementationDetector { // TODO 2.0: Extract into dedicated interface for custom implementation lookup configuration. return detectCustomImplementation(configuration.getImplementationClassName(), // - configuration.getBasePackages(), // + configuration.getImplementationBasePackages(configuration.getRepositoryInterface()), // configuration.getExcludeFilters()); } diff --git a/src/main/java/org/springframework/data/repository/config/DefaultRepositoryConfiguration.java b/src/main/java/org/springframework/data/repository/config/DefaultRepositoryConfiguration.java index a9a22ea30..1c87906b7 100644 --- a/src/main/java/org/springframework/data/repository/config/DefaultRepositoryConfiguration.java +++ b/src/main/java/org/springframework/data/repository/config/DefaultRepositoryConfiguration.java @@ -15,6 +15,8 @@ */ package org.springframework.data.repository.config; +import java.util.Collections; + import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.core.type.filter.TypeFilter; import org.springframework.data.repository.query.QueryLookupStrategy.Key; @@ -26,6 +28,7 @@ import org.springframework.util.StringUtils; * Default implementation of {@link RepositoryConfiguration}. * * @author Oliver Gierke + * @author Mark Paluch */ public class DefaultRepositoryConfiguration implements RepositoryConfiguration { @@ -78,6 +81,15 @@ public class DefaultRepositoryConfiguration getImplementationBasePackages(String interfaceClassName) { + return Collections.singleton(ClassUtils.getPackageName(interfaceClassName)); + } + /* * (non-Javadoc) * @see org.springframework.data.repository.config.RepositoryConfiguration#getRepositoryInterface() diff --git a/src/main/java/org/springframework/data/repository/config/RepositoryConfiguration.java b/src/main/java/org/springframework/data/repository/config/RepositoryConfiguration.java index 106f38f86..67f736141 100644 --- a/src/main/java/org/springframework/data/repository/config/RepositoryConfiguration.java +++ b/src/main/java/org/springframework/data/repository/config/RepositoryConfiguration.java @@ -33,6 +33,15 @@ public interface RepositoryConfiguration getBasePackages(); + /** + * Returns the base packages to scan for repository implementations. + * + * @param interfaceClassName class name of the interface. + * @return + * @since 1.13.8 + */ + Iterable getImplementationBasePackages(String interfaceClassName); + /** * Returns the interface name of the repository. * diff --git a/src/test/java/org/springframework/data/repository/config/DefaultRepositoryConfigurationUnitTests.java b/src/test/java/org/springframework/data/repository/config/DefaultRepositoryConfigurationUnitTests.java index db51f17ad..b234635fe 100644 --- a/src/test/java/org/springframework/data/repository/config/DefaultRepositoryConfigurationUnitTests.java +++ b/src/test/java/org/springframework/data/repository/config/DefaultRepositoryConfigurationUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2013 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. @@ -22,6 +22,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.beans.factory.config.ConstructorArgumentValues; import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.data.repository.query.QueryLookupStrategy.Key; @@ -29,6 +30,7 @@ import org.springframework.data.repository.query.QueryLookupStrategy.Key; * Unit tests for {@link DefaultRepositoryConfiguration}. * * @author Oliver Gierke + * @author Mark Paluch */ @RunWith(MockitoJUnitRunner.class) public class DefaultRepositoryConfigurationUnitTests { @@ -48,4 +50,39 @@ public class DefaultRepositoryConfigurationUnitTests { assertThat(configuration.getQueryLookupStrategyKey(), is((Object) Key.CREATE_IF_NOT_FOUND)); assertThat(configuration.isLazyInit(), is(false)); } + + @Test // DATACMNS-1172 + public void limitsImplementationBasePackages() { + + Iterable packages = getConfiguration(source).getImplementationBasePackages("com.acme.MyRepository"); + + assertThat(packages, hasItem("com.acme")); + } + + @Test // DATACMNS-1172 + public void limitsImplementationBasePackagesOfNestedClass() { + + Iterable packages = getConfiguration(source).getImplementationBasePackages(NestedInterface.class.getName()); + + assertThat(packages, hasItem("org.springframework.data.repository.config")); + } + + private DefaultRepositoryConfiguration getConfiguration( + RepositoryConfigurationSource source) { + RootBeanDefinition beanDefinition = createBeanDefinition(); + return new DefaultRepositoryConfiguration(source, beanDefinition); + } + + private static RootBeanDefinition createBeanDefinition() { + + RootBeanDefinition beanDefinition = new RootBeanDefinition("com.acme.MyRepository"); + + ConstructorArgumentValues constructorArgumentValues = new ConstructorArgumentValues(); + constructorArgumentValues.addGenericArgumentValue(MyRepository.class); + beanDefinition.setConstructorArgumentValues(constructorArgumentValues); + + return beanDefinition; + } + + private interface NestedInterface {} } diff --git a/src/test/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupportUnitTests.java b/src/test/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupportUnitTests.java index 8e39c46a3..5dc164cc0 100644 --- a/src/test/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupportUnitTests.java +++ b/src/test/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupportUnitTests.java @@ -15,7 +15,8 @@ */ package org.springframework.data.repository.config; -import static org.mockito.Matchers.*; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.*; import java.lang.annotation.Annotation; @@ -31,12 +32,14 @@ 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.PersonRepositoryImpl; import org.springframework.data.repository.core.support.RepositoryFactoryBeanSupport; /** * Integration test for {@link RepositoryBeanDefinitionRegistrarSupport}. * * @author Oliver Gierke + * @author Mark Paluch */ @RunWith(MockitoJUnitRunner.class) public class RepositoryBeanDefinitionRegistrarSupportUnitTests { @@ -66,6 +69,17 @@ public class RepositoryBeanDefinitionRegistrarSupportUnitTests { assertNoBeanDefinitionRegisteredFor("profileRepository"); } + @Test // DATACMNS-1172 + public void shouldLimitImplementationBasePackages() { + + AnnotationMetadata metadata = new StandardAnnotationMetadata(LimitsImplementationBasePackages.class, true); + + registrar.registerBeanDefinitions(metadata, registry); + + assertBeanDefinitionRegisteredFor("personRepository"); + assertNoBeanDefinitionRegisteredFor("personRepositoryImpl"); + } + /** * @see DATACMNS-360 */ @@ -133,4 +147,7 @@ public class RepositoryBeanDefinitionRegistrarSupportUnitTests { return "commons"; } } + + @EnableRepositories(basePackageClasses = PersonRepositoryImpl.class) + static class LimitsImplementationBasePackages {} } diff --git a/src/test/java/org/springframework/data/repository/config/SampleConfiguration.java b/src/test/java/org/springframework/data/repository/config/SampleConfiguration.java index 9f30af2a9..e4927e059 100644 --- a/src/test/java/org/springframework/data/repository/config/SampleConfiguration.java +++ b/src/test/java/org/springframework/data/repository/config/SampleConfiguration.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. @@ -17,8 +17,12 @@ package org.springframework.data.repository.config; import org.springframework.context.annotation.ComponentScan.Filter; import org.springframework.context.annotation.FilterType; +import org.springframework.data.repository.config.basepackage.repo.PersonRepository; -@EnableRepositories(excludeFilters = @Filter(type = FilterType.ASSIGNABLE_TYPE, value = MyOtherRepository.class), basePackageClasses = AnnotationRepositoryConfigurationSourceUnitTests.class) +@EnableRepositories( + excludeFilters = { @Filter(type = FilterType.ASSIGNABLE_TYPE, value = MyOtherRepository.class), + @Filter(type = FilterType.ASSIGNABLE_TYPE, value = PersonRepository.class) }, + basePackageClasses = AnnotationRepositoryConfigurationSourceUnitTests.class) class SampleConfiguration { } diff --git a/src/test/java/org/springframework/data/repository/config/basepackage/PersonRepositoryImpl.java b/src/test/java/org/springframework/data/repository/config/basepackage/PersonRepositoryImpl.java new file mode 100644 index 000000000..24d8d5984 --- /dev/null +++ b/src/test/java/org/springframework/data/repository/config/basepackage/PersonRepositoryImpl.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.PersonRepositoryCustom; + +/** + * @author Mark Paluch + */ +public class PersonRepositoryImpl implements PersonRepositoryCustom {} diff --git a/src/test/java/org/springframework/data/repository/config/basepackage/repo/PersonRepository.java b/src/test/java/org/springframework/data/repository/config/basepackage/repo/PersonRepository.java new file mode 100644 index 000000000..4116f805a --- /dev/null +++ b/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, PersonRepositoryCustom {} diff --git a/src/test/java/org/springframework/data/repository/config/basepackage/repo/PersonRepositoryCustom.java b/src/test/java/org/springframework/data/repository/config/basepackage/repo/PersonRepositoryCustom.java new file mode 100644 index 000000000..81c02bdc1 --- /dev/null +++ b/src/test/java/org/springframework/data/repository/config/basepackage/repo/PersonRepositoryCustom.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 PersonRepositoryCustom {}