Deprecate mutability of DelegatingDataAccessStrategy.

mp911de · schauder · commit 76ea47ab160b · 2022-10-05T15:34:11.000+02:00
See: #1315 Original pull request: #1324.
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
@@ -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 <T> Object insert(T instance, Class<T> domainType, Identifier identifier, IdValueSource idValueSource) {
 		return delegate.insert(instance, domainType, identifier, idValueSource);
@@ -182,7 +190,9 @@ public <T> long count(Query query, Class<T> domainType) {
 	 * 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
@@ -86,16 +86,6 @@ public static DataAccessStrategy createCombinedAccessStrategy(RelationalMappingC
 			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 static DataAccessStrategy createCombinedAccessStrategy(RelationalMappingC
 				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 <T> Iterable<T> findAll(Class<T> domainType, Pageable pageable) {
 	}
 
 	@Override
-	public <T> Optional<T> selectOne(Query query, Class<T> probeType) {
+	public <T> Optional<T> findOne(Query query, Class<T> probeType) {
 		throw new UnsupportedOperationException("Not implemented");
 	}
 
 	@Override
-	public <T> Iterable<T> select(Query query, Class<T> probeType) {
+	public <T> Iterable<T> findAll(Query query, Class<T> probeType) {
 		throw new UnsupportedOperationException("Not implemented");
 	}
 
 	@Override
-	public <T> Iterable<T> select(Query query, Class<T> probeType, Pageable pageable) {
+	public <T> Iterable<T> findAll(Query query, Class<T> 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
@@ -43,21 +43,21 @@
  * @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,15 +80,15 @@ public void before() {
 	}
 
 	@Test // GH-1159
-	public void insert() {
+	void insert() {
 
 		accessStrategy.insert(new DummyEntity(ORIGINAL_ID), DummyEntity.class, Identifier.empty(), IdValueSource.PROVIDED);
 
 		verify(insertStrategyFactory).insertStrategy(IdValueSource.PROVIDED, SqlIdentifier.quoted("ID"));
 	}
 
 	@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 void batchInsert() {
 	}
 
 	@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 void insertForEntityWithNoId() {
 	}
 
 	@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
@@ -32,12 +32,22 @@
 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 @@
  * @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 void before() {
 
 	@Test // DATAJDBC-99
 	@SuppressWarnings("rawtypes")
-	public void publishesEventsOnSave() {
+	void publishesEventsOnSave() {
 
 		DummyEntity entity = new DummyEntity(23L);
 
@@ -126,7 +137,7 @@ public void publishesEventsOnSave() {
 
 	@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 void publishesEventsOnSaveMany() {
 	}
 
 	@Test // DATAJDBC-99
-	public void publishesEventsOnDelete() {
+	void publishesEventsOnDelete() {
 
 		DummyEntity entity = new DummyEntity(23L);
 
@@ -173,7 +184,7 @@ private Object getEntity(RelationalEvent e) {
 
 	@Test // DATAJDBC-99
 	@SuppressWarnings("rawtypes")
-	public void publishesEventsOnDeleteById() {
+	void publishesEventsOnDeleteById() {
 
 		repository.deleteById(23L);
 
@@ -187,7 +198,7 @@ public void publishesEventsOnDeleteById() {
 
 	@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 void publishesEventsOnFindAll() {
 
 	@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 void publishesEventsOnFindAllById() {
 
 	@Test // DATAJDBC-197
 	@SuppressWarnings("rawtypes")
-	public void publishesEventsOnFindById() {
+	void publishesEventsOnFindById() {
 
 		DummyEntity entity1 = new DummyEntity(23L);
 
@@ -242,7 +253,7 @@ public void publishesEventsOnFindById() {
 
 	@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 void publishesEventsOnFindAllSorted() {
 
 	@Test // DATAJDBC-101
 	@SuppressWarnings("rawtypes")
-	public void publishesEventsOnFindAllPaged() {
+	void publishesEventsOnFindAllPaged() {
 
 		DummyEntity entity1 = new DummyEntity(42L);
 		DummyEntity entity2 = new DummyEntity(23L);
