SPR-6199

org.springframework.context/src/main/java/org/springframework/ui/format/support/GenericFormatterRegistry.java

@@ -17,8 +17,6 @@
 package org.springframework.ui.format.support;
 
 import java.lang.annotation.Annotation;
-import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
 import java.text.ParseException;
 import java.util.LinkedList;
 import java.util.Locale;
@@ -40,7 +38,6 @@ import org.springframework.ui.format.Formatted;
 import org.springframework.ui.format.Formatter;
 import org.springframework.ui.format.FormatterRegistry;
 import org.springframework.util.Assert;
-import org.springframework.util.ReflectionUtils;
 
 /**
  * A generic implementation of {@link org.springframework.ui.format.FormatterRegistry}
@@ -61,9 +58,9 @@ import org.springframework.util.ReflectionUtils;
  */
 public class GenericFormatterRegistry implements FormatterRegistry, ApplicationContextAware, Cloneable {
 
-	private final Map<Class, FormatterHolder> typeFormatters = new ConcurrentHashMap<Class, FormatterHolder>();
+	private final Map<Class<?>, FormatterHolder> typeFormatters = new ConcurrentHashMap<Class<?>, FormatterHolder>();
 
-	private final Map<Class, AnnotationFormatterFactoryHolder> annotationFormatters = new ConcurrentHashMap<Class, AnnotationFormatterFactoryHolder>();
+	private final Map<Class<?>, AnnotationFormatterFactoryHolder> annotationFormatters = new ConcurrentHashMap<Class<?>, AnnotationFormatterFactoryHolder>();
 
 	private ConversionService conversionService;
 
@@ -182,47 +179,47 @@ public class GenericFormatterRegistry implements FormatterRegistry, ApplicationC
 
 	// implementing FormatterRegistry
 
-	public void addFormatterByType(Class<?> type, Formatter<?> formatter) {
-		Class<?> formattedObjectType = GenericTypeResolver.resolveTypeArgument(formatter.getClass(), Formatter.class);
-		if (formattedObjectType != null && !type.isAssignableFrom(formattedObjectType)) {
-			if (this.conversionService == null) {
-				throw new IllegalStateException("Unable to index Formatter " + formatter + " under type ["
-						+ type.getName() + "]; not able to convert from a [" + formattedObjectType.getName()
-						+ "] parsed by the Formatter to [" + type.getName()
-						+ "] because this.conversionService is null");
-			}
-			if (!this.conversionService.canConvert(formattedObjectType, type)) {
-				throw new IllegalArgumentException("Unable to index Formatter " + formatter + " under type ["
-						+ type.getName() + "]; not able to convert from a [" + formattedObjectType.getName()
-						+ "] parsed by the Formatter to [" + type.getName() + "]");
-			}
-			if (!this.conversionService.canConvert(type, formattedObjectType)) {
-				throw new IllegalArgumentException("Unable to index Formatter " + formatter + " under type ["
-						+ type.getName() + "]; not able to convert to [" + formattedObjectType.getName()
-						+ "] to format a [" + type.getName() + "]");
-			}
-		}
-		this.typeFormatters.put(type, new FormatterHolder(formattedObjectType, formatter));
-	}
-
 	public void addFormatterByType(Formatter<?> formatter) {
-		Class<?> formattedObjectType = GenericTypeResolver.resolveTypeArgument(formatter.getClass(), Formatter.class);
+		Class<?> formatterObjectType = GenericTypeResolver.resolveTypeArgument(formatter.getClass(), Formatter.class);
-		if (formattedObjectType == null) {
+		if (formatterObjectType == null) {
 			throw new IllegalArgumentException("Unable to register Formatter " + formatter
 					+ "; cannot determine parameterized object type <T>");
 		}
-		this.typeFormatters.put(formattedObjectType, new FormatterHolder(formattedObjectType, formatter));
+		this.typeFormatters.put(formatterObjectType, new FormatterHolder(formatterObjectType, formatter));
+	}
+
+	public void addFormatterByType(Class<?> type, Formatter<?> formatter) {
+		Class<?> formatterObjectType = GenericTypeResolver.resolveTypeArgument(formatter.getClass(), Formatter.class);
+		if (formatterObjectType != null && !type.isAssignableFrom(formatterObjectType)) {
+			if (this.conversionService == null) {
+				throw new IllegalStateException("Unable to index Formatter " + formatter + " under type ["
+						+ type.getName() + "]; not able to convert from a [" + formatterObjectType.getName()
+						+ "] parsed by the Formatter to [" + type.getName()
+						+ "] because this.conversionService is null");
+			}
+			if (!this.conversionService.canConvert(formatterObjectType, type)) {
+				throw new IllegalArgumentException("Unable to index Formatter " + formatter + " under type ["
+						+ type.getName() + "]; not able to convert from a [" + formatterObjectType.getName()
+						+ "] parsed by the Formatter to [" + type.getName() + "]");
+			}
+			if (!this.conversionService.canConvert(type, formatterObjectType)) {
+				throw new IllegalArgumentException("Unable to index Formatter " + formatter + " under type ["
+						+ type.getName() + "]; not able to convert to [" + formatterObjectType.getName()
+						+ "] to format a [" + type.getName() + "]");
+			}
+		}
+		this.typeFormatters.put(type, new FormatterHolder(formatterObjectType, formatter));
 	}
 
 	public void addFormatterByAnnotation(Class<? extends Annotation> annotationType, Formatter<?> formatter) {
-		Class<?> formattedObjectType = GenericTypeResolver.resolveTypeArgument(formatter.getClass(), Formatter.class);
+		Class<?> formatterObjectType = GenericTypeResolver.resolveTypeArgument(formatter.getClass(), Formatter.class);
 		SimpleAnnotationFormatterFactory factory = new SimpleAnnotationFormatterFactory(formatter);
 		this.annotationFormatters.put(annotationType,
-				new AnnotationFormatterFactoryHolder(formattedObjectType, factory));
+				new AnnotationFormatterFactoryHolder(formatterObjectType, factory));
 	}
 
 	public void addFormatterByAnnotation(AnnotationFormatterFactory<?, ?> factory) {
-		Class[] typeArgs = GenericTypeResolver.resolveTypeArguments(factory.getClass(),
+		Class<?>[] typeArgs = GenericTypeResolver.resolveTypeArguments(factory.getClass(),
 				AnnotationFormatterFactory.class);
 		if (typeArgs == null || typeArgs.length != 2) {
 			throw new IllegalArgumentException(
@@ -236,17 +233,18 @@ public class GenericFormatterRegistry implements FormatterRegistry, ApplicationC
 	public Formatter<Object> getFormatter(TypeDescriptor type) {
 		Assert.notNull(type, "TypeDescriptor is required");
 		FormatterHolder holder = findFormatterHolderForAnnotatedProperty(type.getAnnotations());
+		Class<?> objectType = type.getObjectType();
 		if (holder == null) {
-			holder = findFormatterHolderForType(type.getType());
+			holder = findFormatterHolderForType(objectType);
 		}
 		if (holder == null) {
-			holder = getDefaultFormatterHolder(type);
+			holder = getDefaultFormatterHolder(objectType);
 		}
 		if (holder == null) {
 			return null;
 		}
-		Class formattedObjectType = holder.getFormattedObjectType();
+		Class<?> formatterObjectType = holder.getFormatterObjectType();
-		if (formattedObjectType != null && !type.getType().isAssignableFrom(formattedObjectType)) {
+		if (formatterObjectType != null && !objectType.isAssignableFrom(formatterObjectType)) {
 			if (this.conversionService != null) {
 				return new ConvertingFormatter(type, holder);
 			} else {
@@ -278,7 +276,7 @@ public class GenericFormatterRegistry implements FormatterRegistry, ApplicationC
 			Formatted formattedAnnotation = annotationType.getAnnotation(Formatted.class);
 			if (formattedAnnotation != null) {
 				// property annotation has @Formatted meta-annotation
-				Formatter formatter = createFormatter(formattedAnnotation.value());
+				Formatter<?> formatter = createFormatter(formattedAnnotation.value());
 				addFormatterByAnnotation(annotationType, formatter);
 				return findFormatterHolderForAnnotation(annotation);
 			} else {
@@ -287,11 +285,11 @@ public class GenericFormatterRegistry implements FormatterRegistry, ApplicationC
 		}
 	}
 
-	private FormatterHolder findFormatterHolderForType(Class type) {
+	private FormatterHolder findFormatterHolderForType(Class<?> type) {
-		LinkedList<Class> classQueue = new LinkedList<Class>();
+		LinkedList<Class<?>> classQueue = new LinkedList<Class<?>>();
 		classQueue.addFirst(type);
 		while (!classQueue.isEmpty()) {
-			Class currentClass = classQueue.removeLast();
+			Class<?> currentClass = classQueue.removeLast();
 			FormatterHolder holder = this.typeFormatters.get(currentClass);
 			if (holder != null) {
 				return holder;
@@ -299,19 +297,18 @@ public class GenericFormatterRegistry implements FormatterRegistry, ApplicationC
 			if (currentClass.getSuperclass() != null) {
 				classQueue.addFirst(currentClass.getSuperclass());
 			}
-			Class[] interfaces = currentClass.getInterfaces();
+			Class<?>[] interfaces = currentClass.getInterfaces();
-			for (Class ifc : interfaces) {
+			for (Class<?> ifc : interfaces) {
 				classQueue.addFirst(ifc);
 			}
 		}
 		return null;
 	}
 
-	private FormatterHolder getDefaultFormatterHolder(TypeDescriptor typeDescriptor) {
+	private FormatterHolder getDefaultFormatterHolder(Class<?> type) {
-		Class type = typeDescriptor.getType();
 		Formatted formatted = AnnotationUtils.findAnnotation(type, Formatted.class);
 		if (formatted != null) {
-			Formatter formatter = createFormatter(formatted.value());
+			Formatter<?> formatter = createFormatter(formatted.value());
 			addFormatterByType(type, formatter);
 			return findFormatterHolderForType(type);
 		} else {
@@ -319,21 +316,21 @@ public class GenericFormatterRegistry implements FormatterRegistry, ApplicationC
 		}
 	}
 
-	private Formatter createFormatter(Class<? extends Formatter> formatterClass) {
+	private Formatter<?> createFormatter(Class<? extends Formatter> formatterClass) {
 		return (this.applicationContext != null ? this.applicationContext.getAutowireCapableBeanFactory().createBean(
 				formatterClass) : BeanUtils.instantiate(formatterClass));
 	}
 
 	private abstract static class AbstractFormatterHolder {
 
-		private Class formattedObjectType;
+		private Class<?> formatterObjectType;
 
-		public AbstractFormatterHolder(Class formattedObjectType) {
+		public AbstractFormatterHolder(Class<?> formatterObjectType) {
-			this.formattedObjectType = formattedObjectType;
+			this.formatterObjectType = formatterObjectType;
 		}
 
-		public Class<?> getFormattedObjectType() {
+		public Class<?> getFormatterObjectType() {
-			return formattedObjectType;
+			return formatterObjectType;
 		}
 
 	}
@@ -342,8 +339,8 @@ public class GenericFormatterRegistry implements FormatterRegistry, ApplicationC
 
 		private Formatter formatter;
 
-		public FormatterHolder(Class formattedObjectType, Formatter formatter) {
+		public FormatterHolder(Class<?> formatterObjectType, Formatter<?> formatter) {
-			super(formattedObjectType);
+			super(formatterObjectType);
 			this.formatter = formatter;
 		}
 
@@ -357,13 +354,13 @@ public class GenericFormatterRegistry implements FormatterRegistry, ApplicationC
 
 		private AnnotationFormatterFactory factory;
 
-		public AnnotationFormatterFactoryHolder(Class formattedObjectType, AnnotationFormatterFactory factory) {
+		public AnnotationFormatterFactoryHolder(Class<?> formatterObjectType, AnnotationFormatterFactory<?, ?> factory) {
-			super(formattedObjectType);
+			super(formatterObjectType);
 			this.factory = factory;
 		}
 
 		public FormatterHolder getFormatterHolder(Annotation annotation) {
-			return new FormatterHolder(getFormattedObjectType(), this.factory.getFormatter(annotation));
+			return new FormatterHolder(getFormatterObjectType(), this.factory.getFormatter(annotation));
 		}
 
 	}
@@ -372,7 +369,7 @@ public class GenericFormatterRegistry implements FormatterRegistry, ApplicationC
 
 		private final Formatter instance;
 
-		public SimpleAnnotationFormatterFactory(Formatter instance) {
+		public SimpleAnnotationFormatterFactory(Formatter<?> instance) {
 			this.instance = instance;
 		}
 
@@ -381,28 +378,6 @@ public class GenericFormatterRegistry implements FormatterRegistry, ApplicationC
 		}
 	}
 
-	private static class ValueOfMethodFormatter implements Formatter {
-
-		private Method valueOfMethod;
-
-		public ValueOfMethodFormatter(Method valueOfMethod) {
-			this.valueOfMethod = valueOfMethod;
-		}
-
-		public String format(Object object, Locale locale) {
-			if (object == null) {
-				return "";
-			} else {
-				return object.toString();
-			}
-		}
-
-		public Object parse(String formatted, Locale locale) throws ParseException {
-			return ReflectionUtils.invokeMethod(valueOfMethod, null, formatted);
-		}
-
-	}
-
 	private class ConvertingFormatter implements Formatter {
 
 		private final TypeDescriptor type;
@@ -416,14 +391,14 @@ public class GenericFormatterRegistry implements FormatterRegistry, ApplicationC
 
 		public String format(Object object, Locale locale) {
 			object = GenericFormatterRegistry.this.conversionService.convert(object, this.formatterHolder
-					.getFormattedObjectType());
+					.getFormatterObjectType());
 			return this.formatterHolder.getFormatter().format(object, locale);
 		}
 
 		public Object parse(String formatted, Locale locale) throws ParseException {
 			Object parsed = this.formatterHolder.getFormatter().parse(formatted, locale);
 			parsed = GenericFormatterRegistry.this.conversionService.convert(parsed, TypeDescriptor
-					.valueOf(this.formatterHolder.getFormattedObjectType()), this.type);
+					.valueOf(this.formatterHolder.getFormatterObjectType()), this.type);
 			return parsed;
 		}
 	}

org.springframework.context/src/test/java/org/springframework/ui/format/support/GenericFormatterRegistryTests.java

@@ -63,6 +63,16 @@ public class GenericFormatterRegistryTests {
 		Integer i = (Integer) formatter.parse("3", Locale.US);
 		assertEquals(new Integer(3), i);
 	}
+	
+	@Test
+	public void testAddLookupByPrimitive() throws ParseException {
+		registry.addFormatterByType(new IntegerFormatter());
+		Formatter formatter = registry.getFormatter(TypeDescriptor.valueOf(int.class));
+		String formatted = formatter.format(3, Locale.US);
+		assertEquals("3", formatted);
+		int integer = (Integer) formatter.parse("3", Locale.US);
+		assertEquals(3, integer);
+	}
 
 	@Test
 	public void testAddByObjectType() {

org.springframework.core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java

@@ -317,14 +317,14 @@ public class GenericConversionService implements ConversionService, ConverterReg
 						classQueue.addFirst(Array.newInstance(componentType.getSuperclass(), 0).getClass());
 					}
 				} else {
+					Class[] interfaces = currentClass.getInterfaces();
+					for (Class ifc : interfaces) {
+						classQueue.addFirst(ifc);
+					}
 					if (currentClass.getSuperclass() != null) {
 						classQueue.addFirst(currentClass.getSuperclass());
 					}
 				}
-				Class[] interfaces = currentClass.getInterfaces();
-				for (Class ifc : interfaces) {
-					classQueue.addFirst(ifc);
-				}
 			}
 			return null;
 		}
@@ -378,14 +378,14 @@ public class GenericConversionService implements ConversionService, ConverterReg
 						classQueue.addFirst(Array.newInstance(componentType.getSuperclass(), 0).getClass());
 					}
 				} else {
+					Class[] interfaces = currentClass.getInterfaces();
+					for (Class ifc : interfaces) {
+						classQueue.addFirst(ifc);
+					}
 					if (currentClass.getSuperclass() != null) {
 						classQueue.addFirst(currentClass.getSuperclass());
 					}
 				}
-				Class[] interfaces = currentClass.getInterfaces();
-				for (Class ifc : interfaces) {
-					classQueue.addFirst(ifc);
-				}
 			}
 			return null;
 		}

org.springframework.core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java

@@ -25,6 +25,7 @@ import static org.junit.Assert.assertTrue;
 import java.util.AbstractList;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
@@ -124,6 +125,20 @@ public class GenericConversionServiceTests {
 
 	@Test
 	public void convertObjectToPrimitive() {
+		assertFalse(conversionService.canConvert(String.class, boolean.class));
+		conversionService.addConverter(new StringToBooleanConverter());
+		assertTrue(conversionService.canConvert(String.class, boolean.class));
+		Boolean b = conversionService.convert("true", boolean.class);
+		assertEquals(Boolean.TRUE, b);
+		assertTrue(conversionService.canConvert(TypeDescriptor.valueOf(String.class), TypeDescriptor
+				.valueOf(boolean.class)));
+		b = (Boolean) conversionService.convert("true", TypeDescriptor.valueOf(String.class), TypeDescriptor
+				.valueOf(boolean.class));
+		assertEquals(Boolean.TRUE, b);
+	}
+
+	@Test
+	public void convertObjectToPrimitiveViaConverterFactory() {
 		conversionService.addConverterFactory(new StringToNumberConverterFactory());
 		Integer three = conversionService.convert("3", int.class);
 		assertEquals(3, three.intValue());
@@ -262,11 +277,11 @@ public class GenericConversionServiceTests {
 		assertEquals(new Integer(2), bar.get(1));
 		assertEquals(new Integer(3), bar.get(2));
 	}
-	
+
 	@Test
 	public void convertCollectionToCollectionNull() throws Exception {
-		List<Integer> bar = (List<Integer>) conversionService.convert(null, TypeDescriptor.valueOf(LinkedHashSet.class),
+		List<Integer> bar = (List<Integer>) conversionService.convert(null,
-				new TypeDescriptor(getClass().getField("genericList")));
+				TypeDescriptor.valueOf(LinkedHashSet.class), new TypeDescriptor(getClass().getField("genericList")));
 		assertNull(bar);
 	}
 
@@ -284,6 +299,22 @@ public class GenericConversionServiceTests {
 		assertEquals("3", bar.get(2));
 	}
 
+	@Test
+	public void convertCollectionToCollectionSpecialCaseSourceImpl() throws Exception {
+		conversionService.addConverterFactory(new StringToNumberConverterFactory());
+		Map map = new LinkedHashMap();
+		map.put("1", "1");
+		map.put("2", "2");
+		map.put("3", "3");
+		Collection values = map.values();
+		List<Integer> bar = (List<Integer>) conversionService.convert(values,
+				TypeDescriptor.valueOf(values.getClass()), new TypeDescriptor(getClass().getField("genericList")));
+		assertEquals(3, bar.size());
+		assertEquals(new Integer(1), bar.get(0));
+		assertEquals(new Integer(2), bar.get(1));
+		assertEquals(new Integer(3), bar.get(2));
+	}
+
 	@Test
 	public void convertCollectionToString() {
 		List<String> list = Arrays.asList(new String[] { "foo", "bar" });