diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/ConverterRegistration.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/ConverterRegistration.java new file mode 100644 index 000000000..3365c2359 --- /dev/null +++ b/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); + } +} \ No newline at end of file diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/CustomConversions.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/CustomConversions.java index 64338166e..71f80b8ee 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/CustomConversions.java +++ b/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 readingPairs; private final Set writingPairs; private final Set> 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; diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/ConverterRegistrationUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/ConverterRegistrationUnitTests.java new file mode 100644 index 000000000..069540006 --- /dev/null +++ b/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)); + } +} diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/CustomConversionsUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/CustomConversionsUnitTests.java index 9db2be6a2..8049d5fa1 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/CustomConversionsUnitTests.java +++ b/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() { 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 { INSTANCE; @@ -155,7 +168,7 @@ public class CustomConversionsUnitTests { return 0L; } } - + enum StringToIntegerConverter implements Converter { INSTANCE; public Integer convert(String source) {