Skip to content

Commit 96c494c

Browse files
committed
Update error format in MethodArgumentNotValidException
1. Remove list markers (those can be provided in message). 2. Use ", and " between errors for readability. 3. Remove single quotes around errors. 4. If MessageSource is provided, use resolved message as is since in that case applications have full control over each message. Closes gh-30198
1 parent 48548b2 commit 96c494c

File tree

5 files changed

+149
-39
lines changed

5 files changed

+149
-39
lines changed

spring-web/spring-web.gradle

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ dependencies {
8989
testRuntimeOnly("com.sun.xml.bind:jaxb-impl")
9090
testRuntimeOnly("jakarta.json:jakarta.json-api")
9191
testRuntimeOnly("org.eclipse:yasson")
92+
testRuntimeOnly("org.glassfish:jakarta.el")
93+
testRuntimeOnly("org.hibernate:hibernate-validator")
9294
testFixturesApi("jakarta.servlet:jakarta.servlet-api")
9395
testFixturesApi("org.junit.jupiter:junit-jupiter-api")
9496
testFixturesApi("org.junit.jupiter:junit-jupiter-params")

spring-web/src/main/java/org/springframework/web/bind/MethodArgumentNotValidException.java

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,18 @@
1616

1717
package org.springframework.web.bind;
1818

19-
import java.util.ArrayList;
2019
import java.util.LinkedHashMap;
2120
import java.util.List;
2221
import java.util.Locale;
2322
import java.util.Map;
24-
import java.util.function.Function;
2523

2624
import org.springframework.context.MessageSource;
2725
import org.springframework.core.MethodParameter;
2826
import org.springframework.http.HttpStatus;
2927
import org.springframework.http.HttpStatusCode;
3028
import org.springframework.http.ProblemDetail;
3129
import org.springframework.lang.Nullable;
30+
import org.springframework.util.Assert;
3231
import org.springframework.util.StringUtils;
3332
import org.springframework.validation.BindException;
3433
import org.springframework.validation.BindingResult;
@@ -100,21 +99,26 @@ public String getMessage() {
10099

101100
@Override
102101
public Object[] getDetailMessageArguments() {
103-
return new Object[] {errorsToStringList(getGlobalErrors()), errorsToStringList(getFieldErrors())};
102+
return new Object[] {
103+
join(formatErrors(getGlobalErrors(), null, null)),
104+
join(formatErrors(getFieldErrors(), null, null))};
104105
}
105106

106107
@Override
107108
public Object[] getDetailMessageArguments(MessageSource messageSource, Locale locale) {
108109
return new Object[] {
109-
errorsToStringList(getGlobalErrors(), messageSource, locale),
110-
errorsToStringList(getFieldErrors(), messageSource, locale)
111-
};
110+
join(formatErrors(getGlobalErrors(), messageSource, locale)),
111+
join(formatErrors(getFieldErrors(), messageSource, locale))};
112+
}
113+
114+
private static String join(List<String> errors) {
115+
return String.join(", and ", errors);
112116
}
113117

114118
/**
115119
* Resolve global and field errors to messages with the given
116120
* {@link MessageSource} and {@link Locale}.
117-
* @return a Map with errors as key and resolved messages as value
121+
* @return a Map with errors as keys and resolved messages as values
118122
* @since 6.0.3
119123
*/
120124
public Map<ObjectError, String> resolveErrorMessages(MessageSource messageSource, Locale locale) {
@@ -141,8 +145,7 @@ private static void addMessages(
141145
* @since 6.0
142146
*/
143147
public static List<String> errorsToStringList(List<? extends ObjectError> errors) {
144-
return errorsToStringList(errors, error ->
145-
error.getDefaultMessage() != null ? error.getDefaultMessage() : error.getCode());
148+
return formatErrors(errors, null, null);
146149
}
147150

148151
/**
@@ -154,23 +157,28 @@ public static List<String> errorsToStringList(List<? extends ObjectError> errors
154157
public static List<String> errorsToStringList(
155158
List<? extends ObjectError> errors, @Nullable MessageSource source, Locale locale) {
156159

157-
return (source != null ?
158-
errorsToStringList(errors, error -> source.getMessage(error, locale)) :
159-
errorsToStringList(errors));
160+
return formatErrors(errors, source, locale);
161+
}
162+
163+
public static List<String> formatErrors(
164+
List<? extends ObjectError> errors, @Nullable MessageSource messageSource, @Nullable Locale locale) {
165+
166+
return errors.stream()
167+
.map(error -> formatError(error, messageSource, locale))
168+
.filter(StringUtils::hasText)
169+
.toList();
160170
}
161171

162-
private static List<String> errorsToStringList(
163-
List<? extends ObjectError> errors, Function<ObjectError, String> formatter) {
172+
private static String formatError(
173+
ObjectError error, @Nullable MessageSource messageSource, @Nullable Locale locale) {
164174

165-
List<String> result = new ArrayList<>(errors.size());
166-
for (ObjectError error : errors) {
167-
String value = formatter.apply(error);
168-
if (StringUtils.hasText(value)) {
169-
result.add(error instanceof FieldError fieldError ?
170-
fieldError.getField() + ": '" + value + "'" : "'" + value + "'");
171-
}
175+
if (messageSource != null) {
176+
Assert.notNull(locale, "Expected MessageSource and locale");
177+
return messageSource.getMessage(error, locale);
172178
}
173-
return result;
179+
String field = (error instanceof FieldError fieldError ? fieldError.getField() + ": " : "");
180+
String message = (error.getDefaultMessage() != null ? error.getDefaultMessage() : error.getCode());
181+
return (field + message);
174182
}
175183

176184
}

spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeBindException.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -56,11 +56,13 @@ public WebExchangeBindException(MethodParameter parameter, BindingResult binding
5656

5757
private static Object[] initMessageDetailArguments(BindingResult bindingResult) {
5858
return new Object[] {
59-
MethodArgumentNotValidException.errorsToStringList(bindingResult.getGlobalErrors()),
60-
MethodArgumentNotValidException.errorsToStringList(bindingResult.getFieldErrors())
61-
};
59+
join(MethodArgumentNotValidException.errorsToStringList(bindingResult.getGlobalErrors())),
60+
join(MethodArgumentNotValidException.errorsToStringList(bindingResult.getFieldErrors()))};
6261
}
6362

63+
private static String join(List<String> errors) {
64+
return String.join(", and ", errors);
65+
}
6466

6567
/**
6668
* Return the BindingResult that this BindException wraps.
@@ -303,8 +305,8 @@ public String getMessage() {
303305
@Override
304306
public Object[] getDetailMessageArguments(MessageSource source, Locale locale) {
305307
return new Object[] {
306-
MethodArgumentNotValidException.errorsToStringList(getGlobalErrors(), source, locale),
307-
MethodArgumentNotValidException.errorsToStringList(getFieldErrors(), source, locale)
308+
join(MethodArgumentNotValidException.errorsToStringList(getGlobalErrors(), source, locale)),
309+
join(MethodArgumentNotValidException.errorsToStringList(getFieldErrors(), source, locale))
308310
};
309311
}
310312

spring-web/src/test/java/org/springframework/web/ErrorResponseExceptionTests.java

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ public BindingResult initBindingResult() {
444444
BindingResult bindingResult = new BindException(new TestBean(), "myBean");
445445
bindingResult.reject("bean.invalid.A", "Invalid bean message");
446446
bindingResult.reject("bean.invalid.B");
447-
bindingResult.rejectValue("name", "name.required", "Name is required");
447+
bindingResult.rejectValue("name", "name.required", "must be provided");
448448
bindingResult.rejectValue("age", "age.min");
449449
return bindingResult;
450450
}
@@ -456,33 +456,33 @@ private void assertDetailMessage(ErrorResponse ex) {
456456
String message = messageSource.getMessage(
457457
ex.getDetailMessageCode(), ex.getDetailMessageArguments(), Locale.UK);
458458

459-
assertThat(message).isEqualTo("" +
460-
"Failures ['Invalid bean message', 'bean.invalid.B']. " +
461-
"nested failures: [name: 'Name is required', age: 'age.min']");
459+
assertThat(message).isEqualTo(
460+
"Failed because Invalid bean message, and bean.invalid.B. " +
461+
"Also because name: must be provided, and age: age.min");
462462

463463
message = messageSource.getMessage(
464464
ex.getDetailMessageCode(), ex.getDetailMessageArguments(messageSource, Locale.UK), Locale.UK);
465465

466-
assertThat(message).isEqualTo("" +
467-
"Failures ['Bean A message', 'Bean B message']. " +
468-
"nested failures: [name: 'Required name message', age: 'Minimum age message']");
466+
assertThat(message).isEqualTo(
467+
"Failed because Bean A message, and Bean B message. " +
468+
"Also because name is required, and age is below minimum");
469469
}
470470

471471
private void assertErrorMessages(BiFunction<MessageSource, Locale, Map<ObjectError, String>> expectedMessages) {
472472
StaticMessageSource messageSource = initMessageSource();
473473
Map<ObjectError, String> map = expectedMessages.apply(messageSource, Locale.UK);
474474

475475
assertThat(map).hasSize(4).containsValues(
476-
"'Bean A message'", "'Bean B message'", "name: 'Required name message'", "age: 'Minimum age message'");
476+
"Bean A message", "Bean B message", "name is required", "age is below minimum");
477477
}
478478

479479
private StaticMessageSource initMessageSource() {
480480
StaticMessageSource messageSource = new StaticMessageSource();
481-
messageSource.addMessage(this.code, Locale.UK, "Failures {0}. nested failures: {1}");
481+
messageSource.addMessage(this.code, Locale.UK, "Failed because {0}. Also because {1}");
482482
messageSource.addMessage("bean.invalid.A", Locale.UK, "Bean A message");
483483
messageSource.addMessage("bean.invalid.B", Locale.UK, "Bean B message");
484-
messageSource.addMessage("name.required", Locale.UK, "Required name message");
485-
messageSource.addMessage("age.min", Locale.UK, "Minimum age message");
484+
messageSource.addMessage("name.required", Locale.UK, "name is required");
485+
messageSource.addMessage("age.min", Locale.UK, "age is below minimum");
486486
return messageSource;
487487
}
488488
}
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
/*
2+
* Copyright 2002-2023 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.web.bind;
18+
19+
import java.lang.reflect.Method;
20+
import java.util.List;
21+
import java.util.Locale;
22+
23+
import jakarta.validation.constraints.Min;
24+
import jakarta.validation.constraints.Size;
25+
import org.junit.jupiter.api.Test;
26+
27+
import org.springframework.context.support.StaticMessageSource;
28+
import org.springframework.core.MethodParameter;
29+
import org.springframework.validation.BeanPropertyBindingResult;
30+
import org.springframework.validation.BindingResult;
31+
import org.springframework.validation.FieldError;
32+
import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean;
33+
import org.springframework.validation.beanvalidation.SpringValidatorAdapter;
34+
35+
import static org.assertj.core.api.Assertions.assertThat;
36+
37+
/**
38+
* Unit tests for {@link MethodArgumentNotValidException}.
39+
* @author Rossen Stoyanchev
40+
*/
41+
public class MethodArgumentNotValidExceptionTests {
42+
43+
@Test
44+
void errorsToStringList() throws Exception {
45+
Person frederick1234 = new Person("Frederick1234", 24);
46+
MethodArgumentNotValidException ex = createException(frederick1234);
47+
48+
List<FieldError> fieldErrors = ex.getFieldErrors();
49+
List<String> errors = MethodArgumentNotValidException.errorsToStringList(fieldErrors);
50+
51+
assertThat(errors).containsExactlyInAnyOrder(
52+
"name: size must be between 0 and 10", "age: must be greater than or equal to 25");
53+
}
54+
55+
@Test
56+
void errorsToStringListWithMessageSource() throws Exception {
57+
Person frederick1234 = new Person("Frederick1234", 24);
58+
MethodArgumentNotValidException ex = createException(frederick1234);
59+
60+
StaticMessageSource source = new StaticMessageSource();
61+
source.addMessage("Size.name", Locale.UK, "name exceeds {1} characters");
62+
source.addMessage("Min.age", Locale.UK, "age is under {1}");
63+
64+
List<FieldError> fieldErrors = ex.getFieldErrors();
65+
List<String> errors = MethodArgumentNotValidException.errorsToStringList(fieldErrors, source, Locale.UK);
66+
67+
assertThat(errors).containsExactlyInAnyOrder("name exceeds 10 characters", "age is under 25");
68+
}
69+
70+
private static MethodArgumentNotValidException createException(Person person) throws Exception {
71+
LocalValidatorFactoryBean validatorBean = new LocalValidatorFactoryBean();
72+
validatorBean.afterPropertiesSet();
73+
SpringValidatorAdapter validator = new SpringValidatorAdapter(validatorBean);
74+
75+
BindingResult result = new BeanPropertyBindingResult(person, "person");
76+
validator.validate(person, result);
77+
78+
Method method = Handler.class.getDeclaredMethod("handle", Person.class);
79+
MethodParameter parameter = new MethodParameter(method, 0);
80+
81+
return new MethodArgumentNotValidException(parameter, result);
82+
}
83+
84+
85+
@SuppressWarnings("unused")
86+
private static class Handler {
87+
88+
@SuppressWarnings("unused")
89+
void handle(Person person) {
90+
}
91+
}
92+
93+
94+
@SuppressWarnings("unused")
95+
private record Person(@Size(max = 10) String name, @Min(25) int age) {
96+
}
97+
98+
}

0 commit comments

Comments
 (0)