From b4b71ec015b4efd78db93d15996bb98ce803000f Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Mon, 6 Sep 2021 10:25:00 +0200 Subject: [PATCH] Add bind method to ConfigurationProperties bean definition This commit reworks the configuration properties registrar to use RootBeanDefinition and a standard attribute rather than relying on a package private sub-class. This allows other components to inspect the metadata if necessary. Closes gh-27821 --- .../ConfigurationPropertiesBeanRegistrar.java | 24 ++++++-- ...urationPropertiesBindingPostProcessor.java | 8 +-- ...onPropertiesValueObjectBeanDefinition.java | 56 ------------------- ...igurationPropertiesBeanRegistrarTests.java | 20 +++++-- ...igurationPropertiesScanRegistrarTests.java | 28 +++++++--- ...ConfigurationPropertiesRegistrarTests.java | 27 ++++++--- ...nfigurationPropertiesBeanRegistrarTests.kt | 15 ++--- 7 files changed, 84 insertions(+), 94 deletions(-) delete mode 100644 spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesValueObjectBeanDefinition.java diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanRegistrar.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanRegistrar.java index 9967645d5b8..51e72231c16 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanRegistrar.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanRegistrar.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2021 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. @@ -21,7 +21,7 @@ import org.springframework.beans.factory.HierarchicalBeanFactory; import org.springframework.beans.factory.ListableBeanFactory; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.support.BeanDefinitionRegistry; -import org.springframework.beans.factory.support.GenericBeanDefinition; +import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.boot.context.properties.ConfigurationPropertiesBean.BindMethod; import org.springframework.core.annotation.MergedAnnotation; import org.springframework.core.annotation.MergedAnnotations; @@ -89,12 +89,24 @@ final class ConfigurationPropertiesBeanRegistrar { } private BeanDefinition createBeanDefinition(String beanName, Class type) { - if (BindMethod.forType(type) == BindMethod.VALUE_OBJECT) { - return new ConfigurationPropertiesValueObjectBeanDefinition(this.beanFactory, beanName, type); + BindMethod bindMethod = BindMethod.forType(type); + RootBeanDefinition definition = new RootBeanDefinition(type); + definition.setAttribute(BindMethod.class.getName(), bindMethod); + if (bindMethod == BindMethod.VALUE_OBJECT) { + definition.setInstanceSupplier(() -> createValueObject(beanName, type)); } - GenericBeanDefinition definition = new GenericBeanDefinition(); - definition.setBeanClass(type); return definition; } + private Object createValueObject(String beanName, Class beanType) { + ConfigurationPropertiesBean bean = ConfigurationPropertiesBean.forValueObject(beanType, beanName); + ConfigurationPropertiesBinder binder = ConfigurationPropertiesBinder.get(this.beanFactory); + try { + return binder.bindOrCreate(bean); + } + catch (Exception ex) { + throw new ConfigurationPropertiesBindException(bean, ex); + } + } + } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessor.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessor.java index 96bf6421926..a5f57c6086c 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessor.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2021 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. @@ -94,8 +94,8 @@ public class ConfigurationPropertiesBindingPostProcessor } private boolean hasBoundValueObject(String beanName) { - return this.registry.containsBeanDefinition(beanName) && this.registry - .getBeanDefinition(beanName) instanceof ConfigurationPropertiesValueObjectBeanDefinition; + return this.registry.containsBeanDefinition(beanName) && BindMethod.VALUE_OBJECT + .equals(this.registry.getBeanDefinition(beanName).getAttribute(BindMethod.class.getName())); } /** @@ -108,7 +108,7 @@ public class ConfigurationPropertiesBindingPostProcessor Assert.notNull(registry, "Registry must not be null"); if (!registry.containsBeanDefinition(BEAN_NAME)) { BeanDefinition definition = BeanDefinitionBuilder - .genericBeanDefinition(ConfigurationPropertiesBindingPostProcessor.class, + .rootBeanDefinition(ConfigurationPropertiesBindingPostProcessor.class, ConfigurationPropertiesBindingPostProcessor::new) .getBeanDefinition(); definition.setRole(BeanDefinition.ROLE_INFRASTRUCTURE); diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesValueObjectBeanDefinition.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesValueObjectBeanDefinition.java deleted file mode 100644 index a87b06c9de8..00000000000 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesValueObjectBeanDefinition.java +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright 2012-2020 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.boot.context.properties; - -import org.springframework.beans.factory.BeanFactory; -import org.springframework.beans.factory.config.BeanDefinition; -import org.springframework.beans.factory.support.GenericBeanDefinition; - -/** - * {@link BeanDefinition} that is used for registering - * {@link ConfigurationProperties @ConfigurationProperties} value object beans that are - * bound at creation time. - * - * @author Stephane Nicoll - * @author Madhura Bhave - * @author Phillip Webb - */ -final class ConfigurationPropertiesValueObjectBeanDefinition extends GenericBeanDefinition { - - private final BeanFactory beanFactory; - - private final String beanName; - - ConfigurationPropertiesValueObjectBeanDefinition(BeanFactory beanFactory, String beanName, Class beanClass) { - this.beanFactory = beanFactory; - this.beanName = beanName; - setBeanClass(beanClass); - setInstanceSupplier(this::createBean); - } - - private Object createBean() { - ConfigurationPropertiesBean bean = ConfigurationPropertiesBean.forValueObject(getBeanClass(), this.beanName); - ConfigurationPropertiesBinder binder = ConfigurationPropertiesBinder.get(this.beanFactory); - try { - return binder.bindOrCreate(bean); - } - catch (Exception ex) { - throw new ConfigurationPropertiesBindException(bean, ex); - } - } - -} diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanRegistrarTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanRegistrarTests.java index a00c2e9d691..72a44356bec 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanRegistrarTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesBeanRegistrarTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2021 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. @@ -16,12 +16,16 @@ package org.springframework.boot.context.properties; +import java.util.function.Consumer; + import org.junit.jupiter.api.Test; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.beans.factory.support.GenericBeanDefinition; +import org.springframework.beans.factory.support.RootBeanDefinition; +import org.springframework.boot.context.properties.ConfigurationPropertiesBean.BindMethod; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; @@ -70,15 +74,23 @@ class ConfigurationPropertiesBeanRegistrarTests { String beanName = "valuecp-" + ValueObjectConfigurationProperties.class.getName(); this.registrar.register(ValueObjectConfigurationProperties.class); BeanDefinition definition = this.registry.getBeanDefinition(beanName); - assertThat(definition).isInstanceOf(ConfigurationPropertiesValueObjectBeanDefinition.class); + assertThat(definition).satisfies(configurationPropertiesBeanDefinition(BindMethod.VALUE_OBJECT)); } @Test - void registerWhenNotValueObjectRegistersGenericBeanDefinition() { + void registerWhenNotValueObjectRegistersRootBeanDefinitionWithJavaBeanBindMethod() { String beanName = MultiConstructorBeanConfigurationProperties.class.getName(); this.registrar.register(MultiConstructorBeanConfigurationProperties.class); BeanDefinition definition = this.registry.getBeanDefinition(beanName); - assertThat(definition).isInstanceOf(GenericBeanDefinition.class); + assertThat(definition).satisfies(configurationPropertiesBeanDefinition(BindMethod.JAVA_BEAN)); + } + + private Consumer configurationPropertiesBeanDefinition(BindMethod bindMethod) { + return (definition) -> { + assertThat(definition).isExactlyInstanceOf(RootBeanDefinition.class); + assertThat(definition.hasAttribute(BindMethod.class.getName())).isTrue(); + assertThat(definition.getAttribute(BindMethod.class.getName())).isEqualTo(bindMethod); + }; } @ConfigurationProperties(prefix = "beancp") diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesScanRegistrarTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesScanRegistrarTests.java index d861c18b490..9cd33e4e452 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesScanRegistrarTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesScanRegistrarTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2021 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. @@ -17,12 +17,14 @@ package org.springframework.boot.context.properties; import java.io.IOException; +import java.util.function.Consumer; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.support.DefaultListableBeanFactory; -import org.springframework.beans.factory.support.GenericBeanDefinition; +import org.springframework.beans.factory.support.RootBeanDefinition; +import org.springframework.boot.context.properties.ConfigurationPropertiesBean.BindMethod; import org.springframework.boot.context.properties.scan.combined.c.CombinedConfiguration; import org.springframework.boot.context.properties.scan.combined.d.OtherCombinedConfiguration; import org.springframework.boot.context.properties.scan.valid.ConfigurationPropertiesScanConfiguration; @@ -54,9 +56,9 @@ class ConfigurationPropertiesScanRegistrarTests { "foo-org.springframework.boot.context.properties.scan.valid.ConfigurationPropertiesScanConfiguration$FooProperties"); BeanDefinition barDefinition = this.beanFactory.getBeanDefinition( "bar-org.springframework.boot.context.properties.scan.valid.ConfigurationPropertiesScanConfiguration$BarProperties"); - assertThat(bingDefinition).isExactlyInstanceOf(GenericBeanDefinition.class); - assertThat(fooDefinition).isExactlyInstanceOf(GenericBeanDefinition.class); - assertThat(barDefinition).isExactlyInstanceOf(ConfigurationPropertiesValueObjectBeanDefinition.class); + assertThat(bingDefinition).satisfies(configurationPropertiesBeanDefinition(BindMethod.JAVA_BEAN)); + assertThat(fooDefinition).satisfies(configurationPropertiesBeanDefinition(BindMethod.JAVA_BEAN)); + assertThat(barDefinition).satisfies(configurationPropertiesBeanDefinition(BindMethod.VALUE_OBJECT)); } @Test @@ -67,7 +69,7 @@ class ConfigurationPropertiesScanRegistrarTests { getAnnotationMetadata(ConfigurationPropertiesScanConfiguration.TestConfiguration.class), beanFactory); BeanDefinition fooDefinition = beanFactory.getBeanDefinition( "foo-org.springframework.boot.context.properties.scan.valid.ConfigurationPropertiesScanConfiguration$FooProperties"); - assertThat(fooDefinition).isExactlyInstanceOf(GenericBeanDefinition.class); + assertThat(fooDefinition).isExactlyInstanceOf(RootBeanDefinition.class); } @Test @@ -86,11 +88,11 @@ class ConfigurationPropertiesScanRegistrarTests { "b.first-org.springframework.boot.context.properties.scan.valid.b.BScanConfiguration$BFirstProperties"); BeanDefinition bSecondDefinition = beanFactory.getBeanDefinition( "b.second-org.springframework.boot.context.properties.scan.valid.b.BScanConfiguration$BSecondProperties"); - assertThat(aDefinition).isExactlyInstanceOf(GenericBeanDefinition.class); + assertThat(aDefinition).satisfies(configurationPropertiesBeanDefinition(BindMethod.JAVA_BEAN)); // Constructor injection - assertThat(bFirstDefinition).isExactlyInstanceOf(ConfigurationPropertiesValueObjectBeanDefinition.class); + assertThat(bFirstDefinition).satisfies(configurationPropertiesBeanDefinition(BindMethod.VALUE_OBJECT)); // Post-processing injection - assertThat(bSecondDefinition).isExactlyInstanceOf(GenericBeanDefinition.class); + assertThat(bSecondDefinition).satisfies(configurationPropertiesBeanDefinition(BindMethod.JAVA_BEAN)); } @Test @@ -110,6 +112,14 @@ class ConfigurationPropertiesScanRegistrarTests { assertThat(beanFactory.getBeanDefinitionCount()).isEqualTo(0); } + private Consumer configurationPropertiesBeanDefinition(BindMethod bindMethod) { + return (definition) -> { + assertThat(definition).isExactlyInstanceOf(RootBeanDefinition.class); + assertThat(definition.hasAttribute(BindMethod.class.getName())).isTrue(); + assertThat(definition.getAttribute(BindMethod.class.getName())).isEqualTo(bindMethod); + }; + } + private AnnotationMetadata getAnnotationMetadata(Class source) throws IOException { return new SimpleMetadataReaderFactory().getMetadataReader(source.getName()).getAnnotationMetadata(); } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/EnableConfigurationPropertiesRegistrarTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/EnableConfigurationPropertiesRegistrarTests.java index 4fde563278f..32208fec83b 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/EnableConfigurationPropertiesRegistrarTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/EnableConfigurationPropertiesRegistrarTests.java @@ -16,12 +16,15 @@ package org.springframework.boot.context.properties; +import java.util.function.Consumer; + import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.support.DefaultListableBeanFactory; -import org.springframework.beans.factory.support.GenericBeanDefinition; +import org.springframework.beans.factory.support.RootBeanDefinition; +import org.springframework.boot.context.properties.ConfigurationPropertiesBean.BindMethod; import org.springframework.core.type.AnnotationMetadata; import static org.assertj.core.api.Assertions.assertThat; @@ -51,27 +54,27 @@ class EnableConfigurationPropertiesRegistrarTests { } @Test - void typeWithDefaultConstructorShouldRegisterGenericBeanDefinition() { + void typeWithDefaultConstructorShouldRegisterRootBeanDefinition() { register(TestConfiguration.class); - BeanDefinition beanDefinition = this.beanFactory + BeanDefinition definition = this.beanFactory .getBeanDefinition("foo-" + getClass().getName() + "$FooProperties"); - assertThat(beanDefinition).isExactlyInstanceOf(GenericBeanDefinition.class); + assertThat(definition).satisfies(configurationPropertiesBeanDefinition(BindMethod.JAVA_BEAN)); } @Test void typeWithConstructorBindingShouldRegisterConfigurationPropertiesBeanDefinition() { register(TestConfiguration.class); - BeanDefinition beanDefinition = this.beanFactory + BeanDefinition definition = this.beanFactory .getBeanDefinition("bar-" + getClass().getName() + "$BarProperties"); - assertThat(beanDefinition).isExactlyInstanceOf(ConfigurationPropertiesValueObjectBeanDefinition.class); + assertThat(definition).satisfies(configurationPropertiesBeanDefinition(BindMethod.VALUE_OBJECT)); } @Test void typeWithMultipleConstructorsShouldRegisterGenericBeanDefinition() { register(TestConfiguration.class); - BeanDefinition beanDefinition = this.beanFactory + BeanDefinition definition = this.beanFactory .getBeanDefinition("bing-" + getClass().getName() + "$BingProperties"); - assertThat(beanDefinition).isExactlyInstanceOf(GenericBeanDefinition.class); + assertThat(definition).satisfies(configurationPropertiesBeanDefinition(BindMethod.JAVA_BEAN)); } @Test @@ -97,6 +100,14 @@ class EnableConfigurationPropertiesRegistrarTests { } } + private Consumer configurationPropertiesBeanDefinition(BindMethod bindMethod) { + return (definition) -> { + assertThat(definition).isExactlyInstanceOf(RootBeanDefinition.class); + assertThat(definition.hasAttribute(BindMethod.class.getName())).isTrue(); + assertThat(definition.getAttribute(BindMethod.class.getName())).isEqualTo(bindMethod); + }; + } + private void register(Class configuration) { AnnotationMetadata metadata = AnnotationMetadata.introspect(configuration); this.registrar.registerBeanDefinitions(metadata, this.beanFactory); diff --git a/spring-boot-project/spring-boot/src/test/kotlin/org/springframework/boot/context/properties/KotlinConfigurationPropertiesBeanRegistrarTests.kt b/spring-boot-project/spring-boot/src/test/kotlin/org/springframework/boot/context/properties/KotlinConfigurationPropertiesBeanRegistrarTests.kt index 984c9361bc2..92ff037c6c4 100644 --- a/spring-boot-project/spring-boot/src/test/kotlin/org/springframework/boot/context/properties/KotlinConfigurationPropertiesBeanRegistrarTests.kt +++ b/spring-boot-project/spring-boot/src/test/kotlin/org/springframework/boot/context/properties/KotlinConfigurationPropertiesBeanRegistrarTests.kt @@ -3,7 +3,7 @@ package org.springframework.boot.context.properties import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test import org.springframework.beans.factory.support.DefaultListableBeanFactory -import org.springframework.beans.factory.support.GenericBeanDefinition +import org.springframework.beans.factory.support.RootBeanDefinition /** * Tests for `ConfigurationPropertiesBeanRegistrar`. @@ -18,11 +18,11 @@ class KotlinConfigurationPropertiesBeanRegistrarTests { private val registrar = ConfigurationPropertiesBeanRegistrar(beanFactory) @Test - fun `type with default constructor should register generic bean definition`() { + fun `type with default constructor should register root bean definition`() { this.registrar.register(FooProperties::class.java) val beanDefinition = this.beanFactory.getBeanDefinition( "foo-org.springframework.boot.context.properties.KotlinConfigurationPropertiesBeanRegistrarTests\$FooProperties") - assertThat(beanDefinition).isExactlyInstanceOf(GenericBeanDefinition::class.java) + assertThat(beanDefinition).isExactlyInstanceOf(RootBeanDefinition::class.java) } @Test @@ -30,16 +30,17 @@ class KotlinConfigurationPropertiesBeanRegistrarTests { this.registrar.register(BarProperties::class.java) val beanDefinition = this.beanFactory.getBeanDefinition( "bar-org.springframework.boot.context.properties.KotlinConfigurationPropertiesBeanRegistrarTests\$BarProperties") - assertThat(beanDefinition).isExactlyInstanceOf( - ConfigurationPropertiesValueObjectBeanDefinition::class.java) + assertThat(beanDefinition.hasAttribute(ConfigurationPropertiesBean.BindMethod::class.java.name)).isTrue() + assertThat(beanDefinition.getAttribute(ConfigurationPropertiesBean.BindMethod::class.java.name)) + .isEqualTo(ConfigurationPropertiesBean.BindMethod.VALUE_OBJECT) } @Test - fun `type with no primary constructor should register generic bean definition`() { + fun `type with no primary constructor should register root bean definition`() { this.registrar.register(BingProperties::class.java) val beanDefinition = this.beanFactory.getBeanDefinition( "bing-org.springframework.boot.context.properties.KotlinConfigurationPropertiesBeanRegistrarTests\$BingProperties") - assertThat(beanDefinition).isExactlyInstanceOf(GenericBeanDefinition::class.java) + assertThat(beanDefinition).isExactlyInstanceOf(RootBeanDefinition::class.java) } @ConfigurationProperties(prefix = "foo")