From d0e6f0f73fc3c6c31513a0a8c9805b41b102e1f6 Mon Sep 17 00:00:00 2001 From: David Haraburda Date: Wed, 30 Jul 2014 17:18:32 -0500 Subject: [PATCH] Change GenericConversionService to better handle enum Prior to this commit, given an enum which implements some interface, GenericConversionService would select the String -> Enum converter even if a converter for String -> SomeInterface was registered. This also affected converters that were registered for String -> SomeBaseInterface, when SomeInterface extended SomeBaseInterface. This change modifies the behavior of the private method getClassHierarchy() by placing Enum.class as late as possible, pretty much the same way as Object.class is handled. Issue: SPR-12050 --- .../support/GenericConversionService.java | 21 +++- .../GenericConversionServiceTests.java | 97 +++++++++++++++++-- 2 files changed, 107 insertions(+), 11 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 525b9f250d..7ab8813a38 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 @@ -55,6 +55,7 @@ import org.springframework.util.StringUtils; * @author Juergen Hoeller * @author Chris Beams * @author Phillip Webb + * @author David Haraburda * @since 3.0 */ public class GenericConversionService implements ConfigurableConversionService { @@ -567,19 +568,31 @@ public class GenericConversionService implements ConfigurableConversionService { Class candidate = hierarchy.get(i); candidate = (array ? candidate.getComponentType() : ClassUtils.resolvePrimitiveIfNecessary(candidate)); Class superclass = candidate.getSuperclass(); - if (candidate.getSuperclass() != null && superclass != Object.class) { + if (candidate.getSuperclass() != null && superclass != Object.class && superclass != Enum.class) { addToClassHierarchy(i + 1, candidate.getSuperclass(), array, hierarchy, visited); } - for (Class implementedInterface : candidate.getInterfaces()) { - addToClassHierarchy(hierarchy.size(), implementedInterface, array, hierarchy, visited); - } + addInterfacesToClassHierarchy(candidate, array, hierarchy, visited); i++; } + + if (type.isEnum()) { + addToClassHierarchy(hierarchy.size(), Enum.class, array, hierarchy, visited); + addToClassHierarchy(hierarchy.size(), Enum.class, false, hierarchy, visited); + addInterfacesToClassHierarchy(Enum.class, array, hierarchy, visited); + } + addToClassHierarchy(hierarchy.size(), Object.class, array, hierarchy, visited); addToClassHierarchy(hierarchy.size(), Object.class, false, hierarchy, visited); return hierarchy; } + private void addInterfacesToClassHierarchy(Class type, boolean asArray, + List> hierarchy, Set> visited) { + for (Class implementedInterface : type.getInterfaces()) { + addToClassHierarchy(hierarchy.size(), implementedInterface, asArray, hierarchy, visited); + } + } + private void addToClassHierarchy(int index, Class type, boolean asArray, List> hierarchy, Set> visited) { if (asArray) { 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 a9f22acff2..bd88180631 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 @@ -57,6 +57,7 @@ import static org.junit.Assert.*; * @author Keith Donald * @author Juergen Hoeller * @author Phillip Webb + * @author David Haraburda */ public class GenericConversionServiceTests { @@ -758,6 +759,20 @@ public class GenericConversionServiceTests { assertEquals("1", result); } + @Test + public void testStringToEnumWithInterfaceConversion() { + conversionService.addConverterFactory(new StringToEnumConverterFactory()); + conversionService.addConverterFactory(new StringToMyEnumInterfaceConverterFactory()); + assertEquals(MyEnum.A, conversionService.convert("1", MyEnum.class)); + } + + @Test + public void testStringToEnumWithBaseInterfaceConversion() { + conversionService.addConverterFactory(new StringToEnumConverterFactory()); + conversionService.addConverterFactory(new StringToMyEnumBaseInterfaceConverterFactory()); + assertEquals(MyEnum.A, conversionService.convert("base1", MyEnum.class)); + } + @Test public void convertNullAnnotatedStringToString() throws Exception { DefaultConversionService.addDefaultConverters(conversionService); @@ -930,19 +945,34 @@ public class GenericConversionServiceTests { } } + interface MyEnumBaseInterface { + String getBaseCode(); + } - interface MyEnumInterface { - + interface MyEnumInterface extends MyEnumBaseInterface { String getCode(); } public static enum MyEnum implements MyEnumInterface { - A { - @Override - public String getCode() { - return "1"; - } + A("1"), + B("2"), + C("3"); + + private String code; + + MyEnum(String code) { + this.code = code; + } + + @Override + public String getCode() { + return code; + } + + @Override + public String getBaseCode() { + return "base" + code; } } @@ -971,6 +1001,59 @@ public class GenericConversionServiceTests { } } + private static class StringToMyEnumInterfaceConverterFactory implements ConverterFactory { + + @SuppressWarnings("unchecked") + public Converter getConverter(Class targetType) { + return new StringToMyEnumInterfaceConverter(targetType); + } + + private static class StringToMyEnumInterfaceConverter & MyEnumInterface> implements Converter { + private final Class enumType; + + public StringToMyEnumInterfaceConverter(Class enumType) { + this.enumType = enumType; + } + + public T convert(String source) { + for (T value : enumType.getEnumConstants()) { + if (value.getCode().equals(source)) { + return value; + } + } + return null; + } + } + + } + + private static class StringToMyEnumBaseInterfaceConverterFactory implements ConverterFactory { + + @SuppressWarnings("unchecked") + public Converter getConverter(Class targetType) { + return new StringToMyEnumBaseInterfaceConverter(targetType); + } + + private static class StringToMyEnumBaseInterfaceConverter & MyEnumBaseInterface> implements Converter { + private final Class enumType; + + public StringToMyEnumBaseInterfaceConverter(Class enumType) { + this.enumType = enumType; + } + + public T convert(String source) { + for (T value : enumType.getEnumConstants()) { + if (value.getBaseCode().equals(source)) { + return value; + } + } + return null; + } + } + + } + + public static class MyStringToStringCollectionConverter implements Converter> { @Override