DATAMONGO-2195 - Move common logic to ResultOperations.

And guard transaction tests.

Author: christophstrobl

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/EntityOperations.java

@@ -206,12 +206,7 @@ interface Entity<T> {
 		 * @since 2.2
 		 */
 		default Query getRemoveByQuery() {
-
-			if (isVersionedEntity()) {
-				return getQueryForVersion();
-			}
-
-			return getByIdQuery();
+			return isVersionedEntity() ? getQueryForVersion() : getByIdQuery();
 		}
 
 		/**

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java

@@ -1740,29 +1740,25 @@ public DeleteResult doInCollection(MongoCollection<Document> collection)
 			private void checkForOptimisticLockingFailures(DeleteResult result, Document removeQuery,
 					MongoCollection<Document> collectionToUse) {
 
-				if (!multi && entity != null && entity.hasVersionProperty() && result.wasAcknowledged()) {
-
-					if (result.getDeletedCount() == 0) {
-
-						String versionFieldName = entity.getVersionProperty().getFieldName();
-						Document idQuery = new Document(removeQuery);
-						idQuery.remove(versionFieldName);
+				if (multi || !ResultOperations.isUndecidedDeleteResult(result, removeQuery, entity)) {
+					return;
+				}
 
-						Iterator<Document> it = collectionToUse.find(idQuery)
-								.projection(Projections.include("_id", versionFieldName)).limit(1).iterator();
+				String versionFieldName = entity.getVersionProperty().getFieldName();
+				Document idQuery = new Document(removeQuery);
+				idQuery.remove(versionFieldName);
 
-						if (it.hasNext()) {
+				Iterator<Document> it = collectionToUse.find(idQuery).projection(Projections.include("_id", versionFieldName))
+						.limit(1).iterator();
 
-							Document source = it.next();
+				if (it.hasNext()) {
 
-							throw new OptimisticLockingFailureException(String
-									.format("The entity with id %s in %s has changed and cannot be deleted! " + System.lineSeparator() + //
-							"Expected version %s but was %s.", source.get("_id"), collectionName, removeQuery.get(versionFieldName),
-											source.get(versionFieldName)));
-						}
+					Document source = it.next();
+					throw ResultOperations.newDeleteVersionedOptimisticLockingException(source.get("_id"), collectionName,
+							removeQuery.get(versionFieldName), source.get(versionFieldName));
 
-					}
 				}
+
 			}
 		});
 	}

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java

@@ -1688,7 +1688,7 @@ private boolean containsVersionProperty(Document document, @Nullable MongoPersis
 			return false;
 		}
 
-		return document.containsKey(persistentEntity.getRequiredIdProperty().getFieldName());
+		return document.containsKey(persistentEntity.getRequiredVersionProperty().getFieldName());
 	}
 
 	/*
@@ -1826,41 +1826,33 @@ protected <T> Mono<DeleteResult> doRemove(String collectionName, Query query, @N
 
 		}).flatMap(deleteResult -> {
 
-			if (entity != null && entity.hasVersionProperty()) {
-
-				if (deleteResult.wasAcknowledged() && deleteResult.getDeletedCount() == 0) {
-
-					if (query.getQueryObject().containsKey(entity.getVersionProperty().getFieldName())) {
+			if (!ResultOperations.isUndecidedDeleteResult(deleteResult, removeQuery, entity)) {
+				return Mono.just(deleteResult);
+			}
 
-						return execute(collectionName, (coll -> {
+			return execute(collectionName, (coll -> {
 
-							String versionFieldName = entity.getVersionProperty().getFieldName();
+				String versionFieldName = entity.getVersionProperty().getFieldName();
 
-							Document daQuery = new Document(removeQuery);
-							daQuery.remove(versionFieldName);
+				Document queryWithoutVersion = new Document(removeQuery);
+				queryWithoutVersion.remove(versionFieldName);
 
-							Publisher<Document> publisher = coll.find(daQuery)
-									.projection(Projections.include("_id", entity.getVersionProperty().getFieldName())).limit(1).first();
+				Publisher<Document> publisher = coll.find(queryWithoutVersion)
+						.projection(Projections.include("_id", entity.getVersionProperty().getFieldName())).limit(1).first();
 
-							return Mono.from(publisher) //
-									.defaultIfEmpty(new Document()) //
-									.flatMap(it -> {
+				return Mono.from(publisher) //
+						.defaultIfEmpty(new Document()) //
+						.flatMap(it -> {
 
-										if (it.isEmpty()) {
-											return Mono.just(deleteResult);
-										}
-										return Mono.error(new OptimisticLockingFailureException(String.format(
-												"The entity with id %s in %s has changed and cannot be deleted! " + System.lineSeparator() + //
-										"Expected version %s but was %s.", it.get("_id"), collectionName, removeQuery.get(versionFieldName),
-												it.get(versionFieldName))));
+							if (it.isEmpty()) {
+								return Mono.just(deleteResult);
+							}
 
-									});
-						})).next();
-					}
-				}
-			}
+							return Mono.error(ResultOperations.newDeleteVersionedOptimisticLockingException(it.get("_id"),
+									collectionName, removeQuery.get(versionFieldName), it.get(versionFieldName)));
+						});
+			})).next();
 
-			return Mono.just(deleteResult);
 		}).doOnNext(it -> maybeEmitEvent(new AfterDeleteEvent<>(queryObject, entityClass, collectionName))) //
 				.next();
 	}

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ResultOperations.java

@@ -0,0 +1,76 @@
+/*
+ * Copyright 2019 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.mongodb.core;
+
+import org.bson.Document;
+import org.springframework.dao.OptimisticLockingFailureException;
+import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity;
+import org.springframework.lang.Nullable;
+
+import com.mongodb.client.result.DeleteResult;
+import com.mongodb.client.result.UpdateResult;
+
+/**
+ * {@link ResultOperations} is intended to share logic depending on {@link DeleteResult} and
+ * {@link com.mongodb.client.result.UpdateResult} between reactive and imperative implementations.
+ *
+ * @author Christoph Strobl
+ * @since 2.2
+ */
+class ResultOperations {
+
+	/**
+	 * Decide if a given {@link DeleteResult} needs further inspection. Returns {@literal true} if the delete of a
+	 * {@link org.springframework.data.annotation.Version versioned} entity did not remove any documents although the
+	 * operation has been {@link DeleteResult#wasAcknowledged() acknowledged}.
+	 *
+	 * @param deleteResult must not be {@literal null}.
+	 * @param query the actual query used for the delete operation.
+	 * @param entity can be {@literal null}.
+	 * @return {@literal true} if it cannot be decided if nothing got deleted because of a potential
+	 *         {@link org.springframework.data.annotation.Version version} mismatch, or if the document did not exist in
+	 *         first place.
+	 */
+	static boolean isUndecidedDeleteResult(DeleteResult deleteResult, org.bson.Document query,
+			@Nullable MongoPersistentEntity<?> entity) {
+
+		if (!deleteResult.wasAcknowledged() || deleteResult.getDeletedCount() > 0) {
+			return false;
+		}
+
+		if (entity == null) {
+			return false;
+		}
+
+		if (!entity.hasVersionProperty()) {
+			return false;
+		}
+
+		return containsVersionProperty(query, entity);
+	}
+
+	static OptimisticLockingFailureException newDeleteVersionedOptimisticLockingException(Object id,
+			String collectionName, Object expectedVersion, Object actualVersion) {
+
+		throw new OptimisticLockingFailureException(
+				String.format("The entity with id %s in %s has changed and cannot be deleted! " + System.lineSeparator() + //
+						"Expected version %s but was %s.", id, collectionName, expectedVersion, actualVersion));
+	}
+
+	private static boolean containsVersionProperty(Document document, MongoPersistentEntity<?> entity) {
+		return document.containsKey(entity.getRequiredVersionProperty().getFieldName());
+	}
+}

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/support/SimpleMongoRepositoryVersionedEntityTests.java

@@ -16,6 +16,7 @@
 package org.springframework.data.mongodb.repository.support;
 
 import static org.assertj.core.api.Assertions.*;
+import static org.assertj.core.api.Assumptions.assumeThat;
 import static org.springframework.data.mongodb.core.query.Criteria.*;
 import static org.springframework.data.mongodb.core.query.Query.*;
 
@@ -32,6 +33,8 @@
 import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity;
 import org.springframework.data.mongodb.repository.VersionedPerson;
 import org.springframework.data.mongodb.repository.query.MongoEntityInformation;
+import org.springframework.data.mongodb.test.util.MongoVersion;
+import org.springframework.data.mongodb.test.util.ReplicaSet;
 import org.springframework.data.util.ClassTypeInformation;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
@@ -91,8 +94,11 @@ public void deleteWithMatchingVersion() {
 	}
 
 	@Test // DATAMONGO-2195
+	@MongoVersion(asOf = "4.0")
 	public void deleteWithMatchingVersionInTx() {
 
+		assumeThat(ReplicaSet.required().runsAsReplicaSet()).isTrue();
+
 		long countBefore = repository.count();
 
 		initTxTemplate().execute(status -> {
@@ -118,8 +124,11 @@ public void deleteWithVersionMismatch() {
 	}
 
 	@Test // DATAMONGO-2195
+	@MongoVersion(asOf = "4.0")
 	public void deleteWithVersionMismatchInTx() {
 
+		assumeThat(ReplicaSet.required().runsAsReplicaSet()).isTrue();
+
 		long countBefore = repository.count();
 
 		assertThatExceptionOfType(OptimisticLockingFailureException.class)
@@ -141,8 +150,11 @@ public void deleteNonExisting() {
 	}
 
 	@Test // DATAMONGO-2195
+	@MongoVersion(asOf = "4.0")
 	public void deleteNonExistingInTx() {
 
+		assumeThat(ReplicaSet.required().runsAsReplicaSet()).isTrue();
+
 		initTxTemplate().execute(status -> {
 
 			repository.delete(new VersionedPerson("T-800"));