Commit 326a56da authored by Phillip Webb's avatar Phillip Webb

Support validation of bound map key entries

Update `ValidationBindHandler` so that pushed fields that reference
map keys can be used. This fixes a regression that was introduced in
commit 4483f417 when we switched to a `AbstractBindingResult` that no
longer required public getters/setters.

Closes gh-20350
parent 018cc1c8
...@@ -35,14 +35,29 @@ public abstract class DataObjectPropertyName { ...@@ -35,14 +35,29 @@ public abstract class DataObjectPropertyName {
* @return the dashed from * @return the dashed from
*/ */
public static String toDashedForm(String name) { public static String toDashedForm(String name) {
StringBuilder result = new StringBuilder(); StringBuilder result = new StringBuilder(name.length());
String replaced = name.replace('_', '-'); boolean inIndex = false;
for (int i = 0; i < replaced.length(); i++) { for (int i = 0; i < name.length(); i++) {
char ch = replaced.charAt(i); char ch = name.charAt(i);
if (Character.isUpperCase(ch) && result.length() > 0 && result.charAt(result.length() - 1) != '-') { if (inIndex) {
result.append('-'); result.append(ch);
if (ch == ']') {
inIndex = false;
}
}
else {
if (ch == '[') {
inIndex = true;
result.append(ch);
}
else {
ch = (ch != '_') ? ch : '-';
if (Character.isUpperCase(ch) && result.length() > 0 && result.charAt(result.length() - 1) != '-') {
result.append('-');
}
result.append(Character.toLowerCase(ch));
}
} }
result.append(Character.toLowerCase(ch));
} }
return result.toString(); return result.toString();
} }
......
...@@ -29,7 +29,9 @@ import org.springframework.boot.context.properties.bind.Bindable; ...@@ -29,7 +29,9 @@ import org.springframework.boot.context.properties.bind.Bindable;
import org.springframework.boot.context.properties.bind.DataObjectPropertyName; import org.springframework.boot.context.properties.bind.DataObjectPropertyName;
import org.springframework.boot.context.properties.source.ConfigurationProperty; import org.springframework.boot.context.properties.source.ConfigurationProperty;
import org.springframework.boot.context.properties.source.ConfigurationPropertyName; import org.springframework.boot.context.properties.source.ConfigurationPropertyName;
import org.springframework.boot.context.properties.source.ConfigurationPropertyName.Form;
import org.springframework.core.ResolvableType; import org.springframework.core.ResolvableType;
import org.springframework.util.ObjectUtils;
import org.springframework.validation.AbstractBindingResult; import org.springframework.validation.AbstractBindingResult;
import org.springframework.validation.Validator; import org.springframework.validation.Validator;
...@@ -165,28 +167,53 @@ public class ValidationBindHandler extends AbstractBindHandler { ...@@ -165,28 +167,53 @@ public class ValidationBindHandler extends AbstractBindHandler {
@Override @Override
public Class<?> getFieldType(String field) { public Class<?> getFieldType(String field) {
try { ResolvableType type = getBoundField(ValidationBindHandler.this.boundTypes, field);
ResolvableType type = ValidationBindHandler.this.boundTypes.get(getName(field)); Class<?> resolved = (type != null) ? type.resolve() : null;
Class<?> resolved = (type != null) ? type.resolve() : null; if (resolved != null) {
if (resolved != null) { return resolved;
return resolved;
}
}
catch (Exception ex) {
} }
return super.getFieldType(field); return super.getFieldType(field);
} }
@Override @Override
protected Object getActualFieldValue(String field) { protected Object getActualFieldValue(String field) {
return getBoundField(ValidationBindHandler.this.boundResults, field);
}
private <T> T getBoundField(Map<ConfigurationPropertyName, T> boundFields, String field) {
try { try {
return ValidationBindHandler.this.boundResults.get(getName(field)); ConfigurationPropertyName name = getName(field);
T bound = boundFields.get(name);
if (bound != null) {
return bound;
}
if (name.hasIndexedElement()) {
for (Map.Entry<ConfigurationPropertyName, T> entry : boundFields.entrySet()) {
if (isFieldNameMatch(entry.getKey(), name)) {
return entry.getValue();
}
}
}
} }
catch (Exception ex) { catch (Exception ex) {
} }
return null; return null;
} }
private boolean isFieldNameMatch(ConfigurationPropertyName name, ConfigurationPropertyName fieldName) {
if (name.getNumberOfElements() != fieldName.getNumberOfElements()) {
return false;
}
for (int i = 0; i < name.getNumberOfElements(); i++) {
String element = name.getElement(i, Form.ORIGINAL);
String fieldElement = fieldName.getElement(i, Form.ORIGINAL);
if (!ObjectUtils.nullSafeEquals(element, fieldElement)) {
return false;
}
}
return true;
}
private ConfigurationPropertyName getName(String field) { private ConfigurationPropertyName getName(String field) {
return this.name.append(DataObjectPropertyName.toDashedForm(field)); return this.name.append(DataObjectPropertyName.toDashedForm(field));
} }
......
/* /*
* Copyright 2012-2019 the original author or authors. * Copyright 2012-2020 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
...@@ -86,6 +86,20 @@ public final class ConfigurationPropertyName implements Comparable<Configuration ...@@ -86,6 +86,20 @@ public final class ConfigurationPropertyName implements Comparable<Configuration
return (size > 0 && isIndexed(size - 1)); return (size > 0 && isIndexed(size - 1));
} }
/**
* Return {@code true} if any element in the name is indexed.
* @return if the element has one or more indexed elements
* @since 2.2.10
*/
public boolean hasIndexedElement() {
for (int i = 0; i < getNumberOfElements(); i++) {
if (isIndexed(i)) {
return true;
}
}
return false;
}
/** /**
* Return if the element in the name is indexed. * Return if the element in the name is indexed.
* @param elementIndex the index of the element * @param elementIndex the index of the element
......
/* /*
* Copyright 2012-2019 the original author or authors. * Copyright 2012-2020 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
...@@ -29,7 +29,7 @@ import static org.assertj.core.api.Assertions.assertThat; ...@@ -29,7 +29,7 @@ import static org.assertj.core.api.Assertions.assertThat;
class DataObjectPropertyNameTests { class DataObjectPropertyNameTests {
@Test @Test
void toDashedCaseShouldConvertValue() { void toDashedCaseConvertsValue() {
assertThat(DataObjectPropertyName.toDashedForm("Foo")).isEqualTo("foo"); assertThat(DataObjectPropertyName.toDashedForm("Foo")).isEqualTo("foo");
assertThat(DataObjectPropertyName.toDashedForm("foo")).isEqualTo("foo"); assertThat(DataObjectPropertyName.toDashedForm("foo")).isEqualTo("foo");
assertThat(DataObjectPropertyName.toDashedForm("fooBar")).isEqualTo("foo-bar"); assertThat(DataObjectPropertyName.toDashedForm("fooBar")).isEqualTo("foo-bar");
...@@ -38,4 +38,10 @@ class DataObjectPropertyNameTests { ...@@ -38,4 +38,10 @@ class DataObjectPropertyNameTests {
assertThat(DataObjectPropertyName.toDashedForm("foo_Bar")).isEqualTo("foo-bar"); assertThat(DataObjectPropertyName.toDashedForm("foo_Bar")).isEqualTo("foo-bar");
} }
@Test
void toDashedFormWhenContainsIndexedAddsNoDashToIndex() throws Exception {
assertThat(DataObjectPropertyName.toDashedForm("test[fooBar]")).isEqualTo("test[fooBar]");
assertThat(DataObjectPropertyName.toDashedForm("testAgain[fooBar]")).isEqualTo("test-again[fooBar]");
}
} }
...@@ -17,7 +17,9 @@ ...@@ -17,7 +17,9 @@
package org.springframework.boot.context.properties.bind.validation; package org.springframework.boot.context.properties.bind.validation;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List; import java.util.List;
import java.util.Map;
import java.util.Set; import java.util.Set;
import javax.validation.Valid; import javax.validation.Valid;
...@@ -35,11 +37,16 @@ import org.springframework.boot.context.properties.bind.Binder; ...@@ -35,11 +37,16 @@ import org.springframework.boot.context.properties.bind.Binder;
import org.springframework.boot.context.properties.source.ConfigurationProperty; import org.springframework.boot.context.properties.source.ConfigurationProperty;
import org.springframework.boot.context.properties.source.ConfigurationPropertyName; import org.springframework.boot.context.properties.source.ConfigurationPropertyName;
import org.springframework.boot.context.properties.source.ConfigurationPropertySource; import org.springframework.boot.context.properties.source.ConfigurationPropertySource;
import org.springframework.boot.context.properties.source.ConfigurationPropertySources;
import org.springframework.boot.context.properties.source.MockConfigurationPropertySource; import org.springframework.boot.context.properties.source.MockConfigurationPropertySource;
import org.springframework.boot.origin.Origin; import org.springframework.boot.origin.Origin;
import org.springframework.core.convert.ConverterNotFoundException; import org.springframework.core.convert.ConverterNotFoundException;
import org.springframework.core.env.MapPropertySource;
import org.springframework.validation.Errors;
import org.springframework.validation.FieldError; import org.springframework.validation.FieldError;
import org.springframework.validation.ObjectError; import org.springframework.validation.ObjectError;
import org.springframework.validation.ValidationUtils;
import org.springframework.validation.Validator;
import org.springframework.validation.annotation.Validated; import org.springframework.validation.annotation.Validated;
import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean; import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean;
...@@ -210,6 +217,54 @@ class ValidationBindHandlerTests { ...@@ -210,6 +217,54 @@ class ValidationBindHandlerTests {
assertThat(fieldError.getField()).isEqualTo("personAge"); assertThat(fieldError.getField()).isEqualTo("personAge");
} }
@Test
void validateMapValues() throws Exception {
MockConfigurationPropertySource source = new MockConfigurationPropertySource();
source.put("test.items.[itemOne].number", "one");
source.put("test.items.[ITEM2].number", "two");
this.sources.add(source);
Validator validator = getMapValidator();
this.handler = new ValidationBindHandler(validator);
this.binder.bind(ConfigurationPropertyName.of("test"), Bindable.of(ExampleWithMap.class), this.handler);
}
@Test
void validateMapValuesWithNonUniformSource() throws Exception {
Map<String, Object> map = new LinkedHashMap<>();
map.put("test.items.itemOne.number", "one");
map.put("test.items.ITEM2.number", "two");
this.sources.add(ConfigurationPropertySources.from(new MapPropertySource("test", map)).iterator().next());
Validator validator = getMapValidator();
this.handler = new ValidationBindHandler(validator);
this.binder.bind(ConfigurationPropertyName.of("test"), Bindable.of(ExampleWithMap.class), this.handler);
}
private Validator getMapValidator() {
return new Validator() {
@Override
public boolean supports(Class<?> clazz) {
return ExampleWithMap.class == clazz;
}
@Override
public void validate(Object target, Errors errors) {
ExampleWithMap value = (ExampleWithMap) target;
value.getItems().forEach((k, v) -> {
try {
errors.pushNestedPath("items[" + k + "]");
ValidationUtils.rejectIfEmptyOrWhitespace(errors, "number", "NUMBER_ERR");
}
finally {
errors.popNestedPath();
}
});
}
};
}
private BindValidationException bindAndExpectValidationError(Runnable action) { private BindValidationException bindAndExpectValidationError(Runnable action) {
try { try {
action.run(); action.run();
...@@ -358,6 +413,30 @@ class ValidationBindHandlerTests { ...@@ -358,6 +413,30 @@ class ValidationBindHandlerTests {
} }
static class ExampleWithMap {
private Map<String, ExampleMapValue> items = new LinkedHashMap<>();
Map<String, ExampleMapValue> getItems() {
return this.items;
}
}
static class ExampleMapValue {
private String number;
String getNumber() {
return this.number;
}
void setNumber(String number) {
this.number = number;
}
}
static class TestHandler extends AbstractBindHandler { static class TestHandler extends AbstractBindHandler {
private Object result; private Object result;
......
/* /*
* Copyright 2012-2019 the original author or authors. * Copyright 2012-2020 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
...@@ -618,4 +618,14 @@ class ConfigurationPropertyNameTests { ...@@ -618,4 +618,14 @@ class ConfigurationPropertyNameTests {
assertThat(ConfigurationPropertyName.isValid("foo!bar")).isFalse(); assertThat(ConfigurationPropertyName.isValid("foo!bar")).isFalse();
} }
@Test
void hasIndexedElementWhenHasIndexedElementReturnsTrue() throws Exception {
assertThat(ConfigurationPropertyName.of("foo[bar]").hasIndexedElement()).isTrue();
}
@Test
void hasIndexedElementWhenHasNoIndexedElementReturnsFalse() throws Exception {
assertThat(ConfigurationPropertyName.of("foo.bar").hasIndexedElement()).isFalse();
}
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment