DATAMONGO-342 - Introduced support for @ReadingConverter / @WritingConverter.

CustomConversions now evaluates @ReadingConverter / @WritingConverter when adding Converter implementations. See DATACMNS-113 and the appropriate commit for details. Added unit test to verify StringToBigIntegerConverter does not get added as writing converter.

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

@@ -0,0 +1,115 @@
+/*
+ * Copyright 2011 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.mongodb.core.convert;
+
+import org.springframework.core.convert.converter.GenericConverter.ConvertiblePair;
+import org.springframework.data.mongodb.core.mapping.MongoSimpleTypes;
+import org.springframework.util.Assert;
+
+/**
+ * Conversion registration information.
+ * 
+ * @author Oliver Gierke
+ */
+class ConverterRegistration {
+
+	private final ConvertiblePair convertiblePair;
+	private final boolean reading;
+	private final boolean writing;
+
+	/**
+	 * Creates a new {@link ConverterRegistration}.
+	 * 
+	 * @param convertiblePair must not be {@literal null}.
+	 * @param isReading whether to force to consider the converter for reading.
+	 * @param isWritingwhether to force to consider the converter for reading.
+	 */
+	public ConverterRegistration(ConvertiblePair convertiblePair, boolean isReading, boolean isWriting) {
+
+		Assert.notNull(convertiblePair);
+
+		this.convertiblePair = convertiblePair;
+		this.reading = isReading;
+		this.writing = isWriting;
+	}
+
+	/**
+	 * Creates a new {@link ConverterRegistration} from the given source and target type and read/write flags.
+	 * 
+	 * @param source the source type to be converted from, must not be {@literal null}.
+	 * @param target the target type to be converted to, must not be {@literal null}.
+	 * @param isReading whether to force to consider the converter for reading.
+	 * @param isWriting whether to force to consider the converter for writing.
+	 */
+	public ConverterRegistration(Class<?> source, Class<?> target, boolean isReading, boolean isWriting) {
+		this(new ConvertiblePair(source, target), isReading, isWriting);
+	}
+
+	/**
+	 * Returns whether the converter shall be used for writing.
+	 * 
+	 * @return
+	 */
+	public boolean isWriting() {
+		return writing == true || (!reading && isSimpleTargetType());
+	}
+
+	/**
+	 * Returns whether the converter shall be used for reading.
+	 * 
+	 * @return
+	 */
+	public boolean isReading() {
+		return reading == true || (!writing && isSimpleSourceType());
+	}
+
+	/**
+	 * Returns the actual conversion pair.
+	 * 
+	 * @return
+	 */
+	public ConvertiblePair getConvertiblePair() {
+		return convertiblePair;
+	}
+
+	/**
+	 * Returns whether the source type is a Mongo simple one.
+	 * 
+	 * @return
+	 */
+	public boolean isSimpleSourceType() {
+		return isMongoBasicType(convertiblePair.getSourceType());
+	}
+
+	/**
+	 * Returns whether the target type is a Mongo simple one.
+	 * 
+	 * @return
+	 */
+	public boolean isSimpleTargetType() {
+		return isMongoBasicType(convertiblePair.getTargetType());
+	}
+
+	/**
+	 * Returns whether the given type is a type that Mongo can handle basically.
+	 * 
+	 * @param type
+	 * @return
+	 */
+	private static boolean isMongoBasicType(Class<?> type) {
+		return MongoSimpleTypes.HOLDER.isSimpleType(type);
+	}
+}

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

@@ -22,6 +22,8 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Set;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.springframework.core.GenericTypeResolver;
 import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.core.convert.converter.Converter;
@@ -29,6 +31,8 @@ import org.springframework.core.convert.converter.ConverterFactory;
 import org.springframework.core.convert.converter.GenericConverter;
 import org.springframework.core.convert.converter.GenericConverter.ConvertiblePair;
 import org.springframework.core.convert.support.GenericConversionService;
+import org.springframework.data.convert.ReadingConverter;
+import org.springframework.data.convert.WritingConverter;
 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;
