From 9bfe675761b5e13dcc533b4f4a82fe240c7a2595 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 26 Oct 2012 15:32:49 -0700 Subject: [PATCH 1/7] Additional GenericConversionService Tests --- .../GenericConversionServiceTests.java | 49 ++++++++++++++++++- 1 file changed, 47 insertions(+), 2 deletions(-) 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 27f15b76d09..9c87aed2dde 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-2011 the original author or authors. + * Copyright 2002-2012 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. @@ -16,12 +16,12 @@ package org.springframework.core.convert.support; -import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.awt.Color; @@ -52,6 +52,7 @@ import org.springframework.util.StringUtils; /** * @author Keith Donald * @author Juergen Hoeller + * @author Phillip Webb */ public class GenericConversionServiceTests { @@ -599,4 +600,48 @@ public class GenericConversionServiceTests { assertFalse(pair.hashCode() == pairOpposite.hashCode()); } + @Test + public void convertPrimitiveArray() throws Exception { + GenericConversionService conversionService = new DefaultConversionService(); + byte[] byteArray = new byte[] { 1, 2, 3 }; + Byte[] converted = conversionService.convert(byteArray, Byte[].class); + assertTrue(Arrays.equals(converted, new Byte[] { 1, 2, 3 })); + } + + @Test + public void canConvertIllegalArgumentNullTargetTypeFromClass() { + try { + conversionService.canConvert(String.class, null); + fail("Did not thow IllegalArgumentException"); + } catch(IllegalArgumentException e) { + } + } + + @Test + public void canConvertIllegalArgumentNullTargetTypeFromTypeDescriptor() { + try { + conversionService.canConvert(TypeDescriptor.valueOf(String.class), null); + fail("Did not thow IllegalArgumentException"); + } catch(IllegalArgumentException e) { + } + } + + @Test + @SuppressWarnings({ "rawtypes" }) + public void convertHashMapValuesToList() throws Exception { + GenericConversionService conversionService = new DefaultConversionService(); + Map hashMap = new LinkedHashMap(); + hashMap.put("1", 1); + hashMap.put("2", 2); + List converted = conversionService.convert(hashMap.values(), List.class); + assertEquals(Arrays.asList(1, 2), converted); + } + + @Test + public void removeConvertible() throws Exception { + conversionService.addConverter(new ColorConverter()); + assertTrue(conversionService.canConvert(String.class, Color.class)); + conversionService.removeConvertible(String.class, Color.class); + assertFalse(conversionService.canConvert(String.class, Color.class)); + } } From 4dc289592d0072c7472ab455522362f8db42b4a7 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 26 Oct 2012 17:10:42 -0700 Subject: [PATCH 2/7] Refactor GenericConversionService Refactor internal workings of GenericConversionService in order to better support future enhancements. This commit should not affect existing behavior. Issue: SPR-9927 --- .../core/convert/TypeDescriptor.java | 18 + .../support/GenericConversionService.java | 599 ++++++++---------- .../core/convert/TypeDescriptorTests.java | 36 +- 3 files changed, 317 insertions(+), 336 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/convert/TypeDescriptor.java b/spring-core/src/main/java/org/springframework/core/convert/TypeDescriptor.java index 58585070a87..7e119c34548 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/TypeDescriptor.java +++ b/spring-core/src/main/java/org/springframework/core/convert/TypeDescriptor.java @@ -23,6 +23,7 @@ import java.util.HashMap; import java.util.Map; import org.springframework.core.MethodParameter; +import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.ObjectUtils; @@ -249,6 +250,23 @@ public class TypeDescriptor { this.mapKeyTypeDescriptor, this.mapValueTypeDescriptor, this.annotations); } + /** + * Cast this {@link TypeDescriptor} to a superclass or implemented interface + * preserving annotations and nested type context. + * + * @param superType the super type to cast to (can be {@code null} + * @return a new TypeDescriptor for the up-cast type + * @throws IllegalArgumentException if this type is not assignable to the super-type + */ + public TypeDescriptor upcast(Class superType) { + if (superType == null) { + return null; + } + Assert.isAssignable(superType, getType()); + return new TypeDescriptor(superType, this.elementTypeDescriptor, + this.mapKeyTypeDescriptor, this.mapValueTypeDescriptor, this.annotations); + } + /** * Returns the name of this type: the fully qualified class name. */ 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 5856489988d..fa9383e2f2d 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-2011 the original author or authors. + * Copyright 2002-2012 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. @@ -19,9 +19,8 @@ package org.springframework.core.convert.support; import java.lang.reflect.Array; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; -import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; @@ -40,8 +39,11 @@ 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 org.springframework.core.convert.converter.GenericConverter.ConvertiblePair; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; +import org.springframework.util.ObjectUtils; +import org.springframework.util.StringUtils; /** * Base {@link ConversionService} implementation suitable for use in most environments. @@ -51,37 +53,24 @@ import org.springframework.util.ClassUtils; * @author Keith Donald * @author Juergen Hoeller * @author Chris Beams + * @author Phillip Webb * @since 3.0 */ public class GenericConversionService implements ConfigurableConversionService { - private static final GenericConverter NO_OP_CONVERTER = new GenericConverter() { - public Set getConvertibleTypes() { - return null; - } - public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { - return source; - } - public String toString() { - return "NO_OP"; - } - }; + /** + * General NO-OP converter used when conversion is not required. + */ + private static final GenericConverter NO_OP_CONVERTER = new NoOpConverter("NO_OP"); - private static final GenericConverter NO_MATCH = new GenericConverter() { - public Set getConvertibleTypes() { - throw new UnsupportedOperationException(); - } - public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { - throw new UnsupportedOperationException(); - } - public String toString() { - return "NO_MATCH"; - } - }; + /** + * Used as a cache entry when no converter is available. This converter is never + * returned. + */ + private static final GenericConverter NO_MATCH = new NoOpConverter("NO_MATCH"); - private final Map, Map, MatchableConverters>> converters = - new HashMap, Map, MatchableConverters>>(36); + private final Converters converters = new Converters(); private final Map converterCache = new ConcurrentHashMap(); @@ -91,10 +80,8 @@ public class GenericConversionService implements ConfigurableConversionService { public void addConverter(Converter converter) { GenericConverter.ConvertiblePair typeInfo = getRequiredTypeInfo(converter, Converter.class); - if (typeInfo == null) { - throw new IllegalArgumentException("Unable to the determine sourceType and targetType which " + + Assert.notNull(typeInfo, "Unable to the determine sourceType and targetType which " + "your Converter converts between; declare these generic types."); - } addConverter(new ConverterAdapter(typeInfo, converter)); } @@ -104,10 +91,7 @@ public class GenericConversionService implements ConfigurableConversionService { } public void addConverter(GenericConverter converter) { - Set convertibleTypes = converter.getConvertibleTypes(); - for (GenericConverter.ConvertiblePair convertibleType : convertibleTypes) { - getMatchableConverters(convertibleType.getSourceType(), convertibleType.getTargetType()).add(converter); - } + this.converters.add(converter); invalidateCache(); } @@ -121,24 +105,19 @@ public class GenericConversionService implements ConfigurableConversionService { } public void removeConvertible(Class sourceType, Class targetType) { - getSourceConverterMap(sourceType).remove(targetType); + this.converters.remove(sourceType, targetType); invalidateCache(); } - // implementing ConversionService public boolean canConvert(Class sourceType, Class targetType) { - if (targetType == null) { - throw new IllegalArgumentException("The targetType to convert to cannot be null"); - } + Assert.notNull(targetType, "The targetType to convert to cannot be null"); return canConvert(sourceType != null ? TypeDescriptor.valueOf(sourceType) : null, TypeDescriptor.valueOf(targetType)); } public boolean canConvert(TypeDescriptor sourceType, TypeDescriptor targetType) { - if (targetType == null) { - throw new IllegalArgumentException("The targetType to convert to cannot be null"); - } + Assert.notNull(targetType,"The targetType to convert to cannot be null"); if (sourceType == null) { return true; } @@ -148,16 +127,12 @@ public class GenericConversionService implements ConfigurableConversionService { @SuppressWarnings("unchecked") public T convert(Object source, Class targetType) { - if (targetType == null) { - throw new IllegalArgumentException("The targetType to convert to cannot be null"); - } + Assert.notNull(targetType,"The targetType to convert to cannot be null"); return (T) convert(source, TypeDescriptor.forObject(source), TypeDescriptor.valueOf(targetType)); } public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { - if (targetType == null) { - throw new IllegalArgumentException("The targetType to convert to cannot be null"); - } + Assert.notNull(targetType,"The 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)); @@ -171,9 +146,7 @@ public class GenericConversionService implements ConfigurableConversionService { Object result = ConversionUtils.invokeConverter(converter, source, sourceType, targetType); return handleResult(sourceType, targetType, result); } - else { - return handleConverterNotFound(source, sourceType, targetType); - } + return handleConverterNotFound(source, sourceType, targetType); } /** @@ -191,21 +164,7 @@ public class GenericConversionService implements ConfigurableConversionService { } public String toString() { - List converterStrings = new ArrayList(); - for (Map, MatchableConverters> targetConverters : this.converters.values()) { - for (MatchableConverters matchable : targetConverters.values()) { - converterStrings.add(matchable.toString()); - } - } - Collections.sort(converterStrings); - StringBuilder builder = new StringBuilder(); - builder.append("ConversionService converters = ").append("\n"); - for (String converterString : converterStrings) { - builder.append("\t"); - builder.append(converterString); - builder.append("\n"); - } - return builder.toString(); + return this.converters.toString(); } @@ -231,7 +190,7 @@ public class GenericConversionService implements ConfigurableConversionService { * 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 + * @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) { @@ -240,20 +199,19 @@ public class GenericConversionService implements ConfigurableConversionService { if (converter != null) { return (converter != NO_MATCH ? converter : null); } - else { - converter = findConverterForClassPair(sourceType, targetType); - if (converter == null) { - converter = getDefaultConverter(sourceType, targetType); - } - if (converter != null) { - this.converterCache.put(key, converter); - return converter; - } - else { - this.converterCache.put(key, NO_MATCH); - return null; - } + + converter = this.converters.find(sourceType, targetType); + if (converter == null) { + converter = getDefaultConverter(sourceType, targetType); } + + if (converter != null) { + this.converterCache.put(key, converter); + return converter; + } + + this.converterCache.put(key, NO_MATCH); + return null; } /** @@ -276,204 +234,19 @@ public class GenericConversionService implements ConfigurableConversionService { return (args != null ? new GenericConverter.ConvertiblePair(args[0], args[1]) : null); } - private MatchableConverters getMatchableConverters(Class sourceType, Class targetType) { - Map, MatchableConverters> sourceMap = getSourceConverterMap(sourceType); - MatchableConverters matchable = sourceMap.get(targetType); - if (matchable == null) { - matchable = new MatchableConverters(); - sourceMap.put(targetType, matchable); - } - return matchable; - } - private void invalidateCache() { this.converterCache.clear(); } - private Map, MatchableConverters> getSourceConverterMap(Class sourceType) { - Map, MatchableConverters> sourceMap = converters.get(sourceType); - if (sourceMap == null) { - sourceMap = new HashMap, MatchableConverters>(); - this.converters.put(sourceType, sourceMap); - } - return sourceMap; - } - - private GenericConverter findConverterForClassPair(TypeDescriptor sourceType, TypeDescriptor targetType) { - Class sourceObjectType = sourceType.getObjectType(); - if (sourceObjectType.isInterface()) { - LinkedList> classQueue = new LinkedList>(); - classQueue.addFirst(sourceObjectType); - while (!classQueue.isEmpty()) { - Class currentClass = classQueue.removeLast(); - Map, MatchableConverters> converters = getTargetConvertersForSource(currentClass); - GenericConverter converter = getMatchingConverterForTarget(sourceType, targetType, converters); - if (converter != null) { - return converter; - } - Class[] interfaces = currentClass.getInterfaces(); - for (Class ifc : interfaces) { - classQueue.addFirst(ifc); - } - } - Map, MatchableConverters> objectConverters = getTargetConvertersForSource(Object.class); - return getMatchingConverterForTarget(sourceType, targetType, objectConverters); - } - else if (sourceObjectType.isArray()) { - LinkedList> classQueue = new LinkedList>(); - classQueue.addFirst(sourceObjectType); - while (!classQueue.isEmpty()) { - Class currentClass = classQueue.removeLast(); - Map, MatchableConverters> converters = getTargetConvertersForSource(currentClass); - GenericConverter converter = getMatchingConverterForTarget(sourceType, targetType, converters); - if (converter != null) { - return converter; - } - Class componentType = ClassUtils.resolvePrimitiveIfNecessary(currentClass.getComponentType()); - if (componentType.getSuperclass() != null) { - classQueue.addFirst(Array.newInstance(componentType.getSuperclass(), 0).getClass()); - } - else if (componentType.isInterface()) { - classQueue.addFirst(Object[].class); - } - } - return null; - } - else { - HashSet> interfaces = new LinkedHashSet>(); - LinkedList> classQueue = new LinkedList>(); - classQueue.addFirst(sourceObjectType); - while (!classQueue.isEmpty()) { - Class currentClass = classQueue.removeLast(); - Map, MatchableConverters> converters = getTargetConvertersForSource(currentClass); - GenericConverter converter = getMatchingConverterForTarget(sourceType, targetType, converters); - if (converter != null) { - return converter; - } - Class superClass = currentClass.getSuperclass(); - if (superClass != null && superClass != Object.class) { - classQueue.addFirst(superClass); - } - for (Class interfaceType : currentClass.getInterfaces()) { - addInterfaceHierarchy(interfaceType, interfaces); - } - } - for (Class interfaceType : interfaces) { - Map, MatchableConverters> converters = getTargetConvertersForSource(interfaceType); - GenericConverter converter = getMatchingConverterForTarget(sourceType, targetType, converters); - if (converter != null) { - return converter; - } - } - Map, MatchableConverters> objectConverters = getTargetConvertersForSource(Object.class); - return getMatchingConverterForTarget(sourceType, targetType, objectConverters); - } - } - - private Map, MatchableConverters> getTargetConvertersForSource(Class sourceType) { - Map, MatchableConverters> converters = this.converters.get(sourceType); - if (converters == null) { - converters = Collections.emptyMap(); - } - return converters; - } - - private GenericConverter getMatchingConverterForTarget(TypeDescriptor sourceType, TypeDescriptor targetType, - Map, MatchableConverters> converters) { - Class targetObjectType = targetType.getObjectType(); - if (targetObjectType.isInterface()) { - LinkedList> classQueue = new LinkedList>(); - classQueue.addFirst(targetObjectType); - while (!classQueue.isEmpty()) { - Class currentClass = classQueue.removeLast(); - MatchableConverters matchable = converters.get(currentClass); - GenericConverter converter = matchConverter(matchable, sourceType, targetType); - if (converter != null) { - return converter; - } - Class[] interfaces = currentClass.getInterfaces(); - for (Class ifc : interfaces) { - classQueue.addFirst(ifc); - } - } - return matchConverter(converters.get(Object.class), sourceType, targetType); - } - else if (targetObjectType.isArray()) { - LinkedList> classQueue = new LinkedList>(); - classQueue.addFirst(targetObjectType); - while (!classQueue.isEmpty()) { - Class currentClass = classQueue.removeLast(); - MatchableConverters matchable = converters.get(currentClass); - GenericConverter converter = matchConverter(matchable, sourceType, targetType); - if (converter != null) { - return converter; - } - Class componentType = ClassUtils.resolvePrimitiveIfNecessary(currentClass.getComponentType()); - if (componentType.getSuperclass() != null) { - classQueue.addFirst(Array.newInstance(componentType.getSuperclass(), 0).getClass()); - } - else if (componentType.isInterface()) { - classQueue.addFirst(Object[].class); - } - } - return null; - } - else { - Set> interfaces = new LinkedHashSet>(); - LinkedList> classQueue = new LinkedList>(); - classQueue.addFirst(targetObjectType); - while (!classQueue.isEmpty()) { - Class currentClass = classQueue.removeLast(); - MatchableConverters matchable = converters.get(currentClass); - GenericConverter converter = matchConverter(matchable, sourceType, targetType); - if (converter != null) { - return converter; - } - Class superClass = currentClass.getSuperclass(); - if (superClass != null && superClass != Object.class) { - classQueue.addFirst(superClass); - } - for (Class interfaceType : currentClass.getInterfaces()) { - addInterfaceHierarchy(interfaceType, interfaces); - } - } - for (Class interfaceType : interfaces) { - MatchableConverters matchable = converters.get(interfaceType); - GenericConverter converter = matchConverter(matchable, sourceType, targetType); - if (converter != null) { - return converter; - } - } - return matchConverter(converters.get(Object.class), sourceType, targetType); - } - } - - private void addInterfaceHierarchy(Class interfaceType, Set> interfaces) { - interfaces.add(interfaceType); - for (Class inheritedInterface : interfaceType.getInterfaces()) { - addInterfaceHierarchy(inheritedInterface, interfaces); - } - } - - private GenericConverter matchConverter( - MatchableConverters matchable, TypeDescriptor sourceFieldType, TypeDescriptor targetFieldType) { - if (matchable == null) { - return null; - } - return matchable.matchConverter(sourceFieldType, targetFieldType); - } - private Object handleConverterNotFound(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { if (source == null) { assertNotPrimitiveTargetType(sourceType, targetType); return source; } - else if (sourceType.isAssignableTo(targetType) && targetType.getObjectType().isInstance(source)) { + if (sourceType.isAssignableTo(targetType) && targetType.getObjectType().isInstance(source)) { return source; } - else { - throw new ConverterNotFoundException(sourceType, targetType); - } + throw new ConverterNotFoundException(sourceType, targetType); } private Object handleResult(TypeDescriptor sourceType, TypeDescriptor targetType, Object result) { @@ -482,6 +255,7 @@ public class GenericConversionService implements ConfigurableConversionService { } return result; } + private void assertNotPrimitiveTargetType(TypeDescriptor sourceType, TypeDescriptor targetType) { if (targetType.isPrimitive()) { throw new ConversionFailedException(sourceType, targetType, null, @@ -490,24 +264,32 @@ public class GenericConversionService implements ConfigurableConversionService { } + /** + * Adapts a {@link Converter} to a {@link GenericConverter}. + */ @SuppressWarnings("unchecked") - private final class ConverterAdapter implements GenericConverter { + private final class ConverterAdapter implements ConditionalGenericConverter { private final ConvertiblePair typeInfo; private final Converter converter; + public ConverterAdapter(ConvertiblePair typeInfo, Converter converter) { this.converter = (Converter) converter; this.typeInfo = typeInfo; } + public Set getConvertibleTypes() { return Collections.singleton(this.typeInfo); } - public boolean matchesTargetType(TypeDescriptor targetType) { - return this.typeInfo.getTargetType().equals(targetType.getObjectType()); + public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { + if(!this.typeInfo.getTargetType().equals(targetType.getObjectType())) { + return false; + } + return true; } public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { @@ -524,6 +306,9 @@ public class GenericConversionService implements ConfigurableConversionService { } + /** + * Adapts a {@link ConverterFactory} to a {@link GenericConverter}. + */ @SuppressWarnings("unchecked") private final class ConverterFactoryAdapter implements GenericConverter { @@ -531,11 +316,13 @@ public class GenericConversionService implements ConfigurableConversionService { private final ConverterFactory converterFactory; + public ConverterFactoryAdapter(ConvertiblePair typeInfo, ConverterFactory converterFactory) { this.converterFactory = (ConverterFactory) converterFactory; this.typeInfo = typeInfo; } + public Set getConvertibleTypes() { return Collections.singleton(this.typeInfo); } @@ -554,73 +341,22 @@ public class GenericConversionService implements ConfigurableConversionService { } - private static class MatchableConverters { - - private LinkedList conditionalConverters; - - private GenericConverter defaultConverter; - - public void add(GenericConverter converter) { - if (converter instanceof ConditionalGenericConverter) { - if (this.conditionalConverters == null) { - this.conditionalConverters = new LinkedList(); - } - this.conditionalConverters.addFirst((ConditionalGenericConverter) converter); - } - else { - this.defaultConverter = converter; - } - } - - public GenericConverter matchConverter(TypeDescriptor sourceType, TypeDescriptor targetType) { - if (this.conditionalConverters != null) { - for (ConditionalGenericConverter conditional : this.conditionalConverters) { - if (conditional.matches(sourceType, targetType)) { - return conditional; - } - } - } - if (this.defaultConverter instanceof ConverterAdapter) { - ConverterAdapter adapter = (ConverterAdapter) this.defaultConverter; - if (!adapter.matchesTargetType(targetType)) { - return null; - } - } - return this.defaultConverter; - } - - public String toString() { - if (this.conditionalConverters != null) { - StringBuilder builder = new StringBuilder(); - for (Iterator it = this.conditionalConverters.iterator(); it.hasNext();) { - builder.append(it.next()); - if (it.hasNext()) { - builder.append(", "); - } - } - if (this.defaultConverter != null) { - builder.append(", ").append(this.defaultConverter); - } - return builder.toString(); - } - else { - return this.defaultConverter.toString(); - } - } - } - - + /** + * Key for use with the converter cache. + */ private static final class ConverterCacheKey { private final TypeDescriptor sourceType; private final TypeDescriptor targetType; + public ConverterCacheKey(TypeDescriptor sourceType, TypeDescriptor targetType) { this.sourceType = sourceType; this.targetType = targetType; } + public boolean equals(Object other) { if (this == other) { return true; @@ -629,16 +365,221 @@ public class GenericConversionService implements ConfigurableConversionService { return false; } ConverterCacheKey otherKey = (ConverterCacheKey) other; - return this.sourceType.equals(otherKey.sourceType) && this.targetType.equals(otherKey.targetType); + return ObjectUtils.nullSafeEquals(this.sourceType, otherKey.sourceType) + && ObjectUtils.nullSafeEquals(this.targetType, otherKey.targetType); } public int hashCode() { - return this.sourceType.hashCode() * 29 + this.targetType.hashCode(); + return ObjectUtils.nullSafeHashCode(this.sourceType) * 29 + + ObjectUtils.nullSafeHashCode(this.targetType); } public String toString() { - return "ConverterCacheKey [sourceType = " + this.sourceType + ", targetType = " + this.targetType + "]"; + return "ConverterCacheKey [sourceType = " + this.sourceType + + ", targetType = " + this.targetType + "]"; } } + /** + * Manages all converters registered with the service. + */ + private static class Converters { + + private static final Set> IGNORED_CLASSES; + static { + Set> ignored = new HashSet>(); + ignored.add(Object.class); + ignored.add(Object[].class); + IGNORED_CLASSES = Collections.unmodifiableSet(ignored); + } + + private final Map converters = + new LinkedHashMap(36); + + public void add(GenericConverter converter) { + Set convertibleTypes = converter.getConvertibleTypes(); + Assert.state(converter.getConvertibleTypes() != null, "Converter does not specifiy ConvertibleTypes"); + for (ConvertiblePair convertiblePair : convertibleTypes) { + ConvertersForPair convertersForPair = getMatchableConverters(convertiblePair); + convertersForPair.add(converter); + } + } + + private ConvertersForPair getMatchableConverters(ConvertiblePair convertiblePair) { + ConvertersForPair convertersForPair = this.converters.get(convertiblePair); + if (convertersForPair == null) { + convertersForPair = new ConvertersForPair(); + this.converters.put(convertiblePair, convertersForPair); + } + return convertersForPair; + } + + public void remove(Class sourceType, Class targetType) { + converters.remove(new ConvertiblePair(sourceType, targetType)); + } + + /** + * Find a {@link GenericConverter} given a source and target type. This method will + * attempt to match all possible converters by working though the class and interface + * hierarchy of the types. + * @param sourceType the source type + * @param targetType the target type + * @return a {@link GenericConverter} or null + * @see #getTypeHierarchy(Class) + */ + public GenericConverter find(TypeDescriptor sourceType, TypeDescriptor targetType) { + // Search the full type hierarchy + List sourceCandidates = getTypeHierarchy(sourceType); + List targetCandidates = getTypeHierarchy(targetType); + for (TypeDescriptor sourceCandidate : sourceCandidates) { + for (TypeDescriptor targetCandidate : targetCandidates) { + GenericConverter converter = getRegisteredConverter(sourceType, targetType, sourceCandidate, targetCandidate); + if(converter != null) { + return converter; + } + } + } + return null; + } + + private GenericConverter getRegisteredConverter(TypeDescriptor sourceType, TypeDescriptor targetType, + TypeDescriptor sourceCandidate, TypeDescriptor targetCandidate) { + + // Check specifically registered converters + ConvertersForPair convertersForPair = converters.get(new ConvertiblePair( + sourceCandidate.getType(), targetCandidate.getType())); + GenericConverter converter = convertersForPair == null ? null + : convertersForPair.getConverter(sourceType, targetType); + if (converter != null) { + return converter; + } + + return null; + } + + /** + * Returns an ordered class hierarchy for the given type. + * @param type the type + * @return an ordered list of all classes that the given type extends or + * implements. + */ + private List getTypeHierarchy(TypeDescriptor type) { + if(type.isPrimitive()) { + type = TypeDescriptor.valueOf(type.getObjectType()); + } + Set typeHierarchy = new LinkedHashSet(); + collectTypeHierarchy(typeHierarchy, type); + if(type.isArray()) { + typeHierarchy.add(TypeDescriptor.valueOf(Object[].class)); + } + typeHierarchy.add(TypeDescriptor.valueOf(Object.class)); + return new ArrayList(typeHierarchy); + } + + private void collectTypeHierarchy(Set typeHierarchy, + TypeDescriptor type) { + if(type != null && !IGNORED_CLASSES.contains(type.getType())) { + if(typeHierarchy.add(type)) { + Class superclass = type.getType().getSuperclass(); + if (type.isArray()) { + superclass = ClassUtils.resolvePrimitiveIfNecessary(superclass); + } + collectTypeHierarchy(typeHierarchy, createRelated(type, superclass)); + + for (Class implementsInterface : type.getType().getInterfaces()) { + collectTypeHierarchy(typeHierarchy, createRelated(type, implementsInterface)); + } + } + } + } + + private TypeDescriptor createRelated(TypeDescriptor type, Class relatedType) { + if (relatedType == null && type.isArray()) { + relatedType = Array.newInstance(relatedType, 0).getClass(); + } + if(!type.getType().equals(relatedType)) { + return type.upcast(relatedType); + } + return null; + } + + @Override + public String toString() { + StringBuilder builder = new StringBuilder(); + builder.append("ConversionService converters = ").append("\n"); + for (String converterString : getConverterStrings()) { + builder.append("\t"); + builder.append(converterString); + builder.append("\n"); + } + return builder.toString(); + } + + private List getConverterStrings() { + List converterStrings = new ArrayList(); + for (ConvertersForPair convertersForPair : converters.values()) { + converterStrings.add(convertersForPair.toString()); + } + Collections.sort(converterStrings); + return converterStrings; + } + } + + + /** + * Manages converters registered with a specific {@link ConvertiblePair}. + */ + private static class ConvertersForPair { + + private final LinkedList converters = new LinkedList(); + + public void add(GenericConverter converter) { + this.converters.addFirst(converter); + } + + public GenericConverter getConverter(TypeDescriptor sourceType, + TypeDescriptor targetType) { + for (GenericConverter converter : this.converters) { + if (!(converter instanceof ConditionalGenericConverter) + || ((ConditionalGenericConverter) converter).matches(sourceType, + targetType)) { + return converter; + } + } + return null; + } + + public String toString() { + return StringUtils.collectionToCommaDelimitedString(this.converters); + } + } + + + /** + * Internal converter that performs no operation. + */ + private static class NoOpConverter implements GenericConverter { + + private String name; + + + public NoOpConverter(String name) { + this.name = name; + } + + + public Set getConvertibleTypes() { + return null; + } + + public Object convert(Object source, TypeDescriptor sourceType, + TypeDescriptor targetType) { + return source; + } + + @Override + public String toString() { + return name; + } + } } diff --git a/spring-core/src/test/java/org/springframework/core/convert/TypeDescriptorTests.java b/spring-core/src/test/java/org/springframework/core/convert/TypeDescriptorTests.java index 20cefeeec35..e16a3324262 100644 --- a/spring-core/src/test/java/org/springframework/core/convert/TypeDescriptorTests.java +++ b/spring-core/src/test/java/org/springframework/core/convert/TypeDescriptorTests.java @@ -16,6 +16,13 @@ package org.springframework.core.convert; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + import java.lang.annotation.ElementType; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -28,15 +35,8 @@ import java.util.List; import java.util.Map; import org.junit.Test; - import org.springframework.core.MethodParameter; -import static junit.framework.Assert.assertEquals; -import static junit.framework.Assert.assertTrue; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; - /** * @author Keith Donald * @author Andy Clement @@ -801,4 +801,26 @@ public class TypeDescriptorTests { public Map isAssignableMapKeyValueTypes; + @Test + public void testUpCast() throws Exception { + Property property = new Property(getClass(), getClass().getMethod("getProperty"), + getClass().getMethod("setProperty", Map.class)); + TypeDescriptor typeDescriptor = new TypeDescriptor(property); + TypeDescriptor upCast = typeDescriptor.upcast(Object.class); + assertTrue(upCast.getAnnotation(MethodAnnotation1.class) != null); + } + + @Test + public void testUpCastNotSuper() throws Exception { + Property property = new Property(getClass(), getClass().getMethod("getProperty"), + getClass().getMethod("setProperty", Map.class)); + TypeDescriptor typeDescriptor = new TypeDescriptor(property); + try { + typeDescriptor.upcast(Collection.class); + fail("Did not throw"); + } catch(IllegalArgumentException e) { + assertEquals("interface java.util.Map is not assignable to interface java.util.Collection", e.getMessage()); + } + } + } From f13e3ad72b813d35d3187394dc53f98a35003c9a Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 26 Oct 2012 17:22:10 -0700 Subject: [PATCH 3/7] Extend conditional conversion support Introduce new ConditionalConversion interface that can be applied to Converter, ConverterFactory or GenericConverter interfaces to make them conditional. Prior to this commit the only ConditionalGenericConverter could be conditional. Issue: SPR-9928 --- .../converter/ConditionalConversion.java | 55 +++++++ .../ConditionalGenericConverter.java | 35 ++--- .../core/convert/converter/Converter.java | 7 +- .../convert/converter/ConverterFactory.java | 5 +- .../convert/converter/GenericConverter.java | 12 +- .../support/GenericConversionService.java | 47 +++++- .../GenericConversionServiceTests.java | 134 ++++++++++++++++++ 7 files changed, 259 insertions(+), 36 deletions(-) create mode 100644 spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalConversion.java diff --git a/spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalConversion.java b/spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalConversion.java new file mode 100644 index 00000000000..1196b26fc4f --- /dev/null +++ b/spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalConversion.java @@ -0,0 +1,55 @@ +/* + * Copyright 2012 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.core.convert.converter; + +import org.springframework.core.convert.TypeDescriptor; + +/** + * Allows a {@link Converter}, {@link GenericConverter} or {@link ConverterFactory} to + * conditionally execute based on attributes of the {@code source} and {@code target} + * {@link TypeDescriptor}. + * + *

Often used to selectively match custom conversion logic based on the presence of a + * field or class-level characteristic, such as an annotation or method. For example, when + * converting from a String field to a Date field, an implementation might return + * + * {@code true} if the target field has also been annotated with {@code @DateTimeFormat}. + * + *

As another example, when converting from a String field to an {@code Account} field, an + * implementation might return {@code true} if the target Account class defines a + * {@code public static findAccount(String)} method. + * + * @author Keith Donald + * @author Phillip Webb + * @since 3.2 + * @see Converter + * @see GenericConverter + * @see ConverterFactory + * @see ConditionalGenericConverter + */ +public interface ConditionalConversion { + + /** + * Should the converter from {@code sourceType} to {@code targetType} currently under + * consideration be selected? + * + * @param sourceType the type descriptor of the field we are converting from + * @param targetType the type descriptor of the field we are converting to + * @return true if conversion should be performed, false otherwise + */ + boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType); +} diff --git a/spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalGenericConverter.java b/spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalGenericConverter.java index fd52ca7d871..fc7ce7a1ab0 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalGenericConverter.java +++ b/spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalGenericConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2012 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. @@ -18,34 +18,19 @@ package org.springframework.core.convert.converter; import org.springframework.core.convert.TypeDescriptor; + /** - * A generic converter that conditionally executes. - * - *

Applies a rule that determines if a converter between a set of - * {@link #getConvertibleTypes() convertible types} matches given a client request to - * convert between a source field of convertible type S and a target field of convertible type T. - * - *

Often used to selectively match custom conversion logic based on the presence of - * a field or class-level characteristic, such as an annotation or method. For example, - * when converting from a String field to a Date field, an implementation might return - * true if the target field has also been annotated with @DateTimeFormat. - * - *

As another example, when converting from a String field to an Account field, - * an implementation might return true if the target Account class defines a - * public static findAccount(String) method. + * A {@link GenericConverter} that may conditionally execute based on attributes of the + * {@code source} and {@code target} {@link TypeDescriptor}. See + * {@link ConditionalConversion} for details. * * @author Keith Donald + * @author Phillip Webb * @since 3.0 + * @see GenericConverter + * @see ConditionalConversion */ -public interface ConditionalGenericConverter extends GenericConverter { - - /** - * Should the converter from sourceType to targetType - * currently under consideration be selected? - * @param sourceType the type descriptor of the field we are converting from - * @param targetType the type descriptor of the field we are converting to - * @return true if conversion should be performed, false otherwise - */ - boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType); +public interface ConditionalGenericConverter extends GenericConverter, + ConditionalConversion { } diff --git a/spring-core/src/main/java/org/springframework/core/convert/converter/Converter.java b/spring-core/src/main/java/org/springframework/core/convert/converter/Converter.java index 8417f581bc8..a7b1a68d031 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/converter/Converter.java +++ b/spring-core/src/main/java/org/springframework/core/convert/converter/Converter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2012 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. @@ -20,10 +20,13 @@ package org.springframework.core.convert.converter; * A converter converts a source object of type S to a target of type T. * Implementations of this interface are thread-safe and can be shared. * + *

Implementations may additionally implement {@link ConditionalConversion}. + * * @author Keith Donald + * @since 3.0 + * @see ConditionalConversion * @param The source type * @param The target type - * @since 3.0 */ public interface Converter { diff --git a/spring-core/src/main/java/org/springframework/core/convert/converter/ConverterFactory.java b/spring-core/src/main/java/org/springframework/core/convert/converter/ConverterFactory.java index fd4e5909dce..bece4d3266b 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/converter/ConverterFactory.java +++ b/spring-core/src/main/java/org/springframework/core/convert/converter/ConverterFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2012 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. @@ -19,8 +19,11 @@ package org.springframework.core.convert.converter; /** * A factory for "ranged" converters that can convert objects from S to subtypes of R. * + *

Implementations may additionally implement {@link ConditionalConversion}. + * * @author Keith Donald * @since 3.0 + * @see ConditionalConversion * @param The source type converters created by this factory can convert from * @param The target range (or base) type converters created by this factory can convert to; * for example {@link Number} for a set of number subtypes. diff --git a/spring-core/src/main/java/org/springframework/core/convert/converter/GenericConverter.java b/spring-core/src/main/java/org/springframework/core/convert/converter/GenericConverter.java index 84c93c8b99f..3bbb1149f64 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/converter/GenericConverter.java +++ b/spring-core/src/main/java/org/springframework/core/convert/converter/GenericConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 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. @@ -34,18 +34,24 @@ import java.util.Set; *

This interface should generally not be used when the simpler {@link Converter} or * {@link ConverterFactory} interfaces are sufficient. * + *

Implementations may additionally implement {@link ConditionalConversion}. + * * @author Keith Donald * @author Juergen Hoeller * @since 3.0 * @see TypeDescriptor * @see Converter * @see ConverterFactory + * @see ConditionalConversion */ public interface GenericConverter { /** - * Return the source and target types which this converter can convert between. - *

Each entry is a convertible source-to-target type pair. + * Return the source and target types which this converter can convert between. Each + * entry is a convertible source-to-target type pair. + *

+ * For {@link ConditionalConversion conditional} converters this method may return + * {@code null} to indicate all source-to-target pairs should be considered. * */ Set getConvertibleTypes(); 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 fa9383e2f2d..205c66363e6 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 @@ -34,6 +34,7 @@ import org.springframework.core.convert.ConversionFailedException; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.ConverterNotFoundException; import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.convert.converter.ConditionalConversion; import org.springframework.core.convert.converter.ConditionalGenericConverter; import org.springframework.core.convert.converter.Converter; import org.springframework.core.convert.converter.ConverterFactory; @@ -289,6 +290,10 @@ public class GenericConversionService implements ConfigurableConversionService { if(!this.typeInfo.getTargetType().equals(targetType.getObjectType())) { return false; } + if (this.converter instanceof ConditionalConversion) { + return ((ConditionalConversion) this.converter).matches(sourceType, + targetType); + } return true; } @@ -310,7 +315,7 @@ public class GenericConversionService implements ConfigurableConversionService { * Adapts a {@link ConverterFactory} to a {@link GenericConverter}. */ @SuppressWarnings("unchecked") - private final class ConverterFactoryAdapter implements GenericConverter { + private final class ConverterFactoryAdapter implements ConditionalGenericConverter { private final ConvertiblePair typeInfo; @@ -327,6 +332,21 @@ public class GenericConversionService implements ConfigurableConversionService { return Collections.singleton(this.typeInfo); } + public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { + boolean matches = true; + if (this.converterFactory instanceof ConditionalConversion) { + matches = ((ConditionalConversion) this.converterFactory).matches( + sourceType, targetType); + } + if(matches) { + Converter converter = converterFactory.getConverter(targetType.getType()); + if(converter instanceof ConditionalConversion) { + matches = ((ConditionalConversion) converter).matches(sourceType, targetType); + } + } + return matches; + } + public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { if (source == null) { return convertNullSource(sourceType, targetType); @@ -393,15 +413,23 @@ public class GenericConversionService implements ConfigurableConversionService { IGNORED_CLASSES = Collections.unmodifiableSet(ignored); } + private final Set globalConverters = + new LinkedHashSet(); + private final Map converters = new LinkedHashMap(36); public void add(GenericConverter converter) { Set convertibleTypes = converter.getConvertibleTypes(); - Assert.state(converter.getConvertibleTypes() != null, "Converter does not specifiy ConvertibleTypes"); - for (ConvertiblePair convertiblePair : convertibleTypes) { - ConvertersForPair convertersForPair = getMatchableConverters(convertiblePair); - convertersForPair.add(converter); + if (convertibleTypes == null) { + Assert.state(converter instanceof ConditionalConversion, + "Only conditional converters may return null convertible types"); + globalConverters.add(converter); + } else { + for (ConvertiblePair convertiblePair : convertibleTypes) { + ConvertersForPair convertersForPair = getMatchableConverters(convertiblePair); + convertersForPair.add(converter); + } } } @@ -454,6 +482,15 @@ public class GenericConversionService implements ConfigurableConversionService { return converter; } + // Check ConditionalGenericConverter that match all types + for (GenericConverter globalConverter : this.globalConverters) { + if (((ConditionalConversion)globalConverter).matches( + sourceCandidate, + targetCandidate)) { + return globalConverter; + } + } + return null; } 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 9c87aed2dde..7d72d957b9a 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 @@ -31,7 +31,9 @@ import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -42,7 +44,9 @@ 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.ConditionalConversion; import org.springframework.core.convert.converter.Converter; +import org.springframework.core.convert.converter.ConverterFactory; import org.springframework.core.convert.converter.GenericConverter; import org.springframework.core.io.DescriptiveResource; import org.springframework.core.io.Resource; @@ -644,4 +648,134 @@ public class GenericConversionServiceTests { conversionService.removeConvertible(String.class, Color.class); assertFalse(conversionService.canConvert(String.class, Color.class)); } + + @Test + public void conditionalConverter() throws Exception { + GenericConversionService conversionService = new GenericConversionService(); + MyConditionalConverter converter = new MyConditionalConverter(); + conversionService.addConverter(new ColorConverter()); + conversionService.addConverter(converter); + assertEquals(Color.BLACK, conversionService.convert("#000000", Color.class)); + assertTrue(converter.getMatchAttempts() > 0); + } + + @Test + public void conditionalConverterFactory() throws Exception { + GenericConversionService conversionService = new GenericConversionService(); + MyConditionalConverterFactory converter = new MyConditionalConverterFactory(); + conversionService.addConverter(new ColorConverter()); + conversionService.addConverterFactory(converter); + assertEquals(Color.BLACK, conversionService.convert("#000000", Color.class)); + assertTrue(converter.getMatchAttempts() > 0); + assertTrue(converter.getNestedMatchAttempts() > 0); + } + + @Test + public void shouldNotSuportNullConvertibleTypesFromNonConditionalGenericConverter() + throws Exception { + GenericConversionService conversionService = new GenericConversionService(); + GenericConverter converter = new GenericConverter() { + + public Set getConvertibleTypes() { + return null; + } + + public Object convert(Object source, TypeDescriptor sourceType, + TypeDescriptor targetType) { + return null; + } + }; + try { + conversionService.addConverter(converter); + fail("Did not throw"); + } catch (IllegalStateException e) { + assertEquals("Only conditional converters may return null convertible types", e.getMessage()); + } + } + + @Test + public void conditionalConversionForAllTypes() throws Exception { + GenericConversionService conversionService = new GenericConversionService(); + MyConditionalGenericConverter converter = new MyConditionalGenericConverter(); + conversionService.addConverter(converter); + assertEquals((Integer) 3, conversionService.convert(3, Integer.class)); + Iterator iterator = converter.getSourceTypes().iterator(); + assertEquals(Integer.class, iterator.next().getType()); + assertEquals(Number.class, iterator.next().getType()); + TypeDescriptor last = null; + while (iterator.hasNext()) { + last = iterator.next(); + } + assertEquals(Object.class, last.getType()); + } + + private static class MyConditionalConverter implements Converter, + ConditionalConversion { + + private int matchAttempts = 0; + + public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { + matchAttempts++; + return false; + } + + public Color convert(String source) { + throw new IllegalStateException(); + } + + public int getMatchAttempts() { + return matchAttempts; + } + } + + private static class MyConditionalGenericConverter implements GenericConverter, + ConditionalConversion { + + private Set sourceTypes = new LinkedHashSet(); + + public Set getConvertibleTypes() { + return null; + } + + public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { + sourceTypes.add(sourceType); + return false; + } + + public Object convert(Object source, TypeDescriptor sourceType, + TypeDescriptor targetType) { + return null; + } + + public Set getSourceTypes() { + return sourceTypes; + } + } + + private static class MyConditionalConverterFactory implements + ConverterFactory, ConditionalConversion { + + private MyConditionalConverter converter = new MyConditionalConverter(); + + private int matchAttempts = 0; + + public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { + matchAttempts++; + return true; + } + + @SuppressWarnings("unchecked") + public Converter getConverter(Class targetType) { + return (Converter) converter; + } + + public int getMatchAttempts() { + return matchAttempts; + } + + public int getNestedMatchAttempts() { + return converter.getMatchAttempts(); + } + } + } From a27d1a28ffb69aed618869d3815a3af7120effa2 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 26 Oct 2012 17:43:41 -0700 Subject: [PATCH 4/7] Bypass conversion when possible Prior to this commit conversion between like types would often result in a copy of the object. This can be problematic in the case of large byte arrays and objects that do not have a default constructor. The ConversionService SPI now includes canBypassConvert methods that can be used to deduce when conversion is not needed. Several existing converters have been updated to ensure they only apply when source and target types differ. This change introduces new methods to the ConversionService that will break existing implementations. However, it anticipated that most users are consuming the ConversionService interface rather then extending it. Issue: SPR-9566 --- .../core/convert/ConversionService.java | 25 ++++++++++++++++++- .../support/ArrayToArrayConverter.java | 21 ++++++++++++---- .../FallbackObjectToStringConverter.java | 5 +++- .../support/GenericConversionService.java | 15 +++++++++++ .../NumberToNumberConverterFactory.java | 11 ++++++-- .../GenericConversionServiceTests.java | 25 +++++++++++++++++++ 6 files changed, 93 insertions(+), 9 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/convert/ConversionService.java b/spring-core/src/main/java/org/springframework/core/convert/ConversionService.java index 378016c30b4..b359453d0d0 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/ConversionService.java +++ b/spring-core/src/main/java/org/springframework/core/convert/ConversionService.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 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. @@ -21,6 +21,7 @@ package org.springframework.core.convert; * Call {@link #convert(Object, Class)} to perform a thread-safe type conversion using this system. * * @author Keith Donald + * @author Phillip Webb * @since 3.0 */ public interface ConversionService { @@ -54,6 +55,28 @@ public interface ConversionService { */ 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 + * 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 targetType context about the target type to convert to (required) + * @return true if conversion can be bypassed + * @throws IllegalArgumentException if targetType is null + */ + boolean canBypassConvert(Class sourceType, Class 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 + * 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 targetType context about the target type to convert to (required) + * @return true if conversion can be bypassed + * @throws IllegalArgumentException if targetType is null + */ + boolean canBypassConvert(TypeDescriptor sourceType, TypeDescriptor targetType); + /** * Convert the source to targetType. * @param source the source object to convert (may be null) diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/ArrayToArrayConverter.java b/spring-core/src/main/java/org/springframework/core/convert/support/ArrayToArrayConverter.java index 288072f90b6..9a20642a176 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/ArrayToArrayConverter.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/ArrayToArrayConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 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. @@ -18,6 +18,7 @@ package org.springframework.core.convert.support; import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.Set; import org.springframework.core.convert.ConversionService; @@ -26,18 +27,22 @@ import org.springframework.core.convert.converter.ConditionalGenericConverter; import org.springframework.util.ObjectUtils; /** - * Converts an Array to another Array. - * First adapts the source array to a List, then delegates to {@link CollectionToArrayConverter} to perform the target array conversion. + * Converts an Array to another Array. First adapts the source array to a List, then + * delegates to {@link CollectionToArrayConverter} to perform the target array conversion. * * @author Keith Donald + * @author Phillip Webb * @since 3.0 */ final class ArrayToArrayConverter implements ConditionalGenericConverter { private final CollectionToArrayConverter helperConverter; + private final ConversionService conversionService; + public ArrayToArrayConverter(ConversionService conversionService) { this.helperConverter = new CollectionToArrayConverter(conversionService); + this.conversionService = conversionService; } public Set getConvertibleTypes() { @@ -48,8 +53,14 @@ final class ArrayToArrayConverter implements ConditionalGenericConverter { return this.helperConverter.matches(sourceType, targetType); } - public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { - return this.helperConverter.convert(Arrays.asList(ObjectUtils.toObjectArray(source)), sourceType, targetType); + public Object convert(Object source, TypeDescriptor sourceType, + TypeDescriptor targetType) { + if (conversionService.canBypassConvert(sourceType.getElementTypeDescriptor(), + targetType.getElementTypeDescriptor())) { + return source; + } + List sourceList = Arrays.asList(ObjectUtils.toObjectArray(source)); + return this.helperConverter.convert(sourceList, sourceType, targetType); } } diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/FallbackObjectToStringConverter.java b/spring-core/src/main/java/org/springframework/core/convert/support/FallbackObjectToStringConverter.java index 204a912b0c5..268f0300ab2 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/FallbackObjectToStringConverter.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/FallbackObjectToStringConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2012 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. @@ -41,6 +41,9 @@ final class FallbackObjectToStringConverter implements ConditionalGenericConvert public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { Class sourceClass = sourceType.getObjectType(); + if (String.class.equals(sourceClass)) { + return false; + } return CharSequence.class.isAssignableFrom(sourceClass) || StringWriter.class.isAssignableFrom(sourceClass) || ObjectToObjectConverter.hasValueOfMethodOrConstructor(sourceClass, String.class); } 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 205c66363e6..e7ac42ff9fd 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 @@ -126,6 +126,21 @@ public class GenericConversionService implements ConfigurableConversionService { return (converter != null); } + public boolean canBypassConvert(Class sourceType, Class targetType) { + Assert.notNull(targetType, "The targetType to convert to cannot be null"); + return canBypassConvert(sourceType != null ? TypeDescriptor.valueOf(sourceType) + : null, TypeDescriptor.valueOf(targetType)); + } + + public boolean canBypassConvert(TypeDescriptor sourceType, TypeDescriptor targetType) { + Assert.notNull(targetType, "The targetType to convert to cannot be null"); + if (sourceType == null) { + return true; + } + GenericConverter converter = getConverter(sourceType, targetType); + return (converter == NO_OP_CONVERTER); + } + @SuppressWarnings("unchecked") public T convert(Object source, Class targetType) { Assert.notNull(targetType,"The targetType to convert to cannot be null"); diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/NumberToNumberConverterFactory.java b/spring-core/src/main/java/org/springframework/core/convert/support/NumberToNumberConverterFactory.java index 273d36ab62a..fc9475e8a95 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/NumberToNumberConverterFactory.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/NumberToNumberConverterFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2012 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. @@ -16,6 +16,8 @@ package org.springframework.core.convert.support; +import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.convert.converter.ConditionalConversion; import org.springframework.core.convert.converter.Converter; import org.springframework.core.convert.converter.ConverterFactory; import org.springframework.util.NumberUtils; @@ -38,12 +40,17 @@ import org.springframework.util.NumberUtils; * @see java.math.BigDecimal * @see NumberUtils */ -final class NumberToNumberConverterFactory implements ConverterFactory { +final class NumberToNumberConverterFactory implements ConverterFactory, + ConditionalConversion { public Converter getConverter(Class targetType) { return new NumberToNumber(targetType); } + public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { + return !sourceType.equals(targetType); + } + private final static class NumberToNumber implements Converter { private final Class targetType; 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 7d72d957b9a..b99c76af1d6 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 @@ -19,6 +19,7 @@ package org.springframework.core.convert.support; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; @@ -709,6 +710,30 @@ public class GenericConversionServiceTests { assertEquals(Object.class, last.getType()); } + @Test + public void convertOptimizeArray() throws Exception { + // SPR-9566 + GenericConversionService conversionService = new DefaultConversionService(); + byte[] byteArray = new byte[] { 1, 2, 3 }; + byte[] converted = conversionService.convert(byteArray, byte[].class); + assertSame(byteArray, converted); + } + + @Test + public void convertCannotOptimizeArray() throws Exception { + GenericConversionService conversionService = new GenericConversionService(); + conversionService.addConverter(new Converter() { + public Byte convert(Byte source) { + return (byte) (source + 1); + } + }); + DefaultConversionService.addDefaultConverters(conversionService); + byte[] byteArray = new byte[] { 1, 2, 3 }; + byte[] converted = conversionService.convert(byteArray, byte[].class); + assertNotSame(byteArray, converted); + assertTrue(Arrays.equals(new byte[] { 2, 3, 4 }, converted)); + } + private static class MyConditionalConverter implements Converter, ConditionalConversion { From 138957b1482e262ba3bd981c92ae89c07a4d2e9b Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 26 Oct 2012 18:15:02 -0700 Subject: [PATCH 5/7] Test SpEL unconditional argument conversion Issue: SPR-9566 --- .../spel/MethodInvocationTests.java | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java index 6134533eb1b..56555b209d9 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java @@ -16,7 +16,8 @@ package org.springframework.expression.spel; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; import java.lang.annotation.Annotation; import java.lang.annotation.Retention; @@ -29,6 +30,7 @@ import org.junit.Assert; import org.junit.Test; import org.springframework.core.convert.TypeDescriptor; import org.springframework.expression.AccessException; +import org.springframework.expression.BeanResolver; import org.springframework.expression.EvaluationContext; import org.springframework.expression.Expression; import org.springframework.expression.ExpressionInvocationTargetException; @@ -44,6 +46,7 @@ import org.springframework.expression.spel.testresources.PlaceOfBirth; * Tests invocation of methods. * * @author Andy Clement + * @author Phillip Webb */ public class MethodInvocationTests extends ExpressionTestCase { @@ -369,4 +372,30 @@ public class MethodInvocationTests extends ExpressionTestCase { Object value = expression.getValue(new StandardEvaluationContext(String.class)); assertEquals(value, "java.lang.String"); } + + @Test + public void invokeMethodWithoutConversion() throws Exception { + final BytesService service = new BytesService(); + byte[] bytes = new byte[100]; + StandardEvaluationContext context = new StandardEvaluationContext(bytes); + context.setBeanResolver(new BeanResolver() { + public Object resolve(EvaluationContext context, String beanName) + throws AccessException { + if ("service".equals(beanName)) { + return service; + } + return null; + } + }); + Expression expression = parser.parseExpression("@service.handleBytes(#root)"); + byte[] outBytes = expression.getValue(context, byte[].class); + assertSame(bytes, outBytes); + } + + public static class BytesService { + + public byte[] handleBytes(byte[] bytes) { + return bytes; + } + } } From f82c6ed7cc8639991bac0afde787ea00ad4331ae Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Sat, 27 Oct 2012 09:06:25 -0700 Subject: [PATCH 6/7] Support conversion from Enum Interface EnumToStringConverter in now conditional and only matches Enums that do not implement interfaces that can be converted. Issue: SPR-9692 --- .../support/DefaultConversionService.java | 3 +- .../support/EnumToStringConverter.java | 27 +++++++++++++-- .../GenericConversionServiceTests.java | 34 +++++++++++++++++++ 3 files changed, 61 insertions(+), 3 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/DefaultConversionService.java b/spring-core/src/main/java/org/springframework/core/convert/support/DefaultConversionService.java index fe510e9a534..86e580983b3 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/DefaultConversionService.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/DefaultConversionService.java @@ -59,6 +59,7 @@ public class DefaultConversionService extends GenericConversionService { // internal helpers private static void addScalarConverters(ConverterRegistry converterRegistry) { + ConversionService conversionService = (ConversionService) converterRegistry; converterRegistry.addConverter(new StringToBooleanConverter()); converterRegistry.addConverter(Boolean.class, String.class, new ObjectToStringConverter()); @@ -74,7 +75,7 @@ public class DefaultConversionService extends GenericConversionService { converterRegistry.addConverterFactory(new CharacterToNumberFactory()); converterRegistry.addConverterFactory(new StringToEnumConverterFactory()); - converterRegistry.addConverter(Enum.class, String.class, new EnumToStringConverter()); + converterRegistry.addConverter(Enum.class, String.class, new EnumToStringConverter(conversionService)); converterRegistry.addConverter(new StringToLocaleConverter()); converterRegistry.addConverter(Locale.class, String.class, new ObjectToStringConverter()); diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/EnumToStringConverter.java b/spring-core/src/main/java/org/springframework/core/convert/support/EnumToStringConverter.java index bb06f1773af..b9eede1abdc 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/EnumToStringConverter.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/EnumToStringConverter.java @@ -16,14 +16,37 @@ package org.springframework.core.convert.support; +import org.springframework.core.convert.ConversionService; +import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.convert.converter.ConditionalConversion; import org.springframework.core.convert.converter.Converter; +import org.springframework.util.ClassUtils; +import org.springframework.util.ReflectionUtils; /** - * Simply calls {@link Enum#name()} to convert a source Enum to a String. + * Calls {@link Enum#name()} to convert a source Enum to a String. This converter will + * not match enums with interfaces that can be converterd. * @author Keith Donald + * @author Phillip Webb * @since 3.0 */ -final class EnumToStringConverter implements Converter, String> { +final class EnumToStringConverter implements Converter, String>, ConditionalConversion { + + private final ConversionService conversionService; + + public EnumToStringConverter(ConversionService conversionService) { + this.conversionService = conversionService; + } + + public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { + for (Class interfaceType : ClassUtils.getAllInterfacesForClass(sourceType.getType())) { + if (conversionService.canConvert(TypeDescriptor.valueOf(interfaceType), + targetType)) { + return false; + } + } + return true; + } public String convert(Enum source) { return source.name(); 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 b99c76af1d6..bba7379395d 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 @@ -734,6 +734,22 @@ public class GenericConversionServiceTests { assertTrue(Arrays.equals(new byte[] { 2, 3, 4 }, converted)); } + @Test + public void testEnumToStringConversion() { + conversionService.addConverter(new EnumToStringConverter(conversionService)); + String result = conversionService.convert(MyEnum.A, String.class); + assertEquals("A", result); + } + + @Test + public void testEnumWithInterfaceToStringConversion() { + // SPR-9692 + conversionService.addConverter(new EnumToStringConverter(conversionService)); + conversionService.addConverter(new MyEnumInterfaceToStringConverter()); + String result = conversionService.convert(MyEnum.A, String.class); + assertEquals("1", result); + } + private static class MyConditionalConverter implements Converter, ConditionalConversion { @@ -803,4 +819,22 @@ public class GenericConversionServiceTests { } } + interface MyEnumInterface { + String getCode(); + } + + public static enum MyEnum implements MyEnumInterface { + A { + public String getCode() { + return "1"; + } + }; + } + + private static class MyEnumInterfaceToStringConverter + implements Converter { + public String convert(T source) { + return source.getCode(); + } + } } From 222eec58cde4ac87a6de2cbbf2deb63ef70baea3 Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Mon, 29 Oct 2012 14:25:12 +0100 Subject: [PATCH 7/7] Review and polish pull request #132 Content: - Rename Conditional{Conversion=>Converter} - Add @since tags where appropriate - Update Apache date headers to read 2002-2012 (not just 2012) - Correct minor Javadoc typo Style: - Polish line breaks / whitespace - Use wildcard static imports where appropriate Issue: SPR-9566, SPR-9692, SPR-9928, SPR-9927 --- .../core/convert/ConversionService.java | 2 + .../core/convert/TypeDescriptor.java | 1 + ...version.java => ConditionalConverter.java} | 8 +-- .../ConditionalGenericConverter.java | 7 +- .../core/convert/converter/Converter.java | 4 +- .../convert/converter/ConverterFactory.java | 4 +- .../convert/converter/GenericConverter.java | 6 +- .../support/EnumToStringConverter.java | 10 ++- .../support/GenericConversionService.java | 66 +++++++++++-------- .../NumberToNumberConverterFactory.java | 4 +- .../core/convert/TypeDescriptorTests.java | 9 +-- .../GenericConversionServiceTests.java | 20 ++---- 12 files changed, 69 insertions(+), 72 deletions(-) rename spring-core/src/main/java/org/springframework/core/convert/converter/{ConditionalConversion.java => ConditionalConverter.java} (91%) diff --git a/spring-core/src/main/java/org/springframework/core/convert/ConversionService.java b/spring-core/src/main/java/org/springframework/core/convert/ConversionService.java index b359453d0d0..8dd40490e9b 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/ConversionService.java +++ b/spring-core/src/main/java/org/springframework/core/convert/ConversionService.java @@ -63,6 +63,7 @@ public interface ConversionService { * @param targetType context about the target type to convert to (required) * @return true if conversion can be bypassed * @throws IllegalArgumentException if targetType is null + * @since 3.2 */ boolean canBypassConvert(Class sourceType, Class targetType); @@ -74,6 +75,7 @@ public interface ConversionService { * @param targetType context about the target type to convert to (required) * @return true if conversion can be bypassed * @throws IllegalArgumentException if targetType is null + * @since 3.2 */ boolean canBypassConvert(TypeDescriptor sourceType, TypeDescriptor targetType); diff --git a/spring-core/src/main/java/org/springframework/core/convert/TypeDescriptor.java b/spring-core/src/main/java/org/springframework/core/convert/TypeDescriptor.java index 7e119c34548..c4969fdb568 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/TypeDescriptor.java +++ b/spring-core/src/main/java/org/springframework/core/convert/TypeDescriptor.java @@ -257,6 +257,7 @@ public class TypeDescriptor { * @param superType the super type to cast to (can be {@code null} * @return a new TypeDescriptor for the up-cast type * @throws IllegalArgumentException if this type is not assignable to the super-type + * @since 3.2 */ public TypeDescriptor upcast(Class superType) { if (superType == null) { diff --git a/spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalConversion.java b/spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalConverter.java similarity index 91% rename from spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalConversion.java rename to spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalConverter.java index 1196b26fc4f..94aacd6261f 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalConversion.java +++ b/spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2012 the original author or authors. + * Copyright 2002-2012 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. @@ -33,18 +33,18 @@ import org.springframework.core.convert.TypeDescriptor; * implementation might return {@code true} if the target Account class defines a * {@code public static findAccount(String)} method. * - * @author Keith Donald * @author Phillip Webb + * @author Keith Donald * @since 3.2 * @see Converter * @see GenericConverter * @see ConverterFactory * @see ConditionalGenericConverter */ -public interface ConditionalConversion { +public interface ConditionalConverter { /** - * Should the converter from {@code sourceType} to {@code targetType} currently under + * Should the conversion from {@code sourceType} to {@code targetType} currently under * consideration be selected? * * @param sourceType the type descriptor of the field we are converting from diff --git a/spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalGenericConverter.java b/spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalGenericConverter.java index fc7ce7a1ab0..d65d7bb3e9a 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalGenericConverter.java +++ b/spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalGenericConverter.java @@ -18,19 +18,18 @@ package org.springframework.core.convert.converter; import org.springframework.core.convert.TypeDescriptor; - /** * A {@link GenericConverter} that may conditionally execute based on attributes of the * {@code source} and {@code target} {@link TypeDescriptor}. See - * {@link ConditionalConversion} for details. + * {@link ConditionalConverter} for details. * * @author Keith Donald * @author Phillip Webb * @since 3.0 * @see GenericConverter - * @see ConditionalConversion + * @see ConditionalConverter */ public interface ConditionalGenericConverter extends GenericConverter, - ConditionalConversion { + ConditionalConverter { } diff --git a/spring-core/src/main/java/org/springframework/core/convert/converter/Converter.java b/spring-core/src/main/java/org/springframework/core/convert/converter/Converter.java index a7b1a68d031..6710b666abf 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/converter/Converter.java +++ b/spring-core/src/main/java/org/springframework/core/convert/converter/Converter.java @@ -20,11 +20,11 @@ package org.springframework.core.convert.converter; * A converter converts a source object of type S to a target of type T. * Implementations of this interface are thread-safe and can be shared. * - *

Implementations may additionally implement {@link ConditionalConversion}. + *

Implementations may additionally implement {@link ConditionalConverter}. * * @author Keith Donald * @since 3.0 - * @see ConditionalConversion + * @see ConditionalConverter * @param The source type * @param The target type */ diff --git a/spring-core/src/main/java/org/springframework/core/convert/converter/ConverterFactory.java b/spring-core/src/main/java/org/springframework/core/convert/converter/ConverterFactory.java index bece4d3266b..97ae018d449 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/converter/ConverterFactory.java +++ b/spring-core/src/main/java/org/springframework/core/convert/converter/ConverterFactory.java @@ -19,11 +19,11 @@ package org.springframework.core.convert.converter; /** * A factory for "ranged" converters that can convert objects from S to subtypes of R. * - *

Implementations may additionally implement {@link ConditionalConversion}. + *

Implementations may additionally implement {@link ConditionalConverter}. * * @author Keith Donald * @since 3.0 - * @see ConditionalConversion + * @see ConditionalConverter * @param The source type converters created by this factory can convert from * @param The target range (or base) type converters created by this factory can convert to; * for example {@link Number} for a set of number subtypes. diff --git a/spring-core/src/main/java/org/springframework/core/convert/converter/GenericConverter.java b/spring-core/src/main/java/org/springframework/core/convert/converter/GenericConverter.java index 3bbb1149f64..83923a778e9 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/converter/GenericConverter.java +++ b/spring-core/src/main/java/org/springframework/core/convert/converter/GenericConverter.java @@ -34,7 +34,7 @@ import java.util.Set; *

This interface should generally not be used when the simpler {@link Converter} or * {@link ConverterFactory} interfaces are sufficient. * - *

Implementations may additionally implement {@link ConditionalConversion}. + *

Implementations may additionally implement {@link ConditionalConverter}. * * @author Keith Donald * @author Juergen Hoeller @@ -42,7 +42,7 @@ import java.util.Set; * @see TypeDescriptor * @see Converter * @see ConverterFactory - * @see ConditionalConversion + * @see ConditionalConverter */ public interface GenericConverter { @@ -50,7 +50,7 @@ public interface GenericConverter { * Return the source and target types which this converter can convert between. Each * entry is a convertible source-to-target type pair. *

- * For {@link ConditionalConversion conditional} converters this method may return + * For {@link ConditionalConverter conditional} converters this method may return * {@code null} to indicate all source-to-target pairs should be considered. * */ Set getConvertibleTypes(); diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/EnumToStringConverter.java b/spring-core/src/main/java/org/springframework/core/convert/support/EnumToStringConverter.java index b9eede1abdc..e391b75bb30 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/EnumToStringConverter.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/EnumToStringConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2012 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. @@ -18,10 +18,9 @@ package org.springframework.core.convert.support; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.TypeDescriptor; -import org.springframework.core.convert.converter.ConditionalConversion; +import org.springframework.core.convert.converter.ConditionalConverter; import org.springframework.core.convert.converter.Converter; import org.springframework.util.ClassUtils; -import org.springframework.util.ReflectionUtils; /** * Calls {@link Enum#name()} to convert a source Enum to a String. This converter will @@ -30,7 +29,7 @@ import org.springframework.util.ReflectionUtils; * @author Phillip Webb * @since 3.0 */ -final class EnumToStringConverter implements Converter, String>, ConditionalConversion { +final class EnumToStringConverter implements Converter, String>, ConditionalConverter { private final ConversionService conversionService; @@ -40,8 +39,7 @@ final class EnumToStringConverter implements Converter, String>, Conditi public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { for (Class interfaceType : ClassUtils.getAllInterfacesForClass(sourceType.getType())) { - if (conversionService.canConvert(TypeDescriptor.valueOf(interfaceType), - targetType)) { + if (conversionService.canConvert(TypeDescriptor.valueOf(interfaceType), targetType)) { return false; } } 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 e7ac42ff9fd..fe86c128dab 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 @@ -34,7 +34,7 @@ import org.springframework.core.convert.ConversionFailedException; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.ConverterNotFoundException; import org.springframework.core.convert.TypeDescriptor; -import org.springframework.core.convert.converter.ConditionalConversion; +import org.springframework.core.convert.converter.ConditionalConverter; import org.springframework.core.convert.converter.ConditionalGenericConverter; import org.springframework.core.convert.converter.Converter; import org.springframework.core.convert.converter.ConverterFactory; @@ -81,8 +81,8 @@ public class GenericConversionService implements ConfigurableConversionService { public void addConverter(Converter converter) { GenericConverter.ConvertiblePair typeInfo = getRequiredTypeInfo(converter, Converter.class); - Assert.notNull(typeInfo, "Unable to the determine sourceType and targetType which " + - "your Converter converts between; declare these generic types."); + Assert.notNull(typeInfo, "Unable to the determine sourceType and targetType " + + " which your Converter converts between; declare these generic types."); addConverter(new ConverterAdapter(typeInfo, converter)); } @@ -99,8 +99,9 @@ public class GenericConversionService implements ConfigurableConversionService { public void addConverterFactory(ConverterFactory converterFactory) { GenericConverter.ConvertiblePair typeInfo = getRequiredTypeInfo(converterFactory, ConverterFactory.class); if (typeInfo == null) { - throw new IllegalArgumentException("Unable to the determine sourceType and targetRangeType R which " + - "your ConverterFactory converts between; declare these generic types."); + throw new IllegalArgumentException("Unable to the determine sourceType and " + + "targetRangeType R which your ConverterFactory converts between; " + + "declare these generic types."); } addConverter(new ConverterFactoryAdapter(typeInfo, converterFactory)); } @@ -114,7 +115,9 @@ public class GenericConversionService implements ConfigurableConversionService { public boolean canConvert(Class sourceType, Class targetType) { Assert.notNull(targetType, "The targetType to convert to cannot be null"); - return canConvert(sourceType != null ? TypeDescriptor.valueOf(sourceType) : null, TypeDescriptor.valueOf(targetType)); + return canConvert(sourceType != null ? + TypeDescriptor.valueOf(sourceType) : null, + TypeDescriptor.valueOf(targetType)); } public boolean canConvert(TypeDescriptor sourceType, TypeDescriptor targetType) { @@ -128,8 +131,9 @@ public class GenericConversionService implements ConfigurableConversionService { public boolean canBypassConvert(Class sourceType, Class targetType) { Assert.notNull(targetType, "The targetType to convert to cannot be null"); - return canBypassConvert(sourceType != null ? TypeDescriptor.valueOf(sourceType) - : null, TypeDescriptor.valueOf(targetType)); + return canBypassConvert(sourceType != null ? + TypeDescriptor.valueOf(sourceType) : null, + TypeDescriptor.valueOf(targetType)); } public boolean canBypassConvert(TypeDescriptor sourceType, TypeDescriptor targetType) { @@ -166,8 +170,11 @@ public class GenericConversionService implements ConfigurableConversionService { } /** - * Convenience operation for converting a source object to the specified targetType, where the targetType is a descriptor that provides additional conversion context. - * Simply delegates to {@link #convert(Object, TypeDescriptor, TypeDescriptor)} and encapsulates the construction of the sourceType descriptor using {@link TypeDescriptor#forObject(Object)}. + * Convenience operation for converting a source object to the specified targetType, + * where the targetType is a descriptor that provides additional conversion context. + * Simply delegates to {@link #convert(Object, TypeDescriptor, TypeDescriptor)} and + * encapsulates the construction of the sourceType descriptor using + * {@link TypeDescriptor#forObject(Object)}. * @param source the source object * @param targetType the target type * @return the converted value @@ -206,7 +213,8 @@ public class GenericConversionService implements ConfigurableConversionService { * 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) { @@ -305,9 +313,8 @@ public class GenericConversionService implements ConfigurableConversionService { if(!this.typeInfo.getTargetType().equals(targetType.getObjectType())) { return false; } - if (this.converter instanceof ConditionalConversion) { - return ((ConditionalConversion) this.converter).matches(sourceType, - targetType); + if (this.converter instanceof ConditionalConverter) { + return ((ConditionalConverter) this.converter).matches(sourceType, targetType); } return true; } @@ -320,8 +327,9 @@ public class GenericConversionService implements ConfigurableConversionService { } public String toString() { - return this.typeInfo.getSourceType().getName() + " -> " + this.typeInfo.getTargetType().getName() + - " : " + this.converter.toString(); + return this.typeInfo.getSourceType().getName() + " -> " + + this.typeInfo.getTargetType().getName() + " : " + + this.converter.toString(); } } @@ -349,14 +357,13 @@ public class GenericConversionService implements ConfigurableConversionService { public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { boolean matches = true; - if (this.converterFactory instanceof ConditionalConversion) { - matches = ((ConditionalConversion) this.converterFactory).matches( - sourceType, targetType); + if (this.converterFactory instanceof ConditionalConverter) { + matches = ((ConditionalConverter) this.converterFactory).matches(sourceType, targetType); } if(matches) { - Converter converter = converterFactory.getConverter(targetType.getType()); - if(converter instanceof ConditionalConversion) { - matches = ((ConditionalConversion) converter).matches(sourceType, targetType); + Converter converter = this.converterFactory.getConverter(targetType.getType()); + if(converter instanceof ConditionalConverter) { + matches = ((ConditionalConverter) converter).matches(sourceType, targetType); } } return matches; @@ -370,8 +377,9 @@ public class GenericConversionService implements ConfigurableConversionService { } public String toString() { - return this.typeInfo.getSourceType().getName() + " -> " + this.typeInfo.getTargetType().getName() + - " : " + this.converterFactory.toString(); + return this.typeInfo.getSourceType().getName() + " -> " + + this.typeInfo.getTargetType().getName() + " : " + + this.converterFactory.toString(); } } @@ -437,7 +445,7 @@ public class GenericConversionService implements ConfigurableConversionService { public void add(GenericConverter converter) { Set convertibleTypes = converter.getConvertibleTypes(); if (convertibleTypes == null) { - Assert.state(converter instanceof ConditionalConversion, + Assert.state(converter instanceof ConditionalConverter, "Only conditional converters may return null convertible types"); globalConverters.add(converter); } else { @@ -476,7 +484,8 @@ public class GenericConversionService implements ConfigurableConversionService { List targetCandidates = getTypeHierarchy(targetType); for (TypeDescriptor sourceCandidate : sourceCandidates) { for (TypeDescriptor targetCandidate : targetCandidates) { - GenericConverter converter = getRegisteredConverter(sourceType, targetType, sourceCandidate, targetCandidate); + GenericConverter converter = getRegisteredConverter( + sourceType, targetType, sourceCandidate, targetCandidate); if(converter != null) { return converter; } @@ -499,9 +508,8 @@ public class GenericConversionService implements ConfigurableConversionService { // Check ConditionalGenericConverter that match all types for (GenericConverter globalConverter : this.globalConverters) { - if (((ConditionalConversion)globalConverter).matches( - sourceCandidate, - targetCandidate)) { + if (((ConditionalConverter)globalConverter).matches( + sourceCandidate, targetCandidate)) { return globalConverter; } } diff --git a/spring-core/src/main/java/org/springframework/core/convert/support/NumberToNumberConverterFactory.java b/spring-core/src/main/java/org/springframework/core/convert/support/NumberToNumberConverterFactory.java index fc9475e8a95..240880d67fa 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/support/NumberToNumberConverterFactory.java +++ b/spring-core/src/main/java/org/springframework/core/convert/support/NumberToNumberConverterFactory.java @@ -17,7 +17,7 @@ package org.springframework.core.convert.support; import org.springframework.core.convert.TypeDescriptor; -import org.springframework.core.convert.converter.ConditionalConversion; +import org.springframework.core.convert.converter.ConditionalConverter; import org.springframework.core.convert.converter.Converter; import org.springframework.core.convert.converter.ConverterFactory; import org.springframework.util.NumberUtils; @@ -41,7 +41,7 @@ import org.springframework.util.NumberUtils; * @see NumberUtils */ final class NumberToNumberConverterFactory implements ConverterFactory, - ConditionalConversion { + ConditionalConverter { public Converter getConverter(Class targetType) { return new NumberToNumber(targetType); diff --git a/spring-core/src/test/java/org/springframework/core/convert/TypeDescriptorTests.java b/spring-core/src/test/java/org/springframework/core/convert/TypeDescriptorTests.java index e16a3324262..3e83b173ae0 100644 --- a/spring-core/src/test/java/org/springframework/core/convert/TypeDescriptorTests.java +++ b/spring-core/src/test/java/org/springframework/core/convert/TypeDescriptorTests.java @@ -16,13 +16,6 @@ package org.springframework.core.convert; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - import java.lang.annotation.ElementType; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -37,6 +30,8 @@ import java.util.Map; import org.junit.Test; import org.springframework.core.MethodParameter; +import static org.junit.Assert.*; + /** * @author Keith Donald * @author Andy Clement 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 bba7379395d..f312f2d71a4 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 @@ -16,15 +16,6 @@ package org.springframework.core.convert.support; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNotSame; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - import java.awt.Color; import java.awt.SystemColor; import java.util.ArrayList; @@ -42,10 +33,11 @@ import java.util.Set; import java.util.UUID; 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.ConditionalConversion; +import org.springframework.core.convert.converter.ConditionalConverter; import org.springframework.core.convert.converter.Converter; import org.springframework.core.convert.converter.ConverterFactory; import org.springframework.core.convert.converter.GenericConverter; @@ -54,6 +46,8 @@ import org.springframework.core.io.Resource; import org.springframework.util.StopWatch; import org.springframework.util.StringUtils; +import static org.junit.Assert.*; + /** * @author Keith Donald * @author Juergen Hoeller @@ -751,7 +745,7 @@ public class GenericConversionServiceTests { } private static class MyConditionalConverter implements Converter, - ConditionalConversion { + ConditionalConverter { private int matchAttempts = 0; @@ -770,7 +764,7 @@ public class GenericConversionServiceTests { } private static class MyConditionalGenericConverter implements GenericConverter, - ConditionalConversion { + ConditionalConverter { private Set sourceTypes = new LinkedHashSet(); @@ -794,7 +788,7 @@ public class GenericConversionServiceTests { } private static class MyConditionalConverterFactory implements - ConverterFactory, ConditionalConversion { + ConverterFactory, ConditionalConverter { private MyConditionalConverter converter = new MyConditionalConverter();