From a48e2d35398b587028c2341876a36ecdf37c1703 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Fri, 12 Jan 2024 11:09:29 +0000 Subject: [PATCH] Fix configuration property conversion for CharSequence inputs Closes gh-39051 --- ...opertiesCharSequenceToObjectConverter.java | 106 ++++++++++++++++ .../properties/ConversionServiceDeducer.java | 4 +- ...iesCharSequenceToObjectConverterTests.java | 116 ++++++++++++++++++ .../ConfigurationPropertiesTests.java | 38 +++++- .../ConversionServiceDeducerTests.java | 16 +++ .../convert/ConversionServiceArguments.java | 12 +- .../boot/convert/ConversionServiceTest.java | 4 +- 7 files changed, 286 insertions(+), 10 deletions(-) create mode 100644 spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesCharSequenceToObjectConverter.java create mode 100644 spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesCharSequenceToObjectConverterTests.java diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesCharSequenceToObjectConverter.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesCharSequenceToObjectConverter.java new file mode 100644 index 000000000000..c97e882102dd --- /dev/null +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesCharSequenceToObjectConverter.java @@ -0,0 +1,106 @@ +/* + * Copyright 2012-2024 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 + * + * https://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.boot.context.properties; + +import java.util.Collections; +import java.util.Set; + +import org.springframework.boot.convert.ApplicationConversionService; +import org.springframework.core.convert.ConversionService; +import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.convert.converter.ConditionalGenericConverter; + +/** + * Copy of package-private + * {@code org.springframework.boot.convert.CharSequenceToObjectConverter}, renamed for + * differentiation. + * + * @author Phillip Webb + * @author Andy Wilkinson + */ +final class ConfigurationPropertiesCharSequenceToObjectConverter implements ConditionalGenericConverter { + + private static final TypeDescriptor STRING = TypeDescriptor.valueOf(String.class); + + private static final TypeDescriptor BYTE_ARRAY = TypeDescriptor.valueOf(byte[].class); + + private static final Set TYPES; + + private final ThreadLocal disable = new ThreadLocal<>(); + + static { + TYPES = Collections.singleton(new ConvertiblePair(CharSequence.class, Object.class)); + } + + private final ConversionService conversionService; + + ConfigurationPropertiesCharSequenceToObjectConverter(ConversionService conversionService) { + this.conversionService = conversionService; + } + + @Override + public Set getConvertibleTypes() { + return TYPES; + } + + @Override + public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { + if (sourceType.getType() == String.class || this.disable.get() == Boolean.TRUE) { + return false; + } + this.disable.set(Boolean.TRUE); + try { + boolean canDirectlyConvertCharSequence = this.conversionService.canConvert(sourceType, targetType); + if (canDirectlyConvertCharSequence && !isStringConversionBetter(sourceType, targetType)) { + return false; + } + return this.conversionService.canConvert(STRING, targetType); + } + finally { + this.disable.remove(); + } + } + + /** + * Return if String based conversion is better based on the target type. This is + * required when ObjectTo... conversion produces incorrect results. + * @param sourceType the source type to test + * @param targetType the target type to test + * @return if string conversion is better + */ + private boolean isStringConversionBetter(TypeDescriptor sourceType, TypeDescriptor targetType) { + if (this.conversionService instanceof ApplicationConversionService applicationConversionService) { + if (applicationConversionService.isConvertViaObjectSourceType(sourceType, targetType)) { + // If an ObjectTo... converter is being used then there might be a + // better StringTo... version + return true; + } + } + if ((targetType.isArray() || targetType.isCollection()) && !targetType.equals(BYTE_ARRAY)) { + // StringToArrayConverter / StringToCollectionConverter are better than + // ObjectToArrayConverter / ObjectToCollectionConverter + return true; + } + return false; + } + + @Override + public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { + return this.conversionService.convert(source.toString(), STRING, targetType); + } + +} diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConversionServiceDeducer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConversionServiceDeducer.java index a80535000f6f..3f29066f03ce 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConversionServiceDeducer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConversionServiceDeducer.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 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. @@ -65,6 +65,8 @@ private List getConversionServices(ConfigurableApplicationCon if (!converterBeans.isEmpty()) { FormattingConversionService beansConverterService = new FormattingConversionService(); DefaultConversionService.addCollectionConverters(beansConverterService); + beansConverterService + .addConverter(new ConfigurationPropertiesCharSequenceToObjectConverter(beansConverterService)); converterBeans.addTo(beansConverterService); conversionServices.add(beansConverterService); } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesCharSequenceToObjectConverterTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesCharSequenceToObjectConverterTests.java new file mode 100644 index 000000000000..3a81830df698 --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesCharSequenceToObjectConverterTests.java @@ -0,0 +1,116 @@ +/* + * Copyright 2012-2024 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 + * + * https://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.boot.context.properties; + +import java.util.List; +import java.util.stream.Stream; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.provider.Arguments; + +import org.springframework.boot.convert.ApplicationConversionService; +import org.springframework.boot.convert.ConversionServiceArguments; +import org.springframework.boot.convert.ConversionServiceTest; +import org.springframework.core.convert.ConversionService; +import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.convert.converter.Converter; +import org.springframework.format.support.DefaultFormattingConversionService; +import org.springframework.format.support.FormattingConversionService; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link ConfigurationPropertiesCharSequenceToObjectConverter} + * + * @author Phillip Webb + * @author Andy Wilkinson + */ +class ConfigurationPropertiesCharSequenceToObjectConverterTests { + + @ConversionServiceTest + void convertWhenCanConvertViaToString(ConversionService conversionService) { + assertThat(conversionService.convert(new StringBuilder("1"), Integer.class)).isOne(); + } + + @ConversionServiceTest + void convertWhenCanConvertDirectlySkipsStringConversion(ConversionService conversionService) { + assertThat(conversionService.convert(new String("1"), Long.class)).isOne(); + if (!ConversionServiceArguments.isApplicationConversionService(conversionService)) { + assertThat(conversionService.convert(new StringBuilder("1"), Long.class)).isEqualTo(2); + } + } + + @Test + @SuppressWarnings("unchecked") + void convertWhenTargetIsList() { + ConversionService conversionService = new ApplicationConversionService(); + StringBuilder source = new StringBuilder("1,2,3"); + TypeDescriptor sourceType = TypeDescriptor.valueOf(StringBuilder.class); + TypeDescriptor targetType = TypeDescriptor.collection(List.class, TypeDescriptor.valueOf(String.class)); + List converted = (List) conversionService.convert(source, sourceType, targetType); + assertThat(converted).containsExactly("1", "2", "3"); + } + + @Test + @SuppressWarnings("unchecked") + void convertWhenTargetIsListAndNotUsingApplicationConversionService() { + FormattingConversionService conversionService = new DefaultFormattingConversionService(); + conversionService.addConverter(new ConfigurationPropertiesCharSequenceToObjectConverter(conversionService)); + StringBuilder source = new StringBuilder("1,2,3"); + TypeDescriptor sourceType = TypeDescriptor.valueOf(StringBuilder.class); + TypeDescriptor targetType = TypeDescriptor.collection(List.class, TypeDescriptor.valueOf(String.class)); + List converted = (List) conversionService.convert(source, sourceType, targetType); + assertThat(converted).containsExactly("1", "2", "3"); + } + + static Stream conversionServices() { + return ConversionServiceArguments.with((conversionService) -> { + conversionService.addConverter(new StringToIntegerConverter()); + conversionService.addConverter(new StringToLongConverter()); + conversionService.addConverter(new CharSequenceToLongConverter()); + conversionService.addConverter(new ConfigurationPropertiesCharSequenceToObjectConverter(conversionService)); + }); + } + + static class StringToIntegerConverter implements Converter { + + @Override + public Integer convert(String source) { + return Integer.valueOf(source); + } + + } + + static class StringToLongConverter implements Converter { + + @Override + public Long convert(String source) { + return Long.valueOf(source); + } + + } + + static class CharSequenceToLongConverter implements Converter { + + @Override + public Long convert(CharSequence source) { + return Long.valueOf(source.toString()) + 1; + } + + } + +} diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesTests.java index 15488c4fdb69..bebaa9ae3703 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 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,6 +83,7 @@ import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.core.env.MapPropertySource; import org.springframework.core.env.MutablePropertySources; +import org.springframework.core.env.PropertySource; import org.springframework.core.env.StandardEnvironment; import org.springframework.core.env.SystemEnvironmentPropertySource; import org.springframework.core.io.ClassPathResource; @@ -653,6 +654,41 @@ void loadShouldUseConverterBean() { assertThat(person.lastName).isEqualTo("Smith"); } + @Test + void loadShouldUseStringConverterBeanWhenValueIsCharSequence() { + this.context.register(PersonConverterConfiguration.class, PersonProperties.class); + PropertySource testProperties = new MapPropertySource("test", Map.of("test.person", new CharSequence() { + + private final String value = "John Smith"; + + @Override + public int length() { + return this.value.length(); + } + + @Override + public char charAt(int index) { + return this.value.charAt(index); + } + + @Override + public CharSequence subSequence(int start, int end) { + return this.value.subSequence(start, end); + } + + @Override + public String toString() { + return this.value; + } + + })); + this.context.getEnvironment().getPropertySources().addLast(testProperties); + this.context.refresh(); + Person person = this.context.getBean(PersonProperties.class).getPerson(); + assertThat(person.firstName).isEqualTo("John"); + assertThat(person.lastName).isEqualTo("Smith"); + } + @Test void loadWhenBeanFactoryConversionServiceAndConverterBeanCanUseBeanFactoryConverter() { DefaultConversionService conversionService = new DefaultConversionService(); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConversionServiceDeducerTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConversionServiceDeducerTests.java index eed7608024a4..0027cb1d7cfe 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConversionServiceDeducerTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConversionServiceDeducerTests.java @@ -78,6 +78,7 @@ void getConversionServiceWhenHasQualifiedConverterBeansContainsCustomizedFormatt assertThat(conversionServices).hasSize(2); assertThat(conversionServices.get(0)).isExactlyInstanceOf(FormattingConversionService.class); assertThat(conversionServices.get(0).canConvert(InputStream.class, OutputStream.class)).isTrue(); + assertThat(conversionServices.get(0).canConvert(CharSequence.class, InputStream.class)).isTrue(); assertThat(conversionServices.get(1)).isSameAs(ApplicationConversionService.getSharedInstance()); } @@ -105,6 +106,12 @@ TestConverter testConverter() { return new TestConverter(); } + @Bean + @ConfigurationPropertiesBinding + StringConverter stringConverter() { + return new StringConverter(); + } + } private static final class TestApplicationConversionService extends ApplicationConversionService { @@ -120,4 +127,13 @@ public OutputStream convert(InputStream source) { } + private static final class StringConverter implements Converter { + + @Override + public InputStream convert(String source) { + throw new UnsupportedOperationException(); + } + + } + } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/convert/ConversionServiceArguments.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/convert/ConversionServiceArguments.java index 90c29e18a019..5afe71d440d3 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/convert/ConversionServiceArguments.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/convert/ConversionServiceArguments.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2022 the original author or authors. + * Copyright 2012-2024 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. @@ -35,20 +35,20 @@ * @author Phillip Webb * @author Andy Wilkinson */ -final class ConversionServiceArguments { +public final class ConversionServiceArguments { private ConversionServiceArguments() { } - static Stream with(Formatter formatter) { + public static Stream with(Formatter formatter) { return with((conversionService) -> conversionService.addFormatter(formatter)); } - static Stream with(GenericConverter converter) { + public static Stream with(GenericConverter converter) { return with((conversionService) -> conversionService.addConverter(converter)); } - static Stream with(Consumer initializer) { + public static Stream with(Consumer initializer) { FormattingConversionService withoutDefaults = new FormattingConversionService(); initializer.accept(withoutDefaults); return Stream.of( @@ -57,7 +57,7 @@ static Stream with(Consumer in "Application conversion service"))); } - static boolean isApplicationConversionService(ConversionService conversionService) { + public static boolean isApplicationConversionService(ConversionService conversionService) { if (conversionService instanceof NamedConversionService namedConversionService) { return isApplicationConversionService(namedConversionService.delegate); } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/convert/ConversionServiceTest.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/convert/ConversionServiceTest.java index 37ba16740fd7..522a010813d8 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/convert/ConversionServiceTest.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/convert/ConversionServiceTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2024 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. @@ -39,6 +39,6 @@ @MethodSource("conversionServices") @Target(ElementType.METHOD) @Retention(RetentionPolicy.RUNTIME) -@interface ConversionServiceTest { +public @interface ConversionServiceTest { }