@@ -48,6 +52,10 @@ import org.springframework.util.Assert;
  */
 public class CustomConversions {
 
+	private static final Log LOG = LogFactory.getLog(CustomConversions.class);
+	private static final String READ_CONVERTER_NOT_SIMPLE = "Registering converter from %s to %s as reading converter although it doesn't convert from a Mongo supported type! You might wanna check you annotation setup at the converter implementation.";
+	private static final String WRITE_CONVERTER_NOT_SIMPLE = "Registering converter from %s to %s as writing converter although it doesn't convert to a Mongo supported type! You might wanna check you annotation setup at the converter implementation.";
+
 	private final Set<ConvertiblePair> readingPairs;
 	private final Set<ConvertiblePair> writingPairs;
 	private final Set<Class<?>> customSimpleTypes;
@@ -151,14 +159,18 @@ public class CustomConversions {
 	 */
 	private void registerConversion(Object converter) {
 
+		Class<?> type = converter.getClass();
+		boolean isWriting = type.isAnnotationPresent(WritingConverter.class);
+		boolean isReading = type.isAnnotationPresent(ReadingConverter.class);
+
 		if (converter instanceof GenericConverter) {
 			GenericConverter genericConverter = (GenericConverter) converter;
 			for (ConvertiblePair pair : genericConverter.getConvertibleTypes()) {
-				register(pair);
+				register(new ConverterRegistration(pair, isReading, isWriting));
 			}
 		} else if (converter instanceof Converter) {
 			Class<?>[] arguments = GenericTypeResolver.resolveTypeArguments(converter.getClass(), Converter.class);
-			register(new ConvertiblePair(arguments[0], arguments[1]));
+			register(new ConverterRegistration(arguments[0], arguments[1], isReading, isWriting));
 		} else {
 			throw new IllegalArgumentException("Unsupported Converter type!");
 		}
@@ -170,15 +182,27 @@ public class CustomConversions {
 	 * 
 	 * @param pair
 	 */
-	private void register(ConvertiblePair pair) {
+	private void register(ConverterRegistration context) {
+
+		ConvertiblePair pair = context.getConvertiblePair();
+
+		if (context.isReading()) {
 
-		if (isMongoBasicType(pair.getSourceType())) {
 			readingPairs.add(pair);
+
+			if (LOG.isWarnEnabled() && !context.isSimpleSourceType()) {
+				LOG.warn(String.format(READ_CONVERTER_NOT_SIMPLE, pair.getSourceType(), pair.getTargetType()));
+			}
 		}
 
-		if (isMongoBasicType(pair.getTargetType())) {
+		if (context.isWriting()) {
+
 			writingPairs.add(pair);
 			customSimpleTypes.add(pair.getSourceType());
+
+			if (LOG.isWarnEnabled() && !context.isSimpleTargetType()) {
+				LOG.warn(String.format(WRITE_CONVERTER_NOT_SIMPLE, pair.getSourceType(), pair.getTargetType()));
+			}
 		}
 	}
 
@@ -269,16 +293,7 @@ public class CustomConversions {
 		return null;
 	}
 
-	/**
-	 * Returns whether the given type is a type that Mongo can handle basically.
-	 * 
-	 * @param type
-	 * @return
-	 */
-	private boolean isMongoBasicType(Class<?> type) {
-		return MongoSimpleTypes.HOLDER.isSimpleType(type);
-	}
-
+	@WritingConverter
 	private enum CustomToStringConverter implements GenericConverter {
 		INSTANCE;
 

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

@@ -0,0 +1,66 @@
+/*
+ * Copyright 2011 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.mongodb.core.convert;
+
+import static org.hamcrest.CoreMatchers.*;
+import static org.junit.Assert.*;
+
+import org.junit.Test;
+import org.springframework.data.mongodb.core.mapping.Person;
+
+/**
+ * Unit tests for {@link ConverterRegistration}.
+ * 
+ * @author Oliver Gierke
+ */
+public class ConverterRegistrationUnitTests {
+
+	@Test
+	public void considersNotExplicitlyReadingDependingOnTypes() {
+
+		ConverterRegistration context = new ConverterRegistration(Person.class, String.class, false, false);
+		assertThat(context.isWriting(), is(true));
+		assertThat(context.isReading(), is(false));
+
+		context = new ConverterRegistration(String.class, Person.class, false, false);
+		assertThat(context.isWriting(), is(false));
+		assertThat(context.isReading(), is(true));
+
+		context = new ConverterRegistration(String.class, Class.class, false, false);
+		assertThat(context.isWriting(), is(true));
+		assertThat(context.isReading(), is(true));
+	}
+
+	@Test
+	public void forcesReadWriteOnlyIfAnnotated() {
+
+		ConverterRegistration context = new ConverterRegistration(String.class, Class.class, false, true);
+		assertThat(context.isWriting(), is(true));
+		assertThat(context.isReading(), is(false));
+
+		context = new ConverterRegistration(String.class, Class.class, true, false);
+		assertThat(context.isWriting(), is(false));
+		assertThat(context.isReading(), is(true));
+	}
+
+	@Test
+	public void considersConverterForReadAndWriteIfBothAnnotated() {
+
+		ConverterRegistration context = new ConverterRegistration(String.class, Class.class, true, true);
+		assertThat(context.isWriting(), is(true));
+		assertThat(context.isReading(), is(true));
+	}
+}

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

@@ -12,6 +12,7 @@ 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.MongoConverters.StringToBigIntegerConverter;
 
 import com.mongodb.DBRef;
 
@@ -40,8 +41,8 @@ public class CustomConversionsUnitTests {
 	@SuppressWarnings("unchecked")
 	public void considersSubtypesCorrectly() {
 
-		CustomConversions conversions = new CustomConversions(Arrays.asList(
-				NumberToStringConverter.INSTANCE, StringToNumberConverter.INSTANCE));
+		CustomConversions conversions = new CustomConversions(Arrays.asList(NumberToStringConverter.INSTANCE,
+				StringToNumberConverter.INSTANCE));
 
 		assertThat(conversions.getCustomWriteTarget(Long.class, null), is(typeCompatibleWith(String.class)));
 		assertThat(conversions.hasCustomReadTarget(String.class, Long.class), is(true));
@@ -59,26 +60,25 @@ public class CustomConversionsUnitTests {
 	 */
 	@Test
 	public void considersObjectIdToBeSimpleType() {
-		
+
 		CustomConversions conversions = new CustomConversions();
 		assertThat(conversions.isSimpleType(ObjectId.class), is(true));
 		assertThat(conversions.hasCustomWriteTarget(ObjectId.class), is(false));
-		
-		
+
 	}
-	
+
 	/**
 	 * @see DATAMONGO-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));
@@ -87,11 +87,11 @@ public class CustomConversionsUnitTests {
 
 	@Test
 	public void considersDBRefsToBeSimpleTypes() {
-		
+
 		CustomConversions conversions = new CustomConversions();
 		assertThat(conversions.isSimpleType(DBRef.class), is(true));
 	}
-	
+
 	@Test
 	public void populatesConversionServiceCorrectly() {
 
@@ -99,10 +99,9 @@ public class CustomConversionsUnitTests {
 		GenericConversionService conversionService = ConversionServiceFactory.createDefaultConversionService();
 		assertThat(conversionService.canConvert(String.class, UUID.class), is(false));
 
-		CustomConversions conversions = new CustomConversions(
-				Arrays.asList(StringToUUIDConverter.INSTANCE));
+		CustomConversions conversions = new CustomConversions(Arrays.asList(StringToUUIDConverter.INSTANCE));
 		conversions.registerConvertersIn(conversionService);
-		
+
 		assertThat(conversionService.canConvert(String.class, UUID.class), is(true));
 	}
 
@@ -114,19 +113,33 @@ public class CustomConversionsUnitTests {
 		CustomConversions conversions = new CustomConversions(Arrays.asList(StringToUUIDConverter.INSTANCE));
 		assertThat(conversions.isSimpleType(UUID.class), is(false));
 	}
-	
-	
+
 	/**
 	 * @see DATAMONGO-298
 	 */
 	@Test
 	public void discoversConvertersForSubtypesOfMongoTypes() {
-		
+
 		CustomConversions conversions = new CustomConversions(Arrays.asList(StringToIntegerConverter.INSTANCE));
 		assertThat(conversions.hasCustomReadTarget(String.class, Integer.class), is(true));
 		assertThat(conversions.hasCustomWriteTarget(String.class, Integer.class), is(true));
 	}
-	
+
+	/**
+	 * @see DATAMONGO-342
+	 */
+	@Test
+	public void doesNotHaveConverterForStringToBigIntegerByDefault() {
+
+		CustomConversions conversions = new CustomConversions();
+		assertThat(conversions.hasCustomWriteTarget(String.class), is(false));
+		assertThat(conversions.getCustomWriteTarget(String.class), is(nullValue()));
+
+		conversions = new CustomConversions(Arrays.asList(StringToBigIntegerConverter.INSTANCE));
+		assertThat(conversions.hasCustomWriteTarget(String.class), is(false));
+		assertThat(conversions.getCustomWriteTarget(String.class), is(nullValue()));
+	}
+
 	enum UuidToStringConverter implements Converter<UUID, String> {
 		INSTANCE;
 
@@ -155,7 +168,7 @@ public class CustomConversionsUnitTests {
 			return 0L;
 		}
 	}
-	
+
 	enum StringToIntegerConverter implements Converter<String, Integer> {
 		INSTANCE;
 		public Integer convert(String source) {