Limit ConfigurationPropertiesBean candidates

Update `ConfigurationPropertiesBean.getAll()` to limit the candidate
beans that will actually need a `getBean()` call. Prior to this commit
calling `getAll()` would trigger a get of every bean in the context,
some of which might fail.

Fixes gh-18714
pull/18727/head
Phillip Webb 5 years ago
parent 985220608a
commit 744dcd9426

@ -17,6 +17,7 @@
package org.springframework.boot.context.properties; package org.springframework.boot.context.properties;
import java.lang.annotation.Annotation; import java.lang.annotation.Annotation;
import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Constructor; import java.lang.reflect.Constructor;
import java.lang.reflect.Method; import java.lang.reflect.Method;
import java.util.Iterator; import java.util.Iterator;
@ -156,19 +157,35 @@ public final class ConfigurationPropertiesBean {
Iterator<String> beanNames = beanFactory.getBeanNamesIterator(); Iterator<String> beanNames = beanFactory.getBeanNamesIterator();
while (beanNames.hasNext()) { while (beanNames.hasNext()) {
String beanName = beanNames.next(); String beanName = beanNames.next();
try { if (isConfigurationPropertiesBean(beanFactory, beanName)) {
Object bean = beanFactory.getBean(beanName); try {
ConfigurationPropertiesBean propertiesBean = get(applicationContext, bean, beanName); Object bean = beanFactory.getBean(beanName);
if (propertiesBean != null) { ConfigurationPropertiesBean propertiesBean = get(applicationContext, bean, beanName);
propertiesBeans.put(beanName, propertiesBean); propertiesBeans.put(beanName, propertiesBean);
} }
} catch (Exception ex) {
catch (NoSuchBeanDefinitionException ex) { }
} }
} }
return propertiesBeans; return propertiesBeans;
} }
private static boolean isConfigurationPropertiesBean(ConfigurableListableBeanFactory beanFactory, String beanName) {
try {
if (beanFactory.getBeanDefinition(beanName).isAbstract()) {
return false;
}
if (beanFactory.findAnnotationOnBean(beanName, ConfigurationProperties.class) != null) {
return true;
}
Method factoryMethod = findFactoryMethod(beanFactory, beanName);
return findMergedAnnotation(factoryMethod, ConfigurationProperties.class).isPresent();
}
catch (NoSuchBeanDefinitionException ex) {
return false;
}
}
/** /**
* Return a {@link ConfigurationPropertiesBean @ConfigurationPropertiesBean} instance * Return a {@link ConfigurationPropertiesBean @ConfigurationPropertiesBean} instance
* for the given bean details or {@code null} if the bean is not a * for the given bean details or {@code null} if the bean is not a
@ -195,7 +212,10 @@ public final class ConfigurationPropertiesBean {
} }
private static Method findFactoryMethod(ConfigurableApplicationContext applicationContext, String beanName) { private static Method findFactoryMethod(ConfigurableApplicationContext applicationContext, String beanName) {
ConfigurableListableBeanFactory beanFactory = applicationContext.getBeanFactory(); return findFactoryMethod(applicationContext.getBeanFactory(), beanName);
}
private static Method findFactoryMethod(ConfigurableListableBeanFactory beanFactory, String beanName) {
if (beanFactory.containsBeanDefinition(beanName)) { if (beanFactory.containsBeanDefinition(beanName)) {
BeanDefinition beanDefinition = beanFactory.getMergedBeanDefinition(beanName); BeanDefinition beanDefinition = beanFactory.getMergedBeanDefinition(beanName);
if (beanDefinition instanceof RootBeanDefinition) { if (beanDefinition instanceof RootBeanDefinition) {
@ -260,10 +280,10 @@ public final class ConfigurationPropertiesBean {
Class<A> annotationType) { Class<A> annotationType) {
MergedAnnotation<A> annotation = MergedAnnotation.missing(); MergedAnnotation<A> annotation = MergedAnnotation.missing();
if (factory != null) { if (factory != null) {
annotation = MergedAnnotations.from(factory, SearchStrategy.TYPE_HIERARCHY).get(annotationType); annotation = findMergedAnnotation(factory, annotationType);
} }
if (!annotation.isPresent()) { if (!annotation.isPresent()) {
annotation = MergedAnnotations.from(type, SearchStrategy.TYPE_HIERARCHY).get(annotationType); annotation = findMergedAnnotation(type, annotationType);
} }
if (!annotation.isPresent() && AopUtils.isAopProxy(instance)) { if (!annotation.isPresent() && AopUtils.isAopProxy(instance)) {
annotation = MergedAnnotations.from(AopUtils.getTargetClass(instance), SearchStrategy.TYPE_HIERARCHY) annotation = MergedAnnotations.from(AopUtils.getTargetClass(instance), SearchStrategy.TYPE_HIERARCHY)
@ -272,6 +292,12 @@ public final class ConfigurationPropertiesBean {
return annotation.isPresent() ? annotation.synthesize() : null; return annotation.isPresent() ? annotation.synthesize() : null;
} }
private static <A extends Annotation> MergedAnnotation<A> findMergedAnnotation(AnnotatedElement element,
Class<A> annotationType) {
return (element != null) ? MergedAnnotations.from(element, SearchStrategy.TYPE_HIERARCHY).get(annotationType)
: MergedAnnotation.missing();
}
private static boolean isBindableConstructor(Constructor<?> constructor) { private static boolean isBindableConstructor(Constructor<?> constructor) {
Class<?> declaringClass = constructor.getDeclaringClass(); Class<?> declaringClass = constructor.getDeclaringClass();
Constructor<?> bindConstructor = BindMethod.findBindConstructor(declaringClass); Constructor<?> bindConstructor = BindMethod.findBindConstructor(declaringClass);

@ -29,6 +29,7 @@ import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Import; import org.springframework.context.annotation.Import;
import org.springframework.context.annotation.ImportSelector; import org.springframework.context.annotation.ImportSelector;
import org.springframework.context.annotation.Lazy;
import org.springframework.core.ResolvableType; import org.springframework.core.ResolvableType;
import org.springframework.core.type.AnnotationMetadata; import org.springframework.core.type.AnnotationMetadata;
import org.springframework.stereotype.Component; import org.springframework.stereotype.Component;
@ -72,6 +73,16 @@ class ConfigurationPropertiesBeanTests {
} }
} }
@Test
void getAllWhenHasBadBeanDoesNotFail() {
try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(
NonAnnotatedComponent.class, AnnotatedComponent.class, AnnotatedBeanConfiguration.class,
ValueObjectConfiguration.class, BadBeanConfiguration.class)) {
Map<String, ConfigurationPropertiesBean> all = ConfigurationPropertiesBean.getAll(context);
assertThat(all).isNotEmpty();
}
}
@Test @Test
void getWhenNotAnnotatedReturnsNull() throws Throwable { void getWhenNotAnnotatedReturnsNull() throws Throwable {
get(NonAnnotatedComponent.class, "nonAnnotatedComponent", get(NonAnnotatedComponent.class, "nonAnnotatedComponent",
@ -387,6 +398,25 @@ class ConfigurationPropertiesBeanTests {
} }
@Configuration(proxyBeanMethods = false)
static class BadBeanConfiguration {
@Bean
@Lazy
BadBean badBean() {
return new BadBean();
}
}
static class BadBean {
BadBean() {
throw new IllegalStateException();
}
}
@ConfigurationProperties @ConfigurationProperties
@ConstructorBinding @ConstructorBinding
static class ValueObject { static class ValueObject {

Loading…
Cancel
Save