From 6e57df85b4c3c49d267f5a7d59d042ec8e0ff9d2 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Thu, 25 Aug 2022 15:56:30 +0200 Subject: [PATCH 1/2] 1313-noop-deletes - Prepare branch --- pom.xml | 2 +- spring-data-jdbc-distribution/pom.xml | 2 +- spring-data-jdbc/pom.xml | 4 ++-- spring-data-r2dbc/pom.xml | 4 ++-- spring-data-relational/pom.xml | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pom.xml b/pom.xml index b2d4b1d1ae..be59de7b30 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-relational-parent - 3.0.0-SNAPSHOT + 3.0.0-1313-noop-deletes-SNAPSHOT pom Spring Data Relational Parent diff --git a/spring-data-jdbc-distribution/pom.xml b/spring-data-jdbc-distribution/pom.xml index db3b7ddd1a..1f95852274 100644 --- a/spring-data-jdbc-distribution/pom.xml +++ b/spring-data-jdbc-distribution/pom.xml @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 3.0.0-SNAPSHOT + 3.0.0-1313-noop-deletes-SNAPSHOT ../pom.xml diff --git a/spring-data-jdbc/pom.xml b/spring-data-jdbc/pom.xml index 547ff62b8b..680fb56a56 100644 --- a/spring-data-jdbc/pom.xml +++ b/spring-data-jdbc/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-jdbc - 3.0.0-SNAPSHOT + 3.0.0-1313-noop-deletes-SNAPSHOT Spring Data JDBC Spring Data module for JDBC repositories. @@ -15,7 +15,7 @@ org.springframework.data spring-data-relational-parent - 3.0.0-SNAPSHOT + 3.0.0-1313-noop-deletes-SNAPSHOT diff --git a/spring-data-r2dbc/pom.xml b/spring-data-r2dbc/pom.xml index e263db3097..7db70fd57c 100644 --- a/spring-data-r2dbc/pom.xml +++ b/spring-data-r2dbc/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-r2dbc - 3.0.0-SNAPSHOT + 3.0.0-1313-noop-deletes-SNAPSHOT Spring Data R2DBC Spring Data module for R2DBC @@ -15,7 +15,7 @@ org.springframework.data spring-data-relational-parent - 3.0.0-SNAPSHOT + 3.0.0-1313-noop-deletes-SNAPSHOT diff --git a/spring-data-relational/pom.xml b/spring-data-relational/pom.xml index 717ac86edf..407f43d770 100644 --- a/spring-data-relational/pom.xml +++ b/spring-data-relational/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-relational - 3.0.0-SNAPSHOT + 3.0.0-1313-noop-deletes-SNAPSHOT Spring Data Relational Spring Data Relational support @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 3.0.0-SNAPSHOT + 3.0.0-1313-noop-deletes-SNAPSHOT From de811b29f1e89457715a2df2ff40e6468602622d Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Thu, 25 Aug 2022 16:15:27 +0200 Subject: [PATCH 2/2] Correct behaviour of NOOP deletes to match the specification in CrudRepository. Delete operations that receive a version attribute throw an `OptimisticFailureException` when they delete zero rows. Otherwise, the NOOP delete gets silently ignored. Note that save operations that are determined to be an update because the aggregate is not new will still throw an `IncorrectUpdateSemanticsDataAccessException` if they fail to update any row. This is somewhat asymmetric to the delete-behaviour. But with a delete the intended result is achieved: the aggregate is gone from the database. For save operations the intended result is not achieved, hence the exception. Closes #1313 See https://github.com/spring-projects/spring-data-commons/issues/2651 --- .../jdbc/core/AggregateChangeExecutor.java | 5 ++++ .../jdbc/core/JdbcAggregateOperations.java | 23 ++++++++++++++++++- ...JdbcAggregateTemplateIntegrationTests.java | 12 +++++----- 3 files changed, 33 insertions(+), 7 deletions(-) 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 e039fb9aba..33ffa81890 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 @@ -15,6 +15,7 @@ */ package org.springframework.data.jdbc.core; +import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.data.jdbc.core.convert.DataAccessStrategy; import org.springframework.data.jdbc.core.convert.JdbcConverter; import org.springframework.data.relational.core.conversion.AggregateChange; @@ -110,6 +111,10 @@ private void execute(DbAction action, JdbcAggregateChangeExecutionContext exe throw new RuntimeException("unexpected action"); } } catch (Exception e) { + + if (e instanceof OptimisticLockingFailureException) { + throw e; + } throw new DbActionExecutionException(action, e); } } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateOperations.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateOperations.java index 1b6c2b5124..9b516a8452 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateOperations.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateOperations.java @@ -17,6 +17,7 @@ import java.util.Optional; +import org.springframework.dao.IncorrectUpdateSemanticsDataAccessException; import org.springframework.data.domain.Example; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; @@ -41,6 +42,8 @@ public interface JdbcAggregateOperations { * @param instance the aggregate root of the aggregate to be saved. Must not be {@code null}. * @param the type of the aggregate root. * @return the saved instance. + * @throws IncorrectUpdateSemanticsDataAccessException when the instance is determined to be not new and the resulting + * update does not update any rows. */ T save(T instance); @@ -50,6 +53,8 @@ public interface JdbcAggregateOperations { * @param instances the aggregate roots to be saved. Must not be {@code null}. * @param the type of the aggregate root. * @return the saved instances. + * @throws IncorrectUpdateSemanticsDataAccessException when at least one instance is determined to be not new and the + * resulting update does not update any rows. * @since 3.0 */ Iterable saveAll(Iterable instances); @@ -78,6 +83,11 @@ public interface JdbcAggregateOperations { /** * Deletes a single Aggregate including all entities contained in that aggregate. + *

