spring-boot icon indicating copy to clipboard operation
spring-boot copied to clipboard

Don't register ConfigurationPropertiesBinding beans as web converters

Open philwebb opened this issue 4 years ago • 2 comments

Whilst looking at #24891 I realized that any @ConfigurationPropertiesBinding beans also get registered as web converters.

philwebb avatar Apr 13 '21 17:04 philwebb

Hi @philwebb , I'm new to this project, I'd like to work on this.

I'm going through the code. Here's my analysis so far:

  • org.springframework.boot.autoconfigure.web.format.WebConversionService This is the registry for Web related Converters
  • org.springframework.boot.convert.ApplicationConversionService This is the registry for application converters/custom converters defined via @ConfigurationPropertiesBinding.
  • As per issue description, Are you saying that WebConversionService also contains all the converters from ApplicationConversionService and we don't want this? (Maybe because these are application specific and will be redundant/won't be helpful in web related operations?)
  • I couldn't find the code reference where application converters are registered with Web converters. Could you please help me with the reference, so that I can dig further.

Please feel free to correct me If I've missed something.

Thanks 😊

lower-case avatar May 10 '21 18:05 lower-case

Hi @lower-case,

Thanks for looking at this. I jotted the issue down whilst working on something else so it's a bit light on details and I'm not really sure how we should go about fixing it yet.

The @ConfigurationPropertiesBinding annotation is designed to allow you to add Converter beans that can be used when binding @ConfigurationProperties. You can see it in use in FlywayAutoConfiguration:

@Bean
@ConfigurationPropertiesBinding
public StringOrNumberToMigrationVersionConverter stringOrNumberMigrationVersionConverter() {
	return new StringOrNumberToMigrationVersionConverter();
}

Another feature of Spring Boot is that you can add Converter beans and they'll be auto-configured for use with Spring MVC. That code is here.

This code currently picks up all Converter beans, including those annotated with @ConfigurationPropertiesBinding. I suspect that most users don't want their @ConfigurationPropertiesBinding beans being used with Spring MVC.

We might be able to filter out @ConfigurationPropertiesBinding beans by looking at the bean definition. This would probably satisfy most users, but it might cause problems if a user wants the same converter to be used as a @ConfigurationPropertiesBinding converter and a Spring MVC converter. I'm not quite sure how we can do that. Perhaps another annotation.

I think we'll probably need to do some team design work on this one before we can accept any contributions. I'm not sure when we'll get to that, probably sometime after 2.5 is out.

philwebb avatar May 13 '21 18:05 philwebb