From b240c810a810c873e84d058614b5d0174591cff9 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 15 Oct 2019 11:13:07 +0100 Subject: [PATCH] Fix @ConfigurationProperties on @Bean methods without metadata caching 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 --- .../ConfigurationPropertiesBean.java | 30 +++++++++- .../ConfigurationPropertiesBeanTests.java | 58 ++++++++++++++++++- 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBean.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBean.java index ed1e85f2e28a..36052a491469 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBean.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBean.java @@ -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; @@ -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; /** @@ -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 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, diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanTests.java index b0f50b451da1..56f9b7c1f607 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanTests.java @@ -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; @@ -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) -> { @@ -218,7 +246,20 @@ void bindTypeForClassWhenConstructorBindingOnMultipleConstructorsThrowsException private void get(Class configuration, String beanName, ThrowingConsumer consumer) throws Throwable { - try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(configuration)) { + get(configuration, beanName, true, consumer); + } + + private void getWithoutBeanMetadataCaching(Class configuration, String beanName, + ThrowingConsumer consumer) throws Throwable { + get(configuration, beanName, false, consumer); + } + + private void get(Class configuration, String beanName, boolean cacheBeanMetadata, + ThrowingConsumer 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)); } @@ -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() }; + } + + } + }