Fixed addConverterFactory assertion

(cherry picked from commit 22670b7)

Author: jhoeller

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 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.
@@ -92,7 +92,7 @@ public class GenericConversionService implements ConfigurableConversionService {
 			new ConcurrentHashMap<ConverterCacheKey, GenericConverter>(64);
 
 
-	// implementing ConverterRegistry
+	// ConverterRegistry implementation
 
 	@Override
 	public void addConverter(Converter<?, ?> converter) {
@@ -104,7 +104,8 @@ public void addConverter(Converter<?, ?> converter) {
 
 	@Override
 	public void addConverter(Class<?> sourceType, Class<?> targetType, Converter<?, ?> converter) {
-		addConverter(new ConverterAdapter(converter, ResolvableType.forClass(sourceType), ResolvableType.forClass(targetType)));
+		addConverter(new ConverterAdapter(
+				converter, ResolvableType.forClass(sourceType), ResolvableType.forClass(targetType)));
 	}
 
 	@Override
@@ -116,7 +117,7 @@ public void addConverter(GenericConverter converter) {
 	@Override
 	public void addConverterFactory(ConverterFactory<?, ?> converterFactory) {
 		ResolvableType[] typeInfo = getRequiredTypeInfo(converterFactory, ConverterFactory.class);
-		Assert.notNull("Unable to the determine sourceType <S> and targetRangeType R which your " +
+		Assert.notNull(typeInfo, "Unable to the determine source type <S> and target range type R which your " +
 				"ConverterFactory<S, R> converts between; declare these generic types.");
 		addConverter(new ConverterFactoryAdapter(converterFactory,
 				new ConvertiblePair(typeInfo[0].resolve(), typeInfo[1].resolve())));
@@ -128,7 +129,8 @@ public void removeConvertible(Class<?> sourceType, Class<?> targetType) {
 		invalidateCache();
 	}
 
-	// implementing ConversionService
+
+	// ConversionService implementation
 
 	@Override
 	public boolean canConvert(Class<?> sourceType, Class<?> targetType) {
@@ -148,17 +150,18 @@ public boolean canConvert(TypeDescriptor sourceType, TypeDescriptor targetType)
 	}
 
 	/**
-	 * Returns true if conversion between the sourceType and targetType can be bypassed.
-	 * More precisely this method will return true if objects of sourceType can be
+	 * Return whether conversion between the sourceType and targetType can be bypassed.
+	 * <p>More precisely, this method will return true if objects of sourceType can be
 	 * converted to the targetType by returning the source object unchanged.
-	 * @param sourceType context about the source type to convert from (may be null if source is null)
+	 * @param sourceType context about the source type to convert from
+	 * (may be {@code null} if source is {@code null})
 	 * @param targetType context about the target type to convert to (required)
-	 * @return true if conversion can be bypassed
-	 * @throws IllegalArgumentException if targetType is null
+	 * @return {@code true} if conversion can be bypassed; {@code false} otherwise
+	 * @throws IllegalArgumentException if targetType is {@code null}
 	 * @since 3.2
 	 */
 	public boolean canBypassConvert(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		Assert.notNull(targetType, "The targetType to convert to cannot be null");
+		Assert.notNull(targetType, "targetType to convert to cannot be null");
 		if (sourceType == null) {
 			return true;
 		}
@@ -169,19 +172,19 @@ public boolean canBypassConvert(TypeDescriptor sourceType, TypeDescriptor target
 	@Override
 	@SuppressWarnings("unchecked")
 	public <T> T convert(Object source, Class<T> targetType) {
-		Assert.notNull(targetType,"The targetType to convert to cannot be null");
+		Assert.notNull(targetType, "targetType to convert to cannot be null");
 		return (T) convert(source, TypeDescriptor.forObject(source), TypeDescriptor.valueOf(targetType));
 	}
 
 	@Override
 	public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
-		Assert.notNull(targetType,"The targetType to convert to cannot be null");
+		Assert.notNull(targetType, "targetType to convert to cannot be null");
 		if (sourceType == null) {
-			Assert.isTrue(source == null, "The source must be [null] if sourceType == [null]");
-			return handleResult(sourceType, targetType, convertNullSource(sourceType, targetType));
+			Assert.isTrue(source == null, "source must be [null] if sourceType == [null]");
+			return handleResult(null, targetType, convertNullSource(null, targetType));
 		}
 		if (source != null && !sourceType.getObjectType().isInstance(source)) {
-			throw new IllegalArgumentException("The source to convert from must be an instance of " +
+			throw new IllegalArgumentException("source to convert from must be an instance of " +
 					sourceType + "; instead it was a " + source.getClass().getName());
 		}
 		GenericConverter converter = getConverter(sourceType, targetType);
@@ -202,8 +205,8 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
 	 * @param targetType the target type
 	 * @return the converted value
 	 * @throws ConversionException if a conversion exception occurred
-	 * @throws IllegalArgumentException if targetType is null,
-	 * or sourceType is null but source is not null
+	 * @throws IllegalArgumentException if targetType is {@code null},
+	 * or sourceType is {@code null} but source is not {@code null}
 	 */
 	public Object convert(Object source, TypeDescriptor targetType) {
 		return convert(source, TypeDescriptor.forObject(source), targetType);
@@ -218,11 +221,11 @@ public String toString() {
 	// Protected template methods
 
 	/**
-	 * Template method to convert a null source.
-	 * <p>Default implementation returns {@code null} or the Java 8
+	 * Template method to convert a {@code null} source.
+	 * <p>The default implementation returns {@code null} or the Java 8
 	 * {@link java.util.Optional#empty()} instance if the target type is
-	 * {@code java.uti.Optional}.
-	 * Subclasses may override to return custom null objects for specific target types.
+	 * {@code java.util.Optional}. Subclasses may override this to return
+	 * custom {@code null} objects for specific target types.
 	 * @param sourceType the sourceType to convert from
 	 * @param targetType the targetType to convert to
 	 * @return the converted null object
@@ -239,11 +242,10 @@ protected Object convertNullSource(TypeDescriptor sourceType, TypeDescriptor tar
 	 * First queries this ConversionService's converter cache.
 	 * On a cache miss, then performs an exhaustive search for a matching converter.
 	 * If no converter matches, returns the default converter.
-	 * Subclasses may override.
 	 * @param sourceType the source type to convert from
 	 * @param targetType the target type to convert to
-	 * @return the generic converter that will perform the conversion, or {@code null} if
-	 * no suitable converter was found
+	 * @return the generic converter that will perform the conversion,
+	 * or {@code null} if no suitable converter was found
 	 * @see #getDefaultConverter(TypeDescriptor, TypeDescriptor)
 	 */
 	protected GenericConverter getConverter(TypeDescriptor sourceType, TypeDescriptor targetType) {
@@ -269,9 +271,8 @@ protected GenericConverter getConverter(TypeDescriptor sourceType, TypeDescripto
 
 	/**
 	 * Return the default converter if no converter is found for the given sourceType/targetType pair.
-	 * Returns a NO_OP Converter if the sourceType is assignable to the targetType.
+	 * <p>Returns a NO_OP Converter if the sourceType is assignable to the targetType.
 	 * Returns {@code null} otherwise, indicating no suitable converter could be found.
-	 * Subclasses may override.
 	 * @param sourceType the source type to convert from
 	 * @param targetType the target type to convert to
 	 * @return the default generic converter that will perform the conversion
@@ -280,7 +281,8 @@ protected GenericConverter getDefaultConverter(TypeDescriptor sourceType, TypeDe
 		return (sourceType.isAssignableTo(targetType) ? NO_OP_CONVERTER : null);
 	}
 
-	// internal helpers
+
+	// Internal helpers
 
 	private ResolvableType[] getRequiredTypeInfo(Object converter, Class<?> genericIfc) {
 		ResolvableType resolvableType = ResolvableType.forClass(converter.getClass()).as(genericIfc);
@@ -303,7 +305,7 @@ private void invalidateCache() {
 	private Object handleConverterNotFound(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
 		if (source == null) {
 			assertNotPrimitiveTargetType(sourceType, targetType);
-			return source;
+			return null;
 		}
 		if (sourceType.isAssignableTo(targetType) && targetType.getObjectType().isInstance(source)) {
 			return source;
@@ -375,7 +377,7 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
 
 		@Override
 		public String toString() {
-			return this.typeInfo + " : " + this.converter;
+			return (this.typeInfo + " : " + this.converter);
 		}
 	}
 
@@ -425,7 +427,7 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
 
 		@Override
 		public String toString() {
-			return this.typeInfo + " : " + this.converterFactory;
+			return (this.typeInfo + " : " + this.converterFactory);
 		}
 	}
 
@@ -453,20 +455,20 @@ public boolean equals(Object other) {
 				return false;
 			}
 			ConverterCacheKey otherKey = (ConverterCacheKey) other;
-			return ObjectUtils.nullSafeEquals(this.sourceType, otherKey.sourceType) &&
-					ObjectUtils.nullSafeEquals(this.targetType, otherKey.targetType);
+			return (ObjectUtils.nullSafeEquals(this.sourceType, otherKey.sourceType) &&
+					ObjectUtils.nullSafeEquals(this.targetType, otherKey.targetType));
 		}
 
 		@Override
 		public int hashCode() {
-			return ObjectUtils.nullSafeHashCode(this.sourceType) * 29 +
-					ObjectUtils.nullSafeHashCode(this.targetType);
+			return (ObjectUtils.nullSafeHashCode(this.sourceType) * 29 +
+					ObjectUtils.nullSafeHashCode(this.targetType));
 		}
 
 		@Override
 		public String toString() {
-			return "ConverterCacheKey [sourceType = " + this.sourceType +
-					", targetType = " + this.targetType + "]";
+			return ("ConverterCacheKey [sourceType = " + this.sourceType +
+					", targetType = " + this.targetType + "]");
 		}
 	}
 
@@ -544,9 +546,9 @@ private GenericConverter getRegisteredConverter(TypeDescriptor sourceType,
 					return converter;
 				}
 			}
-			// Check ConditionalGenericConverter that match all types
+			// Check ConditionalConverters for a dynamic match
 			for (GenericConverter globalConverter : this.globalConverters) {
-				if (((ConditionalConverter)globalConverter).matches(sourceType, targetType)) {
+				if (((ConditionalConverter) globalConverter).matches(sourceType, targetType)) {
 					return globalConverter;
 				}
 			}
@@ -563,6 +565,7 @@ private List<Class<?>> getClassHierarchy(Class<?> type) {
 			Set<Class<?>> visited = new HashSet<Class<?>>(20);
 			addToClassHierarchy(0, ClassUtils.resolvePrimitiveIfNecessary(type), false, hierarchy, visited);
 			boolean array = type.isArray();
+
 			int i = 0;
 			while (i < hierarchy.size()) {
 				Class<?> candidate = hierarchy.get(i);

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 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.
@@ -113,17 +113,17 @@ public void convertNullSource() {
 		assertEquals(null, conversionService.convert(null, Integer.class));
 	}
 
-	@Test(expected=ConversionFailedException.class)
+	@Test(expected = ConversionFailedException.class)
 	public void convertNullSourcePrimitiveTarget() {
 		assertEquals(null, conversionService.convert(null, int.class));
 	}
 
-	@Test(expected=ConversionFailedException.class)
+	@Test(expected = ConversionFailedException.class)
 	public void convertNullSourcePrimitiveTargetTypeDescriptor() {
 		conversionService.convert(null, TypeDescriptor.valueOf(String.class), TypeDescriptor.valueOf(int.class));
 	}
 
-	@Test(expected=IllegalArgumentException.class)
+	@Test(expected = IllegalArgumentException.class)
 	public void convertNotNullSourceNullSourceTypeDescriptor() {
 		conversionService.convert("3", null, TypeDescriptor.valueOf(int.class));
 	}
@@ -177,18 +177,18 @@ public void convertNull() {
 		assertNull(conversionService.convert(null, Integer.class));
 	}
 
-	@Test(expected=IllegalArgumentException.class)
+	@Test(expected = IllegalArgumentException.class)
 	public void convertNullTargetClass() {
 		assertNull(conversionService.convert("3", (Class<?>) null));
 		assertNull(conversionService.convert("3", TypeDescriptor.valueOf(String.class), null));
 	}
 
-	@Test(expected=IllegalArgumentException.class)
+	@Test(expected = IllegalArgumentException.class)
 	public void convertNullTypeDescriptor() {
 		assertNull(conversionService.convert("3", TypeDescriptor.valueOf(String.class), null));
 	}
 
-	@Test(expected=IllegalArgumentException.class)
+	@Test(expected = IllegalArgumentException.class)
 	public void convertWrongSourceTypeDescriptor() {
 		conversionService.convert("3", TypeDescriptor.valueOf(Integer.class), TypeDescriptor.valueOf(Long.class));
 	}
@@ -218,8 +218,7 @@ public Integer convert(CharSequence source) {
 	}
 
 	// SPR-8718
-
-	@Test(expected=ConverterNotFoundException.class)
+	@Test(expected = ConverterNotFoundException.class)
 	public void convertSuperTarget() {
 		conversionService.addConverter(new ColorConverter());
 		conversionService.convert("#000000", SystemColor.class);
@@ -295,7 +294,7 @@ public void testMapToObjectConversion() {
 	@Test
 	public void testInterfaceToString() {
 		GenericConversionService conversionService = new GenericConversionService();
-		conversionService.addConverter(new MyBaseInterfaceConverter());
+		conversionService.addConverter(new MyBaseInterfaceToStringConverter());
 		conversionService.addConverter(new ObjectToStringConverter());
 		Object converted = conversionService.convert(new MyInterfaceImplementer(), String.class);
 		assertEquals("RESULT", converted);
@@ -304,7 +303,7 @@ public void testInterfaceToString() {
 	@Test
 	public void testInterfaceArrayToStringArray() {
 		GenericConversionService conversionService = new GenericConversionService();
-		conversionService.addConverter(new MyBaseInterfaceConverter());
+		conversionService.addConverter(new MyBaseInterfaceToStringConverter());
 		conversionService.addConverter(new ArrayToArrayConverter(conversionService));
 		String[] converted = conversionService.convert(new MyInterface[] {new MyInterfaceImplementer()}, String[].class);
 		assertEquals("RESULT", converted[0]);
@@ -313,7 +312,7 @@ public void testInterfaceArrayToStringArray() {
 	@Test
 	public void testObjectArrayToStringArray() {
 		GenericConversionService conversionService = new GenericConversionService();
-		conversionService.addConverter(new MyBaseInterfaceConverter());
+		conversionService.addConverter(new MyBaseInterfaceToStringConverter());
 		conversionService.addConverter(new ArrayToArrayConverter(conversionService));
 		String[] converted = conversionService.convert(new MyInterfaceImplementer[] {new MyInterfaceImplementer()}, String[].class);
 		assertEquals("RESULT", converted[0]);
@@ -813,7 +812,7 @@ private static class MyInterfaceImplementer implements MyInterface {
 	}
 
 
-	private static class MyBaseInterfaceConverter implements Converter<MyBaseInterface, String> {
+	private static class MyBaseInterfaceToStringConverter implements Converter<MyBaseInterface, String> {
 
 		@Override
 		public String convert(MyBaseInterface source) {