From 1a011616d9b6927fa33ea36dd48f8dbf18a23591 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Sat, 1 Feb 2014 12:12:19 +0100 Subject: [PATCH] Polishing Issue: SPR-11369 --- .../converter/ConditionalConverter.java | 9 +-- .../convert/converter/GenericConverter.java | 12 ++- .../support/GenericConversionService.java | 80 ++++++++----------- .../GenericConversionServiceTests.java | 64 ++++++++++++++- 4 files changed, 108 insertions(+), 57 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalConverter.java b/spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalConverter.java index 94aacd6261f..964139e0a2e 100644 --- a/spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalConverter.java +++ b/spring-core/src/main/java/org/springframework/core/convert/converter/ConditionalConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2014 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. @@ -26,11 +26,10 @@ import org.springframework.core.convert.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 + *

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 Phillip Webb @@ -46,10 +45,10 @@ public interface ConditionalConverter { /** * 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 * @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/GenericConverter.java b/spring-core/src/main/java/org/springframework/core/convert/converter/GenericConverter.java index 68261559898..5258497be22 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-2012 the original author or authors. + * Copyright 2002-2014 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,11 +16,11 @@ package org.springframework.core.convert.converter; +import java.util.Set; + import org.springframework.core.convert.TypeDescriptor; import org.springframework.util.Assert; -import java.util.Set; - /** * Generic converter interface for converting between two or more types. * @@ -104,13 +104,17 @@ public interface GenericConverter { } ConvertiblePair other = (ConvertiblePair) obj; return this.sourceType.equals(other.sourceType) && this.targetType.equals(other.targetType); - } @Override public int hashCode() { return this.sourceType.hashCode() * 31 + this.targetType.hashCode(); } + + @Override + public String toString() { + return this.sourceType.getName() + " -> " + this.targetType.getName(); + } } } 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 2f4f4f1ba4b..97ffbbda258 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-2013 the original author or authors. + * Copyright 2002-2014 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. @@ -83,12 +83,12 @@ public class GenericConversionService implements ConfigurableConversionService { 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."); - addConverter(new ConverterAdapter(typeInfo, converter)); + addConverter(new ConverterAdapter(converter, typeInfo)); } public void addConverter(Class sourceType, Class targetType, Converter converter) { GenericConverter.ConvertiblePair typeInfo = new GenericConverter.ConvertiblePair(sourceType, targetType); - addConverter(new ConverterAdapter(typeInfo, converter)); + addConverter(new ConverterAdapter(converter, typeInfo)); } public void addConverter(GenericConverter converter) { @@ -103,7 +103,7 @@ public class GenericConversionService implements ConfigurableConversionService { "targetRangeType R which your ConverterFactory converts between; " + "declare these generic types."); } - addConverter(new ConverterFactoryAdapter(typeInfo, converterFactory)); + addConverter(new ConverterFactoryAdapter(converterFactory, typeInfo)); } public void removeConvertible(Class sourceType, Class targetType) { @@ -114,14 +114,13 @@ public class GenericConversionService implements ConfigurableConversionService { // implementing ConversionService 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, + Assert.notNull(targetType, "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) { - Assert.notNull(targetType,"The targetType to convert to cannot be null"); + Assert.notNull(targetType, "targetType to convert to cannot be null"); if (sourceType == null) { return true; } @@ -189,6 +188,7 @@ public class GenericConversionService implements ConfigurableConversionService { return convert(source, TypeDescriptor.forObject(source), targetType); } + @Override public String toString() { return this.converters.toString(); } @@ -297,17 +297,15 @@ public class GenericConversionService implements ConfigurableConversionService { @SuppressWarnings("unchecked") private final class ConverterAdapter implements ConditionalGenericConverter { - private final ConvertiblePair typeInfo; - private final Converter converter; + private final ConvertiblePair typeInfo; - public ConverterAdapter(ConvertiblePair typeInfo, Converter converter) { + public ConverterAdapter(Converter converter, ConvertiblePair typeInfo) { this.converter = (Converter) converter; this.typeInfo = typeInfo; } - public Set getConvertibleTypes() { return Collections.singleton(this.typeInfo); } @@ -329,10 +327,9 @@ public class GenericConversionService implements ConfigurableConversionService { return this.converter.convert(source); } + @Override public String toString() { - return this.typeInfo.getSourceType().getName() + " -> " + - this.typeInfo.getTargetType().getName() + " : " + - this.converter.toString(); + return this.typeInfo + " : " + this.converter; } } @@ -343,17 +340,15 @@ public class GenericConversionService implements ConfigurableConversionService { @SuppressWarnings("unchecked") private final class ConverterFactoryAdapter implements ConditionalGenericConverter { - private final ConvertiblePair typeInfo; - private final ConverterFactory converterFactory; + private final ConvertiblePair typeInfo; - public ConverterFactoryAdapter(ConvertiblePair typeInfo, ConverterFactory converterFactory) { + public ConverterFactoryAdapter(ConverterFactory converterFactory, ConvertiblePair typeInfo) { this.converterFactory = (ConverterFactory) converterFactory; this.typeInfo = typeInfo; } - public Set getConvertibleTypes() { return Collections.singleton(this.typeInfo); } @@ -379,10 +374,9 @@ public class GenericConversionService implements ConfigurableConversionService { return this.converterFactory.getConverter(targetType.getObjectType()).convert(source); } + @Override public String toString() { - return this.typeInfo.getSourceType().getName() + " -> " + - this.typeInfo.getTargetType().getName() + " : " + - this.converterFactory.toString(); + return this.typeInfo + " : " + this.converterFactory; } } @@ -396,13 +390,11 @@ public class GenericConversionService implements ConfigurableConversionService { 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; @@ -411,18 +403,20 @@ public class GenericConversionService implements ConfigurableConversionService { return false; } ConverterCacheKey otherKey = (ConverterCacheKey) other; - return ObjectUtils.nullSafeEquals(this.sourceType, otherKey.sourceType) - && ObjectUtils.nullSafeEquals(this.targetType, otherKey.targetType); + return ObjectUtils.nullSafeEquals(this.sourceType, otherKey.sourceType) && + ObjectUtils.nullSafeEquals(this.targetType, otherKey.targetType); } + @Override public int hashCode() { - return ObjectUtils.nullSafeHashCode(this.sourceType) * 29 - + ObjectUtils.nullSafeHashCode(this.targetType); + return ObjectUtils.nullSafeHashCode(this.sourceType) * 29 + + ObjectUtils.nullSafeHashCode(this.targetType); } + @Override public String toString() { - return "ConverterCacheKey [sourceType = " + this.sourceType - + ", targetType = " + this.targetType + "]"; + return "ConverterCacheKey [sourceType = " + this.sourceType + + ", targetType = " + this.targetType + "]"; } } @@ -442,7 +436,7 @@ public class GenericConversionService implements ConfigurableConversionService { if (convertibleTypes == null) { Assert.state(converter instanceof ConditionalConverter, "Only conditional converters may return null convertible types"); - globalConverters.add(converter); + this.globalConverters.add(converter); } else { for (ConvertiblePair convertiblePair : convertibleTypes) { @@ -466,12 +460,12 @@ public class GenericConversionService implements ConfigurableConversionService { } /** - * 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. + * Find a {@link GenericConverter} given a source and target type. + *

This method will attempt to match all possible converters by working + * through the class and interface hierarchy of the types. * @param sourceType the source type * @param targetType the target type - * @return a {@link GenericConverter} or null + * @return a matching {@link GenericConverter}, or {@code null} if none found */ public GenericConverter find(TypeDescriptor sourceType, TypeDescriptor targetType) { // Search the full type hierarchy @@ -500,22 +494,19 @@ public class GenericConversionService implements ConfigurableConversionService { return converter; } } - // Check ConditionalGenericConverter that match all types for (GenericConverter globalConverter : this.globalConverters) { if (((ConditionalConverter)globalConverter).matches(sourceType, targetType)) { return globalConverter; } } - 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. + * @return an ordered list of all classes that the given type extends or implements */ private List> getClassHierarchy(Class type) { List> hierarchy = new ArrayList>(20); @@ -525,8 +516,7 @@ public class GenericConversionService implements ConfigurableConversionService { int i = 0; while (i < hierarchy.size()) { Class candidate = hierarchy.get(i); - candidate = (array ? candidate.getComponentType() - : ClassUtils.resolvePrimitiveIfNecessary(candidate)); + candidate = (array ? candidate.getComponentType() : ClassUtils.resolvePrimitiveIfNecessary(candidate)); Class superclass = candidate.getSuperclass(); if (candidate.getSuperclass() != null && superclass != Object.class) { addToClassHierarchy(i + 1, candidate.getSuperclass(), array, hierarchy, visited); @@ -554,11 +544,9 @@ public class GenericConversionService implements ConfigurableConversionService { @Override public String toString() { StringBuilder builder = new StringBuilder(); - builder.append("ConversionService converters = ").append("\n"); + builder.append("ConversionService converters =\n"); for (String converterString : getConverterStrings()) { - builder.append("\t"); - builder.append(converterString); - builder.append("\n"); + builder.append('\t').append(converterString).append('\n'); } return builder.toString(); } @@ -595,6 +583,7 @@ public class GenericConversionService implements ConfigurableConversionService { return null; } + @Override public String toString() { return StringUtils.collectionToCommaDelimitedString(this.converters); } @@ -612,7 +601,6 @@ public class GenericConversionService implements ConfigurableConversionService { this.name = name; } - public Set getConvertibleTypes() { 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 07aa882307a..8841a9e7055 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-2013 the original author or authors. + * Copyright 2002-2014 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. @@ -23,6 +23,7 @@ import java.lang.annotation.RetentionPolicy; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -682,7 +683,7 @@ public class GenericConversionServiceTests { } @Test - public void shouldNotSuportNullConvertibleTypesFromNonConditionalGenericConverter() { + public void shouldNotSupportNullConvertibleTypesFromNonConditionalGenericConverter() { GenericConversionService conversionService = new GenericConversionService(); GenericConverter converter = new GenericConverter() { @Override @@ -766,6 +767,41 @@ public class GenericConversionServiceTests { conversionService.convert(source, sourceType, targetType); } + @Test + public void multipleCollectionTypesFromSameSourceType() throws Exception { + conversionService.addConverter(new MyStringToStringCollectionConverter()); + conversionService.addConverter(new MyStringToIntegerCollectionConverter()); + assertEquals(Collections.singleton(4), // should be "testX" from MyStringToStringCollectionConverter, ideally + conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("stringCollection")))); + assertEquals(Collections.singleton(4), + conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("integerCollection")))); + assertEquals(Collections.singleton(4), + conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("rawCollection")))); + assertEquals(Collections.singleton(4), + conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("genericCollection")))); + assertEquals(Collections.singleton(4), + conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("rawCollection")))); + assertEquals(Collections.singleton(4), // should be "testX" from MyStringToStringCollectionConverter, ideally + conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("stringCollection")))); + } + + @Test + public void adaptedCollectionTypesFromSameSourceType() throws Exception { + conversionService.addConverter(new MyStringToStringCollectionConverter()); + assertEquals(Collections.singleton("testX"), + conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("stringCollection")))); + assertEquals(Collections.singleton("testX"), + conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("genericCollection")))); + assertEquals(Collections.singleton("testX"), + conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("rawCollection")))); + assertEquals(Collections.singleton("testX"), + conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("genericCollection")))); + assertEquals(Collections.singleton("testX"), + conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("stringCollection")))); + assertEquals(Collections.singleton("testX"), + conversionService.convert("test", TypeDescriptor.valueOf(String.class), new TypeDescriptor(getClass().getField("rawCollection")))); + } + @ExampleAnnotation public String annotatedString; @@ -872,4 +908,28 @@ public class GenericConversionServiceTests { } } + public static class MyStringToStringCollectionConverter implements Converter> { + + @Override + public Collection convert(String source) { + return Collections.singleton(source + "X"); + } + } + + public static class MyStringToIntegerCollectionConverter implements Converter> { + + @Override + public Collection convert(String source) { + return Collections.singleton(source.length()); + } + } + + public Collection stringCollection; + + public Collection integerCollection; + + public Collection rawCollection; + + public Collection genericCollection; + }