From 22670b7fad9ecdbf4de2ae1c7a112aae123352ff Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 25 Mar 2015 00:42:51 +0100 Subject: [PATCH] Fixed addConverterFactory assertion --- .../support/GenericConversionService.java | 81 ++++++++++--------- .../GenericConversionServiceTests.java | 25 +++--- 2 files changed, 54 insertions(+), 52 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java b/spring-core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java index f9bb8d3bd1d..0b64b1366e2 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java +++ b/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(64); - // implementing ConverterRegistry + // ConverterRegistry implementation @Override public void addConverter(Converter converter) { @@ -104,7 +104,8 @@ public class GenericConversionService implements ConfigurableConversionService { @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 class GenericConversionService implements ConfigurableConversionService { @Override public void addConverterFactory(ConverterFactory converterFactory) { ResolvableType[] typeInfo = getRequiredTypeInfo(converterFactory, ConverterFactory.class); - Assert.notNull("Unable to the determine sourceType and targetRangeType R which your " + + Assert.notNull(typeInfo, "Unable to the determine source type and target range type R which your " + "ConverterFactory converts between; declare these generic types."); addConverter(new ConverterFactoryAdapter(converterFactory, new ConvertiblePair(typeInfo[0].resolve(), typeInfo[1].resolve()))); @@ -128,7 +129,8 @@ public class GenericConversionService implements ConfigurableConversionService { invalidateCache(); } - // implementing ConversionService + + // ConversionService implementation @Override public boolean canConvert(Class sourceType, Class targetType) { @@ -148,17 +150,18 @@ public class GenericConversionService implements ConfigurableConversionService { } /** - * 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. + *

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 class GenericConversionService implements ConfigurableConversionService { @Override @SuppressWarnings("unchecked") public T convert(Object source, Class 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 class GenericConversionService implements ConfigurableConversionService { * @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 class GenericConversionService implements ConfigurableConversionService { // Protected template methods /** - * Template method to convert a null source. - *

Default implementation returns {@code null} or the Java 8 + * Template method to convert a {@code null} source. + *

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 @@ public class GenericConversionService implements ConfigurableConversionService { * 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 @@ public class GenericConversionService implements ConfigurableConversionService { /** * 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. + *

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 @@ public class GenericConversionService implements ConfigurableConversionService { 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 @@ public class GenericConversionService implements ConfigurableConversionService { 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 class GenericConversionService implements ConfigurableConversionService { @Override public String toString() { - return this.typeInfo + " : " + this.converter; + return (this.typeInfo + " : " + this.converter); } } @@ -425,7 +427,7 @@ public class GenericConversionService implements ConfigurableConversionService { @Override public String toString() { - return this.typeInfo + " : " + this.converterFactory; + return (this.typeInfo + " : " + this.converterFactory); } } @@ -453,20 +455,20 @@ public class GenericConversionService implements ConfigurableConversionService { 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 @@ public class GenericConversionService implements ConfigurableConversionService { 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 @@ public class GenericConversionService implements ConfigurableConversionService { Set> visited = new HashSet>(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); diff --git a/spring-core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java b/spring-core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java index dc87b5f0bcf..68e0509f8da 100644 --- a/spring-core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java +++ b/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 class GenericConversionServiceTests { 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 class GenericConversionServiceTests { 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 class GenericConversionServiceTests { } // 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 class GenericConversionServiceTests { @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 class GenericConversionServiceTests { @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 class GenericConversionServiceTests { @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 @@ public class GenericConversionServiceTests { } - private static class MyBaseInterfaceConverter implements Converter { + private static class MyBaseInterfaceToStringConverter implements Converter { @Override public String convert(MyBaseInterface source) {