From bbb8367afbf38b4a2b9f991c0f604c48c270a500 Mon Sep 17 00:00:00 2001 From: Dmytro Nosan Date: Tue, 16 Jul 2019 09:56:28 +0300 Subject: [PATCH] Use MessageSource to interpolate bean validation messages See gh-17530 --- .../ValidationAutoConfiguration.java | 4 +- .../validation/ValidatorAdapter.java | 5 +- ...onfigurationPropertiesJsr303Validator.java | 4 +- .../MessageInterpolatorFactory.java | 26 +++- .../MessageParameterPlaceholderHelper.java | 105 ++++++++++++++ .../MessageSourceInterpolatorDelegate.java | 57 ++++++++ .../MessageInterpolatorFactoryTests.java | 16 +++ ...latorFactoryWithoutElIntegrationTests.java | 15 ++ ...essageParameterPlaceholderHelperTests.java | 100 +++++++++++++ ...essageSourceInterpolatorDelegateTests.java | 133 ++++++++++++++++++ 10 files changed, 460 insertions(+), 5 deletions(-) create mode 100644 spring-boot-project/spring-boot/src/main/java/org/springframework/boot/validation/MessageParameterPlaceholderHelper.java create mode 100644 spring-boot-project/spring-boot/src/main/java/org/springframework/boot/validation/MessageSourceInterpolatorDelegate.java create mode 100644 spring-boot-project/spring-boot/src/test/java/org/springframework/boot/validation/MessageParameterPlaceholderHelperTests.java create mode 100644 spring-boot-project/spring-boot/src/test/java/org/springframework/boot/validation/MessageSourceInterpolatorDelegateTests.java diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/validation/ValidationAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/validation/ValidationAutoConfiguration.java index 9c01dfc252..87a1dd84f8 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/validation/ValidationAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/validation/ValidationAutoConfiguration.java @@ -28,6 +28,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnResource; import org.springframework.boot.validation.MessageInterpolatorFactory; import org.springframework.boot.validation.beanvalidation.FilteredMethodValidationPostProcessor; import org.springframework.boot.validation.beanvalidation.MethodValidationExcludeFilter; +import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; @@ -54,9 +55,10 @@ public class ValidationAutoConfiguration { @Bean @Role(BeanDefinition.ROLE_INFRASTRUCTURE) @ConditionalOnMissingBean(Validator.class) - public static LocalValidatorFactoryBean defaultValidator() { + public static LocalValidatorFactoryBean defaultValidator(ApplicationContext applicationContext) { LocalValidatorFactoryBean factoryBean = new LocalValidatorFactoryBean(); MessageInterpolatorFactory interpolatorFactory = new MessageInterpolatorFactory(); + interpolatorFactory.setMessageSource(applicationContext); factoryBean.setMessageInterpolator(interpolatorFactory.getObject()); return factoryBean; } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/validation/ValidatorAdapter.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/validation/ValidatorAdapter.java index b7fdd206c2..d3b349431b 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/validation/ValidatorAdapter.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/validation/ValidatorAdapter.java @@ -114,7 +114,7 @@ public class ValidatorAdapter implements SmartValidator, ApplicationContextAware if (existing != null) { return wrap(existing, true); } - return create(); + return create(applicationContext); } private static Validator getExisting(ApplicationContext applicationContext) { @@ -130,10 +130,11 @@ public class ValidatorAdapter implements SmartValidator, ApplicationContextAware } } - private static Validator create() { + private static Validator create(ApplicationContext applicationContext) { OptionalValidatorFactoryBean validator = new OptionalValidatorFactoryBean(); try { MessageInterpolatorFactory factory = new MessageInterpolatorFactory(); + factory.setMessageSource(applicationContext); validator.setMessageInterpolator(factory.getObject()); } catch (ValidationException ex) { diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesJsr303Validator.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesJsr303Validator.java index cc4151e775..4d4f8a6af8 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesJsr303Validator.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesJsr303Validator.java @@ -65,7 +65,9 @@ final class ConfigurationPropertiesJsr303Validator implements Validator { Delegate(ApplicationContext applicationContext) { setApplicationContext(applicationContext); - setMessageInterpolator(new MessageInterpolatorFactory().getObject()); + MessageInterpolatorFactory factory = new MessageInterpolatorFactory(); + factory.setMessageSource(applicationContext); + setMessageInterpolator(factory.getObject()); afterPropertiesSet(); } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/validation/MessageInterpolatorFactory.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/validation/MessageInterpolatorFactory.java index 06a2f13c70..1180e0884c 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/validation/MessageInterpolatorFactory.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/validation/MessageInterpolatorFactory.java @@ -27,6 +27,8 @@ import javax.validation.ValidationException; import org.springframework.beans.BeanUtils; import org.springframework.beans.BeansException; import org.springframework.beans.factory.ObjectFactory; +import org.springframework.context.MessageSource; +import org.springframework.context.MessageSourceAware; import org.springframework.util.ClassUtils; /** @@ -37,7 +39,7 @@ import org.springframework.util.ClassUtils; * @author Phillip Webb * @since 1.5.0 */ -public class MessageInterpolatorFactory implements ObjectFactory { +public class MessageInterpolatorFactory implements ObjectFactory, MessageSourceAware { private static final Set FALLBACKS; @@ -47,8 +49,30 @@ public class MessageInterpolatorFactory implements ObjectFactory + * If returned value has other message parameters, they will be replaced recursively + * until no replacement is performed; + *

+ * Resolver can return {@code null} to signal that no further actions need to be done + * and replacement should be omitted; + *

+ * The message parameter can be escaped by the {@code '\'} symbol; + * @param message the value containing the parameters to be replaced + * @param parameterResolver the {@code parameterResolver} to use for replacement + * @return the replaced message + */ + String replaceParameters(String message, Function parameterResolver) { + return replaceParameters(message, parameterResolver, new LinkedHashSet<>(4)); + } + + private static String replaceParameters(String message, Function parameterResolver, + Set visitedParameters) { + StringBuilder buf = new StringBuilder(message); + int parentheses = 0; + int startIndex = -1; + int endIndex = -1; + for (int i = 0; i < buf.length(); i++) { + if (buf.charAt(i) == ESCAPE) { + i++; + } + else if (buf.charAt(i) == PREFIX) { + if (startIndex == -1) { + startIndex = i; + } + parentheses++; + } + else if (buf.charAt(i) == SUFFIX) { + if (parentheses > 0) { + parentheses--; + } + endIndex = i; + } + if (parentheses == 0 && startIndex < endIndex) { + String parameter = buf.substring(startIndex + 1, endIndex); + if (!visitedParameters.add(parameter)) { + throw new IllegalArgumentException("Circular reference '{" + parameter + "}'"); + } + String value = replaceParameter(parameter, parameterResolver, visitedParameters); + if (value != null) { + buf.replace(startIndex, endIndex + 1, value); + i = startIndex + value.length() - 1; + } + visitedParameters.remove(parameter); + startIndex = -1; + endIndex = -1; + } + } + return buf.toString(); + } + + private static String replaceParameter(String parameter, Function parameterResolver, + Set visitedParameters) { + parameter = replaceParameters(parameter, parameterResolver, visitedParameters); + String value = parameterResolver.apply(parameter); + if (value != null) { + return replaceParameters(value, parameterResolver, visitedParameters); + } + return null; + } + +} diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/validation/MessageSourceInterpolatorDelegate.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/validation/MessageSourceInterpolatorDelegate.java new file mode 100644 index 0000000000..370f12b2ce --- /dev/null +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/validation/MessageSourceInterpolatorDelegate.java @@ -0,0 +1,57 @@ +/* + * Copyright 2012-2019 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.validation; + +import java.util.Locale; + +import javax.validation.MessageInterpolator; + +import org.springframework.context.MessageSource; +import org.springframework.context.i18n.LocaleContextHolder; + +/** + * Resolves any message parameters via {@link MessageSource} and then interpolates a + * message using the underlying {@link MessageInterpolator}. + * + * @author Dmytro Nosan + */ +class MessageSourceInterpolatorDelegate implements MessageInterpolator { + + private static final MessageParameterPlaceholderHelper helper = new MessageParameterPlaceholderHelper(); + + private final MessageSource messageSource; + + private final MessageInterpolator messageInterpolator; + + MessageSourceInterpolatorDelegate(MessageSource messageSource, MessageInterpolator messageInterpolator) { + this.messageSource = messageSource; + this.messageInterpolator = messageInterpolator; + } + + @Override + public String interpolate(String messageTemplate, Context context) { + return interpolate(messageTemplate, context, LocaleContextHolder.getLocale()); + } + + @Override + public String interpolate(String messageTemplate, Context context, Locale locale) { + String message = helper.replaceParameters(messageTemplate, + (parameter) -> this.messageSource.getMessage(parameter, null, null, locale)); + return this.messageInterpolator.interpolate(message, context, locale); + } + +} diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/validation/MessageInterpolatorFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/validation/MessageInterpolatorFactoryTests.java index 9ca7cf0b62..152e1c8dcb 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/validation/MessageInterpolatorFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/validation/MessageInterpolatorFactoryTests.java @@ -21,7 +21,11 @@ import javax.validation.MessageInterpolator; import org.hibernate.validator.messageinterpolation.ResourceBundleMessageInterpolator; import org.junit.jupiter.api.Test; +import org.springframework.context.MessageSource; +import org.springframework.test.util.ReflectionTestUtils; + import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; /** * Tests for {@link MessageInterpolatorFactory}. @@ -36,4 +40,16 @@ class MessageInterpolatorFactoryTests { assertThat(interpolator).isInstanceOf(ResourceBundleMessageInterpolator.class); } + @Test + void getObjectShouldReturnMessageSourceMessageInterpolatorDelegateWithResourceBundleMessageInterpolator() { + MessageInterpolatorFactory interpolatorFactory = new MessageInterpolatorFactory(); + MessageSource messageSource = mock(MessageSource.class); + interpolatorFactory.setMessageSource(messageSource); + MessageInterpolator interpolator = interpolatorFactory.getObject(); + assertThat(interpolator).isInstanceOf(MessageSourceInterpolatorDelegate.class); + assertThat(interpolator).hasFieldOrPropertyWithValue("messageSource", messageSource); + assertThat(ReflectionTestUtils.getField(interpolator, "messageInterpolator")) + .isInstanceOf(ResourceBundleMessageInterpolator.class); + } + } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/validation/MessageInterpolatorFactoryWithoutElIntegrationTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/validation/MessageInterpolatorFactoryWithoutElIntegrationTests.java index 6aceb755fb..84c7b1adff 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/validation/MessageInterpolatorFactoryWithoutElIntegrationTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/validation/MessageInterpolatorFactoryWithoutElIntegrationTests.java @@ -24,9 +24,12 @@ import org.hibernate.validator.messageinterpolation.ParameterMessageInterpolator import org.junit.jupiter.api.Test; import org.springframework.boot.testsupport.classpath.ClassPathExclusions; +import org.springframework.context.MessageSource; +import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.mockito.Mockito.mock; /** * Integration tests for {@link MessageInterpolatorFactory} without EL. @@ -50,4 +53,16 @@ class MessageInterpolatorFactoryWithoutElIntegrationTests { assertThat(interpolator).isInstanceOf(ParameterMessageInterpolator.class); } + @Test + void getObjectShouldUseMessageSourceMessageInterpolatorDelegateWithFallback() { + MessageInterpolatorFactory interpolatorFactory = new MessageInterpolatorFactory(); + MessageSource messageSource = mock(MessageSource.class); + interpolatorFactory.setMessageSource(messageSource); + MessageInterpolator interpolator = interpolatorFactory.getObject(); + assertThat(interpolator).isInstanceOf(MessageSourceInterpolatorDelegate.class); + assertThat(interpolator).hasFieldOrPropertyWithValue("messageSource", messageSource); + assertThat(ReflectionTestUtils.getField(interpolator, "messageInterpolator")) + .isInstanceOf(ParameterMessageInterpolator.class); + } + } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/validation/MessageParameterPlaceholderHelperTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/validation/MessageParameterPlaceholderHelperTests.java new file mode 100644 index 0000000000..12a73063e1 --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/validation/MessageParameterPlaceholderHelperTests.java @@ -0,0 +1,100 @@ +/* + * Copyright 2012-2019 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.validation; + +import java.util.LinkedHashMap; +import java.util.Map; + +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; + +/** + * Tests for {@link MessageParameterPlaceholderHelper}. + * + * @author Dmytro Nosan + */ +class MessageParameterPlaceholderHelperTests { + + private final MessageParameterPlaceholderHelper resolver = new MessageParameterPlaceholderHelper(); + + @Test + void recursionInParameters() { + Map props = new LinkedHashMap<>(); + props.put("a", "{b}"); + props.put("b", "{c}"); + props.put("c", "{a}"); + assertThatIllegalArgumentException().isThrownBy(() -> this.resolver.replaceParameters("{a}", props::get)) + .withMessage("Circular reference '{a}'"); + } + + @Test + void replaceParameters() { + Map props = new LinkedHashMap<>(); + props.put("foo", "fooValue"); + props.put("bar", ""); + assertThat(this.resolver.replaceParameters("{foo}{bar}", props::get)).isEqualTo("fooValue"); + } + + @Test + void replaceNestedParameters() { + Map props = new LinkedHashMap<>(); + props.put("top", "{child}+{child}"); + props.put("child", "{{differentiator}.grandchild}"); + props.put("differentiator", "first"); + props.put("first.grandchild", "actualValue"); + assertThat(this.resolver.replaceParameters("{top}", props::get)).isEqualTo("actualValue+actualValue"); + } + + @Test + void unresolvedParameters() { + Map props = new LinkedHashMap<>(); + props.put("top", "{child}+{child}"); + assertThat(this.resolver.replaceParameters("{foo}{top}{bar}", props::get)) + .isEqualTo("{foo}{child}+{child}{bar}"); + } + + @Test + void unbalancedParentheses() { + Map props = new LinkedHashMap<>(); + props.put("top", "topValue"); + assertThat(this.resolver.replaceParameters("\\{top}", props::get)).isEqualTo("\\{top}"); + assertThat(this.resolver.replaceParameters("{top\\}", props::get)).isEqualTo("{top\\}"); + assertThat(this.resolver.replaceParameters("{{top}", props::get)).isEqualTo("{{top}"); + assertThat(this.resolver.replaceParameters("{top}}", props::get)).isEqualTo("topValue}"); + } + + @Test + void resolveEscapeParameters() { + Map props = new LinkedHashMap<>(); + props.put("foo", "fooValue"); + props.put("bar", "\\{foo}"); + props.put("bazz\\}", "bazzValue"); + assertThat(this.resolver.replaceParameters("{foo}", props::get)).isEqualTo("fooValue"); + assertThat(this.resolver.replaceParameters("{foo}\\a", props::get)).isEqualTo("fooValue\\a"); + assertThat(this.resolver.replaceParameters("\\\\{foo}", props::get)).isEqualTo("\\\\fooValue"); + assertThat(this.resolver.replaceParameters("\\\\\\{foo}", props::get)).isEqualTo("\\\\\\{foo}"); + assertThat(this.resolver.replaceParameters("\\{foo}", props::get)).isEqualTo("\\{foo}"); + assertThat(this.resolver.replaceParameters("{foo\\}", props::get)).isEqualTo("{foo\\}"); + assertThat(this.resolver.replaceParameters("\\{foo\\}", props::get)).isEqualTo("\\{foo\\}"); + assertThat(this.resolver.replaceParameters("{foo}\\", props::get)).isEqualTo("fooValue\\"); + assertThat(this.resolver.replaceParameters("{bar}", props::get)).isEqualTo("\\{foo}"); + assertThat(this.resolver.replaceParameters("{bazz\\}}", props::get)).isEqualTo("bazzValue"); + } + +} diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/validation/MessageSourceInterpolatorDelegateTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/validation/MessageSourceInterpolatorDelegateTests.java new file mode 100644 index 0000000000..a455a2eee4 --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/validation/MessageSourceInterpolatorDelegateTests.java @@ -0,0 +1,133 @@ +/* + * Copyright 2012-2019 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.validation; + +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; +import java.util.Set; + +import javax.validation.ConstraintViolation; +import javax.validation.Validator; +import javax.validation.constraints.NotNull; + +import org.junit.jupiter.api.Test; + +import org.springframework.context.i18n.LocaleContextHolder; +import org.springframework.context.support.StaticMessageSource; +import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Tests for {@link MessageSourceInterpolatorDelegate}. + * + * @author Dmytro Nosan + */ +class MessageSourceInterpolatorDelegateTests { + + private final Validator validator = buildValidator(); + + @NotNull(message = "{null}") + private String nullable; + + @NotNull(message = "{blank}") + private String blank; + + @NotNull(message = "{unknown}") + private String unknown; + + @NotNull(message = "{recursion}") + private String recursion; + + @NotNull(message = "\\{null}") + private String escapePrefix; + + @NotNull(message = "{null\\}") + private String escapeSuffix; + + @NotNull(message = "\\{null\\}") + private String escapePrefixSuffix; + + @NotNull(message = "\\\\{null}") + private String escapeEscape; + + @Test + void nullable() { + assertThat(validate("nullable")).containsExactly("must not be null"); + } + + @Test + void blank() { + assertThat(validate("blank")).containsExactly("must not be null or must not be blank"); + } + + @Test + void recursion() { + assertThatThrownBy(() -> validate("recursion")).hasStackTraceContaining("Circular reference '{recursion}'"); + } + + @Test + void unknown() { + assertThat(validate("unknown")).containsExactly("{unknown}"); + } + + @Test + void escapePrefix() { + assertThat(validate("escapePrefix")).containsExactly("\\{null}"); + } + + @Test + void escapeSuffix() { + assertThat(validate("escapeSuffix")).containsExactly("{null\\}"); + } + + @Test + void escapePrefixSuffix() { + assertThat(validate("escapePrefixSuffix")).containsExactly("{null}"); + } + + @Test + void escapeEscape() { + assertThat(validate("escapeEscape")).containsExactly("\\must not be null"); + } + + private List validate(String property) { + List messages = new ArrayList<>(); + Set> constraints = this.validator.validateProperty(this, property); + for (ConstraintViolation constraint : constraints) { + messages.add(constraint.getMessage()); + } + return messages; + } + + private static Validator buildValidator() { + Locale locale = LocaleContextHolder.getLocale(); + StaticMessageSource messageSource = new StaticMessageSource(); + messageSource.addMessage("blank", locale, "{null} or {javax.validation.constraints.NotBlank.message}"); + messageSource.addMessage("null", locale, "{javax.validation.constraints.NotNull.message}"); + messageSource.addMessage("recursion", locale, "{recursion}"); + MessageInterpolatorFactory messageInterpolatorFactory = new MessageInterpolatorFactory(); + messageInterpolatorFactory.setMessageSource(messageSource); + LocalValidatorFactoryBean validatorFactory = new LocalValidatorFactoryBean(); + validatorFactory.setMessageInterpolator(messageInterpolatorFactory.getObject()); + validatorFactory.afterPropertiesSet(); + return validatorFactory.getValidator(); + } + +}