From 857572d78bf362e586102d0c0e4bb35436e057a8 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Tue, 5 Feb 2019 13:24:03 +0100 Subject: [PATCH 1/4] DATAJDBC-293 - Prepare branch --- pom.xml | 2 +- spring-data-jdbc-distribution/pom.xml | 2 +- spring-data-jdbc/pom.xml | 4 ++-- spring-data-relational/pom.xml | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pom.xml b/pom.xml index 3d53c917f4..ad30a806ae 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-relational-parent - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-293-SNAPSHOT pom Spring Data Relational Parent diff --git a/spring-data-jdbc-distribution/pom.xml b/spring-data-jdbc-distribution/pom.xml index 1753776a73..3dfb4cbaea 100644 --- a/spring-data-jdbc-distribution/pom.xml +++ b/spring-data-jdbc-distribution/pom.xml @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-293-SNAPSHOT ../pom.xml diff --git a/spring-data-jdbc/pom.xml b/spring-data-jdbc/pom.xml index 9458230173..d5cfa60285 100644 --- a/spring-data-jdbc/pom.xml +++ b/spring-data-jdbc/pom.xml @@ -5,7 +5,7 @@ 4.0.0 spring-data-jdbc - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-293-SNAPSHOT Spring Data JDBC Spring Data module for JDBC repositories. @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-293-SNAPSHOT diff --git a/spring-data-relational/pom.xml b/spring-data-relational/pom.xml index 11e7db737c..8ff5c1279b 100644 --- a/spring-data-relational/pom.xml +++ b/spring-data-relational/pom.xml @@ -5,7 +5,7 @@ 4.0.0 spring-data-relational - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-293-SNAPSHOT Spring Data Relational Spring Data Relational support @@ -13,7 +13,7 @@ org.springframework.data spring-data-relational-parent - 1.1.0.BUILD-SNAPSHOT + 1.1.0.DATAJDBC-293-SNAPSHOT From 9224b5a0aee69f426e664bb80b21d8b2c188822f Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Tue, 5 Feb 2019 14:08:20 +0100 Subject: [PATCH 2/4] DATAJDBC-293 - DI logic now runs after Spring Boot. So far the lookup of `NamedParameterJdbcOperations` and `DataAccessStrategy` happend before Spring Boot injected its beans, resulting in failure to consider those beans. This logic is now in the `JdbcRepositoryFactoryBean` which does see the beans created by Spring Boot. --- .../config/JdbcRepositoryConfigExtension.java | 92 +---------- .../support/JdbcRepositoryFactoryBean.java | 90 ++++++++++- ...tomizingNamespaceHsqlIntegrationTests.java | 2 + .../mybatis/MyBatisHsqlIntegrationTests.java | 3 + ...nableJdbcRepositoriesIntegrationTests.java | 4 +- ...dbcRepositoryConfigExtensionUnitTests.java | 146 ------------------ 6 files changed, 95 insertions(+), 242 deletions(-) delete mode 100644 spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/config/JdbcRepositoryConfigExtensionUnitTests.java diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/JdbcRepositoryConfigExtension.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/JdbcRepositoryConfigExtension.java index f623d9f85f..762305b929 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/JdbcRepositoryConfigExtension.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/JdbcRepositoryConfigExtension.java @@ -87,103 +87,17 @@ public void registerBeansForRoot(BeanDefinitionRegistry registry, RepositoryConf } } - /* + /* * (non-Javadoc) * @see org.springframework.data.repository.config.RepositoryConfigurationExtensionSupport#postProcess(org.springframework.beans.factory.support.BeanDefinitionBuilder, org.springframework.data.repository.config.RepositoryConfigurationSource) */ @Override public void postProcess(BeanDefinitionBuilder builder, RepositoryConfigurationSource source) { - resolveReference(builder, source, "jdbcOperationsRef", "jdbcOperations", NamedParameterJdbcOperations.class, true); - resolveReference(builder, source, "dataAccessStrategyRef", "dataAccessStrategy", DataAccessStrategy.class, false); - } - - private void resolveReference(BeanDefinitionBuilder builder, RepositoryConfigurationSource source, - String attributeName, String propertyName, Class classRef, boolean required) { - - Optional beanNameRef = source.getAttribute(attributeName).filter(StringUtils::hasText); - - String beanName = beanNameRef.orElseGet(() -> determineMatchingBeanName(propertyName, classRef, required)); - - if (beanName != null) { - builder.addPropertyReference(propertyName, beanName); - } else { - Assert.isTrue(!required, - "The beanName must not be null when requested as 'required'. Please report this as a bug."); - } - - } - - @Nullable - private String determineMatchingBeanName(String propertyName, Class classRef, boolean required) { - - if (this.beanFactory == null) { - return nullOrThrowException(required, - () -> new NoSuchBeanDefinitionException(classRef, "No BeanFactory available.")); - } - - List beanNames = Arrays.asList(beanFactory.getBeanNamesForType(classRef)); - - if (beanNames.isEmpty()) { - return nullOrThrowException(required, - () -> new NoSuchBeanDefinitionException(classRef, String.format("No bean of type %s available", classRef))); - } - - if (beanNames.size() == 1) { - return beanNames.get(0); - } - - if (!(beanFactory instanceof ConfigurableListableBeanFactory)) { - - return nullOrThrowException(required, - () -> new NoSuchBeanDefinitionException(String.format( - "BeanFactory does not implement ConfigurableListableBeanFactory when trying to find bean of type %s.", - classRef))); - } - - List primaryBeanNames = getPrimaryBeanDefinitions(beanNames, (ConfigurableListableBeanFactory) beanFactory); - - if (primaryBeanNames.size() == 1) { - return primaryBeanNames.get(0); - } - - if (primaryBeanNames.size() > 1) { - throw new NoUniqueBeanDefinitionException(classRef, primaryBeanNames.size(), - "more than one 'primary' bean found among candidates: " + primaryBeanNames); - } - - for (String beanName : beanNames) { - - if (propertyName.equals(beanName) - || ObjectUtils.containsElement(beanFactory.getAliases(beanName), propertyName)) { - return beanName; - } - } - - return nullOrThrowException(required, - () -> new NoSuchBeanDefinitionException(String.format("No bean of name %s found.", propertyName))); + builder.addPropertyValue("jdbcOperationsRef", source.getAttribute("jdbcOperationsRef").orElse(null)); + builder.addPropertyValue("dataAccessStrategyRef", source.getAttribute("dataAccessStrategyRef").orElse(null)); } - private static List getPrimaryBeanDefinitions(List beanNames, - ConfigurableListableBeanFactory beanFactory) { - - ArrayList primaryBeanNames = new ArrayList<>(); - for (String name : beanNames) { - if (beanFactory.getBeanDefinition(name).isPrimary()) { - primaryBeanNames.add(name); - } - } - return primaryBeanNames; - } - - @Nullable - private static String nullOrThrowException(boolean required, Supplier exception) { - - if (required) { - throw exception.get(); - } - return null; - } } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBean.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBean.java index b8ef8f3cc1..3e0d25efc1 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBean.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBean.java @@ -16,8 +16,12 @@ package org.springframework.data.jdbc.repository.support; import java.io.Serializable; +import java.util.Map; +import org.springframework.beans.factory.BeanFactory; +import org.springframework.beans.factory.NoUniqueBeanDefinitionException; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.ApplicationEventPublisherAware; import org.springframework.data.jdbc.core.DataAccessStrategy; @@ -31,7 +35,9 @@ import org.springframework.data.repository.core.support.RepositoryFactorySupport; import org.springframework.data.repository.core.support.TransactionalRepositoryFactoryBeanSupport; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; /** * Special adapter for Springs {@link org.springframework.beans.factory.FactoryBean} interface to allow easy setup of @@ -47,11 +53,14 @@ public class JdbcRepositoryFactoryBean, S, ID extend extends TransactionalRepositoryFactoryBeanSupport implements ApplicationEventPublisherAware { private ApplicationEventPublisher publisher; + private ConfigurableListableBeanFactory beanFactory; private RelationalMappingContext mappingContext; private RelationalConverter converter; private DataAccessStrategy dataAccessStrategy; private QueryMappingConfiguration queryMappingConfiguration = QueryMappingConfiguration.EMPTY; private NamedParameterJdbcOperations operations; + private String dataAccessStrategyRef; + private String jdbcOperationsRef; /** * Creates a new {@link JdbcRepositoryFactoryBean} for the given repository interface. @@ -101,6 +110,10 @@ public void setDataAccessStrategy(DataAccessStrategy dataAccessStrategy) { this.dataAccessStrategy = dataAccessStrategy; } + public void setDataAccessStrategyRef(String dataAccessStrategyRef) { + this.dataAccessStrategyRef = dataAccessStrategyRef; + } + /** * @param queryMappingConfiguration can be {@literal null}. {@link #afterPropertiesSet()} defaults to * {@link QueryMappingConfiguration#EMPTY} if {@literal null}. @@ -126,11 +139,23 @@ public void setJdbcOperations(NamedParameterJdbcOperations operations) { this.operations = operations; } + public void setJdbcOperationsRef(String jdbcOperationsRef) { + this.jdbcOperationsRef = jdbcOperationsRef; + } + @Autowired public void setConverter(RelationalConverter converter) { this.converter = converter; } + @Override + public void setBeanFactory(BeanFactory beanFactory) { + + super.setBeanFactory(beanFactory); + + this.beanFactory = (ConfigurableListableBeanFactory) beanFactory; + } + /* * (non-Javadoc) * @see org.springframework.data.repository.core.support.RepositoryFactoryBeanSupport#afterPropertiesSet() @@ -141,12 +166,8 @@ public void afterPropertiesSet() { Assert.state(this.mappingContext != null, "MappingContext is required and must not be null!"); Assert.state(this.converter != null, "RelationalConverter is required and must not be null!"); - if (dataAccessStrategy == null) { - - SqlGeneratorSource sqlGeneratorSource = new SqlGeneratorSource(mappingContext); - this.dataAccessStrategy = new DefaultDataAccessStrategy(sqlGeneratorSource, mappingContext, converter, - operations); - } + ensureJdbcOperationsIsInitialized(); + ensureDataAccessStrategyIsInitialized(); if (queryMappingConfiguration == null) { this.queryMappingConfiguration = QueryMappingConfiguration.EMPTY; @@ -154,4 +175,61 @@ public void afterPropertiesSet() { super.afterPropertiesSet(); } + + private void ensureJdbcOperationsIsInitialized() { + + if (operations != null) { + return; + } + + operations = findBeanByNameOrType(NamedParameterJdbcOperations.class, jdbcOperationsRef); + + } + + private void ensureDataAccessStrategyIsInitialized() { + + if (dataAccessStrategy != null) { + return; + } + + dataAccessStrategy = findBeanByNameOrType(DataAccessStrategy.class, dataAccessStrategyRef); + + if (dataAccessStrategy != null) { + return; + } + + SqlGeneratorSource sqlGeneratorSource = new SqlGeneratorSource(mappingContext); + this.dataAccessStrategy = new DefaultDataAccessStrategy(sqlGeneratorSource, mappingContext, converter, + operations); + } + + @Nullable + private B findBeanByNameOrType(Class beanType, String beanName) { + + if (beanFactory == null) { + return null; + } + + if (!StringUtils.isEmpty(beanName)) { + return beanFactory.getBean(beanName, beanType); + } + + Map beansOfType = beanFactory.getBeansOfType(beanType); + + if (beansOfType.size() == 0) { + return null; + } + + if (beansOfType.size() == 1) { + return beansOfType.values().iterator().next(); + } + + for (Map.Entry entry : beansOfType.entrySet()) { + if (beanFactory.getBeanDefinition(entry.getKey()).isPrimary()) { + return entry.getValue(); + } + } + + throw new NoUniqueBeanDefinitionException(beanType, beansOfType.keySet()); + } } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisCustomizingNamespaceHsqlIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisCustomizingNamespaceHsqlIntegrationTests.java index ae0ecb83a1..72fcf593ec 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisCustomizingNamespaceHsqlIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisCustomizingNamespaceHsqlIntegrationTests.java @@ -32,6 +32,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Import; +import org.springframework.context.annotation.Primary; import org.springframework.core.io.support.PathMatchingResourcePatternResolver; import org.springframework.data.jdbc.repository.config.EnableJdbcRepositories; import org.springframework.data.jdbc.testing.TestConfiguration; @@ -105,6 +106,7 @@ SqlSessionTemplate sqlSessionTemplate(SqlSessionFactory factory) { } @Bean + @Primary MyBatisDataAccessStrategy dataAccessStrategy(SqlSession sqlSession) { MyBatisDataAccessStrategy strategy = new MyBatisDataAccessStrategy(sqlSession); diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisHsqlIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisHsqlIntegrationTests.java index ea397aa227..db310f16f6 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisHsqlIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisHsqlIntegrationTests.java @@ -30,6 +30,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Import; +import org.springframework.context.annotation.Primary; import org.springframework.data.jdbc.core.DataAccessStrategy; import org.springframework.data.jdbc.repository.config.EnableJdbcRepositories; import org.springframework.data.jdbc.testing.TestConfiguration; @@ -88,8 +89,10 @@ SqlSessionTemplate sqlSessionTemplate(SqlSessionFactory factory) { } @Bean + @Primary DataAccessStrategy dataAccessStrategy(RelationalMappingContext context, RelationalConverter converter, SqlSession sqlSession, EmbeddedDatabase db) { + return MyBatisDataAccessStrategy.createCombinedAccessStrategy(context, converter, new NamedParameterJdbcTemplate(db), sqlSession); } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/config/EnableJdbcRepositoriesIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/config/EnableJdbcRepositoriesIntegrationTests.java index 2ee86b7ff6..6e0dca18fb 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/config/EnableJdbcRepositoriesIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/config/EnableJdbcRepositoriesIntegrationTests.java @@ -79,6 +79,7 @@ public class EnableJdbcRepositoriesIntegrationTests { @BeforeClass public static void setup() { + MAPPER_MAP.setAccessible(true); OPERATIONS.setAccessible(true); DATA_ACCESS_STRATEGY.setAccessible(true); @@ -105,8 +106,9 @@ public void customRowMapperConfigurationGetsPickedUp() { @Test // DATAJDBC-293 public void jdbcOperationsRef() { + NamedParameterJdbcOperations operations = (NamedParameterJdbcOperations) ReflectionUtils.getField(OPERATIONS, factoryBean); - assertThat(operations).isNotSameAs(defaultDataAccessStrategy).isSameAs(qualifierJdbcOperations); + assertThat(operations).isNotSameAs(defaultOperations).isSameAs(qualifierJdbcOperations); DataAccessStrategy dataAccessStrategy = (DataAccessStrategy) ReflectionUtils.getField(DATA_ACCESS_STRATEGY, factoryBean); assertThat(dataAccessStrategy).isNotSameAs(defaultDataAccessStrategy).isSameAs(qualifierDataAccessStrategy); diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/config/JdbcRepositoryConfigExtensionUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/config/JdbcRepositoryConfigExtensionUnitTests.java deleted file mode 100644 index 43fec77565..0000000000 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/config/JdbcRepositoryConfigExtensionUnitTests.java +++ /dev/null @@ -1,146 +0,0 @@ -/* - * Copyright 2018-2019 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.jdbc.repository.config; - -import static org.assertj.core.api.Assertions.*; -import static org.mockito.Mockito.*; - -import org.junit.Test; -import org.springframework.beans.factory.NoSuchBeanDefinitionException; -import org.springframework.beans.factory.config.RuntimeBeanReference; -import org.springframework.beans.factory.support.BeanDefinitionBuilder; -import org.springframework.beans.factory.support.DefaultListableBeanFactory; -import org.springframework.context.support.GenericApplicationContext; -import org.springframework.data.repository.config.RepositoryConfigurationSource; -import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; - -/** - * @author Jens Schauder - */ -public class JdbcRepositoryConfigExtensionUnitTests { - - BeanDefinitionBuilder definitionBuilder = BeanDefinitionBuilder.genericBeanDefinition(); - RepositoryConfigurationSource configSource = mock(RepositoryConfigurationSource.class); - DefaultListableBeanFactory listableBeanFactory = new DefaultListableBeanFactory(); - - JdbcRepositoryConfigExtension extension = new JdbcRepositoryConfigExtension(); - - @Test // DATAJDBC-293 - public void exceptionIsThrownOnPostProcessIfNoBeanFactoryIsAvailable() { - - assertThatThrownBy( // - () -> extension.postProcess(definitionBuilder, configSource)) // - .isInstanceOf(NoSuchBeanDefinitionException.class) // - .hasMessageContaining("No BeanFactory"); - - } - - @Test // DATAJDBC-293 - public void exceptionIsThrownOnPostProcessIfNoJdbcOperationsBeanIsAvailable() { - - extension.registerBeansForRoot(listableBeanFactory, null); - - assertThatThrownBy( // - () -> extension.postProcess(definitionBuilder, configSource)) // - .isInstanceOf(NoSuchBeanDefinitionException.class) // - .hasMessageContaining("NamedParameterJdbcOperations"); // - - } - - @Test // DATAJDBC-293 - public void exceptionIsThrownOnPostProcessIfMultipleJdbcOperationsBeansAreAvailableAndNoConfigurableBeanFactoryAvailable() { - - GenericApplicationContext applicationContext = new GenericApplicationContext(); - - applicationContext.registerBean( // - "one", // - NamedParameterJdbcOperations.class, // - () -> mock(NamedParameterJdbcOperations.class)); - applicationContext.registerBean( // - "two", // - NamedParameterJdbcOperations.class, // - () -> mock(NamedParameterJdbcOperations.class)); - - applicationContext.refresh(); - - extension.registerBeansForRoot(applicationContext, null); - - assertThatThrownBy( // - () -> extension.postProcess(definitionBuilder, configSource)) // - .isInstanceOf(NoSuchBeanDefinitionException.class) // - .hasMessageContaining("NamedParameterJdbcOperations"); // - - } - - @Test // DATAJDBC-293 - public void exceptionIsThrownOnPostProcessIfMultiplePrimaryNoJdbcOperationsBeansAreAvailable() { - - registerJdbcOperations("one", true); - registerJdbcOperations("two", true); - - extension.registerBeansForRoot(listableBeanFactory, null); - - assertThatThrownBy( // - () -> extension.postProcess(definitionBuilder, configSource)) // - .isInstanceOf(NoSuchBeanDefinitionException.class) // - .hasMessageContaining("NamedParameterJdbcOperations"); // - - } - - @Test // DATAJDBC-293 - public void uniquePrimaryBeanIsUsedOfNamedParameterJdbcOperations() { - - registerJdbcOperations("one", false); - registerJdbcOperations("two", true); - - extension.registerBeansForRoot(listableBeanFactory, null); - - extension.postProcess(definitionBuilder, configSource); - - Object jdbcOperations = definitionBuilder.getBeanDefinition().getPropertyValues().get("jdbcOperations"); - - assertThat(jdbcOperations) // - .isInstanceOf(RuntimeBeanReference.class) // - .extracting(rbr -> ((RuntimeBeanReference) rbr).getBeanName()).contains("two"); - } - - @Test // DATAJDBC-293 - public void matchesByNameAsLastResort() { - - registerJdbcOperations("jdbcOperations", false); - registerJdbcOperations("two", false); - - extension.registerBeansForRoot(listableBeanFactory, null); - - extension.postProcess(definitionBuilder, configSource); - - Object jdbcOperations = definitionBuilder.getBeanDefinition().getPropertyValues().get("jdbcOperations"); - - assertThat(jdbcOperations) // - .isInstanceOf(RuntimeBeanReference.class) // - .extracting(rbr -> ((RuntimeBeanReference) rbr).getBeanName()).contains("jdbcOperations"); - - } - - private void registerJdbcOperations(String name, boolean primary) { - - listableBeanFactory.registerBeanDefinition(name, BeanDefinitionBuilder.genericBeanDefinition( // - NamedParameterJdbcOperations.class, // - () -> mock(NamedParameterJdbcOperations.class)) // - .applyCustomizers(bd -> bd.setPrimary(primary)) // - .getBeanDefinition()); - } -} From f0999679458db21306c113b5c24895389f2f14b8 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Wed, 6 Feb 2019 10:13:45 +0100 Subject: [PATCH 3/4] DATAJDBC-293 - Applying feedback from review. The logic is now split in two parts: If a bean name is given, this is configured on the BeanDefinition level in the JdbcRepositoryConfigExtension. If no name is give a bean is looked up by type in the JdbcRepositoryFactoryBean. This makes the code even simpler and uses standard Spring features instead of reimplementing them. --- .../config/JdbcRepositoryConfigExtension.java | 24 ++----- .../support/JdbcRepositoryFactoryBean.java | 64 ++----------------- .../JdbcRepositoryFactoryBeanUnitTests.java | 20 +++++- 3 files changed, 32 insertions(+), 76 deletions(-) diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/JdbcRepositoryConfigExtension.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/JdbcRepositoryConfigExtension.java index 762305b929..9024bba5eb 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/JdbcRepositoryConfigExtension.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/JdbcRepositoryConfigExtension.java @@ -15,27 +15,14 @@ */ package org.springframework.data.jdbc.repository.config; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; import java.util.Locale; -import java.util.Optional; -import java.util.function.Supplier; import org.springframework.beans.factory.ListableBeanFactory; -import org.springframework.beans.factory.NoSuchBeanDefinitionException; -import org.springframework.beans.factory.NoUniqueBeanDefinitionException; -import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.BeanDefinitionRegistry; -import org.springframework.data.jdbc.core.DataAccessStrategy; import org.springframework.data.jdbc.repository.support.JdbcRepositoryFactoryBean; import org.springframework.data.repository.config.RepositoryConfigurationExtensionSupport; import org.springframework.data.repository.config.RepositoryConfigurationSource; -import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; -import org.springframework.lang.Nullable; -import org.springframework.util.Assert; -import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; /** @@ -94,10 +81,13 @@ public void registerBeansForRoot(BeanDefinitionRegistry registry, RepositoryConf @Override public void postProcess(BeanDefinitionBuilder builder, RepositoryConfigurationSource source) { - builder.addPropertyValue("jdbcOperationsRef", source.getAttribute("jdbcOperationsRef").orElse(null)); - builder.addPropertyValue("dataAccessStrategyRef", source.getAttribute("dataAccessStrategyRef").orElse(null)); - } - + source.getAttribute("jdbcOperationsRef") // + .filter(s -> !StringUtils.isEmpty(s)) // + .ifPresent(s -> builder.addPropertyReference("jdbcOperations", s)); + source.getAttribute("dataAccessStrategyRef") // + .filter(s -> !StringUtils.isEmpty(s)) // + .ifPresent(s -> builder.addPropertyReference("dataAccessStrategy", s)); + } } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBean.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBean.java index 3e0d25efc1..ed1481b3c2 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBean.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBean.java @@ -16,12 +16,9 @@ package org.springframework.data.jdbc.repository.support; import java.io.Serializable; -import java.util.Map; import org.springframework.beans.factory.BeanFactory; -import org.springframework.beans.factory.NoUniqueBeanDefinitionException; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.ApplicationEventPublisherAware; import org.springframework.data.jdbc.core.DataAccessStrategy; @@ -35,9 +32,7 @@ import org.springframework.data.repository.core.support.RepositoryFactorySupport; import org.springframework.data.repository.core.support.TransactionalRepositoryFactoryBeanSupport; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; -import org.springframework.lang.Nullable; import org.springframework.util.Assert; -import org.springframework.util.StringUtils; /** * Special adapter for Springs {@link org.springframework.beans.factory.FactoryBean} interface to allow easy setup of @@ -53,14 +48,12 @@ public class JdbcRepositoryFactoryBean, S, ID extend extends TransactionalRepositoryFactoryBeanSupport implements ApplicationEventPublisherAware { private ApplicationEventPublisher publisher; - private ConfigurableListableBeanFactory beanFactory; + private BeanFactory beanFactory; private RelationalMappingContext mappingContext; private RelationalConverter converter; private DataAccessStrategy dataAccessStrategy; private QueryMappingConfiguration queryMappingConfiguration = QueryMappingConfiguration.EMPTY; private NamedParameterJdbcOperations operations; - private String dataAccessStrategyRef; - private String jdbcOperationsRef; /** * Creates a new {@link JdbcRepositoryFactoryBean} for the given repository interface. @@ -110,10 +103,6 @@ public void setDataAccessStrategy(DataAccessStrategy dataAccessStrategy) { this.dataAccessStrategy = dataAccessStrategy; } - public void setDataAccessStrategyRef(String dataAccessStrategyRef) { - this.dataAccessStrategyRef = dataAccessStrategyRef; - } - /** * @param queryMappingConfiguration can be {@literal null}. {@link #afterPropertiesSet()} defaults to * {@link QueryMappingConfiguration#EMPTY} if {@literal null}. @@ -126,7 +115,6 @@ public void setQueryMappingConfiguration(QueryMappingConfiguration queryMappingC /** * @param rowMapperMap can be {@literal null}. {@link #afterPropertiesSet()} defaults to {@link RowMapperMap#EMPTY} if * {@literal null}. - * * @deprecated use {@link #setQueryMappingConfiguration(QueryMappingConfiguration)} instead. */ @Deprecated @@ -139,10 +127,6 @@ public void setJdbcOperations(NamedParameterJdbcOperations operations) { this.operations = operations; } - public void setJdbcOperationsRef(String jdbcOperationsRef) { - this.jdbcOperationsRef = jdbcOperationsRef; - } - @Autowired public void setConverter(RelationalConverter converter) { this.converter = converter; @@ -153,7 +137,7 @@ public void setBeanFactory(BeanFactory beanFactory) { super.setBeanFactory(beanFactory); - this.beanFactory = (ConfigurableListableBeanFactory) beanFactory; + this.beanFactory = beanFactory; } /* @@ -182,8 +166,7 @@ private void ensureJdbcOperationsIsInitialized() { return; } - operations = findBeanByNameOrType(NamedParameterJdbcOperations.class, jdbcOperationsRef); - + operations = beanFactory.getBean(NamedParameterJdbcOperations.class); } private void ensureDataAccessStrategyIsInitialized() { @@ -192,44 +175,11 @@ private void ensureDataAccessStrategyIsInitialized() { return; } - dataAccessStrategy = findBeanByNameOrType(DataAccessStrategy.class, dataAccessStrategyRef); + dataAccessStrategy = beanFactory.getBeanProvider(DataAccessStrategy.class).getIfAvailable(() -> { - if (dataAccessStrategy != null) { - return; - } - - SqlGeneratorSource sqlGeneratorSource = new SqlGeneratorSource(mappingContext); - this.dataAccessStrategy = new DefaultDataAccessStrategy(sqlGeneratorSource, mappingContext, converter, - operations); + SqlGeneratorSource sqlGeneratorSource = new SqlGeneratorSource(mappingContext); + return new DefaultDataAccessStrategy(sqlGeneratorSource, mappingContext, converter, operations); + }); } - @Nullable - private B findBeanByNameOrType(Class beanType, String beanName) { - - if (beanFactory == null) { - return null; - } - - if (!StringUtils.isEmpty(beanName)) { - return beanFactory.getBean(beanName, beanType); - } - - Map beansOfType = beanFactory.getBeansOfType(beanType); - - if (beansOfType.size() == 0) { - return null; - } - - if (beansOfType.size() == 1) { - return beansOfType.values().iterator().next(); - } - - for (Map.Entry entry : beansOfType.entrySet()) { - if (beanFactory.getBeanDefinition(entry.getKey()).isPrimary()) { - return entry.getValue(); - } - } - - throw new NoUniqueBeanDefinitionException(beanType, beansOfType.keySet()); - } } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBeanUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBeanUnitTests.java index 736ad2a0cd..7146e29559 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBeanUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBeanUnitTests.java @@ -21,9 +21,13 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Answers; import org.mockito.Mock; +import org.mockito.invocation.InvocationOnMock; import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.stubbing.Answer; import org.springframework.beans.factory.BeanFactory; +import org.springframework.beans.factory.ListableBeanFactory; import org.springframework.context.ApplicationEventPublisher; import org.springframework.data.annotation.Id; import org.springframework.data.jdbc.core.DataAccessStrategy; @@ -33,8 +37,11 @@ import org.springframework.data.relational.core.conversion.BasicRelationalConverter; import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.repository.CrudRepository; +import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; import org.springframework.test.util.ReflectionTestUtils; +import java.util.function.Supplier; + /** * Tests the dependency injection for {@link JdbcRepositoryFactoryBean}. * @@ -44,7 +51,6 @@ * @author Oliver Gierke * @author Mark Paluch * @author Evgeni Dimitrov - * */ @RunWith(MockitoJUnitRunner.class) public class JdbcRepositoryFactoryBeanUnitTests { @@ -53,6 +59,7 @@ public class JdbcRepositoryFactoryBeanUnitTests { @Mock DataAccessStrategy dataAccessStrategy; @Mock ApplicationEventPublisher publisher; + @Mock(answer = Answers.RETURNS_DEEP_STUBS) ListableBeanFactory beanFactory; RelationalMappingContext mappingContext; @@ -63,6 +70,11 @@ public void setUp() { // Setup standard configuration factoryBean = new JdbcRepositoryFactoryBean<>(DummyEntityRepository.class); + + + when(beanFactory.getBean(NamedParameterJdbcOperations.class)).thenReturn(mock(NamedParameterJdbcOperations.class)); + when(beanFactory.getBeanProvider(DataAccessStrategy.class).getIfAvailable(any())) + .then((Answer) invocation -> ((Supplier)invocation.getArgument(0)).get()); } @Test @@ -72,6 +84,7 @@ public void setsUpBasicInstanceCorrectly() { factoryBean.setMappingContext(mappingContext); factoryBean.setConverter(new BasicRelationalConverter(mappingContext)); factoryBean.setApplicationEventPublisher(publisher); + factoryBean.setBeanFactory(beanFactory); factoryBean.afterPropertiesSet(); assertThat(factoryBean.getObject()).isNotNull(); @@ -88,6 +101,7 @@ public void afterPropertiesThowsExceptionWhenNoMappingContextSet() { factoryBean.setMappingContext(null); factoryBean.setApplicationEventPublisher(publisher); + factoryBean.setBeanFactory(beanFactory); factoryBean.afterPropertiesSet(); } @@ -97,12 +111,14 @@ public void afterPropertiesSetDefaultsNullablePropertiesCorrectly() { factoryBean.setMappingContext(mappingContext); factoryBean.setConverter(new BasicRelationalConverter(mappingContext)); factoryBean.setApplicationEventPublisher(publisher); + factoryBean.setBeanFactory(beanFactory); factoryBean.afterPropertiesSet(); assertThat(factoryBean.getObject()).isNotNull(); assertThat(ReflectionTestUtils.getField(factoryBean, "dataAccessStrategy")) .isInstanceOf(DefaultDataAccessStrategy.class); - assertThat(ReflectionTestUtils.getField(factoryBean, "queryMappingConfiguration")).isEqualTo(QueryMappingConfiguration.EMPTY); + assertThat(ReflectionTestUtils.getField(factoryBean, "queryMappingConfiguration")) + .isEqualTo(QueryMappingConfiguration.EMPTY); } private static class DummyEntity { From d91344751d35d6e0d9534f26f3c52b9e254c8773 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Wed, 6 Feb 2019 11:09:38 +0100 Subject: [PATCH 4/4] DATAJDBC-293 - Remove usage of RETURNS_DEEP_STUBS. This is broken from JDK9 on for generic types. See also: https://github.com/mockito/mockito/issues/357 --- .../support/JdbcRepositoryFactoryBeanUnitTests.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBeanUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBeanUnitTests.java index 7146e29559..8f6774f9fd 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBeanUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBeanUnitTests.java @@ -28,6 +28,7 @@ import org.mockito.stubbing.Answer; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.ListableBeanFactory; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.context.ApplicationEventPublisher; import org.springframework.data.annotation.Id; import org.springframework.data.jdbc.core.DataAccessStrategy; @@ -59,7 +60,7 @@ public class JdbcRepositoryFactoryBeanUnitTests { @Mock DataAccessStrategy dataAccessStrategy; @Mock ApplicationEventPublisher publisher; - @Mock(answer = Answers.RETURNS_DEEP_STUBS) ListableBeanFactory beanFactory; + @Mock ListableBeanFactory beanFactory; RelationalMappingContext mappingContext; @@ -73,7 +74,10 @@ public void setUp() { when(beanFactory.getBean(NamedParameterJdbcOperations.class)).thenReturn(mock(NamedParameterJdbcOperations.class)); - when(beanFactory.getBeanProvider(DataAccessStrategy.class).getIfAvailable(any())) + + ObjectProvider provider = mock(ObjectProvider.class); + when(beanFactory.getBeanProvider(DataAccessStrategy.class)).thenReturn(provider); + when(provider.getIfAvailable(any())) .then((Answer) invocation -> ((Supplier)invocation.getArgument(0)).get()); }