From 29a81b8795def239b885dafd792ea2fc05897980 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Wed, 22 May 2013 15:21:07 +0100 Subject: [PATCH] [bs-130] Prevent double registration * A simple check to see if it is already registered fixed the original problem * Also removed the need to have @ConfigurationProperties on a bean class (unless you want to specify the target name etc.) [Fixes #50256421] [bs-130] When actuator app starts the @ConfigurationProperties are registered with the bean factory several times --- .../src/main/resources/logging.properties | 6 + .../ServiceBootstrapApplicationTests.java | 2 +- ...ConfigurationPropertiesImportSelector.java | 31 ++++- .../PropertySourcesBindingPostProcessor.java | 23 ++-- .../EnableConfigurationPropertiesTests.java | 106 ++++++++++++++++++ 5 files changed, 158 insertions(+), 10 deletions(-) create mode 100644 spring-bootstrap-samples/spring-bootstrap-actuator-sample/src/main/resources/logging.properties create mode 100644 spring-bootstrap/src/test/java/org/springframework/bootstrap/context/annotation/EnableConfigurationPropertiesTests.java diff --git a/spring-bootstrap-samples/spring-bootstrap-actuator-sample/src/main/resources/logging.properties b/spring-bootstrap-samples/spring-bootstrap-actuator-sample/src/main/resources/logging.properties new file mode 100644 index 0000000000..4e73ce1eb7 --- /dev/null +++ b/spring-bootstrap-samples/spring-bootstrap-actuator-sample/src/main/resources/logging.properties @@ -0,0 +1,6 @@ +handlers = java.util.logging.ConsoleHandler +.level = INFO + +java.util.logging.ConsoleHandler.level = FINE +sun.net.www.protocol.http.HttpURLConnection.level = ALL +org.springframework.bootstrap.level = ALL diff --git a/spring-bootstrap-samples/spring-bootstrap-actuator-sample/src/test/java/org/springframework/bootstrap/sample/service/ServiceBootstrapApplicationTests.java b/spring-bootstrap-samples/spring-bootstrap-actuator-sample/src/test/java/org/springframework/bootstrap/sample/service/ServiceBootstrapApplicationTests.java index 8908e70a88..e87798f7ef 100644 --- a/spring-bootstrap-samples/spring-bootstrap-actuator-sample/src/test/java/org/springframework/bootstrap/sample/service/ServiceBootstrapApplicationTests.java +++ b/spring-bootstrap-samples/spring-bootstrap-actuator-sample/src/test/java/org/springframework/bootstrap/sample/service/ServiceBootstrapApplicationTests.java @@ -50,7 +50,7 @@ public class ServiceBootstrapApplicationTests { .run(ServiceBootstrapApplication.class); } }); - context = future.get(10, TimeUnit.SECONDS); + context = future.get(30, TimeUnit.SECONDS); } @AfterClass diff --git a/spring-bootstrap/src/main/java/org/springframework/bootstrap/context/annotation/EnableConfigurationPropertiesImportSelector.java b/spring-bootstrap/src/main/java/org/springframework/bootstrap/context/annotation/EnableConfigurationPropertiesImportSelector.java index 8cc973c641..3742ea76e7 100644 --- a/spring-bootstrap/src/main/java/org/springframework/bootstrap/context/annotation/EnableConfigurationPropertiesImportSelector.java +++ b/spring-bootstrap/src/main/java/org/springframework/bootstrap/context/annotation/EnableConfigurationPropertiesImportSelector.java @@ -23,6 +23,7 @@ import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.context.annotation.ImportBeanDefinitionRegistrar; import org.springframework.context.annotation.ImportSelector; +import org.springframework.core.annotation.AnnotationUtils; import org.springframework.core.type.AnnotationMetadata; import org.springframework.util.MultiValueMap; @@ -64,8 +65,20 @@ public class EnableConfigurationPropertiesImportSelector implements ImportSelect EnableConfigurationProperties.class.getName(), false); List> types = collectClasses(attributes.get("value")); for (Class type : types) { - registry.registerBeanDefinition(type.getName(), BeanDefinitionBuilder - .genericBeanDefinition(type).getBeanDefinition()); + String name = type.getName(); + if (!registry.containsBeanDefinition(name)) { + registry.registerBeanDefinition(name, BeanDefinitionBuilder + .genericBeanDefinition(type).getBeanDefinition()); + ConfigurationProperties properties = AnnotationUtils.findAnnotation( + type, ConfigurationProperties.class); + if (properties == null) { + BeanDefinitionBuilder builder = BeanDefinitionBuilder + .genericBeanDefinition(ConfigurationPropertiesHolder.class); + builder.addConstructorArgReference(name); + registry.registerBeanDefinition(name + ".HOLDER", + builder.getBeanDefinition()); + } + } } } @@ -83,4 +96,18 @@ public class EnableConfigurationPropertiesImportSelector implements ImportSelect } + public static class ConfigurationPropertiesHolder { + + private Object target; + + public ConfigurationPropertiesHolder(Object target) { + this.target = target; + } + + public Object getTarget() { + return this.target; + } + + } + } diff --git a/spring-bootstrap/src/main/java/org/springframework/bootstrap/context/annotation/PropertySourcesBindingPostProcessor.java b/spring-bootstrap/src/main/java/org/springframework/bootstrap/context/annotation/PropertySourcesBindingPostProcessor.java index 0d958877f3..7cc681c0c0 100644 --- a/spring-bootstrap/src/main/java/org/springframework/bootstrap/context/annotation/PropertySourcesBindingPostProcessor.java +++ b/spring-bootstrap/src/main/java/org/springframework/bootstrap/context/annotation/PropertySourcesBindingPostProcessor.java @@ -20,6 +20,7 @@ import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.config.BeanPostProcessor; import org.springframework.bootstrap.bind.PropertiesConfigurationFactory; +import org.springframework.bootstrap.context.annotation.EnableConfigurationPropertiesImportSelector.ConfigurationPropertiesHolder; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.core.convert.ConversionService; import org.springframework.core.env.PropertySources; @@ -68,19 +69,27 @@ public class PropertySourcesBindingPostProcessor implements BeanPostProcessor { throws BeansException { ConfigurationProperties annotation = AnnotationUtils.findAnnotation( bean.getClass(), ConfigurationProperties.class); - if (annotation != null) { + Object target = bean; + if (annotation != null || bean instanceof ConfigurationPropertiesHolder) { + if (bean instanceof ConfigurationPropertiesHolder) { + target = ((ConfigurationPropertiesHolder) bean).getTarget(); + } PropertiesConfigurationFactory factory = new PropertiesConfigurationFactory( - bean); + target); factory.setPropertySources(this.propertySources); factory.setValidator(this.validator); factory.setConversionService(this.conversionService); - factory.setIgnoreInvalidFields(annotation.ignoreInvalidFields()); - factory.setIgnoreUnknownFields(annotation.ignoreUnknownFields()); - String targetName = "".equals(annotation.value()) ? ("".equals(annotation - .name()) ? null : annotation.name()) : annotation.value(); + String targetName = null; + if (annotation != null) { + factory.setIgnoreInvalidFields(annotation.ignoreInvalidFields()); + factory.setIgnoreUnknownFields(annotation.ignoreUnknownFields()); + targetName = "".equals(annotation.value()) ? ("" + .equals(annotation.name()) ? null : annotation.name()) + : annotation.value(); + } factory.setTargetName(targetName); try { - bean = factory.getObject(); + target = factory.getObject(); // throwaway } catch (BeansException e) { throw e; } catch (Exception e) { diff --git a/spring-bootstrap/src/test/java/org/springframework/bootstrap/context/annotation/EnableConfigurationPropertiesTests.java b/spring-bootstrap/src/test/java/org/springframework/bootstrap/context/annotation/EnableConfigurationPropertiesTests.java new file mode 100644 index 0000000000..151d8ff221 --- /dev/null +++ b/spring-bootstrap/src/test/java/org/springframework/bootstrap/context/annotation/EnableConfigurationPropertiesTests.java @@ -0,0 +1,106 @@ +/* + * Copyright 2012-2013 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 + * + * http://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.bootstrap.context.annotation; + +import java.util.Collections; + +import org.junit.Test; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.env.MapPropertySource; + +import static org.junit.Assert.assertEquals; + +/** + * @author Dave Syer + * + */ +public class EnableConfigurationPropertiesTests { + + private AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); + + @Test + public void testBasicPropertiesBinding() { + this.context.register(TestConfiguration.class); + this.context + .getEnvironment() + .getPropertySources() + .addFirst( + new MapPropertySource("test", Collections + . singletonMap("name", "foo"))); + this.context.refresh(); + assertEquals(1, this.context.getBeanNamesForType(TestProperties.class).length); + assertEquals("foo", this.context.getBean(TestProperties.class).getName()); + } + + @Test + public void testPropertiesBindingWithoutAnnotation() { + this.context.register(MoreConfiguration.class); + this.context + .getEnvironment() + .getPropertySources() + .addFirst( + new MapPropertySource("test", Collections + . singletonMap("name", "foo"))); + this.context.refresh(); + assertEquals(1, this.context.getBeanNamesForType(MoreProperties.class).length); + assertEquals("foo", this.context.getBean(MoreProperties.class).getName()); + } + + @Test + public void testBindingWithTwoBeans() { + this.context.register(MoreConfiguration.class, TestConfiguration.class); + this.context.refresh(); + assertEquals(1, this.context.getBeanNamesForType(TestProperties.class).length); + assertEquals(1, this.context.getBeanNamesForType(MoreProperties.class).length); + } + + @Configuration + @EnableConfigurationProperties(TestProperties.class) + protected static class TestConfiguration { + } + + @Configuration + @EnableConfigurationProperties(MoreProperties.class) + protected static class MoreConfiguration { + } + + @ConfigurationProperties + protected static class TestProperties { + private String name; + + public String getName() { + return this.name; + } + + public void setName(String name) { + this.name = name; + } + } + + protected static class MoreProperties { + private String name; + + public String getName() { + return this.name; + } + + public void setName(String name) { + this.name = name; + } + } + +}