From 27a8a8df91b72c9e842d8024d6e370648d4bc4fd Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 8 Aug 2012 18:58:01 +0200 Subject: [PATCH] DATACMNS-47 - Guard against @EnableRepositories annotations not carrying @Inherited. If an @EnableRepository annotation does not carry an @Inherited annotation and the annotation is used in a JavaConfig inheritance scenario the ImportBeanDefinitionRegistrar gets invoked without the annotation metadata available. The RepositoryBeanDefinitionRegistrarSupport now guards against that case and will not invoke registration of repository bean definitions. It's strongly recommended to equip @EnableRepository annotations with @Inherited to accommodate this scenario. --- ...ositoryBeanDefinitionRegistrarSupport.java | 5 ++ .../repository/config/EnableRepositories.java | 9 +- ...itionRegistrarSupportIntegrationTests.java | 65 ++++---------- ...anDefinitionRegistrarSupportUnitTests.java | 84 ++++++++++++++++++ .../core/support/DummyRepositoryFactory.java | 86 +++++++++++++++++++ .../support/DummyRepositoryFactoryBean.java | 40 +++++++++ .../RepositoryFactorySupportUnitTests.java | 53 ++---------- 7 files changed, 244 insertions(+), 98 deletions(-) create mode 100644 spring-data-commons-core/src/test/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupportUnitTests.java create mode 100644 spring-data-commons-core/src/test/java/org/springframework/data/repository/core/support/DummyRepositoryFactory.java create mode 100644 spring-data-commons-core/src/test/java/org/springframework/data/repository/core/support/DummyRepositoryFactoryBean.java diff --git a/spring-data-commons-core/src/main/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupport.java b/spring-data-commons-core/src/main/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupport.java index faa7a7536..82a102ac9 100644 --- a/spring-data-commons-core/src/main/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupport.java +++ b/spring-data-commons-core/src/main/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupport.java @@ -42,6 +42,11 @@ public abstract class RepositoryBeanDefinitionRegistrarSupport implements Import Assert.notNull(annotationMetadata); Assert.notNull(registry); + // Guard against calls for sub-classes + if (annotationMetadata.getAnnotationAttributes(getAnnotation().getName()) == null) { + return; + } + ResourceLoader resourceLoader = new DefaultResourceLoader(); AnnotationRepositoryConfigurationSource configuration = new AnnotationRepositoryConfigurationSource( annotationMetadata, getAnnotation()); diff --git a/spring-data-commons-core/src/test/java/org/springframework/data/repository/config/EnableRepositories.java b/spring-data-commons-core/src/test/java/org/springframework/data/repository/config/EnableRepositories.java index f3a99e600..431694aee 100644 --- a/spring-data-commons-core/src/test/java/org/springframework/data/repository/config/EnableRepositories.java +++ b/spring-data-commons-core/src/test/java/org/springframework/data/repository/config/EnableRepositories.java @@ -15,13 +15,18 @@ */ package org.springframework.data.repository.config; +import java.lang.annotation.Inherited; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import org.springframework.context.annotation.ComponentScan.Filter; -import org.springframework.data.repository.core.support.RepositoryFactoryBeanSupport; +import org.springframework.context.annotation.Import; +import org.springframework.data.repository.config.RepositoryBeanDefinitionRegistrarSupportUnitTests.DummyRegistrar; +import org.springframework.data.repository.core.support.DummyRepositoryFactoryBean; @Retention(RetentionPolicy.RUNTIME) +@Import(DummyRegistrar.class) +@Inherited public @interface EnableRepositories { String[] value() default {}; @@ -34,7 +39,7 @@ public @interface EnableRepositories { Filter[] excludeFilters() default {}; - Class repositoryFactoryBeanClass() default RepositoryFactoryBeanSupport.class; + Class repositoryFactoryBeanClass() default DummyRepositoryFactoryBean.class; String namedQueriesLocation() default ""; diff --git a/spring-data-commons-core/src/test/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupportIntegrationTests.java b/spring-data-commons-core/src/test/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupportIntegrationTests.java index fb209cf89..8cf158d08 100644 --- a/spring-data-commons-core/src/test/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupportIntegrationTests.java +++ b/spring-data-commons-core/src/test/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupportIntegrationTests.java @@ -15,70 +15,35 @@ */ package org.springframework.data.repository.config; -import static org.mockito.Matchers.*; -import static org.mockito.Mockito.*; - -import java.lang.annotation.Annotation; +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; -import org.springframework.beans.factory.config.BeanDefinition; -import org.springframework.beans.factory.support.BeanDefinitionRegistry; -import org.springframework.core.type.AnnotationMetadata; -import org.springframework.core.type.StandardAnnotationMetadata; -import org.springframework.data.repository.core.support.RepositoryFactoryBeanSupport; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Configuration; /** - * Integration test for {@link RepositoryBeanDefinitionRegistrarSupport}. - * * @author Oliver Gierke */ -@RunWith(MockitoJUnitRunner.class) public class RepositoryBeanDefinitionRegistrarSupportIntegrationTests { - @Mock - BeanDefinitionRegistry registry; - - @Test - public void registersBeanDefinitionForFoundBean() { - - AnnotationMetadata metadata = new StandardAnnotationMetadata(SampleConfiguration.class, true); - DummyRegistrar registrar = new DummyRegistrar(); - registrar.registerBeanDefinitions(metadata, registry); + @Configuration + @EnableRepositories + static class SampleConfig { - verify(registry, times(1)).registerBeanDefinition(eq("myRepository"), any(BeanDefinition.class)); } - private static class DummyRegistrar extends RepositoryBeanDefinitionRegistrarSupport { + @Configuration + static class TestConfig extends SampleConfig { - /* - * (non-Javadoc) - * @see org.springframework.data.repository.config.RepositoryBeanDefinitionRegistrarSupport#getAnnotation() - */ - @Override - protected Class getAnnotation() { - return EnableRepositories.class; - } + } - /* - * (non-Javadoc) - * @see org.springframework.data.repository.config.RepositoryBeanDefinitionRegistrarSupport#getExtension() - */ - @Override - protected RepositoryConfigurationExtension getExtension() { - return new RepositoryConfigurationExtensionSupport() { + @Test + public void testBootstrappingWithInheritedConfigClasses() { - public String getRepositoryFactoryClassName() { - return RepositoryFactoryBeanSupport.class.getName(); - } + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(TestConfig.class); - @Override - protected String getModulePrefix() { - return "commons"; - } - }; - } + assertThat(context.getBean(MyRepository.class), is(notNullValue())); + assertThat(context.getBean(MyOtherRepository.class), is(notNullValue())); } } diff --git a/spring-data-commons-core/src/test/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupportUnitTests.java b/spring-data-commons-core/src/test/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupportUnitTests.java new file mode 100644 index 000000000..627692eea --- /dev/null +++ b/spring-data-commons-core/src/test/java/org/springframework/data/repository/config/RepositoryBeanDefinitionRegistrarSupportUnitTests.java @@ -0,0 +1,84 @@ +/* + * Copyright 2012 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; + +import static org.mockito.Matchers.*; +import static org.mockito.Mockito.*; + +import java.lang.annotation.Annotation; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.beans.factory.config.BeanDefinition; +import org.springframework.beans.factory.support.BeanDefinitionRegistry; +import org.springframework.core.type.AnnotationMetadata; +import org.springframework.core.type.StandardAnnotationMetadata; +import org.springframework.data.repository.core.support.RepositoryFactoryBeanSupport; + +/** + * Integration test for {@link RepositoryBeanDefinitionRegistrarSupport}. + * + * @author Oliver Gierke + */ +@RunWith(MockitoJUnitRunner.class) +public class RepositoryBeanDefinitionRegistrarSupportUnitTests { + + @Mock + BeanDefinitionRegistry registry; + + @Test + public void registersBeanDefinitionForFoundBean() { + + AnnotationMetadata metadata = new StandardAnnotationMetadata(SampleConfiguration.class, true); + DummyRegistrar registrar = new DummyRegistrar(); + registrar.registerBeanDefinitions(metadata, registry); + + verify(registry, times(1)).registerBeanDefinition(eq("myRepository"), any(BeanDefinition.class)); + } + + static class DummyRegistrar extends RepositoryBeanDefinitionRegistrarSupport { + + /* + * (non-Javadoc) + * @see org.springframework.data.repository.config.RepositoryBeanDefinitionRegistrarSupport#getAnnotation() + */ + @Override + protected Class getAnnotation() { + return EnableRepositories.class; + } + + /* + * (non-Javadoc) + * @see org.springframework.data.repository.config.RepositoryBeanDefinitionRegistrarSupport#getExtension() + */ + @Override + protected RepositoryConfigurationExtension getExtension() { + return new RepositoryConfigurationExtensionSupport() { + + public String getRepositoryFactoryClassName() { + return RepositoryFactoryBeanSupport.class.getName(); + } + + @Override + protected String getModulePrefix() { + return "commons"; + } + }; + } + } +} diff --git a/spring-data-commons-core/src/test/java/org/springframework/data/repository/core/support/DummyRepositoryFactory.java b/spring-data-commons-core/src/test/java/org/springframework/data/repository/core/support/DummyRepositoryFactory.java new file mode 100644 index 000000000..7eab792c0 --- /dev/null +++ b/spring-data-commons-core/src/test/java/org/springframework/data/repository/core/support/DummyRepositoryFactory.java @@ -0,0 +1,86 @@ +/* + * Copyright 2012 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.core.support; + +import static org.mockito.Mockito.*; + +import java.io.Serializable; +import java.lang.reflect.Method; + +import org.mockito.Mockito; +import org.springframework.data.repository.core.EntityInformation; +import org.springframework.data.repository.core.NamedQueries; +import org.springframework.data.repository.core.RepositoryMetadata; +import org.springframework.data.repository.core.support.RepositoryFactorySupportUnitTests.MyRepositoryQuery; +import org.springframework.data.repository.query.QueryLookupStrategy; +import org.springframework.data.repository.query.QueryLookupStrategy.Key; +import org.springframework.data.repository.query.RepositoryQuery; + +public class DummyRepositoryFactory extends RepositoryFactorySupport { + + private final Object repository; + + public DummyRepositoryFactory(Object repository) { + this.repository = repository; + } + + /* + * (non-Javadoc) + * @see org.springframework.data.repository.core.support.RepositoryFactorySupport#getEntityInformation(java.lang.Class) + */ + @Override + @SuppressWarnings("unchecked") + public EntityInformation getEntityInformation(Class domainClass) { + + return mock(EntityInformation.class); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.repository.core.support.RepositoryFactorySupport#getTargetRepository(org.springframework.data.repository.core.RepositoryMetadata) + */ + @Override + protected Object getTargetRepository(RepositoryMetadata metadata) { + return repository; + } + + /* + * (non-Javadoc) + * @see org.springframework.data.repository.core.support.RepositoryFactorySupport#getRepositoryBaseClass(org.springframework.data.repository.core.RepositoryMetadata) + */ + @Override + protected Class getRepositoryBaseClass(RepositoryMetadata metadata) { + return repository.getClass(); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.repository.core.support.RepositoryFactorySupport#getQueryLookupStrategy(org.springframework.data.repository.query.QueryLookupStrategy.Key) + */ + @Override + protected QueryLookupStrategy getQueryLookupStrategy(Key key) { + + MyRepositoryQuery queryOne = mock(MyRepositoryQuery.class); + RepositoryQuery queryTwo = mock(RepositoryQuery.class); + + QueryLookupStrategy strategy = mock(QueryLookupStrategy.class); + when( + strategy.resolveQuery(Mockito.any(Method.class), Mockito.any(RepositoryMetadata.class), + Mockito.any(NamedQueries.class))).thenReturn(queryOne, queryTwo); + + return strategy; + } +} diff --git a/spring-data-commons-core/src/test/java/org/springframework/data/repository/core/support/DummyRepositoryFactoryBean.java b/spring-data-commons-core/src/test/java/org/springframework/data/repository/core/support/DummyRepositoryFactoryBean.java new file mode 100644 index 000000000..5201e9df2 --- /dev/null +++ b/spring-data-commons-core/src/test/java/org/springframework/data/repository/core/support/DummyRepositoryFactoryBean.java @@ -0,0 +1,40 @@ +/* + * Copyright 2012 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.core.support; + +import static org.mockito.Mockito.*; + +import java.io.Serializable; + +import org.springframework.data.repository.Repository; + +/** + * @author Oliver Gierke + */ +public class DummyRepositoryFactoryBean, S, ID extends Serializable> extends + RepositoryFactoryBeanSupport { + + /* + * (non-Javadoc) + * @see org.springframework.data.repository.core.support.RepositoryFactoryBeanSupport#createRepositoryFactory() + */ + @Override + protected RepositoryFactorySupport createRepositoryFactory() { + + Repository repository = mock(Repository.class); + return new DummyRepositoryFactory(repository); + } +} diff --git a/spring-data-commons-core/src/test/java/org/springframework/data/repository/core/support/RepositoryFactorySupportUnitTests.java b/spring-data-commons-core/src/test/java/org/springframework/data/repository/core/support/RepositoryFactorySupportUnitTests.java index ac8280566..17da693fa 100644 --- a/spring-data-commons-core/src/test/java/org/springframework/data/repository/core/support/RepositoryFactorySupportUnitTests.java +++ b/spring-data-commons-core/src/test/java/org/springframework/data/repository/core/support/RepositoryFactorySupportUnitTests.java @@ -20,9 +20,9 @@ import static org.junit.Assert.*; import static org.mockito.Mockito.*; import java.io.Serializable; -import java.lang.reflect.Method; import java.util.List; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -35,11 +35,6 @@ import org.springframework.data.domain.Sort; import org.springframework.data.repository.PagingAndSortingRepository; import org.springframework.data.repository.Repository; import org.springframework.data.repository.RepositoryDefinition; -import org.springframework.data.repository.core.EntityInformation; -import org.springframework.data.repository.core.NamedQueries; -import org.springframework.data.repository.core.RepositoryMetadata; -import org.springframework.data.repository.query.QueryLookupStrategy; -import org.springframework.data.repository.query.QueryLookupStrategy.Key; import org.springframework.data.repository.query.RepositoryQuery; /** @@ -50,7 +45,7 @@ import org.springframework.data.repository.query.RepositoryQuery; @RunWith(MockitoJUnitRunner.class) public class RepositoryFactorySupportUnitTests { - RepositoryFactorySupport factory = new DummyRepositoryFactory(); + RepositoryFactorySupport factory; @Mock PagingAndSortingRepository backingRepo; @@ -62,6 +57,11 @@ public class RepositoryFactorySupportUnitTests { @Mock PlainQueryCreationListener otherListener; + @Before + public void setUp() { + factory = new DummyRepositoryFactory(backingRepo); + } + @Test public void invokesCustomQueryCreationListenerForSpecialRepositoryQueryOnly() throws Exception { @@ -113,45 +113,6 @@ public class RepositoryFactorySupportUnitTests { assertThat(factory.getRepository(foo), is(notNullValue())); } - class DummyRepositoryFactory extends RepositoryFactorySupport { - - /* (non-Javadoc) - * @see org.springframework.data.repository.support.RepositoryFactorySupport#getEntityInformation(java.lang.Class) - */ - @Override - @SuppressWarnings("unchecked") - public EntityInformation getEntityInformation(Class domainClass) { - - return mock(EntityInformation.class); - } - - @Override - protected Object getTargetRepository(RepositoryMetadata metadata) { - - return backingRepo; - } - - @Override - protected Class getRepositoryBaseClass(RepositoryMetadata metadata) { - - return backingRepo.getClass(); - } - - @Override - protected QueryLookupStrategy getQueryLookupStrategy(Key key) { - - MyRepositoryQuery queryOne = mock(MyRepositoryQuery.class); - RepositoryQuery queryTwo = mock(RepositoryQuery.class); - - QueryLookupStrategy strategy = mock(QueryLookupStrategy.class); - when( - strategy.resolveQuery(Mockito.any(Method.class), Mockito.any(RepositoryMetadata.class), - Mockito.any(NamedQueries.class))).thenReturn(queryOne, queryTwo); - - return strategy; - } - } - interface ObjectRepository extends Repository, ObjectRepositoryCustom { Object findByClass(Class clazz);