From b838dc5422205170d7b1f547db06a10c5acd93f4 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Sun, 7 Jul 2013 00:08:18 -0700 Subject: [PATCH] Remove tangles from core + minor polish --- .../EndpointWebMvcAutoConfiguration.java | 2 +- .../ManagementServerProperties.java | 2 +- .../ServerPropertiesAutoConfiguration.java | 2 +- ...erverPropertiesAutoConfigurationTests.java | 2 +- .../bind/PropertiesConfigurationFactory.java | 153 +++++++++--------- .../{ => properties}/ServerProperties.java | 6 +- ...urationPropertiesBindingPostProcessor.java | 65 ++++---- .../ConfigurationPropertiesHolder.java | 37 +++++ ...ConfigurationPropertiesImportSelector.java | 38 +++-- .../ServerPropertiesTests.java | 3 +- 10 files changed, 173 insertions(+), 137 deletions(-) rename spring-zero-core/src/main/java/org/springframework/zero/context/embedded/{ => properties}/ServerProperties.java (90%) create mode 100644 spring-zero-core/src/main/java/org/springframework/zero/context/properties/ConfigurationPropertiesHolder.java rename spring-zero-core/src/test/java/org/springframework/zero/context/embedded/{ => properties}/ServerPropertiesTests.java (94%) diff --git a/spring-zero-actuator/src/main/java/org/springframework/zero/actuate/autoconfigure/EndpointWebMvcAutoConfiguration.java b/spring-zero-actuator/src/main/java/org/springframework/zero/actuate/autoconfigure/EndpointWebMvcAutoConfiguration.java index 10a6e439b1..2f7e2b91ee 100644 --- a/spring-zero-actuator/src/main/java/org/springframework/zero/actuate/autoconfigure/EndpointWebMvcAutoConfiguration.java +++ b/spring-zero-actuator/src/main/java/org/springframework/zero/actuate/autoconfigure/EndpointWebMvcAutoConfiguration.java @@ -43,7 +43,7 @@ import org.springframework.zero.context.annotation.EnableAutoConfiguration; import org.springframework.zero.context.condition.ConditionalOnClass; import org.springframework.zero.context.condition.ConditionalOnMissingBean; import org.springframework.zero.context.embedded.AnnotationConfigEmbeddedWebApplicationContext; -import org.springframework.zero.context.embedded.ServerProperties; +import org.springframework.zero.context.embedded.properties.ServerProperties; /** * {@link EnableAutoConfiguration Auto-configuration} to enable Spring MVC to handle diff --git a/spring-zero-actuator/src/main/java/org/springframework/zero/actuate/properties/ManagementServerProperties.java b/spring-zero-actuator/src/main/java/org/springframework/zero/actuate/properties/ManagementServerProperties.java index 8bc21dcc3a..cc0bd0e0f4 100644 --- a/spring-zero-actuator/src/main/java/org/springframework/zero/actuate/properties/ManagementServerProperties.java +++ b/spring-zero-actuator/src/main/java/org/springframework/zero/actuate/properties/ManagementServerProperties.java @@ -20,7 +20,7 @@ import java.net.InetAddress; import javax.validation.constraints.NotNull; -import org.springframework.zero.context.embedded.ServerProperties; +import org.springframework.zero.context.embedded.properties.ServerProperties; import org.springframework.zero.context.properties.ConfigurationProperties; /** diff --git a/spring-zero-autoconfigure/src/main/java/org/springframework/zero/autoconfigure/web/ServerPropertiesAutoConfiguration.java b/spring-zero-autoconfigure/src/main/java/org/springframework/zero/autoconfigure/web/ServerPropertiesAutoConfiguration.java index 1a0e7c3c42..50d62ad19d 100644 --- a/spring-zero-autoconfigure/src/main/java/org/springframework/zero/autoconfigure/web/ServerPropertiesAutoConfiguration.java +++ b/spring-zero-autoconfigure/src/main/java/org/springframework/zero/autoconfigure/web/ServerPropertiesAutoConfiguration.java @@ -27,7 +27,7 @@ import org.springframework.zero.context.annotation.EnableAutoConfiguration; import org.springframework.zero.context.condition.ConditionalOnMissingBean; import org.springframework.zero.context.embedded.ConfigurableEmbeddedServletContainerFactory; import org.springframework.zero.context.embedded.EmbeddedServletContainerCustomizer; -import org.springframework.zero.context.embedded.ServerProperties; +import org.springframework.zero.context.embedded.properties.ServerProperties; import org.springframework.zero.context.properties.EnableConfigurationProperties; /** diff --git a/spring-zero-autoconfigure/src/test/java/org/springframework/zero/autoconfigure/web/ServerPropertiesAutoConfigurationTests.java b/spring-zero-autoconfigure/src/test/java/org/springframework/zero/autoconfigure/web/ServerPropertiesAutoConfigurationTests.java index 387372a89d..d543131c2c 100644 --- a/spring-zero-autoconfigure/src/test/java/org/springframework/zero/autoconfigure/web/ServerPropertiesAutoConfigurationTests.java +++ b/spring-zero-autoconfigure/src/test/java/org/springframework/zero/autoconfigure/web/ServerPropertiesAutoConfigurationTests.java @@ -33,7 +33,7 @@ import org.springframework.zero.context.embedded.AnnotationConfigEmbeddedWebAppl import org.springframework.zero.context.embedded.ConfigurableEmbeddedServletContainerFactory; import org.springframework.zero.context.embedded.EmbeddedServletContainerCustomizerBeanPostProcessor; import org.springframework.zero.context.embedded.EmbeddedServletContainerFactory; -import org.springframework.zero.context.embedded.ServerProperties; +import org.springframework.zero.context.embedded.properties.ServerProperties; import org.springframework.zero.context.embedded.tomcat.TomcatEmbeddedServletContainerFactory; import static org.junit.Assert.assertEquals; diff --git a/spring-zero-core/src/main/java/org/springframework/zero/bind/PropertiesConfigurationFactory.java b/spring-zero-core/src/main/java/org/springframework/zero/bind/PropertiesConfigurationFactory.java index 3eef9aa712..d22a8d8c48 100644 --- a/spring-zero-core/src/main/java/org/springframework/zero/bind/PropertiesConfigurationFactory.java +++ b/spring-zero-core/src/main/java/org/springframework/zero/bind/PropertiesConfigurationFactory.java @@ -59,13 +59,13 @@ public class PropertiesConfigurationFactory implements FactoryBean, private PropertySources propertySources; - private T configuration; + private T target; private Validator validator; private MessageSource messageSource; - private boolean initialized = false; + private boolean hasBeenBound = false; private String targetName; @@ -77,7 +77,7 @@ public class PropertiesConfigurationFactory implements FactoryBean, */ public PropertiesConfigurationFactory(T target) { Assert.notNull(target); - this.configuration = target; + this.target = target; } /** @@ -88,7 +88,7 @@ public class PropertiesConfigurationFactory implements FactoryBean, @SuppressWarnings("unchecked") public PropertiesConfigurationFactory(Class type) { Assert.notNull(type); - this.configuration = (T) BeanUtils.instantiate(type); + this.target = (T) BeanUtils.instantiate(type); } /** @@ -165,99 +165,98 @@ public class PropertiesConfigurationFactory implements FactoryBean, @Override public void afterPropertiesSet() throws Exception { + bindPropertiesToTarget(); + } + + @Override + public Class getObjectType() { + if (this.target == null) { + return Object.class; + } + return this.target.getClass(); + } + @Override + public boolean isSingleton() { + return true; + } + + @Override + public T getObject() throws Exception { + if (!this.hasBeenBound) { + bindPropertiesToTarget(); + } + return this.target; + } + + public void bindPropertiesToTarget() throws BindException { Assert.state(this.properties != null || this.propertySources != null, "Properties or propertySources should not be null"); - try { - if (this.properties != null) { - logger.trace("Properties:\n" + this.properties); - } - else { - logger.trace("Property Sources: " + this.propertySources); - } - this.initialized = true; - - RelaxedDataBinder dataBinder; - if (this.targetName != null) { - dataBinder = new RelaxedDataBinder(this.configuration, this.targetName); - } - else { - dataBinder = new RelaxedDataBinder(this.configuration); - } - if (this.validator != null) { - dataBinder.setValidator(this.validator); - } - if (this.conversionService != null) { - dataBinder.setConversionService(this.conversionService); - } - dataBinder.setIgnoreInvalidFields(this.ignoreInvalidFields); - dataBinder.setIgnoreUnknownFields(this.ignoreUnknownFields); - customizeBinder(dataBinder); - PropertyValues pvs; - if (this.properties != null) { - pvs = new MutablePropertyValues(this.properties); - } - else { - pvs = new PropertySourcesPropertyValues(this.propertySources); - } - dataBinder.bind(pvs); - - if (this.validator != null) { - dataBinder.validate(); - BindingResult errors = dataBinder.getBindingResult(); - - if (errors.hasErrors()) { - logger.error("Properties configuration failed validation"); - for (ObjectError error : errors.getAllErrors()) { - logger.error(this.messageSource != null ? this.messageSource - .getMessage(error, Locale.getDefault()) - + " (" - + error - + ")" : error); - } - if (this.exceptionIfInvalid) { - BindException summary = new BindException(errors); - throw summary; - } + if (logger.isTraceEnabled()) { + if (this.properties != null) { + logger.trace("Properties:\n" + this.properties); + } + else { + logger.trace("Property Sources: " + this.propertySources); } } + this.hasBeenBound = true; + doBindPropertiesToTarget(); } catch (BindException e) { if (this.exceptionIfInvalid) { throw e; } - logger.error( - "Failed to load Properties validation bean. Your Properties may be invalid.", - e); + logger.error("Failed to load Properties validation bean. " + + "Your Properties may be invalid.", e); } } - /** - * @param dataBinder the data binder that will be used to bind and validate - */ - protected void customizeBinder(DataBinder dataBinder) { - } + private void doBindPropertiesToTarget() throws BindException { - @Override - public Class getObjectType() { - if (this.configuration == null) { - return Object.class; + RelaxedDataBinder dataBinder = (this.targetName != null ? new RelaxedDataBinder( + this.target, this.targetName) : new RelaxedDataBinder(this.target)); + if (this.validator != null) { + dataBinder.setValidator(this.validator); } - return this.configuration.getClass(); - } + if (this.conversionService != null) { + dataBinder.setConversionService(this.conversionService); + } + dataBinder.setIgnoreInvalidFields(this.ignoreInvalidFields); + dataBinder.setIgnoreUnknownFields(this.ignoreUnknownFields); + customizeBinder(dataBinder); - @Override - public boolean isSingleton() { - return true; + PropertyValues propertyValues = (this.properties != null ? new MutablePropertyValues( + this.properties) + : new PropertySourcesPropertyValues(this.propertySources)); + dataBinder.bind(propertyValues); + + if (this.validator != null) { + validate(dataBinder); + } } - @Override - public T getObject() throws Exception { - if (!this.initialized) { - afterPropertiesSet(); + private void validate(RelaxedDataBinder dataBinder) throws BindException { + dataBinder.validate(); + BindingResult errors = dataBinder.getBindingResult(); + if (errors.hasErrors()) { + logger.error("Properties configuration failed validation"); + for (ObjectError error : errors.getAllErrors()) { + logger.error(this.messageSource != null ? this.messageSource.getMessage( + error, Locale.getDefault()) + " (" + error + ")" : error); + } + if (this.exceptionIfInvalid) { + BindException summary = new BindException(errors); + throw summary; + } } - return this.configuration; + } + + /** + * @param dataBinder the data binder that will be used to bind and validate + */ + protected void customizeBinder(DataBinder dataBinder) { } } diff --git a/spring-zero-core/src/main/java/org/springframework/zero/context/embedded/ServerProperties.java b/spring-zero-core/src/main/java/org/springframework/zero/context/embedded/properties/ServerProperties.java similarity index 90% rename from spring-zero-core/src/main/java/org/springframework/zero/context/embedded/ServerProperties.java rename to spring-zero-core/src/main/java/org/springframework/zero/context/embedded/properties/ServerProperties.java index c7b1fd7a4f..4d65c39c4d 100644 --- a/spring-zero-core/src/main/java/org/springframework/zero/context/embedded/ServerProperties.java +++ b/spring-zero-core/src/main/java/org/springframework/zero/context/embedded/properties/ServerProperties.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package org.springframework.zero.context.embedded; +package org.springframework.zero.context.embedded.properties; import java.io.File; import java.net.InetAddress; @@ -24,6 +24,10 @@ import javax.validation.constraints.NotNull; import org.apache.catalina.valves.AccessLogValve; import org.apache.catalina.valves.RemoteIpValve; import org.springframework.util.StringUtils; +import org.springframework.zero.context.embedded.ConfigurableEmbeddedServletContainerFactory; +import org.springframework.zero.context.embedded.EmbeddedServletContainerCustomizer; +import org.springframework.zero.context.embedded.EmbeddedServletContainerCustomizerBeanPostProcessor; +import org.springframework.zero.context.embedded.EmbeddedServletContainerFactory; import org.springframework.zero.context.embedded.tomcat.TomcatEmbeddedServletContainerFactory; import org.springframework.zero.context.properties.ConfigurationProperties; diff --git a/spring-zero-core/src/main/java/org/springframework/zero/context/properties/ConfigurationPropertiesBindingPostProcessor.java b/spring-zero-core/src/main/java/org/springframework/zero/context/properties/ConfigurationPropertiesBindingPostProcessor.java index ac66c68fca..1ef09fbb79 100644 --- a/spring-zero-core/src/main/java/org/springframework/zero/context/properties/ConfigurationPropertiesBindingPostProcessor.java +++ b/spring-zero-core/src/main/java/org/springframework/zero/context/properties/ConfigurationPropertiesBindingPostProcessor.java @@ -27,9 +27,9 @@ import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.converter.Converter; import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.core.env.PropertySources; +import org.springframework.util.StringUtils; import org.springframework.validation.Validator; import org.springframework.zero.bind.PropertiesConfigurationFactory; -import org.springframework.zero.context.properties.EnableConfigurationPropertiesImportSelector.ConfigurationPropertiesHolder; /** * {@link BeanPostProcessor} to bind {@link PropertySources} to beans annotated with @@ -89,44 +89,41 @@ public class ConfigurationPropertiesBindingPostProcessor implements BeanPostProc throws BeansException { ConfigurationProperties annotation = AnnotationUtils.findAnnotation( bean.getClass(), ConfigurationProperties.class); - Object target = bean; if (annotation != null || bean instanceof ConfigurationPropertiesHolder) { - if (bean instanceof ConfigurationPropertiesHolder) { - target = ((ConfigurationPropertiesHolder) bean).getTarget(); - } - PropertiesConfigurationFactory factory = new PropertiesConfigurationFactory( - target); - factory.setPropertySources(this.propertySources); - factory.setValidator(this.validator); - // If no explicit conversion service is provided we add one so that (at least) - // comma-separated arrays of convertibles can be bound automatically - factory.setConversionService(this.conversionService == null ? getDefaultConversionService() - : this.conversionService); - 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 { - target = factory.getObject(); // throwaway - } - catch (BeansException e) { - throw e; - } - catch (Exception e) { - throw new BeanCreationException(beanName, "Could not bind", e); - } + postProcessAfterInitialization(bean, beanName, annotation); } return bean; } - /** - * @return - */ + private void postProcessAfterInitialization(Object bean, String beanName, + ConfigurationProperties annotation) { + Object target = (bean instanceof ConfigurationPropertiesHolder ? ((ConfigurationPropertiesHolder) bean) + .getTarget() : bean); + PropertiesConfigurationFactory factory = new PropertiesConfigurationFactory( + target); + factory.setPropertySources(this.propertySources); + factory.setValidator(this.validator); + // If no explicit conversion service is provided we add one so that (at least) + // comma-separated arrays of convertibles can be bound automatically + factory.setConversionService(this.conversionService == null ? getDefaultConversionService() + : this.conversionService); + if (annotation != null) { + factory.setIgnoreInvalidFields(annotation.ignoreInvalidFields()); + factory.setIgnoreUnknownFields(annotation.ignoreUnknownFields()); + String targetName = (StringUtils.hasLength(annotation.value()) ? annotation + .value() : annotation.name()); + if (StringUtils.hasLength(targetName)) { + factory.setTargetName(targetName); + } + } + try { + factory.bindPropertiesToTarget(); + } + catch (Exception ex) { + throw new BeanCreationException(beanName, "Could not bind properties", ex); + } + } + private ConversionService getDefaultConversionService() { if (!this.initialized && this.beanFactory instanceof ListableBeanFactory) { for (Converter converter : ((ListableBeanFactory) this.beanFactory) diff --git a/spring-zero-core/src/main/java/org/springframework/zero/context/properties/ConfigurationPropertiesHolder.java b/spring-zero-core/src/main/java/org/springframework/zero/context/properties/ConfigurationPropertiesHolder.java new file mode 100644 index 0000000000..8ee28ecfb0 --- /dev/null +++ b/spring-zero-core/src/main/java/org/springframework/zero/context/properties/ConfigurationPropertiesHolder.java @@ -0,0 +1,37 @@ +/* + * 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.zero.context.properties; + +/** + * Properties holder registered by {@link EnableConfigurationPropertiesImportSelector} to + * be picked up by {@link ConfigurationPropertiesBindingPostProcessor}. + * + * @author Dave Syer + */ +class ConfigurationPropertiesHolder { + + private Object target; + + public ConfigurationPropertiesHolder(Object target) { + this.target = target; + } + + public Object getTarget() { + return this.target; + } + +} \ No newline at end of file diff --git a/spring-zero-core/src/main/java/org/springframework/zero/context/properties/EnableConfigurationPropertiesImportSelector.java b/spring-zero-core/src/main/java/org/springframework/zero/context/properties/EnableConfigurationPropertiesImportSelector.java index 18be19e0c7..097f21fa95 100644 --- a/spring-zero-core/src/main/java/org/springframework/zero/context/properties/EnableConfigurationPropertiesImportSelector.java +++ b/spring-zero-core/src/main/java/org/springframework/zero/context/properties/EnableConfigurationPropertiesImportSelector.java @@ -19,6 +19,7 @@ package org.springframework.zero.context.properties; import java.util.ArrayList; import java.util.List; +import org.springframework.beans.factory.support.AbstractBeanDefinition; import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.context.annotation.ImportBeanDefinitionRegistrar; @@ -67,17 +68,7 @@ class EnableConfigurationPropertiesImportSelector implements ImportSelector { for (Class type : types) { 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()); - } + registerBeanDefinition(registry, type, name); } } } @@ -94,18 +85,25 @@ class EnableConfigurationPropertiesImportSelector implements ImportSelector { return result; } - } - - public static class ConfigurationPropertiesHolder { + private void registerBeanDefinition(BeanDefinitionRegistry registry, + Class type, String name) { + BeanDefinitionBuilder builder = BeanDefinitionBuilder + .genericBeanDefinition(type); + AbstractBeanDefinition beanDefinition = builder.getBeanDefinition(); + registry.registerBeanDefinition(name, beanDefinition); - private Object target; - - public ConfigurationPropertiesHolder(Object target) { - this.target = target; + ConfigurationProperties properties = AnnotationUtils.findAnnotation(type, + ConfigurationProperties.class); + if (properties == null) { + registerPropertiesHolder(registry, name); + } } - public Object getTarget() { - return this.target; + private void registerPropertiesHolder(BeanDefinitionRegistry registry, String name) { + BeanDefinitionBuilder builder = BeanDefinitionBuilder + .genericBeanDefinition(ConfigurationPropertiesHolder.class); + builder.addConstructorArgReference(name); + registry.registerBeanDefinition(name + ".HOLDER", builder.getBeanDefinition()); } } diff --git a/spring-zero-core/src/test/java/org/springframework/zero/context/embedded/ServerPropertiesTests.java b/spring-zero-core/src/test/java/org/springframework/zero/context/embedded/properties/ServerPropertiesTests.java similarity index 94% rename from spring-zero-core/src/test/java/org/springframework/zero/context/embedded/ServerPropertiesTests.java rename to spring-zero-core/src/test/java/org/springframework/zero/context/embedded/properties/ServerPropertiesTests.java index 1e975c9189..f73888b433 100644 --- a/spring-zero-core/src/test/java/org/springframework/zero/context/embedded/ServerPropertiesTests.java +++ b/spring-zero-core/src/test/java/org/springframework/zero/context/embedded/properties/ServerPropertiesTests.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package org.springframework.zero.context.embedded; +package org.springframework.zero.context.embedded.properties; import java.net.InetAddress; import java.util.Collections; @@ -24,6 +24,7 @@ import java.util.Map; import org.junit.Test; import org.springframework.beans.MutablePropertyValues; import org.springframework.zero.bind.RelaxedDataBinder; +import org.springframework.zero.context.embedded.properties.ServerProperties; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse;