From 94f94c6ba0dc3c2a981e70be926cb7aab4549f85 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Fri, 24 Apr 2020 10:12:42 +0200 Subject: [PATCH] #93 - Polishing. Formatting. Added issue to test comments. Removed the test for presence of the id in case of a potential optimistic locking exception. A deleted row is also a case of a concurrent modification and therefore should trigger the OptimisticLockingException. Original pull request: #314. --- .../r2dbc/core/DefaultDatabaseClient.java | 1 + .../data/r2dbc/core/R2dbcEntityTemplate.java | 30 ++++++++----------- ...SimpleR2dbcRepositoryIntegrationTests.java | 10 +++---- 3 files changed, 18 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/springframework/data/r2dbc/core/DefaultDatabaseClient.java b/src/main/java/org/springframework/data/r2dbc/core/DefaultDatabaseClient.java index 560ab1e..25f5652 100644 --- a/src/main/java/org/springframework/data/r2dbc/core/DefaultDatabaseClient.java +++ b/src/main/java/org/springframework/data/r2dbc/core/DefaultDatabaseClient.java @@ -1297,6 +1297,7 @@ class DefaultDatabaseClient implements DatabaseClient, ConnectionAccessor { DefaultTypedUpdateSpec(Class typeToUpdate, @Nullable SqlIdentifier table, @Nullable T objectToUpdate, @Nullable CriteriaDefinition where) { + this.typeToUpdate = typeToUpdate; this.table = table; this.objectToUpdate = objectToUpdate; diff --git a/src/main/java/org/springframework/data/r2dbc/core/R2dbcEntityTemplate.java b/src/main/java/org/springframework/data/r2dbc/core/R2dbcEntityTemplate.java index 59eb176..75e261a 100644 --- a/src/main/java/org/springframework/data/r2dbc/core/R2dbcEntityTemplate.java +++ b/src/main/java/org/springframework/data/r2dbc/core/R2dbcEntityTemplate.java @@ -17,7 +17,6 @@ package org.springframework.data.r2dbc.core; import io.r2dbc.spi.Row; import io.r2dbc.spi.RowMetadata; -import org.springframework.dao.OptimisticLockingFailureException; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -33,6 +32,7 @@ import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.core.convert.ConversionService; import org.springframework.dao.DataAccessException; +import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.dao.TransientDataAccessResourceException; import org.springframework.data.mapping.IdentifierAccessor; import org.springframework.data.mapping.MappingException; @@ -377,7 +377,7 @@ public class R2dbcEntityTemplate implements R2dbcEntityOperations, BeanFactoryAw RelationalPersistentEntity persistentEntity = getRequiredEntity(entity); - setVersionIfNecessary(persistentEntity, entity); + setVersionIfNecessary(persistentEntity, entity); return this.databaseClient.insert() // .into(persistentEntity.getType()) // @@ -388,6 +388,7 @@ public class R2dbcEntityTemplate implements R2dbcEntityOperations, BeanFactoryAw } private void setVersionIfNecessary(RelationalPersistentEntity persistentEntity, T entity) { + RelationalPersistentProperty versionProperty = persistentEntity.getVersionProperty(); if (versionProperty == null) { return; @@ -418,45 +419,37 @@ public class R2dbcEntityTemplate implements R2dbcEntityOperations, BeanFactoryAw DatabaseClient.UpdateSpec updateSpec = updateMatchingSpec; if (persistentEntity.hasVersionProperty()) { + updateSpec = updateMatchingSpec.matching(createMatchingVersionCriteria(entity, persistentEntity)); incrementVersion(entity, persistentEntity); } return updateSpec.fetch() // .rowsUpdated() // - .flatMap(rowsUpdated -> rowsUpdated == 0 - ? handleMissingUpdate(entity, persistentEntity) : Mono.just(entity)); + .flatMap(rowsUpdated -> rowsUpdated == 0 ? handleMissingUpdate(entity, persistentEntity) : Mono.just(entity)); } private Mono handleMissingUpdate(T entity, RelationalPersistentEntity persistentEntity) { - if (!persistentEntity.hasVersionProperty()) { - return Mono.error(new TransientDataAccessResourceException( - formatTransientEntityExceptionMessage(entity, persistentEntity))); - } - return doCount(getByIdQuery(entity, persistentEntity), entity.getClass(), persistentEntity.getTableName()) - .map(count -> { - if (count == 0) { - throw new TransientDataAccessResourceException( - formatTransientEntityExceptionMessage(entity, persistentEntity)); - } else { - throw new OptimisticLockingFailureException( - formatOptimisticLockingExceptionMessage(entity, persistentEntity)); - } - }); + return Mono.error(persistentEntity.hasVersionProperty() + ? new OptimisticLockingFailureException(formatOptimisticLockingExceptionMessage(entity, persistentEntity)) + : new TransientDataAccessResourceException(formatTransientEntityExceptionMessage(entity, persistentEntity))); } private String formatOptimisticLockingExceptionMessage(T entity, RelationalPersistentEntity persistentEntity) { + return String.format("Failed to update table [%s]. Version does not match for row with Id [%s].", persistentEntity.getTableName(), persistentEntity.getIdentifierAccessor(entity).getIdentifier()); } private String formatTransientEntityExceptionMessage(T entity, RelationalPersistentEntity persistentEntity) { + return String.format("Failed to update table [%s]. Row with Id [%s] does not exist.", persistentEntity.getTableName(), persistentEntity.getIdentifierAccessor(entity).getIdentifier()); } private void incrementVersion(T entity, RelationalPersistentEntity persistentEntity) { + PersistentPropertyAccessor propertyAccessor = persistentEntity.getPropertyAccessor(entity); RelationalPersistentProperty versionProperty = persistentEntity.getVersionProperty(); @@ -471,6 +464,7 @@ public class R2dbcEntityTemplate implements R2dbcEntityOperations, BeanFactoryAw } private Criteria createMatchingVersionCriteria(T entity, RelationalPersistentEntity persistentEntity) { + PersistentPropertyAccessor propertyAccessor = persistentEntity.getPropertyAccessor(entity); RelationalPersistentProperty versionProperty = persistentEntity.getVersionProperty(); diff --git a/src/test/java/org/springframework/data/r2dbc/repository/support/AbstractSimpleR2dbcRepositoryIntegrationTests.java b/src/test/java/org/springframework/data/r2dbc/repository/support/AbstractSimpleR2dbcRepositoryIntegrationTests.java index 2d4bc4a..cacbb18 100644 --- a/src/test/java/org/springframework/data/r2dbc/repository/support/AbstractSimpleR2dbcRepositoryIntegrationTests.java +++ b/src/test/java/org/springframework/data/r2dbc/repository/support/AbstractSimpleR2dbcRepositoryIntegrationTests.java @@ -119,7 +119,7 @@ public abstract class AbstractSimpleR2dbcRepositoryIntegrationTests extends R2db assertThat(map).containsEntry("name", "SCHAUFELRADBAGGER").containsEntry("manual", 12).containsKey("id"); } - @Test + @Test // gh-93 public void shouldSaveNewObjectAndSetVersionIfWrapperVersionPropertyExists() { LegoSetVersionable legoSet = new LegoSetVersionable(null, "SCHAUFELRADBAGGER", 12, null); @@ -137,10 +137,10 @@ public abstract class AbstractSimpleR2dbcRepositoryIntegrationTests extends R2db .containsKey("id"); } - @Test + @Test // gh-93 public void shouldSaveNewObjectAndSetVersionIfPrimitiveVersionPropertyExists() { - LegoSetPrimitiveVersionable legoSet = new LegoSetPrimitiveVersionable(null, "SCHAUFELRADBAGGER", 12, -1); + LegoSetPrimitiveVersionable legoSet = new LegoSetPrimitiveVersionable(null, "SCHAUFELRADBAGGER", 12, 0); repository.save(legoSet) // .as(StepVerifier::create) // @@ -173,7 +173,7 @@ public abstract class AbstractSimpleR2dbcRepositoryIntegrationTests extends R2db assertThat(map).containsEntry("name", "SCHAUFELRADBAGGER").containsEntry("manual", 14).containsKey("id"); } - @Test + @Test // gh-93 public void shouldUpdateVersionableObjectAndIncreaseVersion() { jdbc.execute("INSERT INTO legoset (name, manual, version) VALUES('SCHAUFELRADBAGGER', 12, 42)"); @@ -197,7 +197,7 @@ public abstract class AbstractSimpleR2dbcRepositoryIntegrationTests extends R2db .containsKey("id"); } - @Test + @Test // gh-93 public void shouldFailWithOptimistickLockingWhenVersionDoesNotMatchOnUpdate() { jdbc.execute("INSERT INTO legoset (name, manual, version) VALUES('SCHAUFELRADBAGGER', 12, 42)");