From a3533cf9369e25f93284e7549fced1fb1c8f7496 Mon Sep 17 00:00:00 2001 From: Oleg Zhurakousky Date: Wed, 10 Apr 2019 17:59:31 +0200 Subject: [PATCH] GH-1681 Fixed configuration property validation Fixed validation of configuration properties to ensure that only @Validated are validated Resolves #1681 --- pom.xml | 2 +- .../stream/config/BindingHandlerAdvise.java | 15 ++-- .../config/BindingServiceConfiguration.java | 5 +- .../config/BindingHandlerAdviseTests.java | 85 +++++++++++++++++-- 4 files changed, 86 insertions(+), 21 deletions(-) diff --git a/pom.xml b/pom.xml index 1e70cd954..5a53180cf 100644 --- a/pom.xml +++ b/pom.xml @@ -28,7 +28,7 @@ Californium-SR5 3.0.3 2.1 - 2.1.0.RC1 + 2.1.0.BUILD-SNAPSHOT true true diff --git a/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/config/BindingHandlerAdvise.java b/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/config/BindingHandlerAdvise.java index d32db07f8..63737a49f 100644 --- a/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/config/BindingHandlerAdvise.java +++ b/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/config/BindingHandlerAdvise.java @@ -20,16 +20,14 @@ import java.util.LinkedHashMap; import java.util.Map; import org.springframework.boot.context.properties.ConfigurationPropertiesBindHandlerAdvisor; +import org.springframework.boot.context.properties.bind.AbstractBindHandler; import org.springframework.boot.context.properties.bind.BindContext; import org.springframework.boot.context.properties.bind.BindHandler; import org.springframework.boot.context.properties.bind.BindResult; import org.springframework.boot.context.properties.bind.Bindable; -import org.springframework.boot.context.properties.bind.validation.ValidationBindHandler; import org.springframework.boot.context.properties.source.ConfigurationPropertyName; import org.springframework.boot.context.properties.source.ConfigurationPropertyName.Form; -import org.springframework.lang.Nullable; import org.springframework.util.CollectionUtils; -import org.springframework.validation.Validator; /** * @author Oleg Zhurakousky @@ -40,25 +38,22 @@ public class BindingHandlerAdvise implements ConfigurationPropertiesBindHandlerA private final Map mappings; - private final Validator[] validator; BindingHandlerAdvise( - Map additionalMappings, - @Nullable Validator validator) { + Map additionalMappings) { this.mappings = new LinkedHashMap<>(); this.mappings.put(ConfigurationPropertyName.of("spring.cloud.stream.bindings"), ConfigurationPropertyName.of("spring.cloud.stream.default")); if (!CollectionUtils.isEmpty(additionalMappings)) { this.mappings.putAll(additionalMappings); } - this.validator = validator != null ? new Validator[] { validator } - : new Validator[] {}; } @Override public BindHandler apply(BindHandler bindHandler) { - System.out.println("Hello " + bindHandler); - BindHandler handler = new ValidationBindHandler(this.validator) { + + + BindHandler handler = new AbstractBindHandler(bindHandler) { @Override public Bindable onStart(ConfigurationPropertyName name, Bindable target, BindContext context) { diff --git a/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/config/BindingServiceConfiguration.java b/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/config/BindingServiceConfiguration.java index 5196c6fe9..3f8398a49 100644 --- a/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/config/BindingServiceConfiguration.java +++ b/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/config/BindingServiceConfiguration.java @@ -63,7 +63,6 @@ import org.springframework.messaging.core.DestinationResolver; import org.springframework.scheduling.TaskScheduler; import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; -import org.springframework.validation.Validator; /** * Configuration class that provides necessary beans for {@link MessageChannel} binding. @@ -163,7 +162,7 @@ public class BindingServiceConfiguration { @Bean public BindingHandlerAdvise BindingHandlerAdvise( - @Nullable MappingsProvider[] providers, @Nullable Validator validator) { + @Nullable MappingsProvider[] providers) { Map additionalMappings = new HashMap<>(); if (!ObjectUtils.isEmpty(providers)) { for (int i = 0; i < providers.length; i++) { @@ -171,7 +170,7 @@ public class BindingServiceConfiguration { additionalMappings.putAll(mappingsProvider.getDefaultMappings()); } } - return new BindingHandlerAdvise(additionalMappings, validator); + return new BindingHandlerAdvise(additionalMappings); } @Bean diff --git a/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/config/BindingHandlerAdviseTests.java b/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/config/BindingHandlerAdviseTests.java index 0557a1df7..1a8f08f36 100644 --- a/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/config/BindingHandlerAdviseTests.java +++ b/spring-cloud-stream/src/test/java/org/springframework/cloud/stream/config/BindingHandlerAdviseTests.java @@ -17,6 +17,7 @@ package org.springframework.cloud.stream.config; import javax.validation.constraints.Min; +import javax.validation.constraints.NotNull; import org.junit.Test; @@ -25,14 +26,18 @@ import org.springframework.boot.WebApplicationType; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.ConfigurationPropertiesBindException; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.stream.annotation.EnableBinding; import org.springframework.cloud.stream.binder.test.TestChannelBinderConfiguration; +import org.springframework.cloud.stream.messaging.Processor; import org.springframework.cloud.stream.messaging.Sink; +import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Import; import org.springframework.validation.annotation.Validated; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.fail; // see https://github.com/spring-cloud/spring-cloud-stream/issues/1573 for more details /** @@ -43,20 +48,86 @@ public class BindingHandlerAdviseTests { @Test(expected = BeanCreationException.class) public void testFailureWithWrongValue() { - new SpringApplicationBuilder(SampleConfiguration.class) - .web(WebApplicationType.NONE) - .run("--props.value=-1", "--spring.jmx.enabled=false"); + new SpringApplicationBuilder(SampleConfiguration.class).web(WebApplicationType.NONE).run("--props.value=-1", + "--spring.jmx.enabled=false"); } @Test public void testValidatedValueValue() { - ValidatedProps validatedProps = new SpringApplicationBuilder( - SampleConfiguration.class).web(WebApplicationType.NONE) - .run("--props.value=2", "--spring.jmx.enabled=false") - .getBean(ValidatedProps.class); + ValidatedProps validatedProps = new SpringApplicationBuilder(SampleConfiguration.class) + .web(WebApplicationType.NONE).run("--props.value=2", "--spring.jmx.enabled=false") + .getBean(ValidatedProps.class); assertThat(validatedProps.getValue()).isEqualTo(2); } + @Test + public void nonValidatedConfigProperties() { + new SpringApplicationBuilder(NonValidatedConfiguration.class).web(WebApplicationType.NONE) + .run("--spring.jmx.enabled=false"); + // simply should not fail + } + + @Test(expected = ConfigurationPropertiesBindException.class) + public void validatedConfigProperties() { + new SpringApplicationBuilder(ValidatedConfiguration.class).web(WebApplicationType.NONE) + .run("--spring.jmx.enabled=false"); + + fail(); + } + + @EnableBinding(Processor.class) + @Import(TestChannelBinderConfiguration.class) + @EnableAutoConfiguration + public static class NonValidatedConfiguration { + + @Bean + @ConfigurationProperties + public NonValidatedClass nonValidatedClass() { + return new NonValidatedClass(); + } + } + + public static class NonValidatedClass { + + @NotNull + private String id; + + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + } + + @EnableBinding(Processor.class) + @Import(TestChannelBinderConfiguration.class) + @EnableAutoConfiguration + public static class ValidatedConfiguration { + + @Bean + @ConfigurationProperties + public ValidatedClass nonValidatedClass() { + return new ValidatedClass(); + } + } + + @Validated + public static class ValidatedClass { + + @NotNull + private String id; + + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + } + } @EnableBinding(Sink.class)