#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.

Author: schauder

src/main/java/org/springframework/data/r2dbc/core/DefaultDatabaseClient.java

@@ -1297,6 +1297,7 @@ class DefaultTypedUpdateSpec<T> implements TypedUpdateSpec<T>, UpdateMatchingSpe
 
 		DefaultTypedUpdateSpec(Class<T> typeToUpdate, @Nullable SqlIdentifier table, @Nullable T objectToUpdate,
 				@Nullable CriteriaDefinition where) {
+
 			this.typeToUpdate = typeToUpdate;
 			this.table = table;
 			this.objectToUpdate = objectToUpdate;

src/main/java/org/springframework/data/r2dbc/core/R2dbcEntityTemplate.java

@@ -17,7 +17,6 @@
 
 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.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 @@ <T> Mono<T> doInsert(T entity, SqlIdentifier tableName) {
 
 		RelationalPersistentEntity<T> persistentEntity = getRequiredEntity(entity);
 
-		setVersionIfNecessary(persistentEntity,	entity);
+		setVersionIfNecessary(persistentEntity, entity);
 
 		return this.databaseClient.insert() //
 				.into(persistentEntity.getType()) //
@@ -388,6 +388,7 @@ <T> Mono<T> doInsert(T entity, SqlIdentifier tableName) {
 	}
 
 	private <T> void setVersionIfNecessary(RelationalPersistentEntity<T> persistentEntity, T entity) {
+
 		RelationalPersistentProperty versionProperty = persistentEntity.getVersionProperty();
 		if (versionProperty == null) {
 			return;
@@ -418,45 +419,37 @@ public <T> Mono<T> update(T entity) throws DataAccessException {
 
 		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 <T> Mono<? extends T> handleMissingUpdate(T entity, RelationalPersistentEntity<T> 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 <T> String formatOptimisticLockingExceptionMessage(T entity, RelationalPersistentEntity<T> 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 <T> String formatTransientEntityExceptionMessage(T entity, RelationalPersistentEntity<T> persistentEntity) {
+
 		return String.format("Failed to update table [%s]. Row with Id [%s] does not exist.",
 				persistentEntity.getTableName(), persistentEntity.getIdentifierAccessor(entity).getIdentifier());
 	}
 
 	private <T> void incrementVersion(T entity, RelationalPersistentEntity<T> persistentEntity) {
+
 		PersistentPropertyAccessor<?> propertyAccessor = persistentEntity.getPropertyAccessor(entity);
 		RelationalPersistentProperty versionProperty = persistentEntity.getVersionProperty();
 
@@ -471,6 +464,7 @@ private <T> void incrementVersion(T entity, RelationalPersistentEntity<T> persis
 	}
 
 	private <T> Criteria createMatchingVersionCriteria(T entity, RelationalPersistentEntity<T> persistentEntity) {
+
 		PersistentPropertyAccessor<?> propertyAccessor = persistentEntity.getPropertyAccessor(entity);
 		RelationalPersistentProperty versionProperty = persistentEntity.getVersionProperty();
 

src/test/java/org/springframework/data/r2dbc/repository/support/AbstractSimpleR2dbcRepositoryIntegrationTests.java

@@ -119,7 +119,7 @@ public void shouldSaveNewObject() {
 		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 void shouldSaveNewObjectAndSetVersionIfWrapperVersionPropertyExists() {
 				.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 void shouldUpdateObject() {
 		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 void shouldUpdateVersionableObjectAndIncreaseVersion() {
 				.containsKey("id");
 	}
 
-	@Test
+	@Test // gh-93
 	public void shouldFailWithOptimistickLockingWhenVersionDoesNotMatchOnUpdate() {
 
 		jdbc.execute("INSERT INTO legoset (name, manual, version) VALUES('SCHAUFELRADBAGGER', 12, 42)");