ObjectToObjectConverter caches detected methods/constructors

Includes polishing of related conversion exception messages.

Issue: SPR-13703

spring-core/src/main/java/org/springframework/core/convert/ConversionFailedException.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2010 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -43,7 +43,8 @@ public final class ConversionFailedException extends ConversionException {
 	 * @param cause the cause of the conversion failure
 	 */
 	public ConversionFailedException(TypeDescriptor sourceType, TypeDescriptor targetType, Object value, Throwable cause) {
-		super("Failed to convert from type " + sourceType + " to type " + targetType + " for value '" + ObjectUtils.nullSafeToString(value) + "'", cause);
+		super("Failed to convert from type [" + sourceType + "] to type [" + targetType +
+				"] for value '" + ObjectUtils.nullSafeToString(value) + "'", cause);
 		this.sourceType = sourceType;
 		this.targetType = targetType;
 		this.value = value;

spring-core/src/main/java/org/springframework/core/convert/ConverterNotFoundException.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2010 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -36,21 +36,21 @@ public final class ConverterNotFoundException extends ConversionException {
 	 * @param targetType the target type requested to convert to
 	 */
 	public ConverterNotFoundException(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		super("No converter found capable of converting from type " + sourceType + " to type " + targetType);
+		super("No converter found capable of converting from type [" + sourceType + "] to type [" + targetType + "]");
 		this.sourceType = sourceType;
 		this.targetType = targetType;
 	}
 
 
 	/**
-	 * Returns the source type that was requested to convert from.
+	 * Return the source type that was requested to convert from.
 	 */
 	public TypeDescriptor getSourceType() {
 		return this.sourceType;
 	}
 
 	/**
-	 * Returns the target type that was requested to convert to.
+	 * Return the target type that was requested to convert to.
 	 */
 	public TypeDescriptor getTargetType() {
 		return this.targetType;

spring-core/src/main/java/org/springframework/core/convert/support/ObjectToObjectConverter.java

@@ -20,12 +20,14 @@ import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.util.Collections;
+import java.util.Map;
 import java.util.Set;
 
 import org.springframework.core.convert.ConversionFailedException;
 import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.core.convert.converter.ConditionalGenericConverter;
 import org.springframework.util.ClassUtils;
+import org.springframework.util.ConcurrentReferenceHashMap;
 import org.springframework.util.ReflectionUtils;
 
 /**
@@ -60,6 +62,19 @@ import org.springframework.util.ReflectionUtils;
  */
 final class ObjectToObjectConverter implements ConditionalGenericConverter {
 
+	// Cache for the latest to-method resolved on a given Class
+	private static final Map<Class<?>, Method> toMethodCache =
+			new ConcurrentReferenceHashMap<Class<?>, Method>(16);
+
+	// Cache for the latest factory-method resolved on a given Class
+	private static final Map<Class<?>, Method> factoryMethodCache =
+			new ConcurrentReferenceHashMap<Class<?>, Method>(16);
+
+	// Cache for the latest factory-constructor resolved on a given Class
+	private static final Map<Class<?>, Constructor<?>> factoryConstructorCache =
+			new ConcurrentReferenceHashMap<Class<?>, Constructor<?>>(16);
+
+
 	@Override
 	public Set<ConvertiblePair> getConvertibleTypes() {
 		return Collections.singleton(new ConvertiblePair(Object.class, Object.class));
@@ -67,7 +82,7 @@ final class ObjectToObjectConverter implements ConditionalGenericConverter {
 
 	@Override
 	public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		if (sourceType.getType().equals(targetType.getType())) {
+		if (sourceType.getType() == targetType.getType()) {
 			// no conversion required
 			return false;
 		}
@@ -113,35 +128,56 @@ final class ObjectToObjectConverter implements ConditionalGenericConverter {
 
 		// If sourceClass is Number and targetClass is Integer, then the following message
 		// format should expand to:
-		// No toInteger() method exists on java.lang.Number, and no static
-		// valueOf/of/from(java.lang.Number) method or Integer(java.lang.Number)
-		// constructor exists on java.lang.Integer.
+		// No toInteger() method exists on java.lang.Number, and no static valueOf/of/from(java.lang.Number)
+		// method or Integer(java.lang.Number) constructor exists on java.lang.Integer.
 		String message = String.format(
-			"No to%3$s() method exists on %1$s, and no static valueOf/of/from(%1$s) method or %3$s(%1$s) constructor exists on %2$s.",
-			sourceClass.getName(), targetClass.getName(), targetClass.getSimpleName());
+				"No to%3$s() method exists on %1$s, and no static valueOf/of/from(%1$s) method or %3$s(%1$s) constructor exists on %2$s.",
+				sourceClass.getName(), targetClass.getName(), targetClass.getSimpleName());
 
 		throw new IllegalStateException(message);
 	}
 
 
 	private static Method getToMethod(Class<?> targetClass, Class<?> sourceClass) {
-		Method method = ClassUtils.getMethodIfAvailable(sourceClass, "to" + targetClass.getSimpleName());
-		return (method != null && targetClass.equals(method.getReturnType()) ? method : null);
+		Method method = toMethodCache.get(sourceClass);
+		if (method == null || !ClassUtils.isAssignable(targetClass, method.getReturnType())) {
+			method = ClassUtils.getMethodIfAvailable(sourceClass, "to" + targetClass.getSimpleName());
+			if (method == null || !ClassUtils.isAssignable(targetClass, method.getReturnType())) {
+				return null;
+			}
+			toMethodCache.put(sourceClass, method);
+		}
+		return method;
 	}
 
 	private static Method getFactoryMethod(Class<?> targetClass, Class<?> sourceClass) {
-		Method method = ClassUtils.getStaticMethod(targetClass, "valueOf", sourceClass);
-		if (method == null) {
-			method = ClassUtils.getStaticMethod(targetClass, "of", sourceClass);
+		Method method = factoryMethodCache.get(targetClass);
+		if (method == null || method.getParameterTypes()[0] != sourceClass) {
+			method = ClassUtils.getStaticMethod(targetClass, "valueOf", sourceClass);
 			if (method == null) {
-				method = ClassUtils.getStaticMethod(targetClass, "from", sourceClass);
+				method = ClassUtils.getStaticMethod(targetClass, "of", sourceClass);
+				if (method == null) {
+					method = ClassUtils.getStaticMethod(targetClass, "from", sourceClass);
+					if (method == null) {
+						return null;
+					}
+				}
 			}
+			factoryMethodCache.put(targetClass, method);
 		}
 		return method;
 	}
 
 	private static Constructor<?> getFactoryConstructor(Class<?> targetClass, Class<?> sourceClass) {
-		return ClassUtils.getConstructorIfAvailable(targetClass, sourceClass);
+		Constructor<?> ctor = factoryConstructorCache.get(targetClass);
+		if (ctor == null || ctor.getParameterTypes()[0] != sourceClass) {
+			ctor = ClassUtils.getConstructorIfAvailable(targetClass, sourceClass);
+			if (ctor == null) {
+				return null;
+			}
+			factoryConstructorCache.put(targetClass, ctor);
+		}
+		return ctor;
 	}
 
 	private static boolean hasToMethodOrFactoryMethodOrConstructor(Class<?> targetClass, Class<?> sourceClass) {

spring-core/src/test/java/org/springframework/core/convert/support/DefaultConversionServiceTests.java

@@ -617,7 +617,7 @@ public class DefaultConversionServiceTests {
 
 	@Test
 	public void convertArrayToArray() {
-		Integer[] result = conversionService.convert(new String[] { "1", "2", "3" }, Integer[].class);
+		Integer[] result = conversionService.convert(new String[] {"1", "2", "3"}, Integer[].class);
 		assertEquals(new Integer(1), result[0]);
 		assertEquals(new Integer(2), result[1]);
 		assertEquals(new Integer(3), result[2]);
@@ -625,7 +625,7 @@ public class DefaultConversionServiceTests {
 
 	@Test
 	public void convertArrayToPrimitiveArray() {
-		int[] result = conversionService.convert(new String[] { "1", "2", "3" }, int[].class);
+		int[] result = conversionService.convert(new String[] {"1", "2", "3"}, int[].class);
 		assertEquals(1, result[0]);
 		assertEquals(2, result[1]);
 		assertEquals(3, result[2]);
@@ -633,14 +633,14 @@ public class DefaultConversionServiceTests {
 
 	@Test
 	public void convertArrayToWrapperArray() {
-		byte[] byteArray = new byte[] { 1, 2, 3 };
+		byte[] byteArray = new byte[] {1, 2, 3};
 		Byte[] converted = conversionService.convert(byteArray, Byte[].class);
-		assertTrue(Arrays.equals(converted, new Byte[] { 1, 2, 3 }));
+		assertThat(converted, equalTo(new Byte[] {1, 2, 3}));
 	}
 
 	@Test
 	public void convertArrayToArrayAssignable() {
-		int[] result = conversionService.convert(new int[] { 1, 2, 3 }, int[].class);
+		int[] result = conversionService.convert(new int[] {1, 2, 3}, int[].class);
 		assertEquals(1, result[0]);
 		assertEquals(2, result[1]);
 		assertEquals(3, result[2]);
@@ -845,7 +845,7 @@ public class DefaultConversionServiceTests {
 	}
 
 	@Test(expected = ConverterNotFoundException.class)
-	public void convertObjectToObjectNoValueOFMethodOrConstructor() {
+	public void convertObjectToObjectNoValueOfMethodOrConstructor() {
 		conversionService.convert(new Long(3), SSN.class);
 	}
 
@@ -857,27 +857,27 @@ public class DefaultConversionServiceTests {
 
 	@Test
 	public void convertObjectToObjectFinderMethodWithNull() {
-		TestEntity e = (TestEntity) conversionService.convert(null,
+		TestEntity entity = (TestEntity) conversionService.convert(null,
 				TypeDescriptor.valueOf(String.class), TypeDescriptor.valueOf(TestEntity.class));
-		assertNull(e);
+		assertNull(entity);
 	}
 
 	@Test
 	public void convertObjectToObjectFinderMethodWithIdConversion() {
-		TestEntity e = conversionService.convert("1", TestEntity.class);
-		assertEquals(new Long(1), e.getId());
+		TestEntity entity = conversionService.convert("1", TestEntity.class);
+		assertEquals(new Long(1), entity.getId());
 	}
 
 	@Test
 	public void convertCharArrayToString() throws Exception {
-		String converted = conversionService.convert(new char[] { 'a', 'b', 'c' }, String.class);
+		String converted = conversionService.convert(new char[] {'a', 'b', 'c'}, String.class);
 		assertThat(converted, equalTo("a,b,c"));
 	}
 
 	@Test
 	public void convertStringToCharArray() throws Exception {
 		char[] converted = conversionService.convert("a,b,c", char[].class);
-		assertThat(converted, equalTo(new char[] { 'a', 'b', 'c' }));
+		assertThat(converted, equalTo(new char[] {'a', 'b', 'c'}));
 	}
 
 	@Test
@@ -959,6 +959,7 @@ public class DefaultConversionServiceTests {
 		// System.out.println(watch.prettyPrint());
 	}
 
+
 	@SuppressWarnings("serial")
 	public static class CustomNumber extends Number {