DATAMONGO-2195 - Consider version when removing an entity.

We now consider a potential @Version when removing an entity.

MongoOperations#remove(Object) and MongoOperations#remove(Object, String) include the version of the object to remove if the entity is versioned. Opposed to save(Object), remove(Object) does not throw OptimisticLockingFailureException if a versioned entity could not be removed. This behavior is subject to be changed in a future release. Throwing OptimisticLockingFailureException on failed delete on Template API level was not introduced to not break existing application code.

MongoRepository now also considers the entities version, following the very same logic as MongoOperations.
To remove an entity without version check use MongoOperations#remove(Query,…) or MongoRepository#deleteById(…).

Original pull request: #641.

Author: christophstrobl

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

@@ -199,6 +199,16 @@ interface Entity<T> {
 		 */
 		Query getByIdQuery();
 
+		/**
+		 * Returns the {@link Query} to remove an entity by its {@literal id} and if applicable {@literal version}.
+		 *
+		 * @return the {@link Query} to use for removing the entity. Never {@literal null}.
+		 * @since 2.2
+		 */
+		default Query getRemoveByQuery() {
+			return isVersionedEntity() ? getQueryForVersion() : getByIdQuery();
+		}
+
 		/**
 		 * Returns the {@link Query} to find the entity in its current version.
 		 *
@@ -490,10 +500,10 @@ public Query getByIdQuery() {
 		public Query getQueryForVersion() {
 
 			MongoPersistentProperty idProperty = entity.getRequiredIdProperty();
-			MongoPersistentProperty property = entity.getRequiredVersionProperty();
+			MongoPersistentProperty versionProperty = entity.getRequiredVersionProperty();
 
 			return new Query(Criteria.where(idProperty.getName()).is(getId())//
-					.and(property.getName()).is(getVersion()));
+					.and(versionProperty.getName()).is(getVersion()));
 		}
 
 		/*

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

@@ -1350,19 +1350,29 @@ <S, T> T findAndReplace(Query query, S replacement, FindAndReplaceOptions option
 	UpdateResult updateMulti(Query query, Update update, Class<?> entityClass, String collectionName);
 
 	/**
-	 * Remove the given object from the collection by id.
+	 * Remove the given object from the collection by {@literal id} and (if applicable) its
+	 * {@link org.springframework.data.annotation.Version}.
 	 *
 	 * @param object must not be {@literal null}.
 	 * @return the {@link DeleteResult} which lets you access the results of the previous delete.
+	 * @throws org.springframework.dao.OptimisticLockingFailureException if the given {@literal object} is
+	 *           {@link org.springframework.data.annotation.Version versioned} and the current version does not match the
+	 *           one within the store. If no document with matching {@literal _id} exists in the collection the operation
+	 *           completes without error.
 	 */
 	DeleteResult remove(Object object);
 
 	/**
-	 * Removes the given object from the given collection.
+	 * Removes the given object from the given collection by {@literal id} and (if applicable) its
+	 * {@link org.springframework.data.annotation.Version}.
 	 *
 	 * @param object must not be {@literal null}.
 	 * @param collectionName name of the collection where the objects will removed, must not be {@literal null} or empty.
 	 * @return the {@link DeleteResult} which lets you access the results of the previous delete.
+	 * @throws org.springframework.dao.OptimisticLockingFailureException if the given {@literal object} is
+	 *           {@link org.springframework.data.annotation.Version versioned} and the current version does not match the
+	 *           one within the store. If no document with matching {@literal _id} exists in the collection the operation
+	 *           completes without error.
 	 */
 	DeleteResult remove(Object object, String collectionName);
 

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

@@ -68,7 +68,16 @@
 import org.springframework.data.mongodb.core.aggregation.Fields;
 import org.springframework.data.mongodb.core.aggregation.TypeBasedAggregationOperationContext;
 import org.springframework.data.mongodb.core.aggregation.TypedAggregation;
-import org.springframework.data.mongodb.core.convert.*;
+import org.springframework.data.mongodb.core.convert.DbRefResolver;
+import org.springframework.data.mongodb.core.convert.DefaultDbRefResolver;
+import org.springframework.data.mongodb.core.convert.JsonSchemaMapper;
+import org.springframework.data.mongodb.core.convert.MappingMongoConverter;
+import org.springframework.data.mongodb.core.convert.MongoConverter;
+import org.springframework.data.mongodb.core.convert.MongoCustomConversions;
+import org.springframework.data.mongodb.core.convert.MongoJsonSchemaMapper;
+import org.springframework.data.mongodb.core.convert.MongoWriter;
+import org.springframework.data.mongodb.core.convert.QueryMapper;
+import org.springframework.data.mongodb.core.convert.UpdateMapper;
 import org.springframework.data.mongodb.core.index.IndexOperations;
 import org.springframework.data.mongodb.core.index.IndexOperationsProvider;
 import org.springframework.data.mongodb.core.index.MongoMappingEventPublisher;
@@ -1638,9 +1647,7 @@ public DeleteResult remove(Object object) {
 
 		Assert.notNull(object, "Object must not be null!");
 
-		Query query = operations.forEntity(object).getByIdQuery();
-
-		return remove(query, object.getClass());
+		return remove(object, operations.determineCollectionName(object.getClass()));
 	}
 
 	@Override
@@ -1649,7 +1656,7 @@ public DeleteResult remove(Object object, String collectionName) {
 		Assert.notNull(object, "Object must not be null!");
 		Assert.hasText(collectionName, "Collection name must not be null or empty!");
 
-		Query query = operations.forEntity(object).getByIdQuery();
+		Query query = operations.forEntity(object).getRemoveByQuery();
 
 		return doRemove(collectionName, query, object.getClass(), false);
 	}
@@ -1723,10 +1730,36 @@ public DeleteResult doInCollection(MongoCollection<Document> collection)
 				DeleteResult result = multi ? collectionToUse.deleteMany(removeQuery, options)
 						: collectionToUse.deleteOne(removeQuery, options);
 
+				checkForOptimisticLockingFailures(result, removeQuery, collectionToUse);
+
 				maybeEmitEvent(new AfterDeleteEvent<>(queryObject, entityClass, collectionName));
 
 				return result;
 			}
