DATAMONGO-686 - Fixed potential race-condition in QueryMapper.

We now create a new a new DBObject in QueryMapper#getMappedValue() instead of replacing them in the original objects in order to prevent the original query object being manipulated.

Added test case to verify that the original DBObject is not manipulated.

Original pull request: #111.

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2011-2013 the original author or authors.
+ * Copyright 2011-2014 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.
@@ -44,6 +44,7 @@ import com.mongodb.DBRef;
  * @author Jon Brisbin
  * @author Oliver Gierke
  * @author Patryk Wasik
+ * @author Thomas Darimont
  */
 public class QueryMapper {
 
@@ -176,20 +177,22 @@ public class QueryMapper {
 
 			if (value instanceof DBObject) {
 				DBObject valueDbo = (DBObject) value;
+				DBObject resultDbo = new BasicDBObject(valueDbo.toMap());
+
 				if (valueDbo.containsField("$in") || valueDbo.containsField("$nin")) {
 					String inKey = valueDbo.containsField("$in") ? "$in" : "$nin";
 					List<Object> ids = new ArrayList<Object>();
 					for (Object id : (Iterable<?>) valueDbo.get(inKey)) {
 						ids.add(convertId(id));
 					}
-					valueDbo.put(inKey, ids.toArray(new Object[ids.size()]));
+					resultDbo.put(inKey, ids.toArray(new Object[ids.size()]));
 				} else if (valueDbo.containsField("$ne")) {
-					valueDbo.put("$ne", convertId(valueDbo.get("$ne")));
+					resultDbo.put("$ne", convertId(valueDbo.get("$ne")));
 				} else {
-					return getMappedObject((DBObject) value, null);
+					return getMappedObject(resultDbo, null);
 				}
 
-				return valueDbo;
+				return resultDbo;
 
 			} else {
 				return convertId(value);

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

@@ -51,6 +51,7 @@ import org.springframework.dao.OptimisticLockingFailureException;
 import org.springframework.data.annotation.Id;
 import org.springframework.data.annotation.PersistenceConstructor;
 import org.springframework.data.annotation.Version;
+import org.springframework.data.domain.PageRequest;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.domain.Sort.Direction;
 import org.springframework.data.mapping.model.MappingException;
@@ -2158,6 +2159,29 @@ public class MongoTemplateTests {
 		}
 	}
 
+	/**
+	 * @see DATAMONGO-686
+	 */
+	@Test
+	public void itShouldBePossibleToReuseAnExistingQuery() {
+
+		Sample sample = new Sample();
+		sample.id = "42";
+		sample.field = "A";
+
+		template.save(sample);
+
+		Query query = new Query();
+		query.addCriteria(where("_id").in("42", "43"));
+
+		assertThat(template.count(query, Sample.class), is(1L));
+
+		query.with(new PageRequest(0, 10));
+		query.with(new Sort("field"));
+
+		assertThat(template.find(query, Sample.class), is(not(empty())));
+	}
+
 	static interface Model {
 		String value();
 

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2011-2013 the original author or authors.
+ * Copyright 2011-2014 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.
@@ -484,6 +484,23 @@ public class QueryMapperUnitTests {
 		assertThat(fieldsResult.get("reference"), is((Object) 0));
 	}
 
+	/**
+	 * @see DATAMONGO-686
+	 */
+	@Test
+	public void queryMapperShouldNotChangeStateInGivenQueryObjectWhenIdConstrainedByInList() {
+
+		BasicMongoPersistentEntity<?> persistentEntity = context.getPersistentEntity(Sample.class);
+		String idPropertyName = persistentEntity.getIdProperty().getName();
+		DBObject queryObject = query(where(idPropertyName).in("42")).getQueryObject();
+
+		Object idValuesBefore = getAsDBObject(queryObject, idPropertyName).get("$in");
+		mapper.getMappedObject(queryObject, persistentEntity);
+		Object idValuesAfter = getAsDBObject(queryObject, idPropertyName).get("$in");
+
+		assertThat(idValuesAfter, is(idValuesBefore));
+	}
+
 	class IdWrapper {
 		Object id;
 	}