Update SaveBatchingAggregateChange to batch Delete actions.

Original pull request #1229

Author: ctailor2

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/AggregateChangeExecutor.java

@@ -92,6 +92,8 @@ private void execute(DbAction<?> action, JdbcAggregateChangeExecutionContext exe
 				executionContext.executeUpdateRoot((DbAction.UpdateRoot<?>) action);
 			} else if (action instanceof DbAction.Delete) {
 				executionContext.executeDelete((DbAction.Delete<?>) action);
+			} else if (action instanceof DbAction.BatchDelete<?>) {
+				executionContext.executeBatchDelete((DbAction.BatchDelete<?>) action);
 			} else if (action instanceof DbAction.DeleteAll) {
 				executionContext.executeDeleteAll((DbAction.DeleteAll<?>) action);
 			} else if (action instanceof DbAction.DeleteRoot) {

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java

@@ -135,6 +135,12 @@ <T> void executeDelete(DbAction.Delete<T> delete) {
 		accessStrategy.delete(delete.getRootId(), delete.getPropertyPath());
 	}
 
+	<T> void executeBatchDelete(DbAction.BatchDelete<T> batchDelete) {
+
+		List<Object> rootIds = batchDelete.getActions().stream().map(DbAction.Delete::getRootId).toList();
+		accessStrategy.delete(rootIds, batchDelete.getBatchValue());
+	}
+
 	<T> void executeDeleteAllRoot(DbAction.DeleteAllRoot<T> deleteAllRoot) {
 
 		accessStrategy.deleteAll(deleteAllRoot.getEntityType());

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java

@@ -87,6 +87,11 @@ public void delete(Object rootId, PersistentPropertyPath<RelationalPersistentPro
 		collectVoid(das -> das.delete(rootId, propertyPath));
 	}
 
+	@Override
+	public void delete(Iterable<Object> rootIds, PersistentPropertyPath<RelationalPersistentProperty> propertyPath) {
+		collectVoid(das -> das.delete(rootIds, propertyPath));
+	}
+
 	@Override
 	public <T> void deleteAll(Class<T> domainType) {
 		collectVoid(das -> das.deleteAll(domainType));

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DataAccessStrategy.java

@@ -152,6 +152,14 @@ public interface DataAccessStrategy extends RelationResolver {
 	 */
 	void delete(Object rootId, PersistentPropertyPath<RelationalPersistentProperty> propertyPath);
 
+	/**
+	 * Deletes all entities reachable via {@literal propertyPath} from the instances identified by {@literal rootIds}.
+	 *
+	 * @param rootIds Ids of the root objects on which the {@literal propertyPath} is based. Must not be {@code null} or empty.
+	 * @param propertyPath Leading from the root object to the entities to be deleted. Must not be {@code null}.
+	 */
+	void delete(Iterable<Object> rootIds, PersistentPropertyPath<RelationalPersistentProperty> propertyPath);
+
 	/**
 	 * Deletes all entities of the given domain type.
 	 *

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java

@@ -196,6 +196,21 @@ public void delete(Object rootId, PersistentPropertyPath<RelationalPersistentPro
 		operations.update(delete, parameters);
 	}
 
+	@Override
+	public void delete(Iterable<Object> rootIds, PersistentPropertyPath<RelationalPersistentProperty> propertyPath) {
+
+		RelationalPersistentEntity<?> rootEntity = context
+				.getRequiredPersistentEntity(propertyPath.getBaseProperty().getOwner().getType());
+
+		RelationalPersistentProperty referencingProperty = propertyPath.getLeafProperty();
+		Assert.notNull(referencingProperty, "No property found matching the PropertyPath " + propertyPath);
+
+		String delete = sql(rootEntity.getType()).createDeleteInByPath(propertyPath);
+
+		SqlIdentifierParameterSource parameters = sqlParametersFactory.forQueryByIds(rootIds, rootEntity.getType());
+		operations.update(delete, parameters);
+	}
+
 	@Override
 	public <T> void deleteAll(Class<T> domainType) {
 		operations.getJdbcOperations().update(sql(domainType).createDeleteAllSql(null));

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DelegatingDataAccessStrategy.java

@@ -71,6 +71,11 @@ public void delete(Object rootId, PersistentPropertyPath<RelationalPersistentPro
 		delegate.delete(rootId, propertyPath);
 	}
 
+	@Override
+	public void delete(Iterable<Object> rootIds, PersistentPropertyPath<RelationalPersistentProperty> propertyPath) {
+		delegate.delete(rootIds, propertyPath);
+	}
+
 	@Override
 	public void delete(Object id, Class<?> domainType) {
 		delegate.delete(id, domainType);

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java

@@ -360,7 +360,8 @@ String createDeleteAllSql(@Nullable PersistentPropertyPath<RelationalPersistentP
 	}
 
 	/**
-	 * Create a {@code DELETE} query and filter by {@link PersistentPropertyPath}.
+	 * Create a {@code DELETE} query and filter by {@link PersistentPropertyPath} using {@code WHERE} with the {@code =}
+	 * operator.
 	 *
 	 * @param path must not be {@literal null}.
 	 * @return the statement as a {@link String}. Guaranteed to be not {@literal null}.
@@ -370,6 +371,18 @@ String createDeleteByPath(PersistentPropertyPath<RelationalPersistentProperty> p
 				filterColumn -> filterColumn.isEqualTo(getBindMarker(ROOT_ID_PARAMETER)));
 	}
 
+	/**
+	 * Create a {@code DELETE} query and filter by {@link PersistentPropertyPath} using {@code WHERE} with the {@code IN}
+	 * operator.
+	 *
+	 * @param path must not be {@literal null}.
+	 * @return the statement as a {@link String}. Guaranteed to be not {@literal null}.
+	 */
+	String createDeleteInByPath(PersistentPropertyPath<RelationalPersistentProperty> path) {
+		return createDeleteByPathAndCriteria(new PersistentPropertyPathExtension(mappingContext, path),
+				filterColumn -> filterColumn.in(getBindMarker(IDS_SQL_PARAMETER)));
+	}
+
 	private String createFindOneSql() {
 
 		Select select = selectBuilder().where(getIdColumn().isEqualTo(getBindMarker(ID_SQL_PARAMETER))) //

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java

@@ -218,6 +218,11 @@ public void delete(Object rootId, PersistentPropertyPath<RelationalPersistentPro
 		sqlSession().delete(statement, parameter);
 	}
 
+	@Override
+	public void delete(Iterable<Object> rootIds, PersistentPropertyPath<RelationalPersistentProperty> propertyPath) {
+		rootIds.forEach(rootId -> delete(rootId, propertyPath));
+	}
+
 	@Override
 	public <T> void deleteAll(Class<T> domainType) {
 

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java

@@ -147,6 +147,14 @@ void cascadingDeleteFirstLevel() {
 		assertThat(sql).isEqualTo("DELETE FROM referenced_entity WHERE referenced_entity.dummy_entity = :rootId");
 	}
 
+	@Test // GH-537
+	void cascadingDeleteInByPathFirstLevel() {
+
+		String sql = sqlGenerator.createDeleteInByPath(getPath("ref", DummyEntity.class));
+
+		assertThat(sql).isEqualTo("DELETE FROM referenced_entity WHERE referenced_entity.dummy_entity IN (:ids)");
+	}
+
 	@Test // DATAJDBC-112
 	void cascadingDeleteByPathSecondLevel() {
 
@@ -156,6 +164,15 @@ void cascadingDeleteByPathSecondLevel() {
 				"DELETE FROM second_level_referenced_entity WHERE second_level_referenced_entity.referenced_entity IN (SELECT referenced_entity.x_l1id FROM referenced_entity WHERE referenced_entity.dummy_entity = :rootId)");
 	}
 
+	@Test // GH-537
+	void cascadingDeleteInByPathSecondLevel() {
+
+		String sql = sqlGenerator.createDeleteInByPath(getPath("ref.further", DummyEntity.class));
+
+		assertThat(sql).isEqualTo(
+				"DELETE FROM second_level_referenced_entity WHERE second_level_referenced_entity.referenced_entity IN (SELECT referenced_entity.x_l1id FROM referenced_entity WHERE referenced_entity.dummy_entity IN (:ids))");
+	}
+
 	@Test // DATAJDBC-112
 	void deleteAll() {
 

spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java

@@ -422,6 +422,18 @@ public BatchInsertRoot(List<InsertRoot<T>> actions) {
 		}
 	}
 
+	/**
+	 * Represents a batch delete statement for multiple entities that are reachable via a given path from the aggregate root.
+	 *
+	 * @param <T> type of the entity for which this represents a database interaction.
+	 * @since 3.0
+	 */
+	final class BatchDelete<T> extends BatchWithValue<T, Delete<T>, PersistentPropertyPath<RelationalPersistentProperty>> {
+		public BatchDelete(List<Delete<T>> actions) {
+			super(actions, Delete::getPropertyPath);
+		}
+	}
+
 	/**
 	 * An action depending on another action for providing additional information like the id of a parent entity.
 	 *

spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/SaveBatchingAggregateChange.java

@@ -31,8 +31,8 @@
 /**
  * A {@link BatchingAggregateChange} implementation for save changes that can contain actions for any mix of insert and
  * update operations. When consumed, actions are yielded in the appropriate entity tree order with inserts carried out
- * from root to leaves and deletes in reverse. All insert operations are grouped into batches to offer the ability for
- * an optimized batch operation to be used.
+ * from root to leaves and deletes in reverse. All operations that can be batched are grouped and combined to offer the
+ * ability for an optimized batch operation to be used.
  *
  * @author Chirag Tailor
  * @since 3.0
@@ -51,7 +51,7 @@ public class SaveBatchingAggregateChange<T> implements BatchingAggregateChange<T
 	private final List<DbAction.InsertRoot<T>> insertRootBatchCandidates = new ArrayList<>();
 	private final Map<PersistentPropertyPath<RelationalPersistentProperty>, Map<IdValueSource, List<DbAction.Insert<Object>>>> insertActions = //
 			new HashMap<>();
-	private final Map<PersistentPropertyPath<RelationalPersistentProperty>, List<DbAction.Delete<?>>> deleteActions = //
+	private final Map<PersistentPropertyPath<RelationalPersistentProperty>, List<DbAction.Delete<Object>>> deleteActions = //
 			new HashMap<>();
 
 	SaveBatchingAggregateChange(Class<T> entityType) {
@@ -80,9 +80,17 @@ public void forEachAction(Consumer<? super DbAction<?>> consumer) {
 			insertRootBatchCandidates.forEach(consumer);
 		}
 		deleteActions.entrySet().stream().sorted(Map.Entry.comparingByKey(pathLengthComparator.reversed()))
-				.forEach((entry) -> entry.getValue().forEach(consumer));
-		insertActions.entrySet().stream().sorted(Map.Entry.comparingByKey(pathLengthComparator)).forEach((entry) -> entry
-				.getValue().forEach((idValueSource, inserts) -> consumer.accept(new DbAction.BatchInsert<>(inserts))));
+				.forEach((entry) -> {
+					List<DbAction.Delete<Object>> deletes = entry.getValue();
+					if (deletes.size() > 1) {
+						consumer.accept(new DbAction.BatchDelete<>(deletes));
+					} else {
+						deletes.forEach(consumer);
+					}
+				});
+		insertActions.entrySet().stream().sorted(Map.Entry.comparingByKey(pathLengthComparator))
+				.forEach((entry) -> entry.getValue().forEach((idValueSource, inserts) ->
+						consumer.accept(new DbAction.BatchInsert<>(inserts))));
 	}
 
 	@Override
@@ -107,7 +115,8 @@ public void add(RootAggregateChange<T> aggregateChange) {
 				// noinspection unchecked
 				addInsert((DbAction.Insert<Object>) insertAction);
 			} else if (action instanceof DbAction.Delete<?> deleteAction) {
-				addDelete(deleteAction);
+				// noinspection unchecked
+				addDelete((DbAction.Delete<Object>) deleteAction);
 			}
 		});
 	}
@@ -140,7 +149,7 @@ private void addInsert(DbAction.Insert<Object> action) {
 				});
 	}
 
-	private void addDelete(DbAction.Delete<?> action) {
+	private void addDelete(DbAction.Delete<Object> action) {
 
 		PersistentPropertyPath<RelationalPersistentProperty> propertyPath = action.getPropertyPath();
 		deleteActions.merge(propertyPath, new ArrayList<>(singletonList(action)), (actions, defaultValue) -> {

spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/SaveBatchingAggregateChangeTest.java

@@ -277,7 +277,7 @@ void yieldsNestedDeleteActionsInTreeOrderFromLeavesToRoot() {
 		Root root1 = new Root(1L, null);
 		RootAggregateChange<Root> aggregateChange1 = MutableAggregateChange.forSave(root1);
 		aggregateChange1.setRootAction(new DbAction.UpdateRoot<>(root1, null));
-		DbAction.Delete<?> root1IntermediateDelete = new DbAction.Delete<>(1L,
+		DbAction.Delete<Intermediate> root1IntermediateDelete = new DbAction.Delete<>(1L,
 				context.getPersistentPropertyPath("intermediate", Root.class));
 		aggregateChange1.addAction(root1IntermediateDelete);
 
@@ -289,16 +289,46 @@ void yieldsNestedDeleteActionsInTreeOrderFromLeavesToRoot() {
 				context.getPersistentPropertyPath("intermediate.leaf", Root.class));
 		aggregateChange2.addAction(root2LeafDelete);
 
-		DbAction.Delete<?> root2IntermediateDelete = new DbAction.Delete<>(1L,
+		DbAction.Delete<Intermediate> root2IntermediateDelete = new DbAction.Delete<>(1L,
 				context.getPersistentPropertyPath("intermediate", Root.class));
 		aggregateChange2.addAction(root2IntermediateDelete);
 
 		BatchingAggregateChange<Root, RootAggregateChange<Root>> change = BatchingAggregateChange.forSave(Root.class);
 		change.add(aggregateChange1);
 		change.add(aggregateChange2);
 
-		assertThat(extractActions(change)).containsSubsequence(root2LeafDelete, root1IntermediateDelete,
-				root2IntermediateDelete);
+		List<DbAction<?>> actions = extractActions(change);
+		assertThat(actions).extracting(DbAction::getClass, DbAction::getEntityType).containsSubsequence(
+				Tuple.tuple(DbAction.Delete.class, Leaf.class), //
+				Tuple.tuple(DbAction.BatchDelete.class, Intermediate.class));
+		assertThat(getBatchWithValueAction(actions, Intermediate.class, DbAction.BatchDelete.class).getActions())
+				.containsExactly(root1IntermediateDelete, root2IntermediateDelete);
+	}
+
+	@Test
+	void yieldsDeleteActionsAsBatchDeletes_groupedByPath_whenGroupContainsMultipleDeletes() {
+
+		Root root = new Root(1L, null);
+		RootAggregateChange<Root> aggregateChange = MutableAggregateChange.forSave(root);
+		DbAction.UpdateRoot<Root> updateRoot = new DbAction.UpdateRoot<>(root, null);
+		aggregateChange.setRootAction(updateRoot);
+		DbAction.Delete<Intermediate> intermediateDelete1 = new DbAction.Delete<>(1L,
+				context.getPersistentPropertyPath("intermediate", Root.class));
+		DbAction.Delete<Intermediate> intermediateDelete2 = new DbAction.Delete<>(2L,
+				context.getPersistentPropertyPath("intermediate", Root.class));
+		aggregateChange.addAction(intermediateDelete1);
+		aggregateChange.addAction(intermediateDelete2);
+
+		BatchingAggregateChange<Root, RootAggregateChange<Root>> change = BatchingAggregateChange.forSave(Root.class);
+		change.add(aggregateChange);
+
+		List<DbAction<?>> actions = extractActions(change);
+		assertThat(actions).extracting(DbAction::getClass, DbAction::getEntityType) //
+				.containsExactly( //
+						Tuple.tuple(DbAction.UpdateRoot.class, Root.class), //
+						Tuple.tuple(DbAction.BatchDelete.class, Intermediate.class));
+		assertThat(getBatchWithValueAction(actions, Intermediate.class, DbAction.BatchDelete.class).getActions())
+				.containsExactly(intermediateDelete1, intermediateDelete2);
 	}
 
 	@Test