DATAMONGO-2195 - Consider version when removing an entity.

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-mongodb-parent</artifactId>
-	<version>2.2.0.BUILD-SNAPSHOT</version>
+	<version>2.2.0.DATAMONGO-2195-SNAPSHOT</version>
 	<packaging>pom</packaging>
 
 	<name>Spring Data MongoDB</name>

spring-data-mongodb-benchmarks/pom.xml

@@ -7,7 +7,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-mongodb-parent</artifactId>
-		<version>2.2.0.BUILD-SNAPSHOT</version>
+		<version>2.2.0.DATAMONGO-2195-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

spring-data-mongodb-cross-store/pom.xml

@@ -6,7 +6,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-mongodb-parent</artifactId>
-		<version>2.2.0.BUILD-SNAPSHOT</version>
+		<version>2.2.0.DATAMONGO-2195-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 
@@ -50,7 +50,7 @@
 		<dependency>
 			<groupId>org.springframework.data</groupId>
 			<artifactId>spring-data-mongodb</artifactId>
-			<version>2.2.0.BUILD-SNAPSHOT</version>
+			<version>2.2.0.DATAMONGO-2195-SNAPSHOT</version>
 		</dependency>
 
 		<!-- reactive -->

spring-data-mongodb-distribution/pom.xml

@@ -14,7 +14,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-mongodb-parent</artifactId>
-		<version>2.2.0.BUILD-SNAPSHOT</version>
+		<version>2.2.0.DATAMONGO-2195-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

spring-data-mongodb/pom.xml

@@ -11,7 +11,7 @@
 	<parent>
 		<groupId>org.springframework.data</groupId>
 		<artifactId>spring-data-mongodb-parent</artifactId>
-		<version>2.2.0.BUILD-SNAPSHOT</version>
+		<version>2.2.0.DATAMONGO-2195-SNAPSHOT</version>
 		<relativePath>../pom.xml</relativePath>
 	</parent>
 

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.
 		 *
@@ -229,9 +239,11 @@ default boolean isVersionedEntity() {
 		}
 
 		/**
-		 * Returns the value of the version if the entity has a version property, {@literal null} otherwise.
+		 * Returns the value of the version if the entity {@link #isVersionedEntity() has a version property}.
 		 *
-		 * @return
+		 * @return the entity version. Can be {@literal null}.
+		 * @throws IllegalStateException if the entity does not define a {@literal version} property. Make sure to check
+		 *           {@link #isVersionedEntity()}.
 		 */
 		@Nullable
 		Object getVersion();
@@ -287,8 +299,8 @@ interface AdaptibleEntity<T> extends Entity<T> {
 		/**
 		 * Returns the current version value if the entity has a version property.
 		 *
-		 * @return the current version or {@literal null} in case it's uninitialized or the entity doesn't expose a version
-		 *         property.
+		 * @return the current version or {@literal null} in case it's uninitialized.
+		 * @throws IllegalStateException if the entity does not define a {@literal version} property.
 		 */
 		@Nullable
 		Number getVersion();
@@ -490,10 +502,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,15 +1350,21 @@ <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}. <br />
+	 * Use {@link DeleteResult#getDeletedCount()} for insight whether an {@link DeleteResult#wasAcknowledged()
+	 * acknowledged} remove operation was successful or not.
 	 *
 	 * @param object must not be {@literal null}.
 	 * @return the {@link DeleteResult} which lets you access the results of the previous delete.
 	 */
 	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}. <br />
+	 * Use {@link DeleteResult#getDeletedCount()} for insight whether an {@link DeleteResult#wasAcknowledged()
+	 * acknowledged} remove operation was successful or not.
 	 *
 	 * @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.

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);
 	}
@@ -1721,7 +1728,7 @@ public DeleteResult doInCollection(MongoCollection<Document> collection)
 						: collection;
 
 				DeleteResult result = multi ? collectionToUse.deleteMany(removeQuery, options)
-						: collection.deleteOne(removeQuery, options);
+						: collectionToUse.deleteOne(removeQuery, options);
 
 				maybeEmitEvent(new AfterDeleteEvent<>(queryObject, entityClass, collectionName));
 

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,10 @@ 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);
 			}
 
-		}).doOnNext(deleteResult -> maybeEmitEvent(new AfterDeleteEvent<>(queryObject, entityClass, collectionName)))
+		}).doOnNext(it -> maybeEmitEvent(new AfterDeleteEvent<>(queryObject, entityClass, collectionName))) //
 				.next();
 	}
 

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/MongoEntityInformation.java

@@ -16,11 +16,13 @@
 package org.springframework.data.mongodb.repository.query;
 
 import org.springframework.data.repository.core.EntityInformation;
