Add DeleteBatchingAggregateChange to batch DeleteRoot actions.

Original pull request #1231
See #537

Author: ctailor2

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

@@ -98,6 +98,8 @@ private void execute(DbAction<?> action, JdbcAggregateChangeExecutionContext exe
 				executionContext.executeDeleteAll((DbAction.DeleteAll<?>) action);
 			} else if (action instanceof DbAction.DeleteRoot) {
 				executionContext.executeDeleteRoot((DbAction.DeleteRoot<?>) action);
+			} else if (action instanceof DbAction.BatchDeleteRoot) {
+				executionContext.executeBatchDeleteRoot((DbAction.BatchDeleteRoot<?>) action);
 			} else if (action instanceof DbAction.DeleteAllRoot) {
 				executionContext.executeDeleteAllRoot((DbAction.DeleteAllRoot<?>) action);
 			} else if (action instanceof DbAction.AcquireLockRoot) {

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

@@ -131,6 +131,12 @@ <T> void executeDeleteRoot(DbAction.DeleteRoot<T> delete) {
 		}
 	}
 
+	<T> void executeBatchDeleteRoot(DbAction.BatchDeleteRoot<T> batchDelete) {
+
+		List<Object> rootIds = batchDelete.getActions().stream().map(DbAction.DeleteRoot::getId).toList();
+		accessStrategy.delete(rootIds, batchDelete.getEntityType());
+	}
+
 	<T> void executeDelete(DbAction.Delete<T> delete) {
 
 		accessStrategy.delete(delete.getRootId(), delete.getPropertyPath());

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

@@ -79,6 +79,11 @@ public void delete(Object id, Class<?> domainType) {
 		collectVoid(das -> das.delete(id, domainType));
 	}
 
+	@Override
+	public void delete(Iterable<Object> ids, Class<?> domainType) {
+		collectVoid(das -> das.delete(ids, domainType));
+	}
+
 	@Override
 	public <T> void deleteWithVersion(Object id, Class<T> domainType, Number previousVersion) {
 		collectVoid(das -> das.deleteWithVersion(id, domainType, previousVersion));

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

@@ -130,6 +130,20 @@ public interface DataAccessStrategy extends RelationResolver {
 	 */
 	void delete(Object id, Class<?> domainType);
 
+	/**
+	 * Deletes multiple rows identified by the ids, from the table identified by the domainType. Does not handle cascading
+	 * deletes.
+	 * <P>
+	 * The statement will be of the form : {@code DELETE FROM … WHERE ID IN (:ids) } and throw an optimistic record
+	 * locking exception if no rows have been updated.
+	 *
+	 * @param ids the ids of the rows to be deleted. Must not be {@code null}.
+	 * @param domainType the type of entity to be deleted. Implicitly determines the table to operate on. Must not be
+	 *          {@code null}.
+	 * @since 3.0
+	 */
+	void delete(Iterable<Object> ids, Class<?> domainType);
+
 	/**
 	 * Deletes a single entity from the database and enforce optimistic record locking using the version property. Does
 	 * not handle cascading deletes.
@@ -155,7 +169,8 @@ public interface DataAccessStrategy extends RelationResolver {
 	/**
 	 * 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 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);

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

@@ -163,6 +163,15 @@ public void delete(Object id, Class<?> domainType) {
 		operations.update(deleteByIdSql, parameter);
 	}
 
+	@Override
+	public void delete(Iterable<Object> ids, Class<?> domainType) {
+
+		String deleteByIdInSql = sql(domainType).getDeleteByIdIn();
+		SqlParameterSource parameter = sqlParametersFactory.forQueryByIds(ids, domainType);
+
+		operations.update(deleteByIdInSql, parameter);
+	}
+
 	@Override
 	public <T> void deleteWithVersion(Object id, Class<T> domainType, Number previousVersion) {
 

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

@@ -81,6 +81,11 @@ public void delete(Object id, Class<?> domainType) {
 		delegate.delete(id, domainType);
 	}
 
+	@Override
+	public void delete(Iterable<Object> ids, Class<?> domainType) {
+		delegate.delete(ids, domainType);
+	}
+
 	@Override
 	public <T> void deleteWithVersion(Object id, Class<T> domainType, Number previousVersion) {
 		delegate.deleteWithVersion(id, domainType, previousVersion);

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

@@ -79,8 +79,10 @@ class SqlGenerator {
 	private final Lazy<String> updateSql = Lazy.of(this::createUpdateSql);
 	private final Lazy<String> updateWithVersionSql = Lazy.of(this::createUpdateWithVersionSql);
 
-	private final Lazy<String> deleteByIdSql = Lazy.of(this::createDeleteSql);
+	private final Lazy<String> deleteByIdSql = Lazy.of(this::createDeleteByIdSql);
+	private final Lazy<String> deleteByIdInSql = Lazy.of(this::createDeleteByIdInSql);
 	private final Lazy<String> deleteByIdAndVersionSql = Lazy.of(this::createDeleteByIdAndVersionSql);
+	private final Lazy<String> deleteByIdInAndVersionSql = Lazy.of(this::createDeleteByIdInAndVersionSql);
 	private final Lazy<String> deleteByListSql = Lazy.of(this::createDeleteByListSql);
 
 	/**
@@ -322,6 +324,15 @@ String getDeleteById() {
 		return deleteByIdSql.get();
 	}
 
+	/**
+	 * Create a {@code DELETE FROM … WHERE :id IN …} statement.
+	 *
+	 * @return the statement as a {@link String}. Guaranteed to be not {@literal null}.
+	 */
+	String getDeleteByIdIn() {
+		return deleteByIdInSql.get();
+	}
+
 	/**
 	 * Create a {@code DELETE FROM … WHERE :id = … and :___oldOptimisticLockingVersion = ...} statement.
 	 *
@@ -331,6 +342,15 @@ String getDeleteByIdAndVersion() {
 		return deleteByIdAndVersionSql.get();
 	}
 
+	/**
+	 * Create a {@code DELETE FROM … WHERE :id In … and :___oldOptimisticLockingVersion = ...} statement.
+	 *
+	 * @return the statement as a {@link String}. Guaranteed to be not {@literal null}.
+	 */
+	String getDeleteByIdInAndVersion() {
+		return deleteByIdInAndVersionSql.get();
+	}
+
 	/**
 	 * Create a {@code DELETE FROM … WHERE :ids in (…)} statement.
 	 *
@@ -635,10 +655,14 @@ private UpdateBuilder.UpdateWhereAndOr createBaseUpdate() {
 				.where(getIdColumn().isEqualTo(getBindMarker(entity.getIdColumn())));
 	}
 
-	private String createDeleteSql() {
+	private String createDeleteByIdSql() {
 		return render(createBaseDeleteById(getTable()).build());
 	}
 
+	private String createDeleteByIdInSql() {
+		return render(createBaseDeleteByIdIn(getTable()).build());
+	}
+
 	private String createDeleteByIdAndVersionSql() {
 
 		Delete delete = createBaseDeleteById(getTable()) //
@@ -648,11 +672,25 @@ private String createDeleteByIdAndVersionSql() {
 		return render(delete);
 	}
 
+	private String createDeleteByIdInAndVersionSql() {
+
+		Delete delete = createBaseDeleteByIdIn(getTable()) //
+				.and(getVersionColumn().isEqualTo(SQL.bindMarker(":" + renderReference(VERSION_SQL_PARAMETER)))) //
+				.build();
+
+		return render(delete);
+	}
+
 	private DeleteBuilder.DeleteWhereAndOr createBaseDeleteById(Table table) {
 		return Delete.builder().from(table)
 				.where(getIdColumn().isEqualTo(SQL.bindMarker(":" + renderReference(ID_SQL_PARAMETER))));
 	}
 
+	private DeleteBuilder.DeleteWhereAndOr createBaseDeleteByIdIn(Table table) {
+		return Delete.builder().from(table)
+				.where(getIdColumn().in(SQL.bindMarker(":" + renderReference(IDS_SQL_PARAMETER))));
+	}
+
 	private String createDeleteByPathAndCriteria(PersistentPropertyPathExtension path,
 			Function<Column, Condition> rootCondition) {
 

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

@@ -192,6 +192,11 @@ public void delete(Object id, Class<?> domainType) {
 		sqlSession().delete(statement, parameter);
 	}
 
+	@Override
+	public void delete(Iterable<Object> ids, Class<?> domainType) {
+		ids.forEach(id -> delete(id, domainType));
+	}
+
 	@Override
 	public <T> void deleteWithVersion(Object id, Class<T> domainType, Number previousVersion) {
 

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

@@ -32,6 +32,7 @@
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -363,6 +364,25 @@ void saveAndDeleteAllByIdsWithReferencedEntity() {
 		});
 	}
 
+	@Test
+	void saveAndDeleteAllByAggregateRootsWithVersion() {
+		AggregateWithImmutableVersion aggregate1 = new AggregateWithImmutableVersion(null, null);
+		AggregateWithImmutableVersion aggregate2 = new AggregateWithImmutableVersion(null, null);
+		AggregateWithImmutableVersion aggregate3 = new AggregateWithImmutableVersion(null, null);
+		Iterator<AggregateWithImmutableVersion> savedAggregatesIterator = template
+				.saveAll(List.of(aggregate1, aggregate2, aggregate3)).iterator();
+		AggregateWithImmutableVersion savedAggregate1 = savedAggregatesIterator.next();
+		AggregateWithImmutableVersion twiceSavedAggregate2 = template.save(savedAggregatesIterator.next());
+		AggregateWithImmutableVersion twiceSavedAggregate3 = template.save(savedAggregatesIterator.next());
+
+		assertThat(template.count(AggregateWithImmutableVersion.class)).isEqualTo(3);
+
+		template.deleteAll(List.of(savedAggregate1, twiceSavedAggregate2, twiceSavedAggregate3),
+				AggregateWithImmutableVersion.class);
+
+		assertThat(template.count(AggregateWithImmutableVersion.class)).isEqualTo(0);
+	}
+
 	@Test // DATAJDBC-112
 	@EnabledOnFeature({ SUPPORTS_QUOTED_IDS, SUPPORTS_GENERATED_IDS_IN_REFERENCED_ENTITIES })
 	void updateReferencedEntityFromNull() {

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

@@ -432,6 +432,18 @@ public BatchDelete(List<Delete<T>> actions) {
 		}
 	}
 
+	/**
+	 * Represents a batch delete statement for multiple entities that are aggregate roots.
+	 *
+	 * @param <T> type of the entity for which this represents a database interaction.
+	 * @since 3.0
+	 */
+	final class BatchDeleteRoot<T> extends BatchWithValue<T, DeleteRoot<T>, Class<T>> {
+		public BatchDeleteRoot(List<DeleteRoot<T>> actions) {
+			super(actions, DeleteRoot::getEntityType);
+		}
+	}
+
 	/**
 	 * 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/DeleteBatchingAggregateChange.java

@@ -2,12 +2,16 @@
 
 import java.util.ArrayList;
 import java.util.Comparator;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.function.Consumer;
 
 import org.springframework.data.mapping.PersistentPropertyPath;
 import org.springframework.data.relational.core.mapping.RelationalPersistentProperty;
 
+import static java.util.Collections.*;
+
 /**
  * A {@link BatchingAggregateChange} implementation for delete changes that can contain actions for one or more delete
  * operations. When consumed, actions are yielded in the appropriate entity tree order with deletes carried out from
@@ -19,11 +23,9 @@
  */
 public class DeleteBatchingAggregateChange<T> implements BatchingAggregateChange<T, DeleteAggregateChange<T>> {
 
-	private static final Comparator<PersistentPropertyPath<RelationalPersistentProperty>> pathLengthComparator = //
-			Comparator.comparing(PersistentPropertyPath::getLength);
-
 	private final Class<T> entityType;
-	private final List<DbAction.DeleteRoot<T>> rootActions = new ArrayList<>();
+	private final List<DbAction.DeleteRoot<T>> rootActionsWithoutVersion = new ArrayList<>();
+	private final List<DbAction.DeleteRoot<T>> rootActionsWithVersion = new ArrayList<>();
 	private final List<DbAction.AcquireLockRoot<?>> lockActions = new ArrayList<>();
 	private final BatchedActions deleteActions = BatchedActions.batchedDeletes();
 
@@ -46,20 +48,35 @@ public void forEachAction(Consumer<? super DbAction<?>> consumer) {
 
 		lockActions.forEach(consumer);
 		deleteActions.forEach(consumer);
-		rootActions.forEach(consumer);
+		if (rootActionsWithoutVersion.size() > 1) {
+			consumer.accept(new DbAction.BatchDeleteRoot<>(rootActionsWithoutVersion));
+		} else {
+			rootActionsWithoutVersion.forEach(consumer);
+		}
+		rootActionsWithVersion.forEach(consumer);
 	}
 
 	@Override
 	public void add(DeleteAggregateChange<T> aggregateChange) {
 
 		aggregateChange.forEachAction(action -> {
 			if (action instanceof DbAction.DeleteRoot<?> deleteRootAction) {
-				rootActions.add((DbAction.DeleteRoot<T>) deleteRootAction);
+				// noinspection unchecked
+				addDeleteRoot((DbAction.DeleteRoot<T>) deleteRootAction);
 			} else if (action instanceof DbAction.Delete<?> deleteAction) {
 				deleteActions.add(deleteAction);
 			} else if (action instanceof DbAction.AcquireLockRoot<?> lockRootAction) {
 				lockActions.add(lockRootAction);
 			}
 		});
 	}
+
+	private void addDeleteRoot(DbAction.DeleteRoot<T> action) {
+
+		if (action.getPreviousVersion() == null) {
+			rootActionsWithoutVersion.add(action);
+		} else {
+			rootActionsWithVersion.add(action);
+		}
+	}
 }

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

@@ -144,6 +144,38 @@ void yieldsLockRootActionsBeforeDeleteActions() {
 		assertThat(extractActions(change)).containsExactly(lockRootAction, intermediateDelete);
 	}
 
+	@Test // GH-537
+	void yieldsDeleteRootActionsWithoutVersionAsBatchDeleteRoots_whenGroupContainsMultipleDeleteRoots() {
+
+		DeleteAggregateChange<Root> aggregateChange1 = MutableAggregateChange.forDelete(new Root(null, null));
+		DbAction.DeleteRoot<Root> deleteRoot1 = new DbAction.DeleteRoot<>(1L, Root.class, null);
+		aggregateChange1.addAction(deleteRoot1);
+		DeleteAggregateChange<Root> aggregateChange2 = MutableAggregateChange.forDelete(Root.class);
+		DbAction.DeleteRoot<Root> deleteRoot2 = new DbAction.DeleteRoot<>(2L, Root.class, 10);
+		aggregateChange2.addAction(deleteRoot2);
+		DeleteAggregateChange<Root> aggregateChange3 = MutableAggregateChange.forDelete(Root.class);
+		DbAction.DeleteRoot<Root> deleteRoot3 = new DbAction.DeleteRoot<>(3L, Root.class, null);
+		aggregateChange3.addAction(deleteRoot3);
+		DeleteAggregateChange<Root> aggregateChange4 = MutableAggregateChange.forDelete(Root.class);
+		DbAction.DeleteRoot<Root> deleteRoot4 = new DbAction.DeleteRoot<>(4L, Root.class, 10);
+		aggregateChange4.addAction(deleteRoot4);
+
+		BatchingAggregateChange<Root, DeleteAggregateChange<Root>> change = BatchingAggregateChange.forDelete(Root.class);
+		change.add(aggregateChange1);
+		change.add(aggregateChange2);
+		change.add(aggregateChange3);
+		change.add(aggregateChange4);
+
+		List<DbAction<?>> actions = extractActions(change);
+		assertThat(actions).extracting(DbAction::getClass, DbAction::getEntityType).containsExactly( //
+				Tuple.tuple(DbAction.BatchDeleteRoot.class, Root.class), //
+				Tuple.tuple(DbAction.DeleteRoot.class, Root.class), //
+				Tuple.tuple(DbAction.DeleteRoot.class, Root.class));
+		assertThat(getBatchWithValueAction(actions, Root.class, DbAction.BatchDeleteRoot.class).getActions())
+				.containsExactly(deleteRoot1, deleteRoot3);
+		assertThat(actions).containsSubsequence(deleteRoot2, deleteRoot4);
+	}
+
 	private <T> List<DbAction<?>> extractActions(BatchingAggregateChange<T, ? extends MutableAggregateChange<T>> change) {
 
 		List<DbAction<?>> actions = new ArrayList<>();