DATADOC-240, DATADOC-212 - Overhaul of MongoTemplate.doUpdate(…).

Replaced manual ID conversion with delegating to QueryMapper. Added ObjectId as Mongo native type to CustomConversions and added unit tests around its handling.

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

@@ -676,23 +676,12 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware {
 
 		return execute(collectionName, new CollectionCallback<WriteResult>() {
 			public WriteResult doInCollection(DBCollection collection) throws MongoException, DataAccessException {
-				DBObject queryObj = query.getQueryObject();
+				
+				MongoPersistentEntity<?> entity = entityClass == null ? null : getPersistentEntity(entityClass);
+				
+				DBObject queryObj = mapper.getMappedObject(query.getQueryObject(), entity);
 				DBObject updateObj = update.getUpdateObject();
 
-				String idProperty = "id";
-				if (null != entityClass) {
-					idProperty = getPersistentEntity(entityClass).getIdProperty().getName();
-				}
-				for (String key : queryObj.keySet()) {
-					if (idProperty.equals(key)) {
-						// This is an ID field
-						queryObj.put(ID, mongoConverter.convertToMongoType(queryObj.get(key)));
-						queryObj.removeField(key);
-					} else {
-						queryObj.put(key, mongoConverter.convertToMongoType(queryObj.get(key)));
-					}
-				}
-
 				for (String key : updateObj.keySet()) {
 					updateObj.put(key, mongoConverter.convertToMongoType(updateObj.get(key)));
 				}

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

@@ -23,6 +23,7 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Set;
 
+import org.bson.types.ObjectId;
 import org.springframework.core.GenericTypeResolver;
 import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.core.convert.converter.Converter;
@@ -49,7 +50,7 @@ import com.mongodb.DBObject;
 public class CustomConversions {
 
 	@SuppressWarnings({ "unchecked" })
-	private static final List<Class<?>> MONGO_TYPES = Arrays.asList(Number.class, Date.class, String.class,
+	private static final List<Class<?>> MONGO_TYPES = Arrays.asList(Number.class, Date.class, ObjectId.class, String.class,
 			DBObject.class);
 
 	private final Set<ConvertiblePair> readingPairs;
@@ -78,6 +79,7 @@ public class CustomConversions {
 		this.readingPairs = new HashSet<ConvertiblePair>();
 		this.writingPairs = new HashSet<ConvertiblePair>();
 		this.customSimpleTypes = new HashSet<Class<?>>();
+		this.customSimpleTypes.add(ObjectId.class);
 
 		this.converters = new ArrayList<Object>();
 		this.converters.add(CustomToStringConverter.INSTANCE);

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

@@ -18,6 +18,8 @@ package org.springframework.data.mongodb.core;
 import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
 import static org.springframework.data.mongodb.core.query.Criteria.*;
+import static org.springframework.data.mongodb.core.query.Query.*;
+import static org.springframework.data.mongodb.core.query.Update.*;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -750,4 +752,24 @@ public class MongoTemplateTests {
 	public void removingNullIsANoOp() {
 		template.remove(null);
 	}
+
+	/**
+	 * @see DATADOC-240, DATADOC-212
+	 */
+	@Test
+	public void updatesObjectIdsCorrectly() {
+		
+		PersonWithIdPropertyOfTypeObjectId person = new PersonWithIdPropertyOfTypeObjectId();
+		person.setId(new ObjectId());
+		person.setFirstName("Dave");
+		
+		template.save(person);
+		template.updateFirst(query(where("id").is(person.getId())), update("firstName", "Carter"),
+				PersonWithIdPropertyOfTypeObjectId.class);
+		
+		PersonWithIdPropertyOfTypeObjectId result = template.findById(person.getId(), PersonWithIdPropertyOfTypeObjectId.class);
+		assertThat(result, is(notNullValue()));
+		assertThat(result.getId(), is(person.getId()));
+		assertThat(result.getFirstName(), is("Carter"));
+	}
 }

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

@@ -1,17 +1,17 @@
 package org.springframework.data.mongodb.core.convert;
 
-import static org.junit.Assert.*;
 import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
 
 import java.util.Arrays;
 import java.util.Locale;
 import java.util.UUID;
 
+import org.bson.types.ObjectId;
 import org.junit.Test;
 import org.springframework.core.convert.converter.Converter;
 import org.springframework.core.convert.support.ConversionServiceFactory;
 import org.springframework.core.convert.support.GenericConversionService;
-import org.springframework.data.mongodb.core.convert.CustomConversions;
 
 /**
  * Unit tests for {@link CustomConversions}.
@@ -52,6 +52,37 @@ public class CustomConversionsUnitTests {
 		assertThat(conversions.isSimpleType(UUID.class), is(true));
 	}
 
+	/**
+	 * @see DATADOC-240
+	 */
+	@Test
+	public void considersObjectIdToBeSimpleType() {
+		
+		CustomConversions conversions = new CustomConversions();
+		assertThat(conversions.isSimpleType(ObjectId.class), is(true));
+		assertThat(conversions.hasCustomWriteTarget(ObjectId.class), is(false));
+		
+		
+	}
+	
+	/**
+	 * @see DATADOC-240
+	 */
+	@Test
+	public void considersCustomConverterForSimpleType() {
+		
+		CustomConversions conversions = new CustomConversions(Arrays.asList(new Converter<ObjectId, String>() {
+			public String convert(ObjectId source) {
+				return source == null ? null : source.toString();
+			}
+		}));
+		
+		assertThat(conversions.isSimpleType(ObjectId.class), is(true));
+		assertThat(conversions.hasCustomWriteTarget(ObjectId.class), is(true));
+		assertThat(conversions.hasCustomReadTarget(ObjectId.class, String.class), is(true));
+		assertThat(conversions.hasCustomReadTarget(ObjectId.class, Object.class), is(false));
+	}
+	
 	@Test
 	public void populatesConversionServiceCorrectly() {