DATAMONGO-812 - Polishing.

Changed convertToMongoType(…) to forward type hints to recursive calls to make sure type information is written if a TypeInformation was provided initially. Make sure that UpdateMapper hands in an initial type hint to the converter to make sure type information gets written.

Changed the signature of QueryMapper.getMappedObjectForField(…) to allow customizing the entire entry being added to the result. This is in preparation of more advanced mappings that might have to customize the mapped key.

Fixed newly introduced test cases in MongoTemplateTests.

Original pull request: #112.

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

@@ -56,7 +56,6 @@ import org.springframework.data.util.ClassTypeInformation;
 import org.springframework.data.util.TypeInformation;
 import org.springframework.expression.spel.standard.SpelExpressionParser;
 import org.springframework.util.Assert;
-import org.springframework.util.ClassUtils;
 import org.springframework.util.CollectionUtils;
 
 import com.mongodb.BasicDBList;
@@ -904,15 +903,17 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 			return getPotentiallyConvertedSimpleWrite(obj);
 		}
 
+		TypeInformation<?> typeHint = typeInformation == null ? null : ClassTypeInformation.OBJECT;
+
 		if (obj instanceof BasicDBList) {
-			return maybeConvertList((BasicDBList) obj);
+			return maybeConvertList((BasicDBList) obj, typeHint);
 		}
 
 		if (obj instanceof DBObject) {
 			DBObject newValueDbo = new BasicDBObject();
 			for (String vk : ((DBObject) obj).keySet()) {
 				Object o = ((DBObject) obj).get(vk);
-				newValueDbo.put(vk, convertToMongoType(o));
+				newValueDbo.put(vk, convertToMongoType(o, typeHint));
 			}
 			return newValueDbo;
 		}
@@ -920,17 +921,17 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 		if (obj instanceof Map) {
 			DBObject result = new BasicDBObject();
 			for (Map.Entry<Object, Object> entry : ((Map<Object, Object>) obj).entrySet()) {
-				result.put(entry.getKey().toString(), convertToMongoType(entry.getValue()));
+				result.put(entry.getKey().toString(), convertToMongoType(entry.getValue(), typeHint));
 			}
 			return result;
 		}
 
 		if (obj.getClass().isArray()) {
-			return maybeConvertList(Arrays.asList((Object[]) obj));
+			return maybeConvertList(Arrays.asList((Object[]) obj), typeHint);
 		}
 
 		if (obj instanceof Collection) {
-			return maybeConvertList((Collection<?>) obj);
+			return maybeConvertList((Collection<?>) obj, typeHint);
 		}
 
 		DBObject newDbo = new BasicDBObject();
@@ -943,25 +944,16 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 		return !obj.getClass().equals(typeInformation.getType()) ? newDbo : removeTypeInfoRecursively(newDbo);
 	}
 
