From aaae9a5f5c9b2852e5f18989ee071b6d400cb364 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Fri, 23 Mar 2018 08:45:32 +0100 Subject: [PATCH] DATAJDBC-155 - Simplified construction of JdbcRepositoryFactory. Made the DefaultDataAccessStrategy actually the default for JdbcRepositoryFactoryBean. Therefore the injection of a strategy is optional. Simplified constructors of DefaultDataAccessStrategy. Created factory method to construct a correct DataAccessStrategy for use with MyBatis. Did some untangling in the test application context configurations. Original pull request: #54. --- .../jdbc/core/DefaultDataAccessStrategy.java | 11 ++--- .../mybatis/MyBatisDataAccessStrategy.java | 49 +++++++++++++++++-- .../repository/config/JdbcConfiguration.java | 4 +- .../support/JdbcRepositoryFactoryBean.java | 13 ++++- .../DefaultDataAccessStrategyUnitTests.java | 1 - .../mybatis/MyBatisHsqlIntegrationTests.java | 10 ++-- .../SimpleJdbcRepositoryEventsUnitTests.java | 3 +- .../data/jdbc/testing/TestConfiguration.java | 28 +++-------- 8 files changed, 75 insertions(+), 44 deletions(-) diff --git a/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java b/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java index c16b7eeb1..5f06b8f07 100644 --- a/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java +++ b/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java @@ -56,11 +56,11 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy { private final JdbcMappingContext context; private final DataAccessStrategy accessStrategy; - public DefaultDataAccessStrategy(SqlGeneratorSource sqlGeneratorSource, NamedParameterJdbcOperations operations, - JdbcMappingContext context, DataAccessStrategy accessStrategy) { + public DefaultDataAccessStrategy(SqlGeneratorSource sqlGeneratorSource, JdbcMappingContext context, + DataAccessStrategy accessStrategy) { this.sqlGeneratorSource = sqlGeneratorSource; - this.operations = operations; + this.operations = context.getTemplate(); this.context = context; this.accessStrategy = accessStrategy; } @@ -69,11 +69,10 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy { * Creates a {@link DefaultDataAccessStrategy} which references it self for resolution of recursive data accesses. * Only suitable if this is the only access strategy in use. */ - public DefaultDataAccessStrategy(SqlGeneratorSource sqlGeneratorSource, NamedParameterJdbcOperations operations, - JdbcMappingContext context) { + public DefaultDataAccessStrategy(SqlGeneratorSource sqlGeneratorSource, JdbcMappingContext context) { this.sqlGeneratorSource = sqlGeneratorSource; - this.operations = operations; + this.operations = context.getTemplate(); this.context = context; this.accessStrategy = this; } diff --git a/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java b/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java index 4bcae4edc..15770366d 100644 --- a/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java +++ b/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java @@ -15,15 +15,22 @@ */ package org.springframework.data.jdbc.mybatis; +import static java.util.Arrays.*; + +import java.util.Collections; +import java.util.Map; + import org.apache.ibatis.session.SqlSession; import org.mybatis.spring.SqlSessionTemplate; +import org.springframework.data.jdbc.core.CascadingDataAccessStrategy; import org.springframework.data.jdbc.core.DataAccessStrategy; +import org.springframework.data.jdbc.core.DefaultDataAccessStrategy; +import org.springframework.data.jdbc.core.DelegatingDataAccessStrategy; +import org.springframework.data.jdbc.core.SqlGeneratorSource; +import org.springframework.data.jdbc.mapping.model.JdbcMappingContext; import org.springframework.data.jdbc.mapping.model.JdbcPersistentProperty; import org.springframework.data.mapping.PropertyPath; -import java.util.Collections; -import java.util.Map; - /** * {@link DataAccessStrategy} implementation based on MyBatis. Each method gets mapped to a statement. The name of the * statement gets constructed as follows: The namespace is based on the class of the entity plus the suffix "Mapper". @@ -41,11 +48,43 @@ public class MyBatisDataAccessStrategy implements DataAccessStrategy { private final SqlSession sqlSession; + /** + * Create a {@link DataAccessStrategy} that first checks for queries defined by MyBatis and if it doesn't find one + * used a {@link DefaultDataAccessStrategy} + * + * @param context + * @param sqlSession + * @return + */ + public static DataAccessStrategy createCombinedAccessStrategy(JdbcMappingContext context, SqlSession sqlSession) { + + // the DefaultDataAccessStrategy needs a reference to the returned DataAccessStrategy. This creates a dependency + // cycle. In order to create it, we need something that allows to defer closing the cycle until all the elements are + // created. That is the purpose of the DelegatingAccessStrategy. + DelegatingDataAccessStrategy delegatingDataAccessStrategy = new DelegatingDataAccessStrategy(); + MyBatisDataAccessStrategy myBatisDataAccessStrategy = new MyBatisDataAccessStrategy(sqlSession); + + CascadingDataAccessStrategy cascadingDataAccessStrategy = new CascadingDataAccessStrategy( + asList(myBatisDataAccessStrategy, delegatingDataAccessStrategy)); + + DefaultDataAccessStrategy defaultDataAccessStrategy = new DefaultDataAccessStrategy( // + new SqlGeneratorSource(context), // + context, // + cascadingDataAccessStrategy); + + delegatingDataAccessStrategy.setDelegate(defaultDataAccessStrategy); + + return cascadingDataAccessStrategy; + } + /** * Constructs a {@link DataAccessStrategy} based on MyBatis. *

