From df1d1dbaa93aae6cde524163ee2e92307e0e1257 Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Thu, 11 Mar 2021 14:13:13 -0800 Subject: [PATCH] Fallback to BeanPropertyBindingResult for unbound property validation gh-17424 updated `ValidationBindHandler` so that it would only look at bound values when validating. This commit updates `ValidationBindHandler` to use Spring Framework's `BeanPropertyBindingResult`. This means that for fields that are not bound, JavaBean accessor methods can be used to get the value to validate. Fixes gh-25356 --- .../validation/ValidationBindHandler.java | 37 +++++++++---- .../ConfigurationPropertiesTests.java | 7 +++ .../ValidatorPropertiesWithDefaultValues.java | 52 +++++++++++++++++++ src/checkstyle/checkstyle-suppressions.xml | 1 + 4 files changed, 88 insertions(+), 9 deletions(-) create mode 100644 spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ValidatorPropertiesWithDefaultValues.java diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/validation/ValidationBindHandler.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/validation/ValidationBindHandler.java index e4196a1c31..adcdb8f13a 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/validation/ValidationBindHandler.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/validation/ValidationBindHandler.java @@ -22,6 +22,7 @@ import java.util.Map; import java.util.Set; import java.util.stream.Collectors; +import org.springframework.beans.NotReadablePropertyException; import org.springframework.boot.context.properties.bind.AbstractBindHandler; import org.springframework.boot.context.properties.bind.BindContext; import org.springframework.boot.context.properties.bind.BindHandler; @@ -33,6 +34,7 @@ import org.springframework.boot.context.properties.source.ConfigurationPropertyN import org.springframework.core.ResolvableType; import org.springframework.util.ObjectUtils; import org.springframework.validation.AbstractBindingResult; +import org.springframework.validation.BeanPropertyBindingResult; import org.springframework.validation.Validator; /** @@ -143,14 +145,14 @@ public class ValidationBindHandler extends AbstractBindHandler { /** * {@link AbstractBindingResult} implementation backed by the bound properties. */ - private class ValidationResult extends AbstractBindingResult { + private class ValidationResult extends BeanPropertyBindingResult { private final ConfigurationPropertyName name; - private Object target; + private final Object target; protected ValidationResult(ConfigurationPropertyName name, Object target) { - super(null); + super(target, null); this.name = name; this.target = target; } @@ -160,11 +162,6 @@ public class ValidationBindHandler extends AbstractBindHandler { return this.name.toString(); } - @Override - public Object getTarget() { - return this.target; - } - @Override public Class getFieldType(String field) { ResolvableType type = getBoundField(ValidationBindHandler.this.boundTypes, field); @@ -177,7 +174,29 @@ public class ValidationBindHandler extends AbstractBindHandler { @Override protected Object getActualFieldValue(String field) { - return getBoundField(ValidationBindHandler.this.boundResults, field); + Object boundField = getBoundField(ValidationBindHandler.this.boundResults, field); + if (boundField != null) { + return boundField; + } + try { + return super.getActualFieldValue(field); + } + catch (Exception ex) { + if (isPropertyNotReadable(ex)) { + return null; + } + throw ex; + } + } + + private boolean isPropertyNotReadable(Throwable ex) { + while (ex != null) { + if (ex instanceof NotReadablePropertyException) { + return true; + } + ex = ex.getCause(); + } + return false; } private T getBoundField(Map boundFields, String field) { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesTests.java index e9e1e635e1..7898f177ff 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ConfigurationPropertiesTests.java @@ -687,6 +687,13 @@ class ConfigurationPropertiesTests { }); } + @Test + void loadWhenConfigurationPropertiesWithValidDefaultValuesShouldNotFail() { + AnnotationConfigApplicationContext context = load(ValidatorPropertiesWithDefaultValues.class); + ValidatorPropertiesWithDefaultValues bean = context.getBean(ValidatorPropertiesWithDefaultValues.class); + assertThat(bean.getBar()).isEqualTo("a"); + } + @Test void loadWhenSetterThrowsValidationExceptionShouldFail() { assertThatExceptionOfType(BeanCreationException.class) diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ValidatorPropertiesWithDefaultValues.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ValidatorPropertiesWithDefaultValues.java new file mode 100644 index 0000000000..de25994db0 --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/ValidatorPropertiesWithDefaultValues.java @@ -0,0 +1,52 @@ +/* + * 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. + * 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.validation.Errors; +import org.springframework.validation.ValidationUtils; +import org.springframework.validation.Validator; + +/** + * Used for testing validation of properties that have default field values. + * + * @author Madhura Bhave + */ +@EnableConfigurationProperties +@ConfigurationProperties +class ValidatorPropertiesWithDefaultValues implements Validator { + + private String bar = "a"; + + @Override + public boolean supports(Class type) { + return type == ValidatorPropertiesWithDefaultValues.class; + } + + @Override + public void validate(Object target, Errors errors) { + ValidationUtils.rejectIfEmpty(errors, "bar", "foo.empty"); + } + + public String getBar() { + return this.bar; + } + + public void setBar(String bar) { + this.bar = bar; + } + +} diff --git a/src/checkstyle/checkstyle-suppressions.xml b/src/checkstyle/checkstyle-suppressions.xml index 694a83ca8f..8d2e7abac5 100644 --- a/src/checkstyle/checkstyle-suppressions.xml +++ b/src/checkstyle/checkstyle-suppressions.xml @@ -49,4 +49,5 @@ +