DATAMONGO-1870 - Consider skip/limit on MongoOperations.remove(Query, Class).

We now use _id lookup for remove operations that query with limit or skip parameters. This allows more fine grained control over documents removed.

Original pull request: #531.
Related pull request: #532.

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

@@ -649,8 +649,8 @@ public interface MongoOperations {
 	<T> T findById(Object id, Class<T> entityClass, String collectionName);
 
 	/**
-	 * Triggers <a href="http://docs.mongodb.org/manual/reference/method/db.collection.findAndModify/">findAndModify
-	 * <a/> to apply provided {@link Update} on documents matching {@link Criteria} of given {@link Query}.
+	 * Triggers <a href="http://docs.mongodb.org/manual/reference/method/db.collection.findAndModify/">findAndModify <a/>
+	 * to apply provided {@link Update} on documents matching {@link Criteria} of given {@link Query}.
 	 *
 	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a record and also an optional
 	 *          fields specification.
@@ -661,8 +661,8 @@ public interface MongoOperations {
 	<T> T findAndModify(Query query, Update update, Class<T> entityClass);
 
 	/**
-	 * Triggers <a href="http://docs.mongodb.org/manual/reference/method/db.collection.findAndModify/">findAndModify
-	 * <a/> to apply provided {@link Update} on documents matching {@link Criteria} of given {@link Query}.
+	 * Triggers <a href="http://docs.mongodb.org/manual/reference/method/db.collection.findAndModify/">findAndModify <a/>
+	 * to apply provided {@link Update} on documents matching {@link Criteria} of given {@link Query}.
 	 *
 	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a record and also an optional
 	 *          fields specification.
@@ -674,8 +674,8 @@ public interface MongoOperations {
 	<T> T findAndModify(Query query, Update update, Class<T> entityClass, String collectionName);
 
 	/**
-	 * Triggers <a href="http://docs.mongodb.org/manual/reference/method/db.collection.findAndModify/">findAndModify
-	 * <a/> to apply provided {@link Update} on documents matching {@link Criteria} of given {@link Query} taking
+	 * Triggers <a href="http://docs.mongodb.org/manual/reference/method/db.collection.findAndModify/">findAndModify <a/>
+	 * to apply provided {@link Update} on documents matching {@link Criteria} of given {@link Query} taking
 	 * {@link FindAndModifyOptions} into account.
 	 *
 	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a record and also an optional
@@ -688,8 +688,8 @@ public interface MongoOperations {
 	<T> T findAndModify(Query query, Update update, FindAndModifyOptions options, Class<T> entityClass);
 
 	/**
-	 * Triggers <a href="http://docs.mongodb.org/manual/reference/method/db.collection.findAndModify/">findAndModify
-	 * <a/> to apply provided {@link Update} on documents matching {@link Criteria} of given {@link Query} taking
+	 * Triggers <a href="http://docs.mongodb.org/manual/reference/method/db.collection.findAndModify/">findAndModify <a/>
+	 * to apply provided {@link Update} on documents matching {@link Criteria} of given {@link Query} taking
 	 * {@link FindAndModifyOptions} into account.
 	 *
 	 * @param query the {@link Query} class that specifies the {@link Criteria} used to find a record and also an optional
@@ -994,8 +994,9 @@ public interface MongoOperations {
 	 * Remove all documents that match the provided query document criteria from the the collection used to store the
 	 * entityClass. The Class parameter is also used to help convert the Id of the object if it is present in the query.
 	 *
-	 * @param query
-	 * @param entityClass
+	 * @param query must not be {@literal null}.
+	 * @param entityClass must not be {@literal null}.
+	 * @throws IllegalArgumentException when {@literal query} or {@literal entityClass} is {@literal null}.
 	 */
 	WriteResult remove(Query query, Class<?> entityClass);
 
@@ -1006,6 +1007,8 @@ public interface MongoOperations {
 	 * @param query
 	 * @param entityClass
 	 * @param collectionName
+	 * @throws IllegalArgumentException when {@literal query}, {@literal entityClass} or {@literal collectionName} is
+	 *           {@literal null}.
 	 */
 	WriteResult remove(Query query, Class<?> entityClass, String collectionName);
 
@@ -1017,6 +1020,7 @@ public interface MongoOperations {
 	 *
 	 * @param query the query document that specifies the criteria used to remove a record
 	 * @param collectionName name of the collection where the objects will removed
+	 * @throws IllegalArgumentException when {@literal query} or {@literal collectionName} is {@literal null}.
 	 */
 	WriteResult remove(Query query, String collectionName);
 

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

@@ -19,17 +19,8 @@ import static org.springframework.data.mongodb.core.query.Criteria.*;
 import static org.springframework.data.mongodb.core.query.SerializationUtils.*;
 
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 import java.util.Map.Entry;
-import java.util.Scanner;
-import java.util.Set;
 
 import org.bson.types.ObjectId;
 import org.slf4j.Logger;
@@ -108,21 +99,7 @@ import org.springframework.util.ObjectUtils;
 import org.springframework.util.ResourceUtils;
 import org.springframework.util.StringUtils;
 
-import com.mongodb.BasicDBObject;
-import com.mongodb.Bytes;
-import com.mongodb.CommandResult;
-import com.mongodb.Cursor;
-import com.mongodb.DB;
-import com.mongodb.DBCollection;
-import com.mongodb.DBCursor;
-import com.mongodb.DBObject;
-import com.mongodb.MapReduceCommand;
-import com.mongodb.MapReduceOutput;
-import com.mongodb.Mongo;
-import com.mongodb.MongoException;
-import com.mongodb.ReadPreference;
-import com.mongodb.WriteConcern;
-import com.mongodb.WriteResult;
+import com.mongodb.*;
 import com.mongodb.util.JSON;
 import com.mongodb.util.JSONParseException;
 
@@ -1324,35 +1301,46 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware {
 
 	protected <T> WriteResult doRemove(final String collectionName, final Query query, final Class<T> entityClass) {
 
-		if (query == null) {
-			throw new InvalidDataAccessApiUsageException("Query passed in to remove can't be null!");
-		}
-
+		Assert.notNull(query, "Query must not be null!");
 		Assert.hasText(collectionName, "Collection name must not be null or empty!");
 
-		final DBObject queryObject = query.getQueryObject();
 		final MongoPersistentEntity<?> entity = getPersistentEntity(entityClass);
+		final DBObject queryObject = queryMapper.getMappedObject(query.getQueryObject(), entity);
 
 		return execute(collectionName, new CollectionCallback<WriteResult>() {
 			public WriteResult doInCollection(DBCollection collection) throws MongoException, DataAccessException {
 
 				maybeEmitEvent(new BeforeDeleteEvent<T>(queryObject, entityClass, collectionName));
 
-				DBObject dboq = queryMapper.getMappedObject(queryObject, entity);
+				DBObject removeQuery = queryObject;
 
 				MongoAction mongoAction = new MongoAction(writeConcern, MongoActionOperation.REMOVE, collectionName,
-						entityClass, null, queryObject);
+						entityClass, null, removeQuery);
 				WriteConcern writeConcernToUse = prepareWriteConcern(mongoAction);
 
 				if (LOGGER.isDebugEnabled()) {
 					LOGGER.debug("Remove using query: {} in collection: {}.",
-							new Object[] { serializeToJsonSafely(dboq), collectionName });
+							new Object[] { serializeToJsonSafely(removeQuery), collectionName });
+				}
+
+				if (query.getLimit() > 0 || query.getSkip() > 0) {
+
+					DBCursor cursor = new QueryCursorPreparer(query, entityClass)
+							.prepare(collection.find(removeQuery, new BasicDBObject(ID_FIELD, 1)));
+
+					Set<Object> ids = new LinkedHashSet<Object>();
+					Iterator<DBObject> it = cursor.iterator();
+					while (it.hasNext()) {
+						ids.add(it.next().get(ID_FIELD));
+					}
+
+					removeQuery = new BasicDBObject(ID_FIELD, new BasicDBObject("$in", ids));
 				}
 
-				WriteResult wr = writeConcernToUse == null ? collection.remove(dboq)
-						: collection.remove(dboq, writeConcernToUse);
+				WriteResult wr = writeConcernToUse == null ? collection.remove(removeQuery)
+						: collection.remove(removeQuery, writeConcernToUse);
 
-				handleAnyWriteResultErrors(wr, dboq, MongoActionOperation.REMOVE);
+				handleAnyWriteResultErrors(wr, removeQuery, MongoActionOperation.REMOVE);
 
 				maybeEmitEvent(new AfterDeleteEvent<T>(queryObject, entityClass, collectionName));
 
@@ -2582,8 +2570,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware {
 		 */
 		DBObject aggregate(String collectionName, Aggregation aggregation, AggregationOperationContext context) {
 
-			DBObject command = prepareAggregationCommand(collectionName, aggregation,
-					context, batchSize);
+			DBObject command = prepareAggregationCommand(collectionName, aggregation, context, batchSize);
 
 			if (LOGGER.isDebugEnabled()) {
 				LOGGER.debug("Executing aggregation: {}", serializeToJsonSafely(command));

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java

@@ -31,18 +31,7 @@ import lombok.NoArgsConstructor;
 import java.lang.reflect.InvocationTargetException;
 import java.math.BigDecimal;
 import java.math.BigInteger;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Calendar;
-import java.util.Collections;
-import java.util.Date;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Locale;
-import java.util.Map;
-import java.util.UUID;
+import java.util.*;
 
 import org.bson.types.ObjectId;
 import org.joda.time.DateTime;
@@ -97,17 +86,7 @@ import org.springframework.util.ClassUtils;
 import org.springframework.util.ObjectUtils;
 import org.springframework.util.StringUtils;
 
-import com.mongodb.BasicDBObject;
-import com.mongodb.CommandResult;
-import com.mongodb.DBCollection;
-import com.mongodb.DBCursor;
-import com.mongodb.DBObject;
-import com.mongodb.DBRef;
-import com.mongodb.Mongo;
-import com.mongodb.MongoException;
-import com.mongodb.ReadPreference;
-import com.mongodb.WriteConcern;
-import com.mongodb.WriteResult;
+import com.mongodb.*;
 
 /**
  * Integration test for {@link MongoTemplate}.
@@ -3233,6 +3212,34 @@ public class MongoTemplateTests {
 		assertThat(template.count(new BasicQuery("{}"), template.determineCollectionName(Sample.class)), is(equalTo(1L)));
 	}
 
+	@Test // DATAMONGO-1870
+	public void removeShouldConsiderLimit() {
+
+		for (int i = 0; i < 100; i++) {
+			template.save(new Sample("id-" + i, i % 2 == 0 ? "stark" : "lannister"));
+		}
+
+		WriteResult wr = template.remove(query(where("field").is("lannister")).limit(25), Sample.class);
+
+		assertThat(wr.getN(), is(25));
+		assertThat(template.count(new Query(), Sample.class), is(75L));
+	}
+
+	@Test // DATAMONGO-1870
+	public void removeShouldConsiderSkipAndSort() {
+
+		for (int i = 0; i < 100; i++) {
+			template.save(new Sample("id-" + i, i % 2 == 0 ? "stark" : "lannister"));
+		}
+
+		WriteResult wr = template.remove(new Query().skip(25).with(new Sort("field")), Sample.class);
+
+		assertThat(wr.getN(), is(75));
+		assertThat(template.count(new Query(), Sample.class), is(25L));
+		assertThat(template.count(query(where("field").is("lannister")), Sample.class), is(25L));
+		assertThat(template.count(query(where("field").is("stark")), Sample.class), is(0L));
+	}
+
 	static class TypeWithNumbers {
 
 		@Id String id;

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java

@@ -124,7 +124,7 @@ public class MongoTemplateUnitTests extends MongoOperationsUnitTests {
 		new MongoTemplate(null, "database");
 	}
 
-	@Test(expected = DataAccessException.class)
+	@Test(expected = IllegalArgumentException.class) // DATAMONGO-1870
 	public void removeHandlesMongoExceptionProperly() throws Exception {
 		MongoTemplate template = mockOutGetDb();
 		when(db.getCollection("collection")).thenThrow(new MongoException("Exception!"));

src/main/asciidoc/reference/mongodb.adoc

@@ -976,7 +976,25 @@ assertThat(p.getAge(), is(1));
 
 You can use several overloaded methods to remove an object from the database.
 
-* *remove* Remove the given document based on one of the following: a specific object instance, a query document criteria combined with a class or a query document criteria combined with a specific collection name.
+====
+[source,java]
+----
+template.remove(tywin, "GOT");                                              <1>
+
+template.remove(query(where("lastname").is("lannister")), "GOT");           <2>
+
+template.remove(new Query().limit(3), "GOT");                               <3>
+
+template.findAllAndRemove(query(where("lastname").is("lannister"), "GOT");  <4>
+
+template.findAllAndRemove(new Query().limit(3), "GOT");                     <5>
+----
+<1> Remove a single entity via its `id` from the associated collection.
+<2> Remove all documents matching the criteria of the query from the `GOT` collection.
+<3> Rewmove the first 3 documents in the `GOT` collection. Unlike <2> the documents to remove are identified via their `id` using the given query applying `sort`, `limit` and `skip` options and then removed all at once in a seperate step.
+<4> Remove all documents matching the criteria of the query from the `GOT` collection. Unlike <3> documents do not get deleted in a batch but one by one.
+<5> Remove the first 3 documents in the `GOT` collection. Unlike <3> documents do not get deleted in a batch but one by one.
+====
 
 [[mongo-template.optimistic-locking]]
 === Optimistic locking