- * Use a {@link SqlSessionTemplate} for {@link SqlSession} or a similar implementation tying the session to the - * proper transaction. + * Use a {@link SqlSessionTemplate} for {@link SqlSession} or a similar implementation tying the session to the proper + * transaction. Note that the resulting {@link DataAccessStrategy} only handles MyBatis. It does not include the + * functionality of the {@link org.springframework.data.jdbc.core.DefaultDataAccessStrategy} which one normally still + * wants. Use {@link #createCombinedAccessStrategy(JdbcMappingContext, SqlSession)} to create such a + * {@link DataAccessStrategy}. * * @param sqlSession Must be non {@literal null}. */ diff --git a/src/main/java/org/springframework/data/jdbc/repository/config/JdbcConfiguration.java b/src/main/java/org/springframework/data/jdbc/repository/config/JdbcConfiguration.java index 2e1b6bdae..d12a7c329 100644 --- a/src/main/java/org/springframework/data/jdbc/repository/config/JdbcConfiguration.java +++ b/src/main/java/org/springframework/data/jdbc/repository/config/JdbcConfiguration.java @@ -22,18 +22,20 @@ import org.springframework.context.annotation.Configuration; import org.springframework.data.jdbc.mapping.model.ConversionCustomizer; import org.springframework.data.jdbc.mapping.model.JdbcMappingContext; import org.springframework.data.jdbc.mapping.model.NamingStrategy; +import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; /** * Beans that must be registered for Spring Data JDBC to work. * * @author Greg Turnquist + * @author Jens Schauder */ @Configuration public class JdbcConfiguration { @Bean - JdbcMappingContext jdbcMappingContext(NamedParameterJdbcTemplate template, Optional namingStrategy, + JdbcMappingContext jdbcMappingContext(NamedParameterJdbcOperations template, Optional namingStrategy, Optional conversionCustomizer) { return new JdbcMappingContext(namingStrategy.orElse(NamingStrategy.INSTANCE), template, diff --git a/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBean.java b/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBean.java index 4f53754dc..1e3e9820f 100644 --- a/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBean.java +++ b/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactoryBean.java @@ -21,6 +21,8 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.ApplicationEventPublisherAware; import org.springframework.data.jdbc.core.DataAccessStrategy; +import org.springframework.data.jdbc.core.DefaultDataAccessStrategy; +import org.springframework.data.jdbc.core.SqlGeneratorSource; import org.springframework.data.jdbc.mapping.model.JdbcMappingContext; import org.springframework.data.jdbc.repository.RowMapperMap; import org.springframework.data.repository.Repository; @@ -80,7 +82,7 @@ public class JdbcRepositoryFactoryBean, S, ID extend this.mappingContext = mappingContext; } - @Autowired + @Autowired(required = false) public void setDataAccessStrategy(DataAccessStrategy dataAccessStrategy) { this.dataAccessStrategy = dataAccessStrategy; } @@ -93,8 +95,15 @@ public class JdbcRepositoryFactoryBean, S, ID extend @Override public void afterPropertiesSet() { - Assert.notNull(this.dataAccessStrategy, "DataAccessStrategy must not be null!"); Assert.notNull(this.mappingContext, "MappingContext must not be null!"); + + if (dataAccessStrategy == null) { + + dataAccessStrategy = new DefaultDataAccessStrategy( // + new SqlGeneratorSource(mappingContext), // + mappingContext); + } + super.afterPropertiesSet(); } } diff --git a/src/test/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategyUnitTests.java b/src/test/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategyUnitTests.java index 1f4fffcd1..524cbda96 100644 --- a/src/test/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategyUnitTests.java +++ b/src/test/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategyUnitTests.java @@ -48,7 +48,6 @@ public class DefaultDataAccessStrategyUnitTests { DefaultDataAccessStrategy accessStrategy = new DefaultDataAccessStrategy( // new SqlGeneratorSource(context), // - jdbcOperations, // context // ); diff --git a/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisHsqlIntegrationTests.java b/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisHsqlIntegrationTests.java index f3e2ee331..e801ef094 100644 --- a/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisHsqlIntegrationTests.java +++ b/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisHsqlIntegrationTests.java @@ -15,7 +15,7 @@ */ package org.springframework.data.jdbc.mybatis; -import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.*; import junit.framework.AssertionFailedError; @@ -30,6 +30,8 @@ import org.mybatis.spring.SqlSessionTemplate; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Import; +import org.springframework.data.jdbc.core.DataAccessStrategy; +import org.springframework.data.jdbc.mapping.model.JdbcMappingContext; import org.springframework.data.jdbc.repository.config.EnableJdbcRepositories; import org.springframework.data.jdbc.testing.TestConfiguration; import org.springframework.data.repository.CrudRepository; @@ -39,8 +41,6 @@ import org.springframework.test.context.junit4.rules.SpringClassRule; import org.springframework.test.context.junit4.rules.SpringMethodRule; import org.springframework.transaction.annotation.Transactional; -import javax.net.ssl.SSLSocketFactory; - /** * Tests the integration with Mybatis. * @@ -83,8 +83,8 @@ public class MyBatisHsqlIntegrationTests { } @Bean - MyBatisDataAccessStrategy dataAccessStrategy(SqlSession sqlSession) { - return new MyBatisDataAccessStrategy(sqlSession); + DataAccessStrategy dataAccessStrategy(JdbcMappingContext context, SqlSession sqlSession) { + return MyBatisDataAccessStrategy.createCombinedAccessStrategy(context, sqlSession); } } diff --git a/src/test/java/org/springframework/data/jdbc/repository/SimpleJdbcRepositoryEventsUnitTests.java b/src/test/java/org/springframework/data/jdbc/repository/SimpleJdbcRepositoryEventsUnitTests.java index a5b8c5a77..a8dec2533 100644 --- a/src/test/java/org/springframework/data/jdbc/repository/SimpleJdbcRepositoryEventsUnitTests.java +++ b/src/test/java/org/springframework/data/jdbc/repository/SimpleJdbcRepositoryEventsUnitTests.java @@ -46,13 +46,12 @@ public class SimpleJdbcRepositoryEventsUnitTests { @Before public void before() { - final JdbcMappingContext context = new JdbcMappingContext(mock(NamedParameterJdbcOperations.class)); + final JdbcMappingContext context = new JdbcMappingContext(createIdGeneratingOperations()); JdbcRepositoryFactory factory = new JdbcRepositoryFactory( // publisher, // context, // new DefaultDataAccessStrategy( // new SqlGeneratorSource(context), // - createIdGeneratingOperations(), // context // ) // ); diff --git a/src/test/java/org/springframework/data/jdbc/testing/TestConfiguration.java b/src/test/java/org/springframework/data/jdbc/testing/TestConfiguration.java index 00522b330..1be321f94 100644 --- a/src/test/java/org/springframework/data/jdbc/testing/TestConfiguration.java +++ b/src/test/java/org/springframework/data/jdbc/testing/TestConfiguration.java @@ -21,14 +21,12 @@ import javax.sql.DataSource; import org.apache.ibatis.session.SqlSessionFactory; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.ComponentScan; import org.springframework.context.annotation.Configuration; import org.springframework.data.jdbc.core.DataAccessStrategy; import org.springframework.data.jdbc.core.DefaultDataAccessStrategy; -import org.springframework.data.jdbc.core.DelegatingDataAccessStrategy; import org.springframework.data.jdbc.core.SqlGeneratorSource; import org.springframework.data.jdbc.mapping.model.ConversionCustomizer; import org.springframework.data.jdbc.mapping.model.JdbcMappingContext; @@ -54,24 +52,21 @@ public class TestConfiguration { @Autowired(required = false) SqlSessionFactory sqlSessionFactory; @Bean - JdbcRepositoryFactory jdbcRepositoryFactory() { + JdbcRepositoryFactory jdbcRepositoryFactory(DataAccessStrategy dataAccessStrategy) { - NamedParameterJdbcTemplate jdbcTemplate = namedParameterJdbcTemplate(); + NamedParameterJdbcOperations jdbcTemplate = namedParameterJdbcTemplate(); final JdbcMappingContext context = new JdbcMappingContext(NamingStrategy.INSTANCE, jdbcTemplate, __ -> {}); return new JdbcRepositoryFactory( // publisher, // context, // - new DefaultDataAccessStrategy( // - new SqlGeneratorSource(context), // - jdbcTemplate, // - context) // + dataAccessStrategy // ); } @Bean - NamedParameterJdbcTemplate namedParameterJdbcTemplate() { + NamedParameterJdbcOperations namedParameterJdbcTemplate() { return new NamedParameterJdbcTemplate(dataSource); } @@ -81,19 +76,8 @@ public class TestConfiguration { } @Bean - DataAccessStrategy defaultDataAccessStrategy(JdbcMappingContext context, - @Qualifier("namedParameterJdbcTemplate") NamedParameterJdbcOperations operations) { - - DelegatingDataAccessStrategy accessStrategy = new DelegatingDataAccessStrategy(); - - accessStrategy.setDelegate(new DefaultDataAccessStrategy( // - new SqlGeneratorSource(context), // - operations, // - context, // - accessStrategy) // - ); - - return accessStrategy; + DataAccessStrategy defaultDataAccessStrategy(JdbcMappingContext context) { + return new DefaultDataAccessStrategy(new SqlGeneratorSource(context), context); } @Bean