From f0de1c3069aea81a529555e5045ab44f553f586c Mon Sep 17 00:00:00 2001 From: Keith Donald Date: Thu, 5 Nov 2009 14:52:57 +0000 Subject: [PATCH] revised matchable converter lookup algorithm; added conversion service bean container tests --- .../DateTimeFormat.java | 2 +- .../ISODateTimeFormat.java | 2 +- .../ui/format/annotation/package-info.java | 4 ++ ...eTimeFormatAnnotationFormatterFactory.java | 3 +- ...eTimeFormatAnnotationFormatterFactory.java | 3 +- .../context/conversionservice/Bar.java | 14 ++++++ .../ConversionServiceContextConfigTests.java | 21 ++++++++ .../StringToBarConverter.java | 11 +++++ .../context/conversionservice/TestClient.java | 30 ++++++++++++ .../jodatime/JodaTimeFormattingTests.java | 16 +++---- .../FormattingConversionServiceTests.java | 4 +- .../conversionservice/conversionService.xml | 28 +++++++++++ .../support/GenericConversionService.java | 48 +++++++++---------- 13 files changed, 147 insertions(+), 39 deletions(-) rename org.springframework.context/src/main/java/org/springframework/ui/format/{jodatime => annotation}/DateTimeFormat.java (97%) rename org.springframework.context/src/main/java/org/springframework/ui/format/{jodatime => annotation}/ISODateTimeFormat.java (95%) create mode 100644 org.springframework.context/src/main/java/org/springframework/ui/format/annotation/package-info.java create mode 100644 org.springframework.context/src/test/java/org/springframework/context/conversionservice/Bar.java create mode 100644 org.springframework.context/src/test/java/org/springframework/context/conversionservice/ConversionServiceContextConfigTests.java create mode 100644 org.springframework.context/src/test/java/org/springframework/context/conversionservice/StringToBarConverter.java create mode 100644 org.springframework.context/src/test/java/org/springframework/context/conversionservice/TestClient.java create mode 100644 org.springframework.context/src/test/resources/org/springframework/context/conversionservice/conversionService.xml diff --git a/org.springframework.context/src/main/java/org/springframework/ui/format/jodatime/DateTimeFormat.java b/org.springframework.context/src/main/java/org/springframework/ui/format/annotation/DateTimeFormat.java similarity index 97% rename from org.springframework.context/src/main/java/org/springframework/ui/format/jodatime/DateTimeFormat.java rename to org.springframework.context/src/main/java/org/springframework/ui/format/annotation/DateTimeFormat.java index b2ee20439d9..ef91c3eebcf 100644 --- a/org.springframework.context/src/main/java/org/springframework/ui/format/jodatime/DateTimeFormat.java +++ b/org.springframework.context/src/main/java/org/springframework/ui/format/annotation/DateTimeFormat.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.springframework.ui.format.jodatime; +package org.springframework.ui.format.annotation; import java.lang.annotation.ElementType; import java.lang.annotation.Retention; diff --git a/org.springframework.context/src/main/java/org/springframework/ui/format/jodatime/ISODateTimeFormat.java b/org.springframework.context/src/main/java/org/springframework/ui/format/annotation/ISODateTimeFormat.java similarity index 95% rename from org.springframework.context/src/main/java/org/springframework/ui/format/jodatime/ISODateTimeFormat.java rename to org.springframework.context/src/main/java/org/springframework/ui/format/annotation/ISODateTimeFormat.java index dfce7e170b4..f14d42d1d76 100644 --- a/org.springframework.context/src/main/java/org/springframework/ui/format/jodatime/ISODateTimeFormat.java +++ b/org.springframework.context/src/main/java/org/springframework/ui/format/annotation/ISODateTimeFormat.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.springframework.ui.format.jodatime; +package org.springframework.ui.format.annotation; import java.lang.annotation.ElementType; import java.lang.annotation.Retention; diff --git a/org.springframework.context/src/main/java/org/springframework/ui/format/annotation/package-info.java b/org.springframework.context/src/main/java/org/springframework/ui/format/annotation/package-info.java new file mode 100644 index 00000000000..3806a5b7e76 --- /dev/null +++ b/org.springframework.context/src/main/java/org/springframework/ui/format/annotation/package-info.java @@ -0,0 +1,4 @@ +/** + * Annotations for declaratively configuring field formatting rules. + */ +package org.springframework.ui.format.annotation; diff --git a/org.springframework.context/src/main/java/org/springframework/ui/format/jodatime/DateTimeFormatAnnotationFormatterFactory.java b/org.springframework.context/src/main/java/org/springframework/ui/format/jodatime/DateTimeFormatAnnotationFormatterFactory.java index 75ba4be0e46..a2fbe7f9715 100644 --- a/org.springframework.context/src/main/java/org/springframework/ui/format/jodatime/DateTimeFormatAnnotationFormatterFactory.java +++ b/org.springframework.context/src/main/java/org/springframework/ui/format/jodatime/DateTimeFormatAnnotationFormatterFactory.java @@ -16,7 +16,8 @@ package org.springframework.ui.format.jodatime; import org.joda.time.format.DateTimeFormatter; -import org.springframework.ui.format.jodatime.DateTimeFormat.Style; +import org.springframework.ui.format.annotation.DateTimeFormat; +import org.springframework.ui.format.annotation.DateTimeFormat.Style; /** * Formats properties annotated with the {@link DateTimeFormat} annotation. diff --git a/org.springframework.context/src/main/java/org/springframework/ui/format/jodatime/ISODateTimeFormatAnnotationFormatterFactory.java b/org.springframework.context/src/main/java/org/springframework/ui/format/jodatime/ISODateTimeFormatAnnotationFormatterFactory.java index 0e4be37a06e..7aee52e64ea 100644 --- a/org.springframework.context/src/main/java/org/springframework/ui/format/jodatime/ISODateTimeFormatAnnotationFormatterFactory.java +++ b/org.springframework.context/src/main/java/org/springframework/ui/format/jodatime/ISODateTimeFormatAnnotationFormatterFactory.java @@ -16,7 +16,8 @@ package org.springframework.ui.format.jodatime; import org.joda.time.format.DateTimeFormatter; -import org.springframework.ui.format.jodatime.ISODateTimeFormat.Style; +import org.springframework.ui.format.annotation.ISODateTimeFormat; +import org.springframework.ui.format.annotation.ISODateTimeFormat.Style; /** * Formats properties annotated with the {@link ISODateTimeFormat} annotation. diff --git a/org.springframework.context/src/test/java/org/springframework/context/conversionservice/Bar.java b/org.springframework.context/src/test/java/org/springframework/context/conversionservice/Bar.java new file mode 100644 index 00000000000..d61b6100e58 --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/context/conversionservice/Bar.java @@ -0,0 +1,14 @@ +package org.springframework.context.conversionservice; + +public class Bar { + + private String value; + + public Bar(String value) { + this.value = value; + } + + public String getValue() { + return value; + } +} diff --git a/org.springframework.context/src/test/java/org/springframework/context/conversionservice/ConversionServiceContextConfigTests.java b/org.springframework.context/src/test/java/org/springframework/context/conversionservice/ConversionServiceContextConfigTests.java new file mode 100644 index 00000000000..8566f543be4 --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/context/conversionservice/ConversionServiceContextConfigTests.java @@ -0,0 +1,21 @@ +package org.springframework.context.conversionservice; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; +import org.springframework.context.support.ClassPathXmlApplicationContext; + +public class ConversionServiceContextConfigTests { + + @Test + public void testConfigOk() { + ClassPathXmlApplicationContext context = new ClassPathXmlApplicationContext("org/springframework/context/conversionservice/conversionservice.xml"); + TestClient client = context.getBean("testClient", TestClient.class); + assertEquals(2, client.getBars().size()); + assertEquals("value1", client.getBars().get(0).getValue()); + assertEquals("value2", client.getBars().get(1).getValue()); + assertTrue(client.isBool()); + } + +} diff --git a/org.springframework.context/src/test/java/org/springframework/context/conversionservice/StringToBarConverter.java b/org.springframework.context/src/test/java/org/springframework/context/conversionservice/StringToBarConverter.java new file mode 100644 index 00000000000..1d7f911f187 --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/context/conversionservice/StringToBarConverter.java @@ -0,0 +1,11 @@ +package org.springframework.context.conversionservice; + +import org.springframework.core.convert.converter.Converter; + +public class StringToBarConverter implements Converter { + + public Bar convert(String source) { + return new Bar(source); + } + +} diff --git a/org.springframework.context/src/test/java/org/springframework/context/conversionservice/TestClient.java b/org.springframework.context/src/test/java/org/springframework/context/conversionservice/TestClient.java new file mode 100644 index 00000000000..c78f8fdf9c4 --- /dev/null +++ b/org.springframework.context/src/test/java/org/springframework/context/conversionservice/TestClient.java @@ -0,0 +1,30 @@ +package org.springframework.context.conversionservice; + +import java.util.List; + +import org.springframework.beans.factory.annotation.Autowired; + +public class TestClient { + + private List bars; + + private boolean bool; + + public List getBars() { + return bars; + } + + @Autowired + public void setBars(List bars) { + this.bars = bars; + } + + public boolean isBool() { + return bool; + } + + public void setBool(boolean bool) { + this.bool = bool; + } + +} diff --git a/org.springframework.context/src/test/java/org/springframework/ui/format/jodatime/JodaTimeFormattingTests.java b/org.springframework.context/src/test/java/org/springframework/ui/format/jodatime/JodaTimeFormattingTests.java index 504c40a3855..215e96302e6 100644 --- a/org.springframework.context/src/test/java/org/springframework/ui/format/jodatime/JodaTimeFormattingTests.java +++ b/org.springframework.context/src/test/java/org/springframework/ui/format/jodatime/JodaTimeFormattingTests.java @@ -12,11 +12,11 @@ import org.joda.time.LocalDateTime; import org.joda.time.LocalTime; import org.junit.After; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import org.springframework.beans.MutablePropertyValues; import org.springframework.context.i18n.LocaleContextHolder; -import org.springframework.ui.format.jodatime.DateTimeFormat.Style; +import org.springframework.ui.format.annotation.DateTimeFormat; +import org.springframework.ui.format.annotation.DateTimeFormat.Style; import org.springframework.ui.format.support.FormattingConversionService; import org.springframework.validation.DataBinder; @@ -30,7 +30,7 @@ public class JodaTimeFormattingTests { public void setUp() { JodaTimeFormattingConfigurer configurer = new JodaTimeFormattingConfigurer(); configurer.installJodaTimeFormatting(conversionService); - + binder = new DataBinder(new JodaTimeBean()); binder.setConversionService(conversionService); @@ -67,7 +67,7 @@ public class JodaTimeFormattingTests { assertEquals(0, binder.getBindingResult().getErrorCount()); assertEquals("Oct 31, 2009", binder.getBindingResult().getFieldValue("localDateAnnotated")); } - + @Test public void testBindLocalTime() { MutablePropertyValues propertyValues = new MutablePropertyValues(); @@ -101,17 +101,15 @@ public class JodaTimeFormattingTests { propertyValues.addPropertyValue("localDateTimeAnnotated", "Saturday, October 31, 2009 12:00:00 PM "); binder.bind(propertyValues); assertEquals(0, binder.getBindingResult().getErrorCount()); - assertEquals("Saturday, October 31, 2009 12:00:00 PM ", binder.getBindingResult().getFieldValue("localDateTimeAnnotated")); + assertEquals("Saturday, October 31, 2009 12:00:00 PM ", binder.getBindingResult().getFieldValue( + "localDateTimeAnnotated")); } @Test - @Ignore public void testBindDateTime() { MutablePropertyValues propertyValues = new MutablePropertyValues(); propertyValues.addPropertyValue("dateTime", "10/31/09 12:00 PM"); - // this doesn't work because the String->ReadableInstant converter doesn't match due to String->@DateTimeFormat DateTime Matchable taking precedence binder.bind(propertyValues); - System.out.println(binder.getBindingResult()); assertEquals(0, binder.getBindingResult().getErrorCount()); assertEquals("10/31/09 12:00 PM", binder.getBindingResult().getFieldValue("dateTime")); } @@ -125,7 +123,7 @@ public class JodaTimeFormattingTests { assertEquals(0, binder.getBindingResult().getErrorCount()); assertEquals("Oct 31, 2009 12:00 PM", binder.getBindingResult().getFieldValue("dateTimeAnnotated")); } - + @Test public void testBindDate() { MutablePropertyValues propertyValues = new MutablePropertyValues(); diff --git a/org.springframework.context/src/test/java/org/springframework/ui/format/support/FormattingConversionServiceTests.java b/org.springframework.context/src/test/java/org/springframework/ui/format/support/FormattingConversionServiceTests.java index 0643a5d2565..e610a82cbc8 100644 --- a/org.springframework.context/src/test/java/org/springframework/ui/format/support/FormattingConversionServiceTests.java +++ b/org.springframework.context/src/test/java/org/springframework/ui/format/support/FormattingConversionServiceTests.java @@ -31,10 +31,10 @@ import org.junit.Test; import org.springframework.context.i18n.LocaleContextHolder; import org.springframework.core.convert.TypeDescriptor; import org.springframework.core.convert.converter.Converter; +import org.springframework.ui.format.annotation.DateTimeFormat.Style; import org.springframework.ui.format.jodatime.DateTimeFormatAnnotationFormatterFactory; import org.springframework.ui.format.jodatime.DateTimeParser; import org.springframework.ui.format.jodatime.ReadablePartialPrinter; -import org.springframework.ui.format.jodatime.DateTimeFormat.Style; import org.springframework.ui.format.number.IntegerFormatter; /** @@ -105,7 +105,7 @@ public class FormattingConversionServiceTests { private static class Model { @SuppressWarnings("unused") - @org.springframework.ui.format.jodatime.DateTimeFormat(dateStyle = Style.SHORT) + @org.springframework.ui.format.annotation.DateTimeFormat(dateStyle = Style.SHORT) public Date date; } diff --git a/org.springframework.context/src/test/resources/org/springframework/context/conversionservice/conversionService.xml b/org.springframework.context/src/test/resources/org/springframework/context/conversionservice/conversionService.xml new file mode 100644 index 00000000000..64e86fc706f --- /dev/null +++ b/org.springframework.context/src/test/resources/org/springframework/context/conversionservice/conversionService.xml @@ -0,0 +1,28 @@ + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java index 4d060129761..bc247a96e7e 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java +++ b/org.springframework.core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java @@ -51,8 +51,7 @@ public class GenericConversionService implements ConversionService, ConverterReg } }; - private final Map, Map, MatchableConverters>> converters = new HashMap, Map, MatchableConverters>>( - 36); + private final Map, Map, MatchableConverters>> converters = new HashMap, Map, MatchableConverters>>(36); private ConversionService parent; @@ -269,9 +268,7 @@ public class GenericConversionService implements ConversionService, ConverterReg * @return the generic converter that will perform the conversion, or null if no suitable converter was found */ protected GenericConverter getConverter(TypeDescriptor sourceType, TypeDescriptor targetType) { - MatchableConverters matchable = findMatchableConvertersForClassPair(sourceType.getObjectType(), targetType - .getObjectType()); - GenericConverter converter = matchConverter(matchable, sourceType, targetType); + GenericConverter converter = findConverterForClassPair(sourceType, targetType); if (converter != null) { return converter; } else if (this.parent != null && this.parent.canConvert(sourceType, targetType)) { @@ -328,16 +325,17 @@ public class GenericConversionService implements ConversionService, ConverterReg Assert.notNull(targetType, "The targetType to convert to is required"); } - private MatchableConverters findMatchableConvertersForClassPair(Class sourceType, Class targetType) { - if (sourceType.isInterface()) { + private GenericConverter findConverterForClassPair(TypeDescriptor sourceType, TypeDescriptor targetType) { + Class sourceObjectType = sourceType.getObjectType(); + if (sourceObjectType.isInterface()) { LinkedList> classQueue = new LinkedList>(); - classQueue.addFirst(sourceType); + classQueue.addFirst(sourceObjectType); while (!classQueue.isEmpty()) { Class currentClass = classQueue.removeLast(); Map, MatchableConverters> converters = getTargetConvertersForSource(currentClass); - MatchableConverters matchable = getMatchableConvertersForTarget(converters, targetType); - if (matchable != null) { - return matchable; + GenericConverter converter = getMatchingConverterForTarget(sourceType, targetType, converters); + if (converter != null) { + return converter; } Class[] interfaces = currentClass.getInterfaces(); for (Class ifc : interfaces) { @@ -345,16 +343,16 @@ public class GenericConversionService implements ConversionService, ConverterReg } } Map, MatchableConverters> objectConverters = getTargetConvertersForSource(Object.class); - return getMatchableConvertersForTarget(objectConverters, targetType); + return getMatchingConverterForTarget(sourceType, targetType, objectConverters); } else { LinkedList> classQueue = new LinkedList>(); - classQueue.addFirst(sourceType); + classQueue.addFirst(sourceObjectType); while (!classQueue.isEmpty()) { Class currentClass = classQueue.removeLast(); Map, MatchableConverters> converters = getTargetConvertersForSource(currentClass); - MatchableConverters matchable = getMatchableConvertersForTarget(converters, targetType); - if (matchable != null) { - return matchable; + GenericConverter converter = getMatchingConverterForTarget(sourceType, targetType, converters); + if (converter != null) { + return converter; } if (currentClass.isArray()) { Class componentType = ClassUtils.resolvePrimitiveIfNecessary(currentClass.getComponentType()); @@ -383,14 +381,15 @@ public class GenericConversionService implements ConversionService, ConverterReg return converters; } - private MatchableConverters getMatchableConvertersForTarget(Map, MatchableConverters> converters, - Class targetType) { - if (targetType.isInterface()) { + private GenericConverter getMatchingConverterForTarget(TypeDescriptor sourceType, TypeDescriptor targetType, Map, MatchableConverters> converters) { + Class targetObjectType = targetType.getObjectType(); + if (targetObjectType.isInterface()) { LinkedList> classQueue = new LinkedList>(); - classQueue.addFirst(targetType); + classQueue.addFirst(targetObjectType); while (!classQueue.isEmpty()) { Class currentClass = classQueue.removeLast(); - MatchableConverters converter = converters.get(currentClass); + MatchableConverters matchable = converters.get(currentClass); + GenericConverter converter = matchConverter(matchable, sourceType, targetType); if (converter != null) { return converter; } @@ -399,13 +398,14 @@ public class GenericConversionService implements ConversionService, ConverterReg classQueue.addFirst(ifc); } } - return converters.get(Object.class); + return matchConverter(converters.get(Object.class), sourceType, targetType); } else { LinkedList> classQueue = new LinkedList>(); - classQueue.addFirst(targetType); + classQueue.addFirst(targetObjectType); while (!classQueue.isEmpty()) { Class currentClass = classQueue.removeLast(); - MatchableConverters converter = converters.get(currentClass); + MatchableConverters matchable = converters.get(currentClass); + GenericConverter converter = matchConverter(matchable, sourceType, targetType); if (converter != null) { return converter; }