diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/AggregateChangeExecutor.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/AggregateChangeExecutor.java index 8e0637c7..e039fb9a 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/AggregateChangeExecutor.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/AggregateChangeExecutor.java @@ -98,6 +98,8 @@ class AggregateChangeExecutor { 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) { diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java index 0b5f8ea3..d27322cd 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java @@ -131,6 +131,12 @@ class JdbcAggregateChangeExecutionContext { } } + void executeBatchDeleteRoot(DbAction.BatchDeleteRoot batchDelete) { + + List rootIds = batchDelete.getActions().stream().map(DbAction.DeleteRoot::getId).toList(); + accessStrategy.delete(rootIds, batchDelete.getEntityType()); + } + void executeDelete(DbAction.Delete delete) { accessStrategy.delete(delete.getRootId(), delete.getPropertyPath()); diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java index c652fb4e..f68f6bf2 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java @@ -79,6 +79,11 @@ public class CascadingDataAccessStrategy implements DataAccessStrategy { collectVoid(das -> das.delete(id, domainType)); } + @Override + public void delete(Iterable ids, Class domainType) { + collectVoid(das -> das.delete(ids, domainType)); + } + @Override public void deleteWithVersion(Object id, Class domainType, Number previousVersion) { collectVoid(das -> das.deleteWithVersion(id, domainType, previousVersion)); diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DataAccessStrategy.java index fd121673..3e5974fb 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DataAccessStrategy.java +++ b/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. + *

+ * 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 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 rootIds, PersistentPropertyPath propertyPath); diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java index 37fad605..334060da 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java @@ -163,6 +163,15 @@ public class DefaultDataAccessStrategy implements DataAccessStrategy { operations.update(deleteByIdSql, parameter); } + @Override + public void delete(Iterable ids, Class domainType) { + + String deleteByIdInSql = sql(domainType).getDeleteByIdIn(); + SqlParameterSource parameter = sqlParametersFactory.forQueryByIds(ids, domainType); + + operations.update(deleteByIdInSql, parameter); + } + @Override public void deleteWithVersion(Object id, Class domainType, Number previousVersion) { 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 712e38b6..858b0cf9 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 @@ -81,6 +81,11 @@ public class DelegatingDataAccessStrategy implements DataAccessStrategy { delegate.delete(id, domainType); } + @Override + public void delete(Iterable ids, Class domainType) { + delegate.delete(ids, domainType); + } + @Override public void deleteWithVersion(Object id, Class domainType, Number previousVersion) { delegate.deleteWithVersion(id, domainType, previousVersion); diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java index 0ed67139..e1228fe1 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java @@ -79,8 +79,10 @@ class SqlGenerator { private final Lazy updateSql = Lazy.of(this::createUpdateSql); private final Lazy updateWithVersionSql = Lazy.of(this::createUpdateWithVersionSql); - private final Lazy deleteByIdSql = Lazy.of(this::createDeleteSql); + private final Lazy deleteByIdSql = Lazy.of(this::createDeleteByIdSql); + private final Lazy deleteByIdInSql = Lazy.of(this::createDeleteByIdInSql); private final Lazy deleteByIdAndVersionSql = Lazy.of(this::createDeleteByIdAndVersionSql); + private final Lazy deleteByIdInAndVersionSql = Lazy.of(this::createDeleteByIdInAndVersionSql); private final Lazy deleteByListSql = Lazy.of(this::createDeleteByListSql); /** @@ -322,6 +324,15 @@ class SqlGenerator { 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 @@ class SqlGenerator { 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 @@ class SqlGenerator { .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 @@ class SqlGenerator { 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 rootCondition) { 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 e5473128..b4381663 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 @@ -192,6 +192,11 @@ public class MyBatisDataAccessStrategy implements DataAccessStrategy { sqlSession().delete(statement, parameter); } + @Override + public void delete(Iterable ids, Class domainType) { + ids.forEach(id -> delete(id, domainType)); + } + @Override public void deleteWithVersion(Object id, Class domainType, Number previousVersion) { diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java index 16c8b058..ad75859d 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java @@ -32,6 +32,7 @@ import java.util.ArrayList; 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 @@ class JdbcAggregateTemplateIntegrationTests { }); } + @Test + void saveAndDeleteAllByAggregateRootsWithVersion() { + AggregateWithImmutableVersion aggregate1 = new AggregateWithImmutableVersion(null, null); + AggregateWithImmutableVersion aggregate2 = new AggregateWithImmutableVersion(null, null); + AggregateWithImmutableVersion aggregate3 = new AggregateWithImmutableVersion(null, null); + Iterator 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() { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java index 15b72f3c..dc96ac7c 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java @@ -432,6 +432,18 @@ public interface DbAction { } } + /** + * Represents a batch delete statement for multiple entities that are aggregate roots. + * + * @param type of the entity for which this represents a database interaction. + * @since 3.0 + */ + final class BatchDeleteRoot extends BatchWithValue, Class> { + public BatchDeleteRoot(List> actions) { + super(actions, DeleteRoot::getEntityType); + } + } + /** * An action depending on another action for providing additional information like the id of a parent entity. * diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DeleteBatchingAggregateChange.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DeleteBatchingAggregateChange.java index a3104e41..fe7a02a3 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DeleteBatchingAggregateChange.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DeleteBatchingAggregateChange.java @@ -2,12 +2,16 @@ package org.springframework.data.relational.core.conversion; 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 @@ import org.springframework.data.relational.core.mapping.RelationalPersistentProp */ public class DeleteBatchingAggregateChange implements BatchingAggregateChange> { - private static final Comparator> pathLengthComparator = // - Comparator.comparing(PersistentPropertyPath::getLength); - private final Class entityType; - private final List> rootActions = new ArrayList<>(); + private final List> rootActionsWithoutVersion = new ArrayList<>(); + private final List> rootActionsWithVersion = new ArrayList<>(); private final List> lockActions = new ArrayList<>(); private final BatchedActions deleteActions = BatchedActions.batchedDeletes(); @@ -46,7 +48,12 @@ public class DeleteBatchingAggregateChange implements BatchingAggregateChange 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 @@ -54,7 +61,8 @@ public class DeleteBatchingAggregateChange implements BatchingAggregateChange aggregateChange.forEachAction(action -> { if (action instanceof DbAction.DeleteRoot deleteRootAction) { - rootActions.add((DbAction.DeleteRoot) deleteRootAction); + // noinspection unchecked + addDeleteRoot((DbAction.DeleteRoot) deleteRootAction); } else if (action instanceof DbAction.Delete deleteAction) { deleteActions.add(deleteAction); } else if (action instanceof DbAction.AcquireLockRoot lockRootAction) { @@ -62,4 +70,13 @@ public class DeleteBatchingAggregateChange implements BatchingAggregateChange } }); } + + private void addDeleteRoot(DbAction.DeleteRoot action) { + + if (action.getPreviousVersion() == null) { + rootActionsWithoutVersion.add(action); + } else { + rootActionsWithVersion.add(action); + } + } } diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/DeleteBatchingAggregateChangeTest.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/DeleteBatchingAggregateChangeTest.java index d24994b3..6828f2a6 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/DeleteBatchingAggregateChangeTest.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/DeleteBatchingAggregateChangeTest.java @@ -144,6 +144,38 @@ class DeleteBatchingAggregateChangeTest { assertThat(extractActions(change)).containsExactly(lockRootAction, intermediateDelete); } + @Test // GH-537 + void yieldsDeleteRootActionsWithoutVersionAsBatchDeleteRoots_whenGroupContainsMultipleDeleteRoots() { + + DeleteAggregateChange aggregateChange1 = MutableAggregateChange.forDelete(new Root(null, null)); + DbAction.DeleteRoot deleteRoot1 = new DbAction.DeleteRoot<>(1L, Root.class, null); + aggregateChange1.addAction(deleteRoot1); + DeleteAggregateChange aggregateChange2 = MutableAggregateChange.forDelete(Root.class); + DbAction.DeleteRoot deleteRoot2 = new DbAction.DeleteRoot<>(2L, Root.class, 10); + aggregateChange2.addAction(deleteRoot2); + DeleteAggregateChange aggregateChange3 = MutableAggregateChange.forDelete(Root.class); + DbAction.DeleteRoot deleteRoot3 = new DbAction.DeleteRoot<>(3L, Root.class, null); + aggregateChange3.addAction(deleteRoot3); + DeleteAggregateChange aggregateChange4 = MutableAggregateChange.forDelete(Root.class); + DbAction.DeleteRoot deleteRoot4 = new DbAction.DeleteRoot<>(4L, Root.class, 10); + aggregateChange4.addAction(deleteRoot4); + + BatchingAggregateChange> change = BatchingAggregateChange.forDelete(Root.class); + change.add(aggregateChange1); + change.add(aggregateChange2); + change.add(aggregateChange3); + change.add(aggregateChange4); + + List> 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 List> extractActions(BatchingAggregateChange> change) { List> actions = new ArrayList<>();