Skip to content

Commit

Permalink
Fix @ConfigurationProperties on @bean methods without metadata caching
Browse files Browse the repository at this point in the history
Due to a current limitation of Spring Framework, when bean metadata
caching is disabled, a merged bean definition may have a null
resolved factory method that would have been non-null if bean metadata
caching was enabled. Configuration property binding for @bean methods
annotated with @ConfigurationProperties relied upon the resolved
factory method being enabled to find the @ConfigurationProperties
annotation and trigger property binding. As a result, when bean
metadata caching is disabled on the bean factory, such
@ConfigurationProperties beans would not be bound.

This commit works around the limitation by adding a fallback that
performs a reflection-based search for the factory method when the
resolved factory method on the bean definition is null. This allows
the bean's factory method and any @ConfigurationProperties annotation
on it to be found, ensuring that propoerty binding is then performed.

Fixes gh-18440
  • Loading branch information
wilkinsona committed Oct 15, 2019
1 parent 3dac8e9 commit b240c81
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.concurrent.atomic.AtomicReference;

import org.springframework.aop.support.AopUtils;
import org.springframework.beans.BeanUtils;
Expand All @@ -41,6 +42,8 @@
import org.springframework.core.annotation.MergedAnnotations;
import org.springframework.core.annotation.MergedAnnotations.SearchStrategy;
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
import org.springframework.util.ReflectionUtils;
import org.springframework.validation.annotation.Validated;

/**
Expand Down Expand Up @@ -196,12 +199,37 @@ private static Method findFactoryMethod(ConfigurableApplicationContext applicati
if (beanFactory.containsBeanDefinition(beanName)) {
BeanDefinition beanDefinition = beanFactory.getMergedBeanDefinition(beanName);
if (beanDefinition instanceof RootBeanDefinition) {
return ((RootBeanDefinition) beanDefinition).getResolvedFactoryMethod();
Method resolvedFactoryMethod = ((RootBeanDefinition) beanDefinition).getResolvedFactoryMethod();
if (resolvedFactoryMethod != null) {
return resolvedFactoryMethod;
}
}
return findFactoryMethodUsingReflection(beanFactory, beanDefinition);
}
return null;
}

private static Method findFactoryMethodUsingReflection(ConfigurableListableBeanFactory beanFactory,
BeanDefinition beanDefinition) {
String factoryMethodName = beanDefinition.getFactoryMethodName();
String factoryBeanName = beanDefinition.getFactoryBeanName();
if (factoryMethodName == null || factoryBeanName == null) {
return null;
}
System.out.println("***** " + beanDefinition.getFactoryMethodName());
Class<?> factoryType = beanFactory.getType(factoryBeanName);
if (factoryType.getName().contains(ClassUtils.CGLIB_CLASS_SEPARATOR)) {
factoryType = factoryType.getSuperclass();
}
AtomicReference<Method> factoryMethod = new AtomicReference<>();
ReflectionUtils.doWithMethods(factoryType, (method) -> {
if (method.getName().equals(factoryMethodName)) {
factoryMethod.set(method);
}
});
return factoryMethod.get();
}

static ConfigurationPropertiesBean forValueObject(Class<?> beanClass, String beanName) {
ConfigurationPropertiesBean propertiesBean = create(beanName, null, beanClass, null);
Assert.state(propertiesBean != null && propertiesBean.getBindMethod() == BindMethod.VALUE_OBJECT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Import;
import org.springframework.context.annotation.ImportSelector;
import org.springframework.core.ResolvableType;
import org.springframework.core.type.AnnotationMetadata;
import org.springframework.stereotype.Component;
import org.springframework.validation.annotation.Validated;

Expand Down Expand Up @@ -99,6 +102,31 @@ void getWhenFactoryMethodIsAnnotatedReturnsBean() throws Throwable {
});
}

@Test
void getWhenImportedFactoryMethodIsAnnotatedAndMetadataCachingIsDisabledReturnsBean() throws Throwable {
getWithoutBeanMetadataCaching(NonAnnotatedBeanImportConfiguration.class, "nonAnnotatedBean",
(propertiesBean) -> {
assertThat(propertiesBean).isNotNull();
assertThat(propertiesBean.getName()).isEqualTo("nonAnnotatedBean");
assertThat(propertiesBean.getInstance()).isInstanceOf(NonAnnotatedBean.class);
assertThat(propertiesBean.getType()).isEqualTo(NonAnnotatedBean.class);
assertThat(propertiesBean.getAnnotation().prefix()).isEqualTo("prefix");
assertThat(propertiesBean.getBindMethod()).isEqualTo(BindMethod.JAVA_BEAN);
});
}

@Test
void getWhenImportedFactoryMethodIsAnnotatedReturnsBean() throws Throwable {
get(NonAnnotatedBeanImportConfiguration.class, "nonAnnotatedBean", (propertiesBean) -> {
assertThat(propertiesBean).isNotNull();
assertThat(propertiesBean.getName()).isEqualTo("nonAnnotatedBean");
assertThat(propertiesBean.getInstance()).isInstanceOf(NonAnnotatedBean.class);
assertThat(propertiesBean.getType()).isEqualTo(NonAnnotatedBean.class);
assertThat(propertiesBean.getAnnotation().prefix()).isEqualTo("prefix");
assertThat(propertiesBean.getBindMethod()).isEqualTo(BindMethod.JAVA_BEAN);
});
}

@Test
void getWhenHasFactoryMethodBindsUsingMethodReturnType() throws Throwable {
get(NonAnnotatedGenericBeanConfiguration.class, "nonAnnotatedGenericBean", (propertiesBean) -> {
Expand Down Expand Up @@ -218,7 +246,20 @@ void bindTypeForClassWhenConstructorBindingOnMultipleConstructorsThrowsException

private void get(Class<?> configuration, String beanName, ThrowingConsumer<ConfigurationPropertiesBean> consumer)
throws Throwable {
try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(configuration)) {
get(configuration, beanName, true, consumer);
}

private void getWithoutBeanMetadataCaching(Class<?> configuration, String beanName,
ThrowingConsumer<ConfigurationPropertiesBean> consumer) throws Throwable {
get(configuration, beanName, false, consumer);
}

private void get(Class<?> configuration, String beanName, boolean cacheBeanMetadata,
ThrowingConsumer<ConfigurationPropertiesBean> consumer) throws Throwable {
try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext()) {
context.getBeanFactory().setCacheBeanMetadata(cacheBeanMetadata);
context.register(configuration);
context.refresh();
Object bean = context.getBean(beanName);
consumer.accept(ConfigurationPropertiesBean.get(context, bean, beanName));
}
Expand Down Expand Up @@ -404,4 +445,19 @@ static class ConstructorBindingOnMultipleConstructors {

}

@Configuration(proxyBeanMethods = false)
@Import(NonAnnotatedBeanConfigurationImportSelector.class)
static class NonAnnotatedBeanImportConfiguration {

}

static class NonAnnotatedBeanConfigurationImportSelector implements ImportSelector {

@Override
public String[] selectImports(AnnotationMetadata importingClassMetadata) {
return new String[] { NonAnnotatedBeanConfiguration.class.getName() };
}

}

}

0 comments on commit b240c81

Please sign in to comment.