Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow specifying beanname on @EnableConfigurationProperties #19390

Closed
ttddyy opened this issue Dec 17, 2019 · 5 comments
Closed

Allow specifying beanname on @EnableConfigurationProperties #19390

ttddyy opened this issue Dec 17, 2019 · 5 comments
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@ttddyy
Copy link
Contributor

ttddyy commented Dec 17, 2019

When @EnableConfigurationProperties is used, corresponding ConfigurationProperties class is registered as a bean with name <prefix>-<fqcn>.
This bean name is generated by ConfigurationPropertiesBeanRegistrar#getName.

This auto-generated name is very inconvenient when the registered bean needs to be referenced by other places such as SpEL.

Current workaround to specify bean name is to use @ConfigurationProperties with @Bean instead of @EnableConfigurationProperties.

@Bean 
public MyProperties myProperties() {
  return new MyProperties();
}

@ConfigurationProperties("my")
public class MyProperties {
}

If it is declared as a bean, ConfigurationPropertiesBindingPostProcessor performs binding and the configuration properties bean exists with the bean name via @Bean.(myProperties in this example)

I would like to have a capability in @EnableConfigurationProperties to specify bean name (or may be alias to <prefix>-<fqcn>) for binding ConfigurationProperties beans.

For example:

@EnableConfigurationProperties(
                 value = {FooProperties.class, BarProperties.class},
                 beanNames = {"fooProps", "barProps"})

If this sounds ok, then I'll proceed to create a PR.

Thanks,

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 17, 2019
@bclozel
Copy link
Member

bclozel commented Dec 17, 2019

I'm not sure we want to go there. Configuration properties classes are public API by nature, but they're much more likely to change than the rest (because they tend to follow closely changes in 3rd party libraries).
We're already considering updating our documentation to reflect that (see #19199), so I'm wondering if referencing properties beans by their name or using them in a SpEL expression is really advised.

@bclozel bclozel added the for: team-attention An issue we'd like other members of the team to review label Dec 17, 2019
@ttddyy
Copy link
Contributor Author

ttddyy commented Dec 17, 2019

Hi, @bclozel

Just for FYI, my usage is to reference to our own properties. (not 3rd party)

Something like this:

my:
  schedule:
    delay: 30s
@Scheduled(
      initialDelay = "#{@myProperties.getSchedule().getDelay().toMillis() + " +
         "T(java.util.concurrent.ThreadLocalRandom).current().nextInt(3*60*1000)}")

Here, getDelay() returns Duration then call toMillis() to get mili seconds.

@philwebb
Copy link
Member

We discussed this as a team today and we aren't convinced that using a second array in the @EnableConfigurationProperties annotation is a good idea. In this case we'd recommend using @Component on the bean or using an @Bean method as you're already doing. You could also look at @Import with a custom ImportBeanDefinitionRegistrar and use that to register your beans.

One other thing to consider is not trying to use SpEL for this type of configuration. It can be quite brittle as it doesn't fail until runtime. I believe that Spring offers programmatic support for scheduling beans via the SchedulingConfigurer interface which should allow you to move this logic into real code.

@philwebb philwebb added status: declined A suggestion or change that we don't feel we should currently apply and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Dec 18, 2019
@ttddyy
Copy link
Contributor Author

ttddyy commented Dec 18, 2019

ok, thanks @philwebb

I think it would be useful if there is some documentation about this as a guideline or best practice.
So, that I can point other developers about it.

Something like "Referencing @ConfigurationProperties as a bean in SpEL"

  • use @Component or @Bean to give simple beanname
  • or write a custom ImportBeanDefinitionRegistrar to register such beans
  • alternatively, find programmatic way of filling such information
  • etc.

@rehevkor5
Copy link
Contributor

Unfortunately, this tends to make people use cron with a property expression, since it can use the string value directly. eg. @Scheduled(cron = "${something.cron}"). But, it's more complicated to understand than fixedRateString.

Related: spring-projects/spring-framework#22013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

5 participants