From d8705c1cca3908284abd15d972152d9720f59fee Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Wed, 12 Dec 2018 22:27:50 +0100 Subject: [PATCH] DATACMNS-1448 - Repositories now favors primary repositories. If multiple repositories pointing to a single domain type are found we now favor the one registered as primary bean definition. This allows allows reliable disambiguation on which repository is supposed to be used for generic repository interactions. Related tickets: DATAREST-923. --- .../data/repository/support/Repositories.java | 37 +++++++++++++++++-- .../support/RepositoriesUnitTests.java | 32 ++++++++++++++++ 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/springframework/data/repository/support/Repositories.java b/src/main/java/org/springframework/data/repository/support/Repositories.java index 2dd89f6c34..3516b3d294 100644 --- a/src/main/java/org/springframework/data/repository/support/Repositories.java +++ b/src/main/java/org/springframework/data/repository/support/Repositories.java @@ -27,6 +27,8 @@ import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryUtils; import org.springframework.beans.factory.ListableBeanFactory; +import org.springframework.beans.factory.config.BeanDefinition; +import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.repository.core.EntityInformation; @@ -91,7 +93,7 @@ private void populateRepositoryFactoryInformation(ListableBeanFactory factory) { } } - @SuppressWarnings({ "rawtypes", "unchecked" }) + @SuppressWarnings("rawtypes") private synchronized void cacheRepositoryFactory(String name) { RepositoryFactoryInformation repositoryFactoryInformation = beanFactory.get().getBean(name, @@ -101,15 +103,13 @@ private synchronized void cacheRepositoryFactory(String name) { RepositoryInformation information = repositoryFactoryInformation.getRepositoryInformation(); Set> alternativeDomainTypes = information.getAlternativeDomainTypes(); - String beanName = BeanFactoryUtils.transformedBeanName(name); Set> typesToRegister = new HashSet<>(alternativeDomainTypes.size() + 1); typesToRegister.add(domainType); typesToRegister.addAll(alternativeDomainTypes); for (Class type : typesToRegister) { - this.repositoryFactoryInfos.put(type, repositoryFactoryInformation); - this.repositoryBeanNames.put(type, beanName); + cacheFirstOrPrimary(type, repositoryFactoryInformation, BeanFactoryUtils.transformedBeanName(name)); } } @@ -263,6 +263,35 @@ public Iterator> iterator() { return repositoryFactoryInfos.keySet().iterator(); } + /** + * Caches the repository information for the given domain type or overrides existing information in case the bean name + * points to a primary bean definition. + * + * @param type must not be {@literal null}. + * @param information must not be {@literal null}. + * @param name must not be {@literal null}. + */ + @SuppressWarnings({ "rawtypes", "unchecked" }) + private void cacheFirstOrPrimary(Class type, RepositoryFactoryInformation information, String name) { + + if (repositoryBeanNames.containsKey(type)) { + + Boolean presentAndPrimary = beanFactory // + .filter(ConfigurableListableBeanFactory.class::isInstance) // + .map(ConfigurableListableBeanFactory.class::cast) // + .map(it -> it.getBeanDefinition(name)) // + .map(BeanDefinition::isPrimary) // + .orElse(false); + + if (!presentAndPrimary) { + return; + } + } + + this.repositoryFactoryInfos.put(type, information); + this.repositoryBeanNames.put(type, name); + } + /** * Null-object to avoid nasty {@literal null} checks in cache lookups. * diff --git a/src/test/java/org/springframework/data/repository/support/RepositoriesUnitTests.java b/src/test/java/org/springframework/data/repository/support/RepositoriesUnitTests.java index 93a6fe6eef..4b5f2acebe 100755 --- a/src/test/java/org/springframework/data/repository/support/RepositoriesUnitTests.java +++ b/src/test/java/org/springframework/data/repository/support/RepositoriesUnitTests.java @@ -33,6 +33,7 @@ import org.springframework.beans.factory.support.AbstractBeanDefinition; import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.DefaultListableBeanFactory; +import org.springframework.context.annotation.Primary; import org.springframework.context.support.GenericApplicationContext; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.context.SampleMappingContext; @@ -158,6 +159,28 @@ public void exposesRepositoryForProxyType() { assertThat(repositories.getRepositoryFor(proxy.getClass())).isNotEmpty(); } + @Test // DATACMNS-1448 + public void keepsPrimaryRepositoryInCaseOfMultipleOnes() { + + DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(); + beanFactory.registerBeanDefinition("first", getRepositoryBeanDefinition(FirstRepository.class)); + + AbstractBeanDefinition definition = getRepositoryBeanDefinition(PrimaryRepository.class); + definition.setPrimary(true); + + beanFactory.registerBeanDefinition("primary", definition); + beanFactory.registerBeanDefinition("third", getRepositoryBeanDefinition(ThirdRepository.class)); + + context = new GenericApplicationContext(beanFactory); + context.refresh(); + + Repositories repositories = new Repositories(beanFactory); + + assertThat(repositories.getRepositoryFor(SomeEntity.class)).hasValueSatisfying(it -> { + assertThat(it).isInstanceOf(PrimaryRepository.class); + }); + } + class Person {} class Address {} @@ -244,4 +267,13 @@ interface Sample {} static class SampleEntity implements Sample {} interface SampleRepository extends Repository {} + + interface SomeEntity {} + + interface FirstRepository extends Repository {} + + @Primary + interface PrimaryRepository extends Repository {} + + interface ThirdRepository extends Repository {} }