Do not attempt constructor binding for items with an existing value

Update `DefaultBindConstructorProvider` so that it no longer detects
constructors when the `Bindable` has an existing value. This update
allows us change `ConfigurationPropertiesBindingPostProcessor` and
related classes so that all instances created from `@Bean` methods
are treated as `JAVA_BEAN` bindings and will not be accidentally
re-created using constructor binding.

Fixes gh-33710
pull/33732/head
Phillip Webb 2 years ago
parent 2d372edcfc
commit 2e90b71c30

@ -1,5 +1,5 @@
/*
* Copyright 2012-2022 the original author or authors.
* Copyright 2012-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -71,18 +71,13 @@ public final class ConfigurationPropertiesBean {
private final BindMethod bindMethod;
private ConfigurationPropertiesBean(String name, Object instance, ConfigurationProperties annotation,
Bindable<?> bindable) {
this(name, instance, annotation, bindable, BindMethod.get(bindable));
}
private ConfigurationPropertiesBean(String name, Object instance, ConfigurationProperties annotation,
Bindable<?> bindTarget, BindMethod bindMethod) {
this.name = name;
this.instance = instance;
this.annotation = annotation;
this.bindTarget = bindTarget;
this.bindMethod = bindMethod;
this.bindMethod = (bindMethod != null) ? bindMethod : BindMethod.get(bindTarget);
}
/**
@ -168,7 +163,8 @@ public final class ConfigurationPropertiesBean {
if (isConfigurationPropertiesBean(beanFactory, beanName)) {
try {
Object bean = beanFactory.getBean(beanName);
ConfigurationPropertiesBean propertiesBean = get(applicationContext, bean, beanName);
BindMethod bindMethod = getBindMethod(beanFactory, beanName);
ConfigurationPropertiesBean propertiesBean = get(applicationContext, bean, beanName, bindMethod);
if (propertiesBean != null) {
propertiesBeans.put(beanName, propertiesBean);
}
@ -180,6 +176,16 @@ public final class ConfigurationPropertiesBean {
return propertiesBeans;
}
private static BindMethod getBindMethod(ConfigurableListableBeanFactory beanFactory, String beanName) {
try {
BeanDefinition beanDefinition = beanFactory.getBeanDefinition(beanName);
return (BindMethod) beanDefinition.getAttribute(BindMethod.class.getName());
}
catch (NoSuchBeanDefinitionException ex) {
return null;
}
}
private static boolean isConfigurationPropertiesBean(ConfigurableListableBeanFactory beanFactory, String beanName) {
try {
if (beanFactory.getBeanDefinition(beanName).isAbstract()) {
@ -210,8 +216,13 @@ public final class ConfigurationPropertiesBean {
* {@link ConfigurationProperties @ConfigurationProperties}
*/
public static ConfigurationPropertiesBean get(ApplicationContext applicationContext, Object bean, String beanName) {
return get(applicationContext, bean, beanName, null);
}
private static ConfigurationPropertiesBean get(ApplicationContext applicationContext, Object bean, String beanName,
BindMethod bindMethod) {
Method factoryMethod = findFactoryMethod(applicationContext, beanName);
return create(beanName, bean, bean.getClass(), factoryMethod);
return create(beanName, bean, bean.getClass(), factoryMethod, bindMethod);
}
private static Method findFactoryMethod(ApplicationContext applicationContext, String beanName) {
@ -260,13 +271,14 @@ public final class ConfigurationPropertiesBean {
}
static ConfigurationPropertiesBean forValueObject(Class<?> beanClass, String beanName) {
ConfigurationPropertiesBean propertiesBean = create(beanName, null, beanClass, null);
ConfigurationPropertiesBean propertiesBean = create(beanName, null, beanClass, null, null);
Assert.state(propertiesBean != null && propertiesBean.getBindMethod() == BindMethod.VALUE_OBJECT,
() -> "Bean '" + beanName + "' is not a @ConfigurationProperties value object");
return propertiesBean;
}
private static ConfigurationPropertiesBean create(String name, Object instance, Class<?> type, Method factory) {
private static ConfigurationPropertiesBean create(String name, Object instance, Class<?> type, Method factory,
BindMethod bindMethod) {
ConfigurationProperties annotation = findAnnotation(instance, type, factory, ConfigurationProperties.class);
if (annotation == null) {
return null;
@ -277,13 +289,11 @@ public final class ConfigurationPropertiesBean {
ResolvableType bindType = (factory != null) ? ResolvableType.forMethodReturnType(factory)
: ResolvableType.forClass(type);
Bindable<Object> bindable = Bindable.of(bindType).withAnnotations(annotations);
if (instance != null) {
bindMethod = (factory != null) ? BindMethod.JAVA_BEAN : bindMethod;
if (instance != null && bindMethod != BindMethod.VALUE_OBJECT) {
bindable = bindable.withExistingValue(instance);
}
if (factory != null) {
return new ConfigurationPropertiesBean(name, instance, annotation, bindable, BindMethod.JAVA_BEAN);
}
return new ConfigurationPropertiesBean(name, instance, annotation, bindable);
return new ConfigurationPropertiesBean(name, instance, annotation, bindable, bindMethod);
}
private static <A extends Annotation> A findAnnotation(Object instance, Class<?> type, Method factory,

@ -1,5 +1,5 @@
/*
* Copyright 2012-2021 the original author or authors.
* Copyright 2012-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -75,12 +75,19 @@ public class ConfigurationPropertiesBindingPostProcessor
@Override
public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException {
bind(ConfigurationPropertiesBean.get(this.applicationContext, bean, beanName));
if (!hasBoundValueObject(beanName)) {
bind(ConfigurationPropertiesBean.get(this.applicationContext, bean, beanName));
}
return bean;
}
private boolean hasBoundValueObject(String beanName) {
return this.registry.containsBeanDefinition(beanName) && BindMethod.VALUE_OBJECT
.equals(this.registry.getBeanDefinition(beanName).getAttribute(BindMethod.class.getName()));
}
private void bind(ConfigurationPropertiesBean bean) {
if (bean == null || hasBoundValueObject(bean.getName())) {
if (bean == null) {
return;
}
Assert.state(bean.getBindMethod() == BindMethod.JAVA_BEAN, "Cannot bind @ConfigurationProperties for bean '"
@ -93,11 +100,6 @@ public class ConfigurationPropertiesBindingPostProcessor
}
}
private boolean hasBoundValueObject(String beanName) {
return this.registry.containsBeanDefinition(beanName) && BindMethod.VALUE_OBJECT
.equals(this.registry.getBeanDefinition(beanName).getAttribute(BindMethod.class.getName()));
}
/**
* Register a {@link ConfigurationPropertiesBindingPostProcessor} bean if one is not
* already registered.

@ -1,5 +1,5 @@
/*
* Copyright 2012-2021 the original author or authors.
* Copyright 2012-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -90,6 +90,18 @@ public final class Bindable<T> {
return this.value;
}
/**
* Return if the bindable is for an existing non-null value. If this method return
* true then {@link #getValue()} will return a {@link Supplier} that never returns
* {@code null}.
* @return if the {@link Bindable} is for an existing value
* @since 3.0.2
* @see #withExistingValue(Object)
*/
public boolean hasExistingValue() {
return this.value instanceof ExistingValueSupplier;
}
/**
* Return any associated annotations that could affect binding.
* @return the associated annotations
@ -182,7 +194,7 @@ public final class Bindable<T> {
Assert.isTrue(
existingValue == null || this.type.isArray() || this.boxedType.resolve().isInstance(existingValue),
() -> "ExistingValue must be an instance of " + this.type);
Supplier<T> value = (existingValue != null) ? () -> existingValue : null;
Supplier<T> value = (existingValue != null) ? new ExistingValueSupplier(existingValue) : null;
return new Bindable<>(this.type, this.boxedType, value, this.annotations, this.bindRestrictions);
}
@ -307,4 +319,20 @@ public final class Bindable<T> {
}
private class ExistingValueSupplier implements Supplier<T> {
private final T value;
ExistingValueSupplier(T value) {
Assert.notNull(value, "Value must not be null");
this.value = value;
}
@Override
public T get() {
return this.value;
}
}
}

@ -1,5 +1,5 @@
/*
* Copyright 2012-2022 the original author or authors.
* Copyright 2012-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -39,12 +39,18 @@ class DefaultBindConstructorProvider implements BindConstructorProvider {
@Override
public Constructor<?> getBindConstructor(Bindable<?> bindable, boolean isNestedConstructorBinding) {
return getBindConstructor(bindable.getType().resolve(), isNestedConstructorBinding);
return getBindConstructor(bindable.getType().resolve(), bindable.hasExistingValue(),
isNestedConstructorBinding);
}
@Override
public Constructor<?> getBindConstructor(Class<?> type, boolean isNestedConstructorBinding) {
if (type == null) {
return getBindConstructor(type, false, isNestedConstructorBinding);
}
private Constructor<?> getBindConstructor(Class<?> type, boolean hasExistingValue,
boolean isNestedConstructorBinding) {
if (type == null || hasExistingValue) {
return null;
}
Constructors constructors = Constructors.getConstructors(type);

@ -1,5 +1,5 @@
/*
* Copyright 2012-2022 the original author or authors.
* Copyright 2012-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -1113,6 +1113,14 @@ class ConfigurationPropertiesTests {
assertThat(bean.getItem().getValue()).isEqualTo("foo");
}
@Test // gh-33710
void loadWhenConstructorUsedInBeanMethodAndNotAsConstructorBinding() {
load(ConstructorUsedInBeanMethodConfiguration.class, "test.two=bound-2");
ConstructorUsedInBeanMethod bean = this.context.getBean(ConstructorUsedInBeanMethod.class);
assertThat(bean.getOne()).isEqualTo("bean-method-1");
assertThat(bean.getTwo()).isEqualTo("bound-2");
}
private AnnotationConfigApplicationContext load(Class<?> configuration, String... inlinedProperties) {
return load(new Class<?>[] { configuration }, inlinedProperties);
}
@ -2847,4 +2855,45 @@ class ConfigurationPropertiesTests {
}
@Configuration(proxyBeanMethods = false)
@EnableConfigurationProperties
static class ConstructorUsedInBeanMethodConfiguration {
@Bean
@ConfigurationProperties("test")
ConstructorUsedInBeanMethod constructorUsedInBeanMethod() {
return new ConstructorUsedInBeanMethod("bean-method-1", "bean-method-2");
}
}
static class ConstructorUsedInBeanMethod {
private String one;
private String two;
ConstructorUsedInBeanMethod(String one, String two) {
this.one = one;
this.two = two;
}
String getOne() {
return this.one;
}
void setOne(String one) {
this.one = one;
}
String getTwo() {
return this.two;
}
void setTwo(String two) {
this.two = two;
}
}
}

@ -1,5 +1,5 @@
/*
* Copyright 2012-2022 the original author or authors.
* Copyright 2012-2023 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -110,6 +110,14 @@ class DefaultBindConstructorProviderTests {
}
}
@Test
void getBindConstructorWhenHasExistingValueReturnsNull() {
OneConstructorWithConstructorBinding existingValue = new OneConstructorWithConstructorBinding("name", 123);
Bindable<?> bindable = Bindable.of(OneConstructorWithConstructorBinding.class).withExistingValue(existingValue);
Constructor<?> bindConstructor = this.provider.getBindConstructor(bindable, false);
assertThat(bindConstructor).isNull();
}
static class OnlyDefaultConstructor {
}

Loading…
Cancel
Save