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 c0911e8fb6..8e0637c75b 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 @@ -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) { 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 97fbd258b4..22399bb92c 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 @@ -135,6 +135,12 @@ void executeDelete(DbAction.Delete delete) { accessStrategy.delete(delete.getRootId(), delete.getPropertyPath()); } + void executeBatchDelete(DbAction.BatchDelete batchDelete) { + + List rootIds = batchDelete.getActions().stream().map(DbAction.Delete::getRootId).toList(); + accessStrategy.delete(rootIds, batchDelete.getBatchValue()); + } + void executeDeleteAllRoot(DbAction.DeleteAllRoot deleteAllRoot) { accessStrategy.deleteAll(deleteAllRoot.getEntityType()); 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 7987fabf67..f790d67c87 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 @@ -87,6 +87,11 @@ public void delete(Object rootId, PersistentPropertyPath das.delete(rootId, propertyPath)); } + @Override + public void delete(Iterable rootIds, PersistentPropertyPath propertyPath) { + collectVoid(das -> das.delete(rootIds, propertyPath)); + } + @Override public void deleteAll(Class domainType) { collectVoid(das -> das.deleteAll(domainType)); 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 a4c12f0791..e63d71cbbf 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 @@ -151,6 +151,14 @@ public interface DataAccessStrategy extends RelationResolver { */ void delete(Object rootId, PersistentPropertyPath 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 rootIds, PersistentPropertyPath propertyPath); + /** * Deletes all entities of the given domain type. * 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 2521aef521..54b37046a7 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 @@ -196,6 +196,21 @@ public void delete(Object rootId, PersistentPropertyPath rootIds, PersistentPropertyPath 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 void deleteAll(Class domainType) { operations.getJdbcOperations().update(sql(domainType).createDeleteAllSql(null)); 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 3a8f9c308c..712e38b616 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 @@ -71,6 +71,11 @@ public void delete(Object rootId, PersistentPropertyPath rootIds, PersistentPropertyPath propertyPath) { + delegate.delete(rootIds, propertyPath); + } + @Override public void delete(Object id, Class domainType) { delegate.delete(id, domainType); 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 c9e7c25ac3..8ad6cf4a9c 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 @@ -360,7 +360,8 @@ String createDeleteAllSql(@Nullable PersistentPropertyPath 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 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))) // 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 bd66945c6a..67fee8a942 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 @@ -218,6 +218,11 @@ public void delete(Object rootId, PersistentPropertyPath rootIds, PersistentPropertyPath propertyPath) { + rootIds.forEach(rootId -> delete(rootId, propertyPath)); + } + @Override public void deleteAll(Class domainType) { diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java index 7ea24dfebe..37cbdfdbbb 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java +++ b/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() { 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 97e2f53470..4f4bd85072 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 @@ -411,6 +411,18 @@ public BatchInsertRoot(List> actions) { } } + /** + * Represents a batch delete statement for multiple entities that are reachable via a given path from the aggregate root. + * + * @param type of the entity for which this represents a database interaction. + * @since 3.0 + */ + final class BatchDelete extends BatchWithValue, PersistentPropertyPath> { + public BatchDelete(List> actions) { + super(actions, Delete::getPropertyPath); + } + } + /** * 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/SaveBatchingAggregateChange.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/SaveBatchingAggregateChange.java index 49ee0b2727..5719e98642 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/SaveBatchingAggregateChange.java +++ b/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 @@ -47,7 +47,7 @@ public class SaveBatchingAggregateChange implements BatchingAggregateChange> insertRootBatchCandidates = new ArrayList<>(); private final Map, Map>>> insertActions = // new HashMap<>(); - private final Map, List>> deleteActions = // + private final Map, List>> deleteActions = // new HashMap<>(); public SaveBatchingAggregateChange(Class entityType) { @@ -76,15 +76,22 @@ public void forEachAction(Consumer> 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) -> { - if (inserts.size() > 1) { - consumer.accept(new DbAction.BatchInsert<>(inserts)); - } else { - inserts.forEach(consumer); - } - })); + .forEach((entry) -> { + List> 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) -> { + if (inserts.size() > 1) { + consumer.accept(new DbAction.BatchInsert<>(inserts)); + } else { + inserts.forEach(consumer); + } + })); } @Override @@ -95,16 +102,18 @@ public void add(RootAggregateChange aggregateChange) { commitBatchCandidates(); rootActions.add(rootAction); } else if (action instanceof DbAction.InsertRoot rootAction) { - if (!insertRootBatchCandidates.isEmpty() && !insertRootBatchCandidates.get(0).getIdValueSource().equals(rootAction.getIdValueSource())) { + if (!insertRootBatchCandidates.isEmpty() + && !insertRootBatchCandidates.get(0).getIdValueSource().equals(rootAction.getIdValueSource())) { commitBatchCandidates(); } - //noinspection unchecked + // noinspection unchecked insertRootBatchCandidates.add((DbAction.InsertRoot) rootAction); } else if (action instanceof DbAction.Insert insertAction) { // noinspection unchecked addInsert((DbAction.Insert) insertAction); } else if (action instanceof DbAction.Delete deleteAction) { - addDelete(deleteAction); + // noinspection unchecked + addDelete((DbAction.Delete) deleteAction); } }); } @@ -133,7 +142,7 @@ private void addInsert(DbAction.Insert action) { }); } - private void addDelete(DbAction.Delete action) { + private void addDelete(DbAction.Delete action) { PersistentPropertyPath propertyPath = action.getPropertyPath(); deleteActions.merge(propertyPath, new ArrayList<>(singletonList(action)), (actions, defaultValue) -> { diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/SaveBatchingAggregateChangeTest.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/SaveBatchingAggregateChangeTest.java index a56eaf59f9..5d9c55dfd1 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/SaveBatchingAggregateChangeTest.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/SaveBatchingAggregateChangeTest.java @@ -266,17 +266,17 @@ void yieldsNestedDeleteActionsInTreeOrderFromLeavesToRoot() { Root root1 = new Root(1L, null); RootAggregateChange aggregateChange1 = MutableAggregateChange.forSave(root1); aggregateChange1.setRootAction(new DbAction.UpdateRoot<>(root1, null)); - DbAction.Delete root1IntermediateDelete = new DbAction.Delete<>(1L, + DbAction.Delete root1IntermediateDelete = new DbAction.Delete<>(1L, context.getPersistentPropertyPath("intermediate", Root.class)); aggregateChange1.addAction(root1IntermediateDelete); - Root root2 = new Root(1L, null); + Root root2 = new Root(2L, null); RootAggregateChange aggregateChange2 = MutableAggregateChange.forSave(root2); aggregateChange2.setRootAction(new DbAction.UpdateRoot<>(root2, null)); DbAction.Delete root2LeafDelete = new DbAction.Delete<>(1L, context.getPersistentPropertyPath("intermediate.leaf", Root.class)); aggregateChange2.addAction(root2LeafDelete); - DbAction.Delete root2IntermediateDelete = new DbAction.Delete<>(1L, + DbAction.Delete root2IntermediateDelete = new DbAction.Delete<>(1L, context.getPersistentPropertyPath("intermediate", Root.class)); aggregateChange2.addAction(root2IntermediateDelete); @@ -284,8 +284,38 @@ void yieldsNestedDeleteActionsInTreeOrderFromLeavesToRoot() { change.add(aggregateChange1); change.add(aggregateChange2); - assertThat(extractActions(change)).containsSubsequence(root2LeafDelete, root1IntermediateDelete, - root2IntermediateDelete); + List> 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 aggregateChange = MutableAggregateChange.forSave(root); + DbAction.UpdateRoot updateRoot = new DbAction.UpdateRoot<>(root, null); + aggregateChange.setRootAction(updateRoot); + DbAction.Delete intermediateDelete1 = new DbAction.Delete<>(1L, + context.getPersistentPropertyPath("intermediate", Root.class)); + DbAction.Delete intermediateDelete2 = new DbAction.Delete<>(2L, + context.getPersistentPropertyPath("intermediate", Root.class)); + aggregateChange.addAction(intermediateDelete1); + aggregateChange.addAction(intermediateDelete2); + + BatchingAggregateChange> change = BatchingAggregateChange.forSave(Root.class); + change.add(aggregateChange); + + List> 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