+import org.springframework.lang.Nullable;
 
 /**
  * Mongo specific {@link EntityInformation}.
  *
  * @author Oliver Gierke
+ * @author Mark Paluch
  */
 public interface MongoEntityInformation<T, ID> extends EntityInformation<T, ID> {
 
@@ -37,4 +39,26 @@ public interface MongoEntityInformation<T, ID> extends EntityInformation<T, ID>
 	 * @return
 	 */
 	String getIdAttribute();
+
+	/**
+	 * Returns whether the entity uses optimistic locking.
+	 *
+	 * @return true if the entity defines a {@link org.springframework.data.annotation.Version} property.
+	 * @since 2.2
+	 */
+	default boolean isVersioned() {
+		return false;
+	}
+
+	/**
+	 * Returns the version value for the entity or {@literal null} if the entity is not {@link #isVersioned() versioned}.
+	 *
+	 * @param entity must not be {@literal null}
+	 * @return can be {@literal null}.
+	 * @since 2.2
+	 */
+	@Nullable
+	default Object getVersion(T entity) {
+		return null;
+	}
 }

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

@@ -16,6 +16,7 @@
 package org.springframework.data.mongodb.repository.support;
 
 import org.bson.types.ObjectId;
+import org.springframework.data.mapping.PersistentPropertyAccessor;
 import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity;
 import org.springframework.data.mongodb.repository.query.MongoEntityInformation;
 import org.springframework.data.repository.core.support.PersistentEntityInformation;
@@ -87,14 +88,16 @@ private MappingMongoEntityInformation(MongoPersistentEntity<T> entity, @Nullable
 		this.fallbackIdType = idType != null ? idType : (Class<ID>) ObjectId.class;
 	}
 
-	/* (non-Javadoc)
+	/*
+	 * (non-Javadoc)
 	 * @see org.springframework.data.mongodb.repository.MongoEntityInformation#getCollectionName()
 	 */
 	public String getCollectionName() {
 		return customCollectionName == null ? entityMetadata.getCollection() : customCollectionName;
 	}
 
-	/* (non-Javadoc)
+	/*
+	 * (non-Javadoc)
 	 * @see org.springframework.data.mongodb.repository.MongoEntityInformation#getIdAttribute()
 	 */
 	public String getIdAttribute() {
@@ -106,7 +109,6 @@ public String getIdAttribute() {
 	 * @see org.springframework.data.repository.core.support.PersistentEntityInformation#getIdType()
 	 */
 	@Override
-	@SuppressWarnings("unchecked")
 	public Class<ID> getIdType() {
 
 		if (this.entityMetadata.hasIdProperty()) {
@@ -115,4 +117,30 @@ public Class<ID> getIdType() {
 
 		return fallbackIdType;
 	}
+
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.mongodb.repository.MongoEntityInformation#isVersioned()
+	 */
+	@Override
+	public boolean isVersioned() {
+		return this.entityMetadata.hasVersionProperty();
+	}
+
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.mongodb.repository.MongoEntityInformation#getVersion(Object)
+	 */
+	@Override
+	public Object getVersion(T entity) {
+
+		if (!isVersioned()) {
+			return null;
+		}
+
+		PersistentPropertyAccessor<T> accessor = this.entityMetadata.getPropertyAccessor(entity);
+
+		return accessor.getProperty(this.entityMetadata.getRequiredVersionProperty());
+	}
+
 }

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

@@ -23,6 +23,7 @@
 import java.util.Optional;
 import java.util.stream.Collectors;
 
+import org.springframework.dao.OptimisticLockingFailureException;
 import org.springframework.data.domain.Example;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.PageImpl;
@@ -40,6 +41,8 @@
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 
+import com.mongodb.client.result.DeleteResult;
+
 /**
  * Repository base implementation for Mongo.
  *
@@ -161,7 +164,14 @@ public void delete(T entity) {
 
 		Assert.notNull(entity, "The given entity must not be null!");
 
-		deleteById(entityInformation.getRequiredId(entity));
+		DeleteResult deleteResult = mongoOperations.remove(entity, entityInformation.getCollectionName());
+
+		if (entityInformation.isVersioned() && deleteResult.wasAcknowledged() && deleteResult.getDeletedCount() == 0) {
+			throw new OptimisticLockingFailureException(String.format(
+					"The entity with id %s with version %s in %s cannot be deleted! Was it modified or deleted in the meantime?",
+					entityInformation.getId(entity), entityInformation.getVersion(entity),
+					entityInformation.getCollectionName()));
+		}
 	}
 
 	/*