spring-cloud-commons icon indicating copy to clipboard operation
spring-cloud-commons copied to clipboard

Continue rebinding the ConfigurationProperties when any of them fails

Open JordiMartinezVicent opened this issue 2 years ago • 0 comments

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.

JordiMartinezVicent avatar Jul 07 '22 12:07 JordiMartinezVicent