spring-cloud / spring-cloud/spring-cloud-commons
Continue rebinding the ConfigurationProperties when any of them fails
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 751
- Forks
- 744
- Avg merge
- 1d 14h
- Merged PRs (30d)
- 9
Description
Is your feature request related to a problem? Please describe.
Currently when ConfigurationPropertiesRebinder.rebind is executed and any ConfigurationProperties bean fails, the method throws an exception and stops rebinding the following ConfigurationProperties beans.
I do know that exists the property spring.cloud.refresh.never-refreshable which prevents rebind some specific beans. However we are building a common library which uses the rebind functionality and it is necessary for the users of the library to declare FactoryBeans to create their own objects (it is one of the cases which provokes that rebind fails). So we cannot know the classes beforehand.
Besides if you see the code of RefreshScopeHealthIndicator, it accepts a Map with the errors. However with the current implementation of ConfigurationPropertiesRebinder at most only one error will be reported.
Describe the solution you'd like
I would like that even if a single ConfigurationProperties bean fails rebinding, the following ConfigurationProperties still be rebinded and do not throw any exception.
Describe alternatives you've considered
I would suggest to not to throw any exception when there is an exception rebinding a ConfigurationProperties and log the error instead. Thus all the ConfigurationProperties that do not fail will be rebinded and the list of the errors would be complete. Something like:
@ManagedOperation
public boolean rebind(String name) {
if (!this.beans.getBeanNames().contains(name)) {
return false;
}
if (this.applicationContext != null) {
try {
Object bean = this.applicationContext.getBean(name);
if (AopUtils.isAopProxy(bean)) {
bean = ProxyUtils.getTargetObject(bean);
}
if (bean != null) {
// TODO: determine a more general approach to fix this.
// see https://github.com/spring-cloud/spring-cloud-commons/issues/571
if (getNeverRefreshable().contains(bean.getClass().getName())) {
return false; // ignore
}
this.applicationContext.getAutowireCapableBeanFactory().destroyBean(bean);
this.applicationContext.getAutowireCapableBeanFactory().initializeBean(bean, name);
return true;
}
}
catch (RuntimeException e) {
this.errors.put(name, e);
LOG.warn(String.format("Error rebinding the bean '%s'", e);
}
catch (Exception e) {
this.errors.put(name, e);
LOG.warn(String.format("Error rebinding the bean '%s'", e);
}
}
return false;
}
If you agree with the solution I could make a PR with this.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with ConfigurationPropertiesRebinder.rebind in spring-cloud-context/src/main/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinder.java and compare its error handling with RefreshScopeHealthIndicator in spring-cloud-context/src/main/java/org/springframework/cloud/health/RefreshScopeHealthIndicator.java. Done means later ConfigurationProperties continue rebinding after one failure, while each failure is retained in the errors map and logged.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java, spring, spring-boot
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100