-	public BasicDBList maybeConvertList(Iterable<?> source) {
+	public BasicDBList maybeConvertList(Iterable<?> source, TypeInformation<?> typeInformation) {
 
 		BasicDBList newDbl = new BasicDBList();
 		for (Object element : source) {
-			Object convertedTypeDdo = convertToMongoType(element);
-			if (!isSimpleOrCollectionType(element.getClass())) {
-				this.getTypeMapper().writeType(element.getClass(), (DBObject) convertedTypeDdo);
-			}
-			newDbl.add(convertedTypeDdo);
+			newDbl.add(convertToMongoType(element, typeInformation));
 		}
 
 		return newDbl;
 	}
 
-	private boolean isSimpleOrCollectionType(Class<?> type) {
-		return this.conversions.isSimpleType(type) || type.getClass().isArray()
-				|| ClassUtils.isAssignable(Collection.class, type);
-	}
-
 	/**
 	 * Removes the type information from the conversion result.
 	 * 

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

@@ -17,7 +17,9 @@ package org.springframework.data.mongodb.core.convert;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
+import java.util.Map.Entry;
 import java.util.Set;
 
 import org.bson.types.ObjectId;
@@ -106,11 +108,9 @@ public class QueryMapper {
 			}
 
 			Field field = createPropertyField(entity, key, mappingContext);
+			Entry<String, Object> entry = getMappedObjectForField(field, query.get(key));
 
-			Object rawValue = query.get(key);
-			String newKey = field.getMappedKey();
-
-			result.put(newKey, getMappedObjectForField(field, rawValue));
+			result.put(entry.getKey(), entry.getValue());
 		}
 
 		return result;
@@ -123,14 +123,19 @@ public class QueryMapper {
 	 * @param rawValue
 	 * @return
 	 */
-	protected Object getMappedObjectForField(Field field, Object rawValue) {
+	protected Entry<String, Object> getMappedObjectForField(Field field, Object rawValue) {
+
+		String key = field.getMappedKey();
+		Object value;
 
 		if (isNestedKeyword(rawValue) && !field.isIdField()) {
 			Keyword keyword = new Keyword((DBObject) rawValue);
-			return getMappedKeyword(field, keyword);
+			value = getMappedKeyword(field, keyword);
+		} else {
+			value = getMappedValue(field, rawValue);
 		}
 
-		return getMappedValue(field, rawValue);
+		return Collections.singletonMap(key, value).entrySet().iterator().next();
 	}
 
 	/**

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

@@ -16,7 +16,9 @@
 package org.springframework.data.mongodb.core.convert;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Iterator;
+import java.util.Map.Entry;
 
 import org.springframework.core.convert.converter.Converter;
 import org.springframework.data.mapping.context.MappingContext;
@@ -25,6 +27,7 @@ import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty;
 import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty.PropertyToFieldNameConverter;
 import org.springframework.data.mongodb.core.query.Update.Modifier;
 import org.springframework.data.mongodb.core.query.Update.Modifiers;
+import org.springframework.data.util.ClassTypeInformation;
 import org.springframework.util.Assert;
 
 import com.mongodb.BasicDBObject;
@@ -70,33 +73,43 @@ public class UpdateMapper extends QueryMapper {
 	 * @see org.springframework.data.mongodb.core.convert.QueryMapper#getMappedObjectForField(org.springframework.data.mongodb.core.convert.QueryMapper.Field, java.lang.Object)
 	 */
 	@Override
-	protected Object getMappedObjectForField(Field field, Object rawValue) {
+	protected Entry<String, Object> getMappedObjectForField(Field field, Object rawValue) {
 
 		if (!isUpdateModifier(rawValue)) {
 			return super.getMappedObjectForField(field, rawValue);
 		}
 
+		Object value = null;
+
 		if (rawValue instanceof Modifier) {
-			return getMappedValue((Modifier) rawValue);
+
+			value = getMappedValue((Modifier) rawValue);
+
 		} else if (rawValue instanceof Modifiers) {
 
 			DBObject modificationOperations = new BasicDBObject();
+
 			for (Modifier modifier : ((Modifiers) rawValue).getModifiers()) {
 				modificationOperations.putAll(getMappedValue(modifier).toMap());
 			}
 
-			return modificationOperations;
+			value = modificationOperations;
+		} else {
+
+			throw new IllegalArgumentException(String.format("Unable to map value of type '%s'!", rawValue.getClass()));
 		}
 
-		throw new IllegalArgumentException(String.format("Unable to map value of type '%s'!", rawValue.getClass()));
+		return Collections.singletonMap(field.getMappedKey(), value).entrySet().iterator().next();
 	}
 
 	private boolean isUpdateModifier(Object value) {
-		return (value instanceof Modifier || value instanceof Modifiers);
+		return value instanceof Modifier || value instanceof Modifiers;
 	}
 
 	private DBObject getMappedValue(Modifier modifier) {
-		return new BasicDBObject(modifier.getKey(), this.converter.convertToMongoType(modifier.getValue()));
+
+		Object value = converter.convertToMongoType(modifier.getValue(), ClassTypeInformation.OBJECT);
+		return new BasicDBObject(modifier.getKey(), value);
 	}
 
 	/* 

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

@@ -26,6 +26,7 @@ import static org.springframework.data.mongodb.core.query.Update.*;
 import java.math.BigInteger;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -2213,16 +2214,16 @@ public class MongoTemplateTests {
 	@Test
 	public void updatesShouldRetainTypeInformationEvenForCollections() {
 
-		DocumentWithCollection doc = new DocumentWithCollection();
+		List<Model> models = Arrays.<Model> asList(new ModelA("foo"));
+
+		DocumentWithCollection doc = new DocumentWithCollection(models);
 		doc.id = "4711";
-		doc.model = new ArrayList<Model>();
-		doc.model.add(new ModelA("foo"));
 		template.insert(doc);
 
 		Query query = new Query(Criteria.where("id").is(doc.id));
-		query.addCriteria(where("model.value").is("foo"));
+		query.addCriteria(where("models.value").is("foo"));
 		String newModelValue = "bar";
-		Update update = Update.update("model.$", new ModelA(newModelValue));
+		Update update = Update.update("models.$", new ModelA(newModelValue));
 		template.updateFirst(query, update, DocumentWithCollection.class);
 
 		Query findQuery = new Query(Criteria.where("id").is(doc.id));
@@ -2230,9 +2231,9 @@ public class MongoTemplateTests {
 
 		assertThat(result, is(notNullValue()));
 		assertThat(result.id, is(doc.id));
-		assertThat(result.model, is(notNullValue()));
-		assertThat(result.model, hasSize(1));
-		assertThat(result.model.get(0).value(), is(newModelValue));
+		assertThat(result.models, is(notNullValue()));
+		assertThat(result.models, hasSize(1));
+		assertThat(result.models.get(0).value(), is(newModelValue));
 	}
 
 	/**
@@ -2241,15 +2242,15 @@ public class MongoTemplateTests {
 	@Test
 	public void updateMultiShouldAddValuesCorrectlyWhenUsingPushEachWithComplexTypes() {
 
-		DocumentWithCollection document = new DocumentWithCollection(new ModelA("model-a"));
+		DocumentWithCollection document = new DocumentWithCollection(Collections.<Model> emptyList());
 		template.save(document);
 		Query query = query(where("id").is(document.id));
-		assumeThat(template.findOne(query, DocumentWithCollection.class).model, hasSize(1));
+		assumeThat(template.findOne(query, DocumentWithCollection.class).models, hasSize(1));
 
-		Update update = new Update().push("model").each(new ModelA("model-b"), new ModelA("model-c"));
+		Update update = new Update().push("models").each(new ModelA("model-b"), new ModelA("model-c"));
 		template.updateMulti(query, update, DocumentWithCollection.class);
 
-		assertThat(template.findOne(query, DocumentWithCollection.class).model, hasSize(3));
+		assertThat(template.findOne(query, DocumentWithCollection.class).models, hasSize(3));
 	}
 
 	/**
@@ -2273,11 +2274,11 @@ public class MongoTemplateTests {
 
 	static class DocumentWithCollection {
 
-		@Id public String id;
-		public List<Model> model;
+		@Id String id;
+		List<Model> models;
 
-		DocumentWithCollection(Model... models) {
-			this.model = Arrays.asList(models);
+		DocumentWithCollection(List<Model> models) {
+			this.models = models;
 		}
 	}
 

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

@@ -59,6 +59,7 @@ import org.springframework.data.mongodb.core.mapping.Document;
 import org.springframework.data.mongodb.core.mapping.Field;
 import org.springframework.data.mongodb.core.mapping.MongoMappingContext;
 import org.springframework.data.mongodb.core.mapping.PersonPojoStringId;
+import org.springframework.data.util.ClassTypeInformation;
 import org.springframework.test.util.ReflectionTestUtils;
 
 import com.mongodb.BasicDBList;
@@ -1023,7 +1024,7 @@ public class MappingMongoConverterUnitTests {
 		address.city = "London";
 		address.street = "Foo";
 
-		Object result = converter.convertToMongoType(Collections.singleton(address));
+		Object result = converter.convertToMongoType(Collections.singleton(address), ClassTypeInformation.OBJECT);
 		assertThat(result, is(instanceOf(BasicDBList.class)));
 
 		Set<?> readResult = converter.read(Set.class, (BasicDBList) result);
@@ -1383,6 +1384,9 @@ public class MappingMongoConverterUnitTests {
 		assertThat(aValue.get("c"), is((Object) "C"));
 	}
 
+	/**
+	 * @see DATAMONGO-812
+	 */
 	@Test
 	public void convertsListToBasicDBListAndRetainsTypeInformationForComplexObjects() {
 
@@ -1390,15 +1394,19 @@ public class MappingMongoConverterUnitTests {
 		address.city = "London";
 		address.street = "Foo";
 
-		Object result = converter.convertToMongoType(Collections.singletonList(address));
+		Object result = converter.convertToMongoType(Collections.singletonList(address),
+				ClassTypeInformation.from(Address.class));
 
 		assertThat(result, is(instanceOf(BasicDBList.class)));
 
 		BasicDBList dbList = (BasicDBList) result;
 		assertThat(dbList, hasSize(1));
-		assertThat(getTypedValue(getAsDBObject(dbList, 0), ("_class"), String.class), equalTo(Address.class.getName()));
+		assertThat(getTypedValue(getAsDBObject(dbList, 0), "_class", String.class), equalTo(Address.class.getName()));
 	}
 
+	/**
+	 * @see DATAMONGO-812
+	 */
 	@Test
 	public void convertsListToBasicDBListWithoutTypeInformationForSimpleTypes() {
 
@@ -1411,6 +1419,9 @@ public class MappingMongoConverterUnitTests {
 		assertThat(dbList.get(0), instanceOf(String.class));
 	}
 
+	/**
+	 * @see DATAMONGO-812
+	 */
 	@Test
 	public void convertsArrayToBasicDBListAndRetainsTypeInformationForComplexObjects() {
 
@@ -1418,15 +1429,18 @@ public class MappingMongoConverterUnitTests {
 		address.city = "London";
 		address.street = "Foo";
 
-		Object result = converter.convertToMongoType(new Address[] { address });
+		Object result = converter.convertToMongoType(new Address[] { address }, ClassTypeInformation.OBJECT);
 
 		assertThat(result, is(instanceOf(BasicDBList.class)));
 
 		BasicDBList dbList = (BasicDBList) result;
 		assertThat(dbList, hasSize(1));
-		assertThat(getTypedValue(getAsDBObject(dbList, 0), ("_class"), String.class), equalTo(Address.class.getName()));
+		assertThat(getTypedValue(getAsDBObject(dbList, 0), "_class", String.class), equalTo(Address.class.getName()));
 	}
 
+	/**
+	 * @see DATAMONGO-812
+	 */
 	@Test
 	public void convertsArrayToBasicDBListWithoutTypeInformationForSimpleTypes() {
 

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

@@ -258,6 +258,7 @@ public class UpdateMapperUnitTests {
 	 */
 	@Test
 	public void testUpdateShouldAllowMultiplePushEachForDifferentFields() {
+
 		Update update = new Update().push("category").each("spring", "data").push("type").each("mongodb");
 		DBObject mappedObject = mapper.getMappedObject(update.getUpdateObject(), context.getPersistentEntity(Object.class));