diff --git a/spring-web/spring-web.gradle b/spring-web/spring-web.gradle index a6289ac8d9db..30f42af21f9a 100644 --- a/spring-web/spring-web.gradle +++ b/spring-web/spring-web.gradle @@ -89,6 +89,8 @@ dependencies { testRuntimeOnly("com.sun.xml.bind:jaxb-impl") testRuntimeOnly("jakarta.json:jakarta.json-api") testRuntimeOnly("org.eclipse:yasson") + testRuntimeOnly("org.glassfish:jakarta.el") + testRuntimeOnly("org.hibernate:hibernate-validator") testFixturesApi("jakarta.servlet:jakarta.servlet-api") testFixturesApi("org.junit.jupiter:junit-jupiter-api") testFixturesApi("org.junit.jupiter:junit-jupiter-params") diff --git a/spring-web/src/main/java/org/springframework/web/bind/MethodArgumentNotValidException.java b/spring-web/src/main/java/org/springframework/web/bind/MethodArgumentNotValidException.java index 92d882e38c59..9ab4be80d73e 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/MethodArgumentNotValidException.java +++ b/spring-web/src/main/java/org/springframework/web/bind/MethodArgumentNotValidException.java @@ -16,12 +16,10 @@ package org.springframework.web.bind; -import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.function.Function; import org.springframework.context.MessageSource; import org.springframework.core.MethodParameter; @@ -29,6 +27,7 @@ import org.springframework.http.HttpStatusCode; import org.springframework.http.ProblemDetail; import org.springframework.lang.Nullable; +import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.validation.BindException; import org.springframework.validation.BindingResult; @@ -100,21 +99,26 @@ public String getMessage() { @Override public Object[] getDetailMessageArguments() { - return new Object[] {errorsToStringList(getGlobalErrors()), errorsToStringList(getFieldErrors())}; + return new Object[] { + join(formatErrors(getGlobalErrors(), null, null)), + join(formatErrors(getFieldErrors(), null, null))}; } @Override public Object[] getDetailMessageArguments(MessageSource messageSource, Locale locale) { return new Object[] { - errorsToStringList(getGlobalErrors(), messageSource, locale), - errorsToStringList(getFieldErrors(), messageSource, locale) - }; + join(formatErrors(getGlobalErrors(), messageSource, locale)), + join(formatErrors(getFieldErrors(), messageSource, locale))}; + } + + private static String join(List errors) { + return String.join(", and ", errors); } /** * Resolve global and field errors to messages with the given * {@link MessageSource} and {@link Locale}. - * @return a Map with errors as key and resolved messages as value + * @return a Map with errors as keys and resolved messages as values * @since 6.0.3 */ public Map resolveErrorMessages(MessageSource messageSource, Locale locale) { @@ -141,8 +145,7 @@ private static void addMessages( * @since 6.0 */ public static List errorsToStringList(List errors) { - return errorsToStringList(errors, error -> - error.getDefaultMessage() != null ? error.getDefaultMessage() : error.getCode()); + return formatErrors(errors, null, null); } /** @@ -154,23 +157,28 @@ public static List errorsToStringList(List errors public static List errorsToStringList( List errors, @Nullable MessageSource source, Locale locale) { - return (source != null ? - errorsToStringList(errors, error -> source.getMessage(error, locale)) : - errorsToStringList(errors)); + return formatErrors(errors, source, locale); + } + + public static List formatErrors( + List errors, @Nullable MessageSource messageSource, @Nullable Locale locale) { + + return errors.stream() + .map(error -> formatError(error, messageSource, locale)) + .filter(StringUtils::hasText) + .toList(); } - private static List errorsToStringList( - List errors, Function formatter) { + private static String formatError( + ObjectError error, @Nullable MessageSource messageSource, @Nullable Locale locale) { - List result = new ArrayList<>(errors.size()); - for (ObjectError error : errors) { - String value = formatter.apply(error); - if (StringUtils.hasText(value)) { - result.add(error instanceof FieldError fieldError ? - fieldError.getField() + ": '" + value + "'" : "'" + value + "'"); - } + if (messageSource != null) { + Assert.notNull(locale, "Expected MessageSource and locale"); + return messageSource.getMessage(error, locale); } - return result; + String field = (error instanceof FieldError fieldError ? fieldError.getField() + ": " : ""); + String message = (error.getDefaultMessage() != null ? error.getDefaultMessage() : error.getCode()); + return (field + message); } } diff --git a/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeBindException.java b/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeBindException.java index 1c1adcac7257..e1ba3bf46f79 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeBindException.java +++ b/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeBindException.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 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. @@ -56,11 +56,13 @@ public WebExchangeBindException(MethodParameter parameter, BindingResult binding private static Object[] initMessageDetailArguments(BindingResult bindingResult) { return new Object[] { - MethodArgumentNotValidException.errorsToStringList(bindingResult.getGlobalErrors()), - MethodArgumentNotValidException.errorsToStringList(bindingResult.getFieldErrors()) - }; + join(MethodArgumentNotValidException.errorsToStringList(bindingResult.getGlobalErrors())), + join(MethodArgumentNotValidException.errorsToStringList(bindingResult.getFieldErrors()))}; } + private static String join(List errors) { + return String.join(", and ", errors); + } /** * Return the BindingResult that this BindException wraps. @@ -303,8 +305,8 @@ public String getMessage() { @Override public Object[] getDetailMessageArguments(MessageSource source, Locale locale) { return new Object[] { - MethodArgumentNotValidException.errorsToStringList(getGlobalErrors(), source, locale), - MethodArgumentNotValidException.errorsToStringList(getFieldErrors(), source, locale) + join(MethodArgumentNotValidException.errorsToStringList(getGlobalErrors(), source, locale)), + join(MethodArgumentNotValidException.errorsToStringList(getFieldErrors(), source, locale)) }; } diff --git a/spring-web/src/test/java/org/springframework/web/ErrorResponseExceptionTests.java b/spring-web/src/test/java/org/springframework/web/ErrorResponseExceptionTests.java index cbf9cfb71102..2aa4686c7a0d 100644 --- a/spring-web/src/test/java/org/springframework/web/ErrorResponseExceptionTests.java +++ b/spring-web/src/test/java/org/springframework/web/ErrorResponseExceptionTests.java @@ -444,7 +444,7 @@ public BindingResult initBindingResult() { BindingResult bindingResult = new BindException(new TestBean(), "myBean"); bindingResult.reject("bean.invalid.A", "Invalid bean message"); bindingResult.reject("bean.invalid.B"); - bindingResult.rejectValue("name", "name.required", "Name is required"); + bindingResult.rejectValue("name", "name.required", "must be provided"); bindingResult.rejectValue("age", "age.min"); return bindingResult; } @@ -456,16 +456,16 @@ private void assertDetailMessage(ErrorResponse ex) { String message = messageSource.getMessage( ex.getDetailMessageCode(), ex.getDetailMessageArguments(), Locale.UK); - assertThat(message).isEqualTo("" + - "Failures ['Invalid bean message', 'bean.invalid.B']. " + - "nested failures: [name: 'Name is required', age: 'age.min']"); + assertThat(message).isEqualTo( + "Failed because Invalid bean message, and bean.invalid.B. " + + "Also because name: must be provided, and age: age.min"); message = messageSource.getMessage( ex.getDetailMessageCode(), ex.getDetailMessageArguments(messageSource, Locale.UK), Locale.UK); - assertThat(message).isEqualTo("" + - "Failures ['Bean A message', 'Bean B message']. " + - "nested failures: [name: 'Required name message', age: 'Minimum age message']"); + assertThat(message).isEqualTo( + "Failed because Bean A message, and Bean B message. " + + "Also because name is required, and age is below minimum"); } private void assertErrorMessages(BiFunction> expectedMessages) { @@ -473,16 +473,16 @@ private void assertErrorMessages(BiFunction map = expectedMessages.apply(messageSource, Locale.UK); assertThat(map).hasSize(4).containsValues( - "'Bean A message'", "'Bean B message'", "name: 'Required name message'", "age: 'Minimum age message'"); + "Bean A message", "Bean B message", "name is required", "age is below minimum"); } private StaticMessageSource initMessageSource() { StaticMessageSource messageSource = new StaticMessageSource(); - messageSource.addMessage(this.code, Locale.UK, "Failures {0}. nested failures: {1}"); + messageSource.addMessage(this.code, Locale.UK, "Failed because {0}. Also because {1}"); messageSource.addMessage("bean.invalid.A", Locale.UK, "Bean A message"); messageSource.addMessage("bean.invalid.B", Locale.UK, "Bean B message"); - messageSource.addMessage("name.required", Locale.UK, "Required name message"); - messageSource.addMessage("age.min", Locale.UK, "Minimum age message"); + messageSource.addMessage("name.required", Locale.UK, "name is required"); + messageSource.addMessage("age.min", Locale.UK, "age is below minimum"); return messageSource; } } diff --git a/spring-web/src/test/java/org/springframework/web/bind/MethodArgumentNotValidExceptionTests.java b/spring-web/src/test/java/org/springframework/web/bind/MethodArgumentNotValidExceptionTests.java new file mode 100644 index 000000000000..4c05b784dcac --- /dev/null +++ b/spring-web/src/test/java/org/springframework/web/bind/MethodArgumentNotValidExceptionTests.java @@ -0,0 +1,98 @@ +/* + * Copyright 2002-2023 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.web.bind; + +import java.lang.reflect.Method; +import java.util.List; +import java.util.Locale; + +import jakarta.validation.constraints.Min; +import jakarta.validation.constraints.Size; +import org.junit.jupiter.api.Test; + +import org.springframework.context.support.StaticMessageSource; +import org.springframework.core.MethodParameter; +import org.springframework.validation.BeanPropertyBindingResult; +import org.springframework.validation.BindingResult; +import org.springframework.validation.FieldError; +import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean; +import org.springframework.validation.beanvalidation.SpringValidatorAdapter; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Unit tests for {@link MethodArgumentNotValidException}. + * @author Rossen Stoyanchev + */ +public class MethodArgumentNotValidExceptionTests { + + @Test + void errorsToStringList() throws Exception { + Person frederick1234 = new Person("Frederick1234", 24); + MethodArgumentNotValidException ex = createException(frederick1234); + + List fieldErrors = ex.getFieldErrors(); + List errors = MethodArgumentNotValidException.errorsToStringList(fieldErrors); + + assertThat(errors).containsExactlyInAnyOrder( + "name: size must be between 0 and 10", "age: must be greater than or equal to 25"); + } + + @Test + void errorsToStringListWithMessageSource() throws Exception { + Person frederick1234 = new Person("Frederick1234", 24); + MethodArgumentNotValidException ex = createException(frederick1234); + + StaticMessageSource source = new StaticMessageSource(); + source.addMessage("Size.name", Locale.UK, "name exceeds {1} characters"); + source.addMessage("Min.age", Locale.UK, "age is under {1}"); + + List fieldErrors = ex.getFieldErrors(); + List errors = MethodArgumentNotValidException.errorsToStringList(fieldErrors, source, Locale.UK); + + assertThat(errors).containsExactlyInAnyOrder("name exceeds 10 characters", "age is under 25"); + } + + private static MethodArgumentNotValidException createException(Person person) throws Exception { + LocalValidatorFactoryBean validatorBean = new LocalValidatorFactoryBean(); + validatorBean.afterPropertiesSet(); + SpringValidatorAdapter validator = new SpringValidatorAdapter(validatorBean); + + BindingResult result = new BeanPropertyBindingResult(person, "person"); + validator.validate(person, result); + + Method method = Handler.class.getDeclaredMethod("handle", Person.class); + MethodParameter parameter = new MethodParameter(method, 0); + + return new MethodArgumentNotValidException(parameter, result); + } + + + @SuppressWarnings("unused") + private static class Handler { + + @SuppressWarnings("unused") + void handle(Person person) { + } + } + + + @SuppressWarnings("unused") + private record Person(@Size(max = 10) String name, @Min(25) int age) { + } + +}