From 37a66349fe5100285d7a406d8d3c18324986dd29 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 27 Feb 2018 13:05:07 +0000 Subject: [PATCH] Fall back to application conversion service in BindConverter Previously, if a user declared a custom conversionService bean that was not an ApplicationConversionService instance, the binder lost the ability to convert a String to a Duration (along with any other conversions that are specific to ApplicationConversionService). This commit updates BindConverter so that, if the ConversionService with which it is created is not an ApplicationConversionService, it will use one as an additional service when performing conversion. Closes gh-12237 --- .../properties/bind/BindConverter.java | 82 ++++++++++++++++--- .../properties/bind/BindConverterTests.java | 17 +++- 2 files changed, 84 insertions(+), 15 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BindConverter.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BindConverter.java index 75512a09cd..a876135eb1 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BindConverter.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/BindConverter.java @@ -18,12 +18,16 @@ package org.springframework.boot.context.properties.bind; import java.beans.PropertyEditor; import java.lang.annotation.Annotation; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.Consumer; +import java.util.function.Function; +import java.util.function.Predicate; import org.springframework.beans.BeanUtils; import org.springframework.beans.PropertyEditorRegistry; @@ -42,6 +46,7 @@ import org.springframework.util.Assert; * and so a new instance is created for each top-level bind. * * @author Phillip Webb + * @author Andy Wilkinson */ class BindConverter { @@ -52,24 +57,21 @@ class BindConverter { EXCLUDED_EDITORS = Collections.unmodifiableSet(excluded); } - private final ConversionService typeConverterConversionService; - private final ConversionService conversionService; BindConverter(ConversionService conversionService, Consumer propertyEditorInitializer) { Assert.notNull(conversionService, "ConversionService must not be null"); - this.typeConverterConversionService = new TypeConverterConversionService( - propertyEditorInitializer); - this.conversionService = conversionService; + this.conversionService = new CompositeConversionService( + new TypeConverterConversionService(propertyEditorInitializer), + conversionService); } public boolean canConvert(Object value, ResolvableType type, Annotation... annotations) { TypeDescriptor sourceType = TypeDescriptor.forObject(value); TypeDescriptor targetType = new ResolvableTypeDescriptor(type, annotations); - return this.typeConverterConversionService.canConvert(sourceType, targetType) - || this.conversionService.canConvert(sourceType, targetType); + return this.conversionService.canConvert(sourceType, targetType); } public T convert(Object result, Bindable target) { @@ -83,10 +85,6 @@ class BindConverter { } TypeDescriptor sourceType = TypeDescriptor.forObject(value); TypeDescriptor targetType = new ResolvableTypeDescriptor(type, annotations); - if (this.typeConverterConversionService.canConvert(sourceType, targetType)) { - return (T) this.typeConverterConversionService.convert(value, sourceType, - targetType); - } return (T) this.conversionService.convert(value, sourceType, targetType); } @@ -180,4 +178,66 @@ class BindConverter { } + private static final class CompositeConversionService implements ConversionService { + + private final List delegates; + + private CompositeConversionService( + TypeConverterConversionService typeConverterConversionService, + ConversionService conversionService) { + List delegates = new ArrayList(); + delegates.add(typeConverterConversionService); + delegates.add(conversionService); + if (!(conversionService instanceof ApplicationConversionService)) { + delegates.add(ApplicationConversionService.getSharedInstance()); + } + this.delegates = delegates; + } + + @Override + public boolean canConvert(Class sourceType, Class targetType) { + return canConvert((delegate) -> delegate.canConvert(sourceType, targetType)); + } + + @Override + public boolean canConvert(TypeDescriptor sourceType, TypeDescriptor targetType) { + return canConvert((delegate) -> delegate.canConvert(sourceType, targetType)); + } + + private boolean canConvert(Predicate canConvert) { + for (ConversionService delegate : this.delegates) { + if (canConvert.test(delegate)) { + return true; + } + } + return false; + } + + @Override + public T convert(Object source, Class targetType) { + Class sourceType = source.getClass(); + return convert((delegate) -> delegate.canConvert(sourceType, targetType), + (delegate) -> delegate.convert(source, targetType)); + } + + @Override + public Object convert(Object source, TypeDescriptor sourceType, + TypeDescriptor targetType) { + return convert((delegate) -> delegate.canConvert(sourceType, targetType), + (delegate) -> delegate.convert(source, sourceType, targetType)); + } + + public T convert(Predicate canConvert, + Function convert) { + for (int i = 0; i < this.delegates.size() - 1; i++) { + ConversionService delegate = this.delegates.get(i); + if (canConvert.test(delegate)) { + return convert.apply(delegate); + } + } + return convert.apply(this.delegates.get(this.delegates.size() - 1)); + } + + } + } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BindConverterTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BindConverterTests.java index 00e004affc..cc3d1fa612 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BindConverterTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/BindConverterTests.java @@ -18,6 +18,7 @@ package org.springframework.boot.context.properties.bind; import java.beans.PropertyEditorSupport; import java.io.File; +import java.time.Duration; import java.util.List; import java.util.function.Consumer; @@ -45,6 +46,7 @@ import static org.mockito.Mockito.verify; * Tests for {@link BindConverter}. * * @author Phillip Webb + * @author Andy Wilkinson */ public class BindConverterTests { @@ -202,10 +204,17 @@ public class BindConverterTests { // classpath resource reference. See gh-12163 BindConverter bindConverter = new BindConverter(new GenericConversionService(), null); - assertThat(bindConverter.canConvert(".", ResolvableType.forClass(File.class))) - .isFalse(); - this.thrown.expect(ConverterNotFoundException.class); - bindConverter.convert(".", ResolvableType.forClass(File.class)); + File result = bindConverter.convert(".", ResolvableType.forClass(File.class)); + assertThat(result.getPath()).isEqualTo("."); + } + + @Test + public void fallsBackToApplicationConversionService() { + BindConverter bindConverter = new BindConverter(new GenericConversionService(), + null); + Duration result = bindConverter.convert("10s", + ResolvableType.forClass(Duration.class)); + assertThat(result.getSeconds()).isEqualTo(10); } private BindConverter getPropertyEditorOnlyBindConverter(