From 76ea47ab160ba94367aa1eaeab44a4f23f891260 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 5 Oct 2022 14:43:06 +0200 Subject: [PATCH] Deprecate mutability of `DelegatingDataAccessStrategy`. See: #1315 Original pull request: #1324. --- .../convert/DelegatingDataAccessStrategy.java | 10 +++++ .../mybatis/MyBatisDataAccessStrategy.java | 28 ++++++------ .../DefaultDataAccessStrategyUnitTests.java | 28 ++++++------ .../SimpleJdbcRepositoryEventsUnitTests.java | 43 ++++++++++++------- 4 files changed, 65 insertions(+), 44 deletions(-) diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DelegatingDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DelegatingDataAccessStrategy.java index 6261f0d7..422f512b 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DelegatingDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DelegatingDataAccessStrategy.java @@ -43,6 +43,14 @@ public class DelegatingDataAccessStrategy implements DataAccessStrategy { private DataAccessStrategy delegate; + public DelegatingDataAccessStrategy() {} + + public DelegatingDataAccessStrategy(DataAccessStrategy delegate) { + + Assert.notNull(delegate, "DataAccessStrategy must not be null"); + this.delegate = delegate; + } + @Override public Object insert(T instance, Class domainType, Identifier identifier, IdValueSource idValueSource) { return delegate.insert(instance, domainType, identifier, idValueSource); @@ -182,7 +190,9 @@ public class DelegatingDataAccessStrategy implements DataAccessStrategy { * Must be called exactly once before calling any of the other methods. * * @param delegate Must not be {@literal null} + * @deprecated since 3.0, use {@link #DelegatingDataAccessStrategy(DataAccessStrategy)} to avoid mutable state. */ + @Deprecated(since = "3.0", forRemoval = true) public void setDelegate(DataAccessStrategy delegate) { Assert.isNull(this.delegate, "The delegate must be set exactly once"); diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java index 0df402a5..7d063cdd 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java @@ -86,16 +86,6 @@ public class MyBatisDataAccessStrategy implements DataAccessStrategy { JdbcConverter converter, NamedParameterJdbcOperations operations, SqlSession sqlSession, NamespaceStrategy namespaceStrategy, Dialect dialect) { - // 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, - dialect.getIdentifierProcessing()); - myBatisDataAccessStrategy.setNamespaceStrategy(namespaceStrategy); - - CascadingDataAccessStrategy cascadingDataAccessStrategy = new CascadingDataAccessStrategy( - asList(myBatisDataAccessStrategy, delegatingDataAccessStrategy)); SqlGeneratorSource sqlGeneratorSource = new SqlGeneratorSource(context, converter, dialect); SqlParametersFactory sqlParametersFactory = new SqlParametersFactory(context, converter, dialect); @@ -110,7 +100,17 @@ public class MyBatisDataAccessStrategy implements DataAccessStrategy { insertStrategyFactory // ); - delegatingDataAccessStrategy.setDelegate(defaultDataAccessStrategy); + // 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( + defaultDataAccessStrategy); + MyBatisDataAccessStrategy myBatisDataAccessStrategy = new MyBatisDataAccessStrategy(sqlSession, + dialect.getIdentifierProcessing()); + myBatisDataAccessStrategy.setNamespaceStrategy(namespaceStrategy); + + CascadingDataAccessStrategy cascadingDataAccessStrategy = new CascadingDataAccessStrategy( + asList(myBatisDataAccessStrategy, delegatingDataAccessStrategy)); return cascadingDataAccessStrategy; } @@ -316,17 +316,17 @@ public class MyBatisDataAccessStrategy implements DataAccessStrategy { } @Override - public Optional selectOne(Query query, Class probeType) { + public Optional findOne(Query query, Class probeType) { throw new UnsupportedOperationException("Not implemented"); } @Override - public Iterable select(Query query, Class probeType) { + public Iterable findAll(Query query, Class probeType) { throw new UnsupportedOperationException("Not implemented"); } @Override - public Iterable select(Query query, Class probeType, Pageable pageable) { + public Iterable findAll(Query query, Class probeType, Pageable pageable) { throw new UnsupportedOperationException("Not implemented"); } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategyUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategyUnitTests.java index c30e8d2b..82f4fd14 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategyUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategyUnitTests.java @@ -43,21 +43,21 @@ import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; * @author Radim Tlusty * @author Chirag Tailor */ -public class DefaultDataAccessStrategyUnitTests { +class DefaultDataAccessStrategyUnitTests { - public static final long ORIGINAL_ID = 4711L; + static final long ORIGINAL_ID = 4711L; - NamedParameterJdbcOperations namedJdbcOperations = mock(NamedParameterJdbcOperations.class); - JdbcOperations jdbcOperations = mock(JdbcOperations.class); - RelationalMappingContext context = new JdbcMappingContext(); - SqlParametersFactory sqlParametersFactory = mock(SqlParametersFactory.class); - InsertStrategyFactory insertStrategyFactory = mock(InsertStrategyFactory.class); + private NamedParameterJdbcOperations namedJdbcOperations = mock(NamedParameterJdbcOperations.class); + private JdbcOperations jdbcOperations = mock(JdbcOperations.class); + private RelationalMappingContext context = new JdbcMappingContext(); + private SqlParametersFactory sqlParametersFactory = mock(SqlParametersFactory.class); + private InsertStrategyFactory insertStrategyFactory = mock(InsertStrategyFactory.class); - JdbcConverter converter; - DefaultDataAccessStrategy accessStrategy; + private JdbcConverter converter; + private DefaultDataAccessStrategy accessStrategy; @BeforeEach - public void before() { + void before() { DelegatingDataAccessStrategy relationResolver = new DelegatingDataAccessStrategy(); Dialect dialect = HsqlDbDialect.INSTANCE; @@ -80,7 +80,7 @@ public class DefaultDataAccessStrategyUnitTests { } @Test // GH-1159 - public void insert() { + void insert() { accessStrategy.insert(new DummyEntity(ORIGINAL_ID), DummyEntity.class, Identifier.empty(), IdValueSource.PROVIDED); @@ -88,7 +88,7 @@ public class DefaultDataAccessStrategyUnitTests { } @Test // GH-1159 - public void batchInsert() { + void batchInsert() { accessStrategy.insert(singletonList(InsertSubject.describedBy(new DummyEntity(ORIGINAL_ID), Identifier.empty())), DummyEntity.class, IdValueSource.PROVIDED); @@ -97,7 +97,7 @@ public class DefaultDataAccessStrategyUnitTests { } @Test // GH-1159 - public void insertForEntityWithNoId() { + void insertForEntityWithNoId() { accessStrategy.insert(new DummyEntityWithoutIdAnnotation(ORIGINAL_ID), DummyEntityWithoutIdAnnotation.class, Identifier.empty(), IdValueSource.GENERATED); @@ -106,7 +106,7 @@ public class DefaultDataAccessStrategyUnitTests { } @Test // GH-1159 - public void batchInsertForEntityWithNoId() { + void batchInsertForEntityWithNoId() { accessStrategy.insert( singletonList(InsertSubject.describedBy(new DummyEntityWithoutIdAnnotation(ORIGINAL_ID), Identifier.empty())), diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/SimpleJdbcRepositoryEventsUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/SimpleJdbcRepositoryEventsUnitTests.java index 9939313f..12a03fdb 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/SimpleJdbcRepositoryEventsUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/SimpleJdbcRepositoryEventsUnitTests.java @@ -32,12 +32,22 @@ import java.util.List; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.stubbing.Answer; + import org.springframework.context.ApplicationEventPublisher; import org.springframework.data.annotation.Id; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; -import org.springframework.data.jdbc.core.convert.*; +import org.springframework.data.jdbc.core.convert.BasicJdbcConverter; +import org.springframework.data.jdbc.core.convert.BatchJdbcOperations; +import org.springframework.data.jdbc.core.convert.DefaultDataAccessStrategy; +import org.springframework.data.jdbc.core.convert.DefaultJdbcTypeFactory; +import org.springframework.data.jdbc.core.convert.DelegatingDataAccessStrategy; +import org.springframework.data.jdbc.core.convert.InsertStrategyFactory; +import org.springframework.data.jdbc.core.convert.JdbcConverter; +import org.springframework.data.jdbc.core.convert.JdbcCustomConversions; +import org.springframework.data.jdbc.core.convert.SqlGeneratorSource; +import org.springframework.data.jdbc.core.convert.SqlParametersFactory; import org.springframework.data.jdbc.core.mapping.JdbcMappingContext; import org.springframework.data.jdbc.repository.support.JdbcRepositoryFactory; import org.springframework.data.jdbc.repository.support.SimpleJdbcRepository; @@ -73,22 +83,23 @@ import org.springframework.lang.Nullable; * @author Myeonghyeon Lee * @author Chirag Tailor */ -public class SimpleJdbcRepositoryEventsUnitTests { +class SimpleJdbcRepositoryEventsUnitTests { private static final long generatedId = 4711L; - CollectingEventPublisher publisher = new CollectingEventPublisher(); + private CollectingEventPublisher publisher = new CollectingEventPublisher(); - DummyEntityRepository repository; - DefaultDataAccessStrategy dataAccessStrategy; + private DummyEntityRepository repository; + private DefaultDataAccessStrategy dataAccessStrategy; @BeforeEach - public void before() { + void before() { RelationalMappingContext context = new JdbcMappingContext(); NamedParameterJdbcOperations operations = createIdGeneratingOperations(); - DelegatingDataAccessStrategy delegatingDataAccessStrategy = new DelegatingDataAccessStrategy(); + Dialect dialect = HsqlDbDialect.INSTANCE; + DelegatingDataAccessStrategy delegatingDataAccessStrategy = new DelegatingDataAccessStrategy(); JdbcConverter converter = new BasicJdbcConverter(context, delegatingDataAccessStrategy, new JdbcCustomConversions(), new DefaultJdbcTypeFactory(operations.getJdbcOperations()), dialect.getIdentifierProcessing()); SqlGeneratorSource generatorSource = new SqlGeneratorSource(context, converter, dialect); @@ -109,7 +120,7 @@ public class SimpleJdbcRepositoryEventsUnitTests { @Test // DATAJDBC-99 @SuppressWarnings("rawtypes") - public void publishesEventsOnSave() { + void publishesEventsOnSave() { DummyEntity entity = new DummyEntity(23L); @@ -126,7 +137,7 @@ public class SimpleJdbcRepositoryEventsUnitTests { @Test // DATAJDBC-99 @SuppressWarnings("rawtypes") - public void publishesEventsOnSaveMany() { + void publishesEventsOnSaveMany() { DummyEntity entity1 = new DummyEntity(null); DummyEntity entity2 = new DummyEntity(23L); @@ -146,7 +157,7 @@ public class SimpleJdbcRepositoryEventsUnitTests { } @Test // DATAJDBC-99 - public void publishesEventsOnDelete() { + void publishesEventsOnDelete() { DummyEntity entity = new DummyEntity(23L); @@ -173,7 +184,7 @@ public class SimpleJdbcRepositoryEventsUnitTests { @Test // DATAJDBC-99 @SuppressWarnings("rawtypes") - public void publishesEventsOnDeleteById() { + void publishesEventsOnDeleteById() { repository.deleteById(23L); @@ -187,7 +198,7 @@ public class SimpleJdbcRepositoryEventsUnitTests { @Test // DATAJDBC-197 @SuppressWarnings("rawtypes") - public void publishesEventsOnFindAll() { + void publishesEventsOnFindAll() { DummyEntity entity1 = new DummyEntity(42L); DummyEntity entity2 = new DummyEntity(23L); @@ -206,7 +217,7 @@ public class SimpleJdbcRepositoryEventsUnitTests { @Test // DATAJDBC-197 @SuppressWarnings("rawtypes") - public void publishesEventsOnFindAllById() { + void publishesEventsOnFindAllById() { DummyEntity entity1 = new DummyEntity(42L); DummyEntity entity2 = new DummyEntity(23L); @@ -225,7 +236,7 @@ public class SimpleJdbcRepositoryEventsUnitTests { @Test // DATAJDBC-197 @SuppressWarnings("rawtypes") - public void publishesEventsOnFindById() { + void publishesEventsOnFindById() { DummyEntity entity1 = new DummyEntity(23L); @@ -242,7 +253,7 @@ public class SimpleJdbcRepositoryEventsUnitTests { @Test // DATAJDBC-101 @SuppressWarnings("rawtypes") - public void publishesEventsOnFindAllSorted() { + void publishesEventsOnFindAllSorted() { DummyEntity entity1 = new DummyEntity(42L); DummyEntity entity2 = new DummyEntity(23L); @@ -261,7 +272,7 @@ public class SimpleJdbcRepositoryEventsUnitTests { @Test // DATAJDBC-101 @SuppressWarnings("rawtypes") - public void publishesEventsOnFindAllPaged() { + void publishesEventsOnFindAllPaged() { DummyEntity entity1 = new DummyEntity(42L); DummyEntity entity2 = new DummyEntity(23L);