From 20ecbcb151d56e798e9fd52bcb5e6bfcc3e958f6 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Fri, 15 Sep 2017 09:47:44 +0200 Subject: [PATCH] DATAJDBC-123 - Polishing. Incorporating review comments. Code formatting. Improved comments. Better method name. Converted initialiser to before method. --- .../data/jdbc/core/DataAccessStrategy.java | 6 ++++-- .../jdbc/core/DefaultDataAccessStrategy.java | 11 ++++------ .../support/JdbcRepositoryFactoryBean.java | 20 ++++++++++++++++++- .../MyBatisDataAccessStrategyUnitTests.java | 4 +++- .../mybatis/MyBatisHsqlIntegrationTests.java | 1 - 5 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java b/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java index 92037e9b8..61f9b90ec 100644 --- a/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java +++ b/src/main/java/org/springframework/data/jdbc/core/DataAccessStrategy.java @@ -33,7 +33,8 @@ public interface DataAccessStrategy { void delete(Object id, Class domainType); - /** Deletes all entities reachable via {@literal propertyPath} from the instance identified by {@literal rootId}. + /** + * Deletes all entities reachable via {@literal propertyPath} from the instance identified by {@literal rootId}. * * @param rootId Id of the root object on which the {@literal propertyPath} is based. * @param propertyPath Leading from the root object to the entities to be deleted. @@ -42,7 +43,8 @@ public interface DataAccessStrategy { void deleteAll(Class domainType); - /** Deletes all entities reachable via {@literal propertyPath} from any instance. + /** + * Deletes all entities reachable via {@literal propertyPath} from any instance. * * @param propertyPath Leading from the root object to the entities to be deleted. */ 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 296642135..81ebcac45 100644 --- a/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java +++ b/src/main/java/org/springframework/data/jdbc/core/DefaultDataAccessStrategy.java @@ -44,7 +44,7 @@ import org.springframework.jdbc.support.KeyHolder; import org.springframework.util.Assert; /** - * Generates and executes actual SQL statements. + * The default {@link DataAccessStrategy} is to generate SQL statements based on meta data from the entity. * * @author Jens Schauder */ @@ -70,7 +70,7 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy { } /** - * creates a {@link DefaultDataAccessStrategy} which references it self for resolution of recursive data accesses. + * 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. */ @@ -141,18 +141,15 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy { HashMap parameters = new HashMap<>(); parameters.put("rootId", rootId); operations.update(format, parameters); - } @Override public void deleteAll(Class domainType) { - operations.getJdbcOperations().update(sql(domainType).createDeleteAllSql(null)); } @Override public void deleteAll(PropertyPath propertyPath) { - operations.getJdbcOperations().update(sql(propertyPath.getOwningType().getType()).createDeleteAllSql(propertyPath)); } @@ -244,10 +241,10 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy { ID idValue = entityInformation.getId(instance); - return isIdPropertySimpleTypeAndValueZero(idValue, persistentEntity) ? null : idValue; + return isIdPropertyNullOrScalarZero(idValue, persistentEntity) ? null : idValue; } - private boolean isIdPropertySimpleTypeAndValueZero(ID idValue, JdbcPersistentEntity persistentEntity) { + private boolean isIdPropertyNullOrScalarZero(ID idValue, JdbcPersistentEntity persistentEntity) { JdbcPersistentProperty idProperty = persistentEntity.getIdProperty(); return idValue == null // 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 35b64c461..1156e8206 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 @@ -82,6 +82,24 @@ public class JdbcRepositoryFactoryBean, S, ID extend final JdbcMappingContext context = new JdbcMappingContext(findOrCreateNamingStrategy()); + return new JdbcRepositoryFactory(applicationEventPublisher, context, createDataAccessStrategy(context)); + } + + /** + *

+ * Create the {@link DataAccessStrategy}, by combining all applicable strategies into one. + *

+ *

+ * The challenge is that the {@link DefaultDataAccessStrategy} when used for reading needs a + * {@link DataAccessStrategy} for loading referenced entities (see. + * {@link DefaultDataAccessStrategy#getEntityRowMapper(Class)}. But it should use all configured + * {@link DataAccessStrategy}s for this. This creates a cyclic dependency. In order to build this the + * {@link DefaultDataAccessStrategy} gets passed in a {@link DelegatingDataAccessStrategy} which at the end gets set + * to the full {@link CascadingDataAccessStrategy}. + *

+ */ + private CascadingDataAccessStrategy createDataAccessStrategy(JdbcMappingContext context) { + DelegatingDataAccessStrategy delegatingDataAccessStrategy = new DelegatingDataAccessStrategy(); List accessStrategies = Stream.of( // @@ -95,7 +113,7 @@ public class JdbcRepositoryFactoryBean, S, ID extend CascadingDataAccessStrategy strategy = new CascadingDataAccessStrategy(accessStrategies); delegatingDataAccessStrategy.setDelegate(strategy); - return new JdbcRepositoryFactory(applicationEventPublisher, context, strategy); + return strategy; } private Optional createMyBatisDataAccessStrategy() { diff --git a/src/test/java/org/springframework/data/jdbc/core/MyBatisDataAccessStrategyUnitTests.java b/src/test/java/org/springframework/data/jdbc/core/MyBatisDataAccessStrategyUnitTests.java index 5776fd473..b169b13ee 100644 --- a/src/test/java/org/springframework/data/jdbc/core/MyBatisDataAccessStrategyUnitTests.java +++ b/src/test/java/org/springframework/data/jdbc/core/MyBatisDataAccessStrategyUnitTests.java @@ -23,6 +23,7 @@ import java.util.Collections; import org.apache.ibatis.session.SqlSession; import org.apache.ibatis.session.SqlSessionFactory; +import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.Mockito; @@ -44,7 +45,8 @@ public class MyBatisDataAccessStrategyUnitTests { MyBatisDataAccessStrategy accessStrategy = new MyBatisDataAccessStrategy(sessionFactory); - { + @Before + public void before() { doReturn(session).when(sessionFactory).openSession(); doReturn(false).when(session).selectOne(any(), any()); 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 f1b879de3..babb18f21 100644 --- a/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisHsqlIntegrationTests.java +++ b/src/test/java/org/springframework/data/jdbc/mybatis/MyBatisHsqlIntegrationTests.java @@ -50,7 +50,6 @@ public class MyBatisHsqlIntegrationTests { @org.springframework.context.annotation.Configuration @Import(TestConfiguration.class) - @EnableJdbcRepositories(considerNestedRepositories = true) static class Config {