DATAMONGO-321 - Overhaul of id handling.

Cleaned up the id handling on query mapping and mapping in general. We now only try to convert id values into an ObjectId and store it as is using potentially registered custom converters. Register BigInteger<->String converters by default now.

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

@@ -16,7 +16,6 @@
 package org.springframework.data.mongodb.core;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 
@@ -120,22 +119,14 @@ public class QueryMapper {
 	 * @param id
 	 * @return
 	 */
-	@SuppressWarnings("unchecked")
 	public Object convertId(Object id) {
-
+		
-		for (Class<?> type : Arrays.asList(ObjectId.class, String.class)) {
+		try {
-
+			return conversionService.convert(id, ObjectId.class);
-			if (id.getClass().isAssignableFrom(type)) {
+		} catch (ConversionException e) {
-				return id;
+			// Ignore
-			}
-
-			try {
-				return conversionService.convert(id, type);
-			} catch (ConversionException e) {
-				// Ignore
-			}
 		}
-
+		
-		return id;
+		return converter.convertToMongoType(id);
 	}
 }

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

@@ -23,10 +23,8 @@ import org.springframework.core.convert.ConversionService;
 import org.springframework.core.convert.support.ConversionServiceFactory;
 import org.springframework.core.convert.support.GenericConversionService;
 import org.springframework.data.mongodb.core.convert.MongoConverters.BigIntegerToObjectIdConverter;
-import org.springframework.data.mongodb.core.convert.MongoConverters.BigIntegerToStringConverter;
 import org.springframework.data.mongodb.core.convert.MongoConverters.ObjectIdToBigIntegerConverter;
 import org.springframework.data.mongodb.core.convert.MongoConverters.ObjectIdToStringConverter;
-import org.springframework.data.mongodb.core.convert.MongoConverters.StringToBigIntegerConverter;
 import org.springframework.data.mongodb.core.convert.MongoConverters.StringToObjectIdConverter;
 
 /**
@@ -81,12 +79,6 @@ public abstract class AbstractMongoConverter implements MongoConverter, Initiali
 		if (!conversionService.canConvert(BigInteger.class, ObjectId.class)) {
 			conversionService.addConverter(BigIntegerToObjectIdConverter.INSTANCE);
 		}
-		if (!conversionService.canConvert(BigInteger.class, String.class)) {
-			conversionService.addConverter(BigIntegerToStringConverter.INSTANCE);
-		}
-		if (!conversionService.canConvert(String.class, BigInteger.class)) {
-			conversionService.addConverter(StringToBigIntegerConverter.INSTANCE);
-		}
 
 		conversions.registerConvertersIn(conversionService);
 	}

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

@@ -31,7 +31,9 @@ import org.springframework.core.convert.converter.GenericConverter.ConvertiblePa
 import org.springframework.core.convert.support.GenericConversionService;
 import org.springframework.data.mapping.model.SimpleTypeHolder;
 import org.springframework.data.mongodb.core.convert.MongoConverters.BigDecimalToStringConverter;
+import org.springframework.data.mongodb.core.convert.MongoConverters.BigIntegerToStringConverter;
 import org.springframework.data.mongodb.core.convert.MongoConverters.StringToBigDecimalConverter;
+import org.springframework.data.mongodb.core.convert.MongoConverters.StringToBigIntegerConverter;
 import org.springframework.data.mongodb.core.mapping.MongoSimpleTypes;
 import org.springframework.util.Assert;
 
@@ -77,6 +79,8 @@ public class CustomConversions {
 		this.converters.add(CustomToStringConverter.INSTANCE);
 		this.converters.add(BigDecimalToStringConverter.INSTANCE);
 		this.converters.add(StringToBigDecimalConverter.INSTANCE);
+		this.converters.add(BigIntegerToStringConverter.INSTANCE);
+		this.converters.add(StringToBigIntegerConverter.INSTANCE);
 		this.converters.addAll(converters);
 
 		for (Object c : this.converters) {

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

@@ -17,7 +17,6 @@ package org.springframework.data.mongodb.core.convert;
 
 import java.lang.reflect.Array;
 import java.lang.reflect.InvocationTargetException;
-import java.math.BigInteger;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -28,7 +27,6 @@ import java.util.Map.Entry;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-import org.bson.types.ObjectId;
 import org.springframework.beans.BeansException;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextAware;
@@ -80,9 +78,6 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 	private static final TypeInformation<Collection> COLLECTION_TYPE_INFORMATION = ClassTypeInformation
 			.from(Collection.class);
 
-	private static final List<Class<?>> VALID_ID_TYPES = Arrays.asList(new Class<?>[] { ObjectId.class, String.class,
-			BigInteger.class, byte[].class });
-
 	protected static final Log log = LogFactory.getLog(MappingMongoConverter.class);
 
 	protected final MappingContext<? extends MongoPersistentEntity<?>, MongoPersistentProperty> mappingContext;
@@ -362,29 +357,15 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 		// Write the ID
 		final MongoPersistentProperty idProperty = entity.getIdProperty();
 		if (!dbo.containsField("_id") && null != idProperty) {
-			Object idObj = null;
+			
-			Class<?>[] targetClasses = new Class<?>[] { ObjectId.class, String.class, Object.class };
+			try {
-			for (Class<?> targetClass : targetClasses) {
+				Object id = wrapper.getProperty(idProperty, Object.class, useFieldAccessOnly);
-				try {
+				dbo.put("_id", idMapper.convertId(id));
-					idObj = wrapper.getProperty(idProperty, targetClass, useFieldAccessOnly);
+			} catch (ConversionException ignored) {
-					if (null != idObj) {
+			} catch (IllegalAccessException e) {
-						break;
+				throw new MappingException(e.getMessage(), e);
-					}
+			} catch (InvocationTargetException e) {
-				} catch (ConversionException ignored) {
+				throw new MappingException(e.getMessage(), e);
-				} catch (IllegalAccessException e) {
-					throw new MappingException(e.getMessage(), e);
-				} catch (InvocationTargetException e) {
-					throw new MappingException(e.getMessage(), e);
-				}
-			}
-
-			if (null != idObj) {
-				dbo.put("_id", idObj);
-			} else {
-				if (!VALID_ID_TYPES.contains(idProperty.getType())) {
-					throw new MappingException("Invalid data type " + idProperty.getType().getName()
-							+ " for Id property. Should be one of " + VALID_ID_TYPES);
-				}
 			}
 		}
 

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

@@ -844,6 +844,18 @@ public class MappingMongoConverterUnitTests {
 		assertThat((String) inner.get("value"), is("testValue"));
 	}
 	
+	@Test
+	public void writesIntIdCorrectly() {
+		
+		ClassWithIntId value = new ClassWithIntId();
+		value.id = 5;
+		
+		DBObject result = new BasicDBObject();
+		converter.write(value, result);
+		
+		assertThat(result.get("_id"), is((Object) 5));
+	}
+	
 	class GenericType<T> {
 		T content;
 	}
@@ -947,6 +959,12 @@ public class MappingMongoConverterUnitTests {
 			this.value = value;
 		}
 	}
+
+	class ClassWithIntId {
+		
+		@Id
+		int id;
+	}
 	
 	private class LocalDateToDateConverter implements Converter<LocalDate, Date> {
 

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

@@ -91,6 +91,15 @@ public class QueryMapperUnitTests {
 		assertThat(result.get("_id"), is((Object) "1"));
 	}
 	
+	@Test
+	public void handlesObjectIdCapableBigIntegerIdsCorrectly() {
+		
+		ObjectId id = new ObjectId();
+		DBObject dbObject = new BasicDBObject("id", new BigInteger(id.toString(), 16));
+		DBObject result = mapper.getMappedObject(dbObject, null);
+		assertThat(result.get("_id"), is((Object) id));
+	}
+	
 	/**
 	 * @see DATAMONGO-278
 	 */