+
+			private void checkForOptimisticLockingFailures(DeleteResult result, Document removeQuery,
+					MongoCollection<Document> collectionToUse) {
+
+				if (multi || !ResultOperations.isUndecidedDeleteResult(result, removeQuery, entity)) {
+					return;
+				}
+
+				String versionFieldName = entity.getVersionProperty().getFieldName();
+				Document idQuery = new Document(removeQuery);
+				idQuery.remove(versionFieldName);
+
+				Iterator<Document> it = collectionToUse.find(idQuery).projection(Projections.include("_id", versionFieldName))
+						.limit(1).iterator();
+
+				if (it.hasNext()) {
+
+					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());
 	}
 
 	/*
@@ -1717,7 +1717,7 @@ public Mono<DeleteResult> remove(Object object) {
 
 		Assert.notNull(object, "Object must not be null!");
 
-		return remove(operations.forEntity(object).getByIdQuery(), object.getClass());
+		return remove(operations.forEntity(object).getRemoveByQuery(), object.getClass());
 	}
 
 	/*
@@ -1729,7 +1729,7 @@ public Mono<DeleteResult> remove(Object object, String collectionName) {
 		Assert.notNull(object, "Object must not be null!");
 		Assert.hasText(collectionName, "Collection name must not be null or empty!");
 
-		return doRemove(collectionName, operations.forEntity(object).getByIdQuery(), object.getClass());
+		return doRemove(collectionName, operations.forEntity(object).getRemoveByQuery(), object.getClass());
 	}
 
 	private void assertUpdateableIdIfNotSet(Object value) {
@@ -1787,15 +1787,14 @@ protected <T> Mono<DeleteResult> doRemove(String collectionName, Query query, @N
 
 		Document queryObject = query.getQueryObject();
 		MongoPersistentEntity<?> entity = getPersistentEntity(entityClass);
+		Document removeQuery = queryMapper.getMappedObject(queryObject, entity);
 
 		return execute(collectionName, collection -> {
 
-			Document removeQuey = queryMapper.getMappedObject(queryObject, entity);
-
-			maybeEmitEvent(new BeforeDeleteEvent<>(removeQuey, entityClass, collectionName));
+			maybeEmitEvent(new BeforeDeleteEvent<>(removeQuery, entityClass, collectionName));
 
 			MongoAction mongoAction = new MongoAction(writeConcern, MongoActionOperation.REMOVE, collectionName, entityClass,
-					null, removeQuey);
+					null, removeQuery);
 
 			DeleteOptions deleteOptions = new DeleteOptions();
 			query.getCollation().map(Collation::toMongoCollation).ifPresent(deleteOptions::collation);
@@ -1805,13 +1804,13 @@ protected <T> Mono<DeleteResult> doRemove(String collectionName, Query query, @N
 
 			if (LOGGER.isDebugEnabled()) {
 				LOGGER.debug("Remove using query: {} in collection: {}.",
-						new Object[] { serializeToJsonSafely(removeQuey), collectionName });
+						new Object[] { serializeToJsonSafely(removeQuery), collectionName });
 			}
 
 			if (query.getLimit() > 0 || query.getSkip() > 0) {
 
 				FindPublisher<Document> cursor = new QueryFindPublisherPreparer(query, entityClass)
-						.prepare(collection.find(removeQuey)) //
+						.prepare(collection.find(removeQuery)) //
 						.projection(MappedDocument.getIdOnlyProjection());
 
 				return Flux.from(cursor) //
@@ -1822,10 +1821,39 @@ protected <T> Mono<DeleteResult> doRemove(String collectionName, Query query, @N
 							return collectionToUse.deleteMany(MappedDocument.getIdIn(val), deleteOptions);
 						});
 			} else {
-				return collectionToUse.deleteMany(removeQuey, deleteOptions);
+				return collectionToUse.deleteMany(removeQuery, deleteOptions);
+			}
+
+		}).flatMap(deleteResult -> {
+
+			if (!ResultOperations.isUndecidedDeleteResult(deleteResult, removeQuery, entity)) {
+				return Mono.just(deleteResult);
 			}
 
-		}).doOnNext(deleteResult -> maybeEmitEvent(new AfterDeleteEvent<>(queryObject, entityClass, collectionName)))
+			return execute(collectionName, (coll -> {
+
+				String versionFieldName = entity.getVersionProperty().getFieldName();
+
+				Document queryWithoutVersion = new Document(removeQuery);
+				queryWithoutVersion.remove(versionFieldName);
+
+				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 -> {
+
+							if (it.isEmpty()) {
+								return Mono.just(deleteResult);
+							}
+
+							return Mono.error(ResultOperations.newDeleteVersionedOptimisticLockingException(it.get("_id"),
+									collectionName, removeQuery.get(versionFieldName), it.get(versionFieldName)));
+						});
+			})).next();
+
+		}).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/main/java/org/springframework/data/mongodb/repository/support/SimpleMongoRepository.java

@@ -161,7 +161,7 @@ public void delete(T entity) {
 
 		Assert.notNull(entity, "The given entity must not be null!");
 
-		deleteById(entityInformation.getRequiredId(entity));
+		mongoOperations.remove(entity, entityInformation.getCollectionName());
 	}
 
 	/*

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/support/SimpleReactiveMongoRepository.java

@@ -355,7 +355,7 @@ public Mono<Void> delete(T entity) {
 
 		Assert.notNull(entity, "The given entity must not be null!");
 
-		return deleteById(entityInformation.getRequiredId(entity));
+		return mongoOperations.remove(entity, entityInformation.getCollectionName()).then();
 	}
 
 	/*