Use converter beans in preference to ObjectToObjectConverter

Previously, with the converter beans in a conversion service that
appears after the bean factory's conversion service, they would not
be called for a conversion that could be handled by the
ObjectToObjectConverter in the bean factory's conversion service.

This commit creates a new FormattingConversionService that is empty
except for the converter beans and places it first in the list.
It's followed by the bean factory's conversion service. The shared
application conversion service is added to the end of the list to
pick up any conversions that the previous two services could not
handle. This should maintain backwards compatibility with the
previous arrangement where the converter beans were added to an
application conversion service that went after the bean factory's
conversion service.

Fixes gh-34631
This commit is contained in:
Andy Wilkinson 2023-07-06 20:17:10 +01:00
parent 6041fe9b7a
commit f4e05c91c7
3 changed files with 54 additions and 29 deletions

View File

@ -31,6 +31,7 @@ import org.springframework.core.convert.converter.Converter;
import org.springframework.core.convert.converter.GenericConverter; import org.springframework.core.convert.converter.GenericConverter;
import org.springframework.format.Formatter; import org.springframework.format.Formatter;
import org.springframework.format.FormatterRegistry; import org.springframework.format.FormatterRegistry;
import org.springframework.format.support.FormattingConversionService;
/** /**
* Utility to deduce the {@link ConversionService} to use for configuration properties * Utility to deduce the {@link ConversionService} to use for configuration properties
@ -59,14 +60,21 @@ class ConversionServiceDeducer {
private List<ConversionService> getConversionServices(ConfigurableApplicationContext applicationContext) { private List<ConversionService> getConversionServices(ConfigurableApplicationContext applicationContext) {
List<ConversionService> conversionServices = new ArrayList<>(); List<ConversionService> conversionServices = new ArrayList<>();
ConverterBeans converterBeans = new ConverterBeans(applicationContext);
if (!converterBeans.isEmpty()) {
FormattingConversionService beansConverterService = new FormattingConversionService();
converterBeans.addTo(beansConverterService);
conversionServices.add(beansConverterService);
}
if (applicationContext.getBeanFactory().getConversionService() != null) { if (applicationContext.getBeanFactory().getConversionService() != null) {
conversionServices.add(applicationContext.getBeanFactory().getConversionService()); conversionServices.add(applicationContext.getBeanFactory().getConversionService());
} }
ConverterBeans converterBeans = new ConverterBeans(applicationContext);
if (!converterBeans.isEmpty()) { if (!converterBeans.isEmpty()) {
ApplicationConversionService beansConverterService = new ApplicationConversionService(); // Converters beans used to be added to a custom ApplicationConversionService
converterBeans.addTo(beansConverterService); // after the BeanFactory's ConversionService. For backwards compatibility, we
conversionServices.add(beansConverterService); // add an ApplicationConversationService as a fallback in the same place in
// the list.
conversionServices.add(ApplicationConversionService.getSharedInstance());
} }
return conversionServices; return conversionServices;
} }

View File

@ -644,24 +644,36 @@ class ConfigurationPropertiesTests {
@Test @Test
void loadShouldUseConverterBean() { void loadShouldUseConverterBean() {
prepareConverterContext(ConverterConfiguration.class, PersonProperties.class); prepareConverterContext(PersonConverterConfiguration.class, PersonProperties.class);
Person person = this.context.getBean(PersonProperties.class).getPerson(); Person person = this.context.getBean(PersonProperties.class).getPerson();
assertThat(person.firstName).isEqualTo("John"); assertThat(person.firstName).isEqualTo("John");
assertThat(person.lastName).isEqualTo("Smith"); assertThat(person.lastName).isEqualTo("Smith");
} }
@Test @Test
void loadWhenBeanFactoryConversionServiceAndConverterBean() { void loadWhenBeanFactoryConversionServiceAndConverterBeanCanUseBeanFactoryConverter() {
DefaultConversionService conversionService = new DefaultConversionService(); DefaultConversionService conversionService = new DefaultConversionService();
conversionService.addConverter(new AlienConverter()); conversionService.addConverter(new AlienConverter());
this.context.getBeanFactory().setConversionService(conversionService); this.context.getBeanFactory().setConversionService(conversionService);
load(new Class<?>[] { ConverterConfiguration.class, PersonAndAlienProperties.class }, "test.person=John Smith", load(new Class<?>[] { PersonConverterConfiguration.class, PersonAndAlienProperties.class },
"test.alien=Alf Tanner"); "test.person=John Smith", "test.alien=Alf Tanner");
PersonAndAlienProperties properties = this.context.getBean(PersonAndAlienProperties.class); PersonAndAlienProperties properties = this.context.getBean(PersonAndAlienProperties.class);
assertThat(properties.getPerson().firstName).isEqualTo("John"); assertThat(properties.getPerson().firstName).isEqualTo("John");
assertThat(properties.getPerson().lastName).isEqualTo("Smith"); assertThat(properties.getPerson().lastName).isEqualTo("Smith");
assertThat(properties.getAlien().firstName).isEqualTo("Alf"); assertThat(properties.getAlien().name).isEqualTo("rennaT flA");
assertThat(properties.getAlien().lastName).isEqualTo("Tanner"); }
@Test
void loadWhenBeanFactoryConversionServiceAndConverterBeanCanUseConverterBean() {
DefaultConversionService conversionService = new DefaultConversionService();
conversionService.addConverter(new PersonConverter());
this.context.getBeanFactory().setConversionService(conversionService);
load(new Class<?>[] { AlienConverterConfiguration.class, PersonAndAlienProperties.class },
"test.person=John Smith", "test.alien=Alf Tanner");
PersonAndAlienProperties properties = this.context.getBean(PersonAndAlienProperties.class);
assertThat(properties.getPerson().firstName).isEqualTo("John");
assertThat(properties.getPerson().lastName).isEqualTo("Smith");
assertThat(properties.getAlien().name).isEqualTo("rennaT flA");
} }
@Test @Test
@ -1440,7 +1452,7 @@ class ConfigurationPropertiesTests {
} }
@Configuration(proxyBeanMethods = false) @Configuration(proxyBeanMethods = false)
static class ConverterConfiguration { static class PersonConverterConfiguration {
@Bean @Bean
@ConfigurationPropertiesBinding @ConfigurationPropertiesBinding
@ -1450,6 +1462,17 @@ class ConfigurationPropertiesTests {
} }
@Configuration(proxyBeanMethods = false)
static class AlienConverterConfiguration {
@Bean
@ConfigurationPropertiesBinding
Converter<String, Alien> alienConverter() {
return new AlienConverter();
}
}
@Configuration(proxyBeanMethods = false) @Configuration(proxyBeanMethods = false)
static class NonQualifiedConverterConfiguration { static class NonQualifiedConverterConfiguration {
@ -2398,8 +2421,7 @@ class ConfigurationPropertiesTests {
@Override @Override
public Alien convert(String source) { public Alien convert(String source) {
String[] content = StringUtils.split(source, " "); return new Alien(new StringBuilder(source).reverse().toString());
return new Alien(content[0], content[1]);
} }
} }
@ -2467,21 +2489,14 @@ class ConfigurationPropertiesTests {
static class Alien { static class Alien {
private final String firstName; private final String name;
private final String lastName; Alien(String name) {
this.name = name;
Alien(String firstName, String lastName) {
this.firstName = firstName;
this.lastName = lastName;
} }
String getFirstName() { String getName() {
return this.firstName; return this.name;
}
String getLastName() {
return this.lastName;
} }
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2012-2021 the original author or authors. * Copyright 2012-2023 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -30,6 +30,7 @@ import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Configuration;
import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.ConversionService;
import org.springframework.core.convert.converter.Converter; import org.springframework.core.convert.converter.Converter;
import org.springframework.format.support.FormattingConversionService;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
@ -69,14 +70,15 @@ class ConversionServiceDeducerTests {
} }
@Test @Test
void getConversionServiceWhenHasQualifiedConverterBeansContainsCustomizedApplicationService() { void getConversionServiceWhenHasQualifiedConverterBeansContainsCustomizedFormattingService() {
ApplicationContext applicationContext = new AnnotationConfigApplicationContext( ApplicationContext applicationContext = new AnnotationConfigApplicationContext(
CustomConverterConfiguration.class); CustomConverterConfiguration.class);
ConversionServiceDeducer deducer = new ConversionServiceDeducer(applicationContext); ConversionServiceDeducer deducer = new ConversionServiceDeducer(applicationContext);
List<ConversionService> conversionServices = deducer.getConversionServices(); List<ConversionService> conversionServices = deducer.getConversionServices();
assertThat(conversionServices).hasSize(1); assertThat(conversionServices).hasSize(2);
assertThat(conversionServices.get(0)).isNotSameAs(ApplicationConversionService.getSharedInstance()); assertThat(conversionServices.get(0)).isExactlyInstanceOf(FormattingConversionService.class);
assertThat(conversionServices.get(0).canConvert(InputStream.class, OutputStream.class)).isTrue(); assertThat(conversionServices.get(0).canConvert(InputStream.class, OutputStream.class)).isTrue();
assertThat(conversionServices.get(1)).isSameAs(ApplicationConversionService.getSharedInstance());
} }
@Configuration(proxyBeanMethods = false) @Configuration(proxyBeanMethods = false)