From f13e3ad72b813d35d3187394dc53f98a35003c9a Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 26 Oct 2012 17:22:10 -0700 Subject: [PATCH] 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 0000000000..1196b26fc4 --- /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 fd52ca7d87..fc7ce7a1ab 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 8417f581bc..a7b1a68d03 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 fd4e5909dc..bece4d3266 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 84c93c8b99..3bbb1149f6 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 fa9383e2f2..205c66363e 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 9c87aed2dd..7d72d957b9 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(); + } + } + }