From ed5de705ac2dab5457c8dad13986ffd700d4223b Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 9 Dec 2009 16:16:55 +0000 Subject: [PATCH] GenericConversionService prefers matches against inherited interfaces over superclasses (SPR-6297) git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@2610 50f2f4bb-b051-0410-bef5-90022cba6387 --- .../support/GenericConversionService.java | 75 +++++++++------- .../GenericConversionServiceTests.java | 86 +++++++++++++++++-- 2 files changed, 124 insertions(+), 37 deletions(-) diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java index 81dfa12df4b..3b132493841 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java +++ b/org.springframework.core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java @@ -16,8 +16,6 @@ package org.springframework.core.convert.support; -import static org.springframework.core.convert.support.ConversionUtils.invokeConverter; - import java.lang.reflect.Array; import java.util.ArrayList; import java.util.Collections; @@ -29,6 +27,7 @@ import java.util.Map; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; + import org.springframework.core.GenericTypeResolver; import org.springframework.core.convert.ConversionFailedException; import org.springframework.core.convert.ConversionService; @@ -39,11 +38,14 @@ import org.springframework.core.convert.converter.Converter; import org.springframework.core.convert.converter.ConverterFactory; import org.springframework.core.convert.converter.ConverterRegistry; import org.springframework.core.convert.converter.GenericConverter; +import static org.springframework.core.convert.support.ConversionUtils.*; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; /** - * Base ConversionService implementation suitable for use in most environments. + * Base {@link ConversionService} implementation suitable for use in most environments. + * Implements {@link ConverterRegistry} as registration API. + * * @author Keith Donald * @author Juergen Hoeller * @since 3.0 @@ -56,21 +58,23 @@ public class GenericConversionService implements ConversionService, ConverterReg public Class[][] getConvertibleTypes() { return null; } - public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { return source; } }; - private final Map, Map, MatchableConverters>> converters = new HashMap, Map, MatchableConverters>>(36); + + private final Map, Map, MatchableConverters>> converters = + new HashMap, Map, MatchableConverters>>(36); + // implementing ConverterRegistry public void addConverter(Converter converter) { Class[] typeInfo = getRequiredTypeInfo(converter, Converter.class); if (typeInfo == null) { - throw new IllegalArgumentException( - "Unable to the determine sourceType and targetType your Converter converts between; declare these types or implement ConverterInfo"); + throw new IllegalArgumentException("Unable to the determine sourceType and targetType which " + + "your Converter converts between; declare these generic types."); } addGenericConverter(new ConverterAdapter(typeInfo, converter)); } @@ -78,8 +82,8 @@ public class GenericConversionService implements ConversionService, ConverterReg public void addConverterFactory(ConverterFactory converterFactory) { Class[] typeInfo = getRequiredTypeInfo(converterFactory, ConverterFactory.class); if (typeInfo == null) { - throw new IllegalArgumentException( - "Unable to the determine sourceType and targetRangeType R your ConverterFactory converts between; declare these types or implement ConverterInfo"); + throw new IllegalArgumentException("Unable to the determine sourceType and targetRangeType R which " + + "your ConverterFactory converts between; declare these generic types."); } addGenericConverter(new ConverterFactoryAdapter(typeInfo, converterFactory)); } @@ -95,6 +99,7 @@ public class GenericConversionService implements ConversionService, ConverterReg getSourceConverterMap(sourceType).remove(targetType); } + // implementing ConversionService public boolean canConvert(Class sourceType, Class targetType) { @@ -148,6 +153,7 @@ public class GenericConversionService implements ConversionService, ConverterReg return builder.toString(); } + // subclassing hooks /** @@ -161,8 +167,8 @@ public class GenericConversionService implements ConversionService, ConverterReg */ protected Object convertNullSource(TypeDescriptor sourceType, TypeDescriptor targetType) { if (targetType.isPrimitive()) { - throw new ConversionFailedException(sourceType, targetType, null, new IllegalArgumentException( - "A null value cannot be assigned to a primitive type")); + throw new ConversionFailedException(sourceType, targetType, null, + new IllegalArgumentException("A null value cannot be assigned to a primitive type")); } return null; } @@ -170,9 +176,8 @@ public class GenericConversionService implements ConversionService, ConverterReg /** * Hook method to lookup the converter for a given sourceType/targetType pair. * First queries this ConversionService's converter map. - * If no suitable Converter is found, and a {@link #setParent parent} is set, then queries the parent. - * Returns null if this ConversionService simply cannot convert between sourceType and targetType. - * Subclasses may override. + *

Returns null if this ConversionService simply cannot convert + * between sourceType and targetType. 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 null if no suitable converter was found @@ -278,7 +283,7 @@ public class GenericConversionService implements ConversionService, ConverterReg else { Class[] interfaces = currentClass.getInterfaces(); for (Class ifc : interfaces) { - classQueue.addFirst(ifc); + addInterfaceHierarchy(ifc, classQueue); } if (currentClass.getSuperclass() != null) { classQueue.addFirst(currentClass.getSuperclass()); @@ -333,10 +338,11 @@ public class GenericConversionService implements ConversionService, ConverterReg if (componentType.getSuperclass() != null) { classQueue.addFirst(Array.newInstance(componentType.getSuperclass(), 0).getClass()); } - } else { + } + else { Class[] interfaces = currentClass.getInterfaces(); for (Class ifc : interfaces) { - classQueue.addFirst(ifc); + addInterfaceHierarchy(ifc, classQueue); } if (currentClass.getSuperclass() != null) { classQueue.addFirst(currentClass.getSuperclass()); @@ -347,10 +353,17 @@ public class GenericConversionService implements ConversionService, ConverterReg } } - private GenericConverter matchConverter(MatchableConverters matchable, TypeDescriptor sourceFieldType, - TypeDescriptor targetFieldType) { + private void addInterfaceHierarchy(Class ifc, LinkedList> classQueue) { + classQueue.addFirst(ifc); + for (Class inheritedIfc : ifc.getInterfaces()) { + addInterfaceHierarchy(inheritedIfc, classQueue); + } + } - return matchable != null ? matchable.matchConverter(sourceFieldType, targetFieldType) : null; + private GenericConverter matchConverter( + MatchableConverters matchable, TypeDescriptor sourceFieldType, TypeDescriptor targetFieldType) { + + return (matchable != null ? matchable.matchConverter(sourceFieldType, targetFieldType) : null); } @@ -367,7 +380,7 @@ public class GenericConversionService implements ConversionService, ConverterReg } public Class[][] getConvertibleTypes() { - return new Class[][] { this.typeInfo }; + return new Class[][] {this.typeInfo}; } public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { @@ -396,7 +409,7 @@ public class GenericConversionService implements ConversionService, ConverterReg } public Class[][] getConvertibleTypes() { - return new Class[][] { this.typeInfo }; + return new Class[][] {this.typeInfo}; } public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { @@ -407,7 +420,8 @@ public class GenericConversionService implements ConversionService, ConverterReg } public String toString() { - return this.typeInfo[0].getName() + " -> " + this.typeInfo[1].getName() + " : " + this.converterFactory.toString(); + return this.typeInfo[0].getName() + " -> " + this.typeInfo[1].getName() + " : " + + this.converterFactory.toString(); } } @@ -424,7 +438,8 @@ public class GenericConversionService implements ConversionService, ConverterReg this.conditionalConverters = new LinkedList(); } this.conditionalConverters.addFirst((ConditionalGenericConverter) converter); - } else { + } + else { this.defaultConverter = converter; } } @@ -434,18 +449,19 @@ public class GenericConversionService implements ConversionService, ConverterReg for (ConditionalGenericConverter conditional : this.conditionalConverters) { if (conditional.matches(sourceType, targetType)) { if (logger.isDebugEnabled()) { - logger.debug("Converter Lookup [MATCHED] " + conditional); + logger.debug("Converter lookup [MATCHED] " + conditional); } return conditional; - } else { + } + else { if (logger.isDebugEnabled()) { - logger.debug("Converter Lookup [DID NOT MATCH] " + conditional); + logger.debug("Converter lookup [DID NOT MATCH] " + conditional); } } } } if (logger.isDebugEnabled()) { - logger.debug("Converter Lookup [MATCHED] " + this.defaultConverter); + logger.debug("Converter lookup [MATCHED] " + this.defaultConverter); } return this.defaultConverter; } @@ -463,7 +479,8 @@ public class GenericConversionService implements ConversionService, ConverterReg builder.append(", ").append(this.defaultConverter); } return builder.toString(); - } else { + } + else { return this.defaultConverter.toString(); } } diff --git a/org.springframework.core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java b/org.springframework.core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java index f38083f7b96..4a44fa99e42 100644 --- a/org.springframework.core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java +++ b/org.springframework.core/src/test/java/org/springframework/core/convert/support/GenericConversionServiceTests.java @@ -16,18 +16,23 @@ package org.springframework.core.convert.support; -import static junit.framework.Assert.assertEquals; -import static junit.framework.Assert.assertNull; -import static junit.framework.Assert.fail; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import static org.junit.Assert.*; import org.junit.Test; + import org.springframework.core.convert.ConversionFailedException; import org.springframework.core.convert.ConverterNotFoundException; import org.springframework.core.convert.TypeDescriptor; import org.springframework.core.convert.converter.Converter; +/** + * @author Keith Donald + * @author Juergen Hoeller + */ public class GenericConversionServiceTests { private GenericConversionService conversionService = new GenericConversionService(); @@ -53,7 +58,8 @@ public class GenericConversionServiceTests { try { conversionService.convert("3", Integer.class); fail("Should have thrown an exception"); - } catch (ConverterNotFoundException e) { + } + catch (ConverterNotFoundException e) { } } @@ -91,7 +97,8 @@ public class GenericConversionServiceTests { try { conversionService.convert("BOGUS", Integer.class); fail("Should have failed"); - } catch (ConversionFailedException e) { + } + catch (ConversionFailedException e) { } } @@ -136,9 +143,72 @@ public class GenericConversionServiceTests { conversionService.addGenericConverter(new ObjectToArrayConverter(conversionService)); conversionService.convert("1", Integer[].class); fail("Should hace failed"); - } catch (ConversionFailedException e) { + } + catch (ConversionFailedException e) { assertTrue(e.getCause() instanceof ConverterNotFoundException); } } + @Test + public void testListToIterableConversion() { + GenericConversionService conversionService = new GenericConversionService(); + List raw = new ArrayList(); + raw.add("one"); + raw.add("two"); + Object converted = conversionService.convert(raw, Iterable.class); + assertSame(raw, converted); + } + + @Test + public void testListToObjectConversion() { + GenericConversionService conversionService = new GenericConversionService(); + List raw = new ArrayList(); + raw.add("one"); + raw.add("two"); + Object converted = conversionService.convert(raw, Object.class); + assertSame(raw, converted); + } + + @Test + public void testMapToObjectConversionBug() { + GenericConversionService conversionService = new GenericConversionService(); + Map raw = new HashMap(); + raw.put("key", "value"); + Object converted = conversionService.convert(raw, Object.class); + assertSame(raw, converted); + } + + @Test + public void testInterfaceToString() { + GenericConversionService conversionService = new GenericConversionService(); + conversionService.addConverter(new MyBaseInterfaceConverter()); + conversionService.addConverter(new ObjectToStringConverter()); + Object converted = conversionService.convert(new MyInterfaceImplementer(), String.class); + assertEquals("RESULT", converted); + } + + + private interface MyBaseInterface { + + } + + + private interface MyInterface extends MyBaseInterface { + + } + + + private static class MyInterfaceImplementer implements MyInterface { + + } + + + private class MyBaseInterfaceConverter implements Converter { + + public String convert(MyBaseInterface source) { + return "RESULT"; + } + } + + }