revised matchable converter lookup algorithm; added conversion service bean container tests

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;

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;

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;

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.

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.

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;
+	}
+}

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());
+	}
+
+}

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<String, Bar> {
+
+	public Bar convert(String source) {
+		return new Bar(source);
+	}
+
+}

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<Bar> bars;
+
+	private boolean bool;
+
+	public List<Bar> getBars() {
+		return bars;
+	}
+
+	@Autowired
+	public void setBars(List<Bar> bars) {
+		this.bars = bars;
+	}
+
+	public boolean isBool() {
+		return bool;
+	}
+
+	public void setBool(boolean bool) {
+		this.bool = bool;
+	}
+
+}

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();

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;
 
 	}

org.springframework.context/src/test/resources/org/springframework/context/conversionservice/conversionService.xml

@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<beans xmlns="http://www.springframework.org/schema/beans" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+	   xmlns:context="http://www.springframework.org/schema/context"
+	   xmlns:util="http://www.springframework.org/schema/util" xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.0.xsd
+				http://www.springframework.org/schema/context http://www.springframework.org/schema/context/spring-context-3.0.xsd
+				http://www.springframework.org/schema/util http://www.springframework.org/schema/util/spring-util-3.0.xsd">
+
+	<bean id="conversionService" class="org.springframework.core.convert.support.DefaultConversionService">
+		<property name="converters">
+			<bean class="org.springframework.context.conversionservice.StringToBarConverter" />
+		</property>
+	</bean>
+
+	<bean id="testClient" class="org.springframework.context.conversionservice.TestClient">
+		<property name="bool" value="true"/>
+	</bean>
+
+	<bean class="org.springframework.context.conversionservice.Bar">
+		<constructor-arg value ="value1" />
+	</bean>
+
+	<bean class="org.springframework.context.conversionservice.Bar">
+		<constructor-arg value ="value2" />
+	</bean>
+	
+	<context:annotation-config />
+	
+</beans>

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<Class<?>, Map<Class<?>, MatchableConverters>> converters = new HashMap<Class<?>, Map<Class<?>, MatchableConverters>>(
-			36);
+	private final Map<Class<?>, Map<Class<?>, MatchableConverters>> converters = new HashMap<Class<?>, Map<Class<?>, 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 <code>null</code> 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<Class<?>> classQueue = new LinkedList<Class<?>>();
-			classQueue.addFirst(sourceType);
+			classQueue.addFirst(sourceObjectType);
 			while (!classQueue.isEmpty()) {
 				Class<?> currentClass = classQueue.removeLast();
 				Map<Class<?>, 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<Class<?>, MatchableConverters> objectConverters = getTargetConvertersForSource(Object.class);
-			return getMatchableConvertersForTarget(objectConverters, targetType);
+			return getMatchingConverterForTarget(sourceType, targetType, objectConverters);
 		} else {
 			LinkedList<Class<?>> classQueue = new LinkedList<Class<?>>();
-			classQueue.addFirst(sourceType);
+			classQueue.addFirst(sourceObjectType);
 			while (!classQueue.isEmpty()) {
 				Class<?> currentClass = classQueue.removeLast();
 				Map<Class<?>, 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<Class<?>, MatchableConverters> converters,
-			Class<?> targetType) {
-		if (targetType.isInterface()) {
+	private GenericConverter getMatchingConverterForTarget(TypeDescriptor sourceType, TypeDescriptor targetType, Map<Class<?>, MatchableConverters> converters) {
+		Class<?> targetObjectType = targetType.getObjectType();
+		if (targetObjectType.isInterface()) {
 			LinkedList<Class<?>> classQueue = new LinkedList<Class<?>>();
-			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<Class<?>> classQueue = new LinkedList<Class<?>>();
-			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;
 				}