+ * Since no version attribute is provided this method will never throw a + * {@link org.springframework.dao.OptimisticLockingFailureException}. If no rows match the generated delete operation + * this fact will be silently ignored. + *

* * @param id the id of the aggregate root of the aggregate to be deleted. Must not be {@code null}. * @param domainType the type of the aggregate root. @@ -87,7 +97,12 @@ public interface JdbcAggregateOperations { /** * Deletes all aggregates identified by their aggregate root ids. - * + *

+ * Since no version attribute is provided this method will never throw a + * {@link org.springframework.dao.OptimisticLockingFailureException}. If no rows match the generated delete operation + * this fact will be silently ignored. + *

+ * * @param ids the ids of the aggregate roots of the aggregates to be deleted. Must not be {@code null}. * @param domainType the type of the aggregate root. * @param the type of the aggregate root. @@ -100,6 +115,9 @@ public interface JdbcAggregateOperations { * @param aggregateRoot to delete. Must not be {@code null}. * @param domainType the type of the aggregate root. Must not be {@code null}. * @param the type of the aggregate root. + * @throws org.springframework.dao.OptimisticLockingFailureException when {@literal T} has a version attribute and the + * version attribute of the provided entity does not match the version attribute in the database, or when + * there is no aggregate root with matching id. In other cases a NOOP delete is silently ignored. */ void delete(T aggregateRoot, Class domainType); @@ -116,6 +134,9 @@ public interface JdbcAggregateOperations { * @param aggregateRoots to delete. Must not be {@code null}. * @param domainType type of the aggregate roots to be deleted. Must not be {@code null}. * @param the type of the aggregate roots. + * @throws org.springframework.dao.OptimisticLockingFailureException when {@literal T} has a version attribute and for at least on entity the + * version attribute of the entity does not match the version attribute in the database, or when + * there is no aggregate root with matching id. In other cases a NOOP delete is silently ignored. */ void deleteAll(Iterable aggregateRoots, Class domainType); 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 f825f5c6be..dd2987aac0 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 @@ -866,11 +866,11 @@ void saveAndUpdateAggregateWithImmutableVersion() { assertThatThrownBy(() -> template.save(new AggregateWithImmutableVersion(id, 0L))) .describedAs("saving an aggregate with an outdated version should raise an exception") - .hasRootCauseInstanceOf(OptimisticLockingFailureException.class); + .isInstanceOf(OptimisticLockingFailureException.class); assertThatThrownBy(() -> template.save(new AggregateWithImmutableVersion(id, 2L))) .describedAs("saving an aggregate with a future version should raise an exception") - .hasRootCauseInstanceOf(OptimisticLockingFailureException.class); + .isInstanceOf(OptimisticLockingFailureException.class); } @Test // GH-1137 @@ -915,12 +915,12 @@ void deleteAggregateWithVersion() { assertThatThrownBy( () -> template.delete(new AggregateWithImmutableVersion(id, 0L), AggregateWithImmutableVersion.class)) .describedAs("deleting an aggregate with an outdated version should raise an exception") - .hasRootCauseInstanceOf(OptimisticLockingFailureException.class); + .isInstanceOf(OptimisticLockingFailureException.class); assertThatThrownBy( () -> template.delete(new AggregateWithImmutableVersion(id, 2L), AggregateWithImmutableVersion.class)) .describedAs("deleting an aggregate with a future version should raise an exception") - .hasRootCauseInstanceOf(OptimisticLockingFailureException.class); + .isInstanceOf(OptimisticLockingFailureException.class); // This should succeed template.delete(aggregate, AggregateWithImmutableVersion.class); @@ -1060,12 +1060,12 @@ private void saveAndUpdateAggregateWithVersion(VersionedAggre reloadedAggregate.setVersion(toConcreteNumber.apply(initialId)); assertThatThrownBy(() -> template.save(reloadedAggregate)) .withFailMessage("saving an aggregate with an outdated version should raise an exception") - .hasRootCauseInstanceOf(OptimisticLockingFailureException.class); + .isInstanceOf(OptimisticLockingFailureException.class); reloadedAggregate.setVersion(toConcreteNumber.apply(initialId + 2)); assertThatThrownBy(() -> template.save(reloadedAggregate)) .withFailMessage("saving an aggregate with a future version should raise an exception") - .hasRootCauseInstanceOf(OptimisticLockingFailureException.class); + .isInstanceOf(OptimisticLockingFailureException.class); } private Long count(String tableName) {