Ensure that `@ConfigurationProperties` is mandatory

Previously it was possible to bind a bean to the root prefix by just
adding `@EnableConfigurationProperties` with the class of said bean.

 This use case is misleading and prevents any meta-data to be generated
 for that object since the annotation processor reacts on the presence of
 the `@ConfigurationProperties` annotation.

 If a class is included in the list of configuration properties bean to
 create via the `@EnableConfigurationProperties` annotation we now make
 sure that the `@configurationProperties` annotation is present on it.

 Closes gh-3460
This commit is contained in:
Stephane Nicoll 2015-10-28 20:12:13 +01:00
parent 6491eafc4a
commit 3740c817d3
8 changed files with 80 additions and 62 deletions

View File

@ -42,7 +42,6 @@ import org.springframework.boot.autoconfigure.template.TemplateAvailabilityProvi
import org.springframework.boot.context.embedded.ConfigurableEmbeddedServletContainer;
import org.springframework.boot.context.embedded.EmbeddedServletContainerCustomizer;
import org.springframework.boot.context.embedded.ErrorPage;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.ConditionContext;
import org.springframework.context.annotation.Conditional;
@ -74,7 +73,6 @@ import org.springframework.web.util.HtmlUtils;
// Ensure this loads before the main WebMvcAutoConfiguration so that the error View is
// available
@AutoConfigureBefore(WebMvcAutoConfiguration.class)
@EnableConfigurationProperties(ErrorProperties.class)
@Configuration
public class ErrorMvcAutoConfiguration {

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2014 the original author or authors.
* Copyright 2012-2015 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.
@ -32,6 +32,7 @@ import java.lang.annotation.Target;
*
* @author Dave Syer
* @see ConfigurationPropertiesBindingPostProcessor
* @see EnableConfigurationProperties
*/
@Target({ ElementType.TYPE, ElementType.METHOD })
@Retention(RetentionPolicy.RUNTIME)

View File

@ -269,7 +269,7 @@ public class ConfigurationPropertiesBindingPostProcessor implements BeanPostProc
throws BeansException {
ConfigurationProperties annotation = AnnotationUtils
.findAnnotation(bean.getClass(), ConfigurationProperties.class);
if (annotation != null || bean instanceof ConfigurationPropertiesHolder) {
if (annotation != null) {
postProcessBeforeInitialization(bean, beanName, annotation);
}
annotation = this.beans.findFactoryAnnotation(beanName,
@ -288,8 +288,7 @@ public class ConfigurationPropertiesBindingPostProcessor implements BeanPostProc
private void postProcessBeforeInitialization(Object bean, String beanName,
ConfigurationProperties annotation) {
Object target = (bean instanceof ConfigurationPropertiesHolder
? ((ConfigurationPropertiesHolder) bean).getTarget() : bean);
Object target = bean;
PropertiesConfigurationFactory<Object> factory = new PropertiesConfigurationFactory<Object>(
target);
if (annotation != null && annotation.locations().length != 0) {

View File

@ -1,37 +0,0 @@
/*
* 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.boot.context.properties;
/**
* Properties holder registered by {@link EnableConfigurationPropertiesImportSelector} to
* be picked up by {@link ConfigurationPropertiesBindingPostProcessor}.
*
* @author Dave Syer
*/
class ConfigurationPropertiesHolder {
private final Object target;
ConfigurationPropertiesHolder(Object target) {
this.target = target;
}
public Object getTarget() {
return this.target;
}
}

View File

@ -40,9 +40,10 @@ import org.springframework.context.annotation.Import;
public @interface EnableConfigurationProperties {
/**
* Convenient way to quickly register {@link ConfigurationProperties} beans with
* Spring. Standard Spring Beans will also be scanned regardless of this value.
* @return {@link ConfigurationProperties} beans to register
* Convenient way to quickly register {@link ConfigurationProperties} annotated
* beans with Spring. Standard Spring Beans will also be scanned regardless of
* this value.
* @return {@link ConfigurationProperties} annotated beans to register
*/
Class<?>[] value() default {};

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2014 the original author or authors.
* Copyright 2012-2015 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.
@ -26,6 +26,7 @@ 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.Assert;
import org.springframework.util.MultiValueMap;
import org.springframework.util.StringUtils;
@ -41,6 +42,7 @@ import org.springframework.util.StringUtils;
*
* @author Dave Syer
* @author Christian Dupuis
* @author Stephane Nicoll
*/
class EnableConfigurationPropertiesImportSelector implements ImportSelector {
@ -113,18 +115,8 @@ class EnableConfigurationPropertiesImportSelector implements ImportSelector {
ConfigurationProperties properties = AnnotationUtils.findAnnotation(type,
ConfigurationProperties.class);
if (properties == null) {
registerPropertiesHolder(registry, name);
}
}
private void registerPropertiesHolder(BeanDefinitionRegistry registry,
String name) {
BeanDefinitionBuilder builder = BeanDefinitionBuilder
.genericBeanDefinition(ConfigurationPropertiesHolder.class);
builder.addConstructorArgReference(name);
registry.registerBeanDefinition(name + ".HOLDER",
builder.getBeanDefinition());
Assert.notNull(properties, "No " + ConfigurationProperties.class.getSimpleName()
+ " annotation found on '" + type.getName() + "'.");
}
}

View File

@ -286,6 +286,17 @@ public class ConfigurationPropertiesBindingPostProcessorTests {
.getValue(), equalTo("test1"));
}
@Test
public void bindWithoutConfigurationPropertiesAnnotation() {
this.context = new AnnotationConfigApplicationContext();
EnvironmentTestUtils.addEnvironment(this.context, "name:foo");
this.context.register(ConfigurationPropertiesWithoutAnnotation.class);
this.thrown.expect(IllegalArgumentException.class);
this.thrown.expectMessage("No ConfigurationProperties annotation found");
this.context.refresh();
}
private void assertBindingFailure(int errorCount) {
try {
this.context.refresh();
@ -666,4 +677,24 @@ public class ConfigurationPropertiesBindingPostProcessorTests {
}
@Configuration
@EnableConfigurationProperties(PropertyWithoutConfigurationPropertiesAnnotation.class)
public static class ConfigurationPropertiesWithoutAnnotation {
}
public static class PropertyWithoutConfigurationPropertiesAnnotation {
private String name;
public String getName() {
return this.name;
}
public void setName(String name) {
this.name = name;
}
}
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2014 the original author or authors.
* Copyright 2012-2015 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.
@ -47,13 +47,14 @@ import static org.junit.Assert.assertNotNull;
* Tests for {@link EnableConfigurationProperties}.
*
* @author Dave Syer
* @author Stephane Nicoll
*/
public class EnableConfigurationPropertiesTests {
private final AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext();
@Rule
public ExpectedException expected = ExpectedException.none();
public ExpectedException thrown = ExpectedException.none();
@After
public void close() {
@ -158,7 +159,7 @@ public class EnableConfigurationPropertiesTests {
public void testExceptionOnValidation() {
this.context.register(ExceptionIfInvalidTestConfiguration.class);
EnvironmentTestUtils.addEnvironment(this.context, "name:foo");
this.expected.expectCause(Matchers.<Throwable>instanceOf(BindException.class));
this.thrown.expectCause(Matchers.<Throwable>instanceOf(BindException.class));
this.context.refresh();
}
@ -211,6 +212,16 @@ public class EnableConfigurationPropertiesTests {
@Test
public void testPropertiesBindingWithoutAnnotation() {
this.context.register(InvalidConfiguration.class);
EnvironmentTestUtils.addEnvironment(this.context, "name:foo");
this.thrown.expect(IllegalArgumentException.class);
this.thrown.expectMessage("No ConfigurationProperties annotation found");
this.context.refresh();
}
@Test
public void testPropertiesBindingWithoutAnnotationValue() {
this.context.register(MoreConfiguration.class);
EnvironmentTestUtils.addEnvironment(this.context, "name:foo");
this.context.refresh();
@ -548,6 +559,13 @@ public class EnableConfigurationPropertiesTests {
protected static class MoreConfiguration {
}
@Configuration
@EnableConfigurationProperties(InvalidConfiguration.class)
protected static class InvalidConfiguration {
}
@ConfigurationProperties
protected static class NestedProperties {
@ -662,6 +680,7 @@ public class EnableConfigurationPropertiesTests {
}
@ConfigurationProperties
protected static class MoreProperties {
private String name;
@ -673,6 +692,20 @@ public class EnableConfigurationPropertiesTests {
// No getter - you should be able to bind to a write-only bean
}
// No annotation
protected static class InvalidProperties {
private String name;
public String getName() {
return this.name;
}
public void setName(String name) {
this.name = name;
}
}
@ConfigurationProperties(locations = "${binding.location:classpath:name.yml}")
protected static class ResourceBindingProperties {