Skip to content

Commit 52c3f71

Browse files
snicollrstoyanchev
snicoll
authored and
rstoyanchev
committed
Fix handling of required payload.
A payload that is required will now throw an appropriate exception regardless of if a conversion is required or not. isEmptyPayload now takes the payload instead of the message so that both the original payload and the converted payload, if necessary, share the same logic. JSR-303 validation is now consistently applied. Issue: SPR-11577
1 parent 4cd818b commit 52c3f71

File tree

4 files changed

+188
-65
lines changed

4 files changed

+188
-65
lines changed

‎spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/support/MethodArgumentNotValidException.java‎

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -23,31 +23,39 @@
2323
import org.springframework.validation.ObjectError;
2424

2525
/**
26-
* Exception to be thrown when validation on an method parameter annotated with {@code @Valid} fails.
26+
* Exception to be thrown when a method argument is not valid. For instance, this
27+
* can be issued if a validation on a method parameter annotated with
28+
* {@code @Valid} fails.
29+
*
2730
* @author Brian Clozel
2831
* @since 4.0.1
2932
*/
3033
@SuppressWarnings("serial")
3134
public class MethodArgumentNotValidException extends MessagingException {
3235

33-
private final MethodParameter parameter;
34-
35-
private final BindingResult bindingResult;
36-
36+
/**
37+
* Create a new message with the given description.
38+
* @see #getMessage()
39+
*/
40+
public MethodArgumentNotValidException(Message<?> message, String description) {
41+
super(message, description);
42+
}
3743

38-
public MethodArgumentNotValidException(Message<?> message, MethodParameter parameter, BindingResult bindingResult) {
39-
super(message);
40-
this.parameter = parameter;
41-
this.bindingResult = bindingResult;
44+
/**
45+
* Create a new instance with a failed validation described by
46+
* the given {@link BindingResult}.
47+
*/
48+
public MethodArgumentNotValidException(Message<?> message,
49+
MethodParameter parameter, BindingResult bindingResult) {
50+
this(message, generateMessage(parameter, bindingResult));
4251
}
4352

44-
@Override
45-
public String getMessage() {
53+
private static String generateMessage(MethodParameter parameter, BindingResult bindingResult) {
4654
StringBuilder sb = new StringBuilder("Validation failed for parameter at index ")
47-
.append(this.parameter.getParameterIndex()).append(" in method: ")
48-
.append(this.parameter.getMethod().toGenericString())
49-
.append(", with ").append(this.bindingResult.getErrorCount()).append(" error(s): ");
50-
for (ObjectError error : this.bindingResult.getAllErrors()) {
55+
.append(parameter.getParameterIndex()).append(" in method: ")
56+
.append(parameter.getMethod().toGenericString())
57+
.append(", with ").append(bindingResult.getErrorCount()).append(" error(s): ");
58+
for (ObjectError error : bindingResult.getAllErrors()) {
5159
sb.append("[").append(error).append("] ");
5260
}
5361
return sb.toString();

‎spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/support/PayloadArgumentResolver.java‎

Lines changed: 53 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
*
4343
* @author Rossen Stoyanchev
4444
* @author Brian Clozel
45+
* @author Stephane Nicoll
4546
* @since 4.0
4647
*/
4748
public class PayloadArgumentResolver implements HandlerMethodArgumentResolver {
@@ -65,36 +66,53 @@ public boolean supportsParameter(MethodParameter parameter) {
6566

6667
@Override
6768
public Object resolveArgument(MethodParameter parameter, Message<?> message) throws Exception {
68-
69-
Class<?> sourceClass = message.getPayload().getClass();
70-
Class<?> targetClass = parameter.getParameterType();
71-
72-
if (ClassUtils.isAssignable(targetClass,sourceClass)) {
73-
return message.getPayload();
74-
}
75-
7669
Payload annot = parameter.getParameterAnnotation(Payload.class);
70+
if ((annot != null) && StringUtils.hasText(annot.value())) {
71+
throw new IllegalStateException("@Payload SpEL expressions not supported by this resolver.");
72+
}
7773

78-
if (isEmptyPayload(message)) {
79-
if ((annot != null) && !annot.required()) {
74+
Object target = getTargetPayload(parameter, message);
75+
if (annot != null && isEmptyPayload(target)) {
76+
if (annot.required()) {
77+
throw new MethodArgumentNotValidException(message, createPayloadRequiredExceptionMessage(parameter, target));
78+
}
79+
else {
8080
return null;
8181
}
8282
}
8383

84-
if ((annot != null) && StringUtils.hasText(annot.value())) {
85-
throw new IllegalStateException("@Payload SpEL expressions not supported by this resolver.");
84+
if (annot != null) { // Only validate @Payload
85+
validate(message, parameter, target);
8686
}
87-
88-
Object target = this.converter.fromMessage(message, targetClass);
89-
validate(message, parameter, target);
90-
9187
return target;
9288
}
9389

94-
protected boolean isEmptyPayload(Message<?> message) {
95-
Object payload = message.getPayload();
96-
if (payload instanceof byte[]) {
97-
return ((byte[]) message.getPayload()).length == 0;
90+
/**
91+
* Return the target payload to handle for the specified message. Can either
92+
* be the payload itself if the parameter type supports it or the converted
93+
* one otherwise. While the payload of a {@link Message} cannot be null by
94+
* design, this method may return a {@code null} payload if the conversion
95+
* result is {@code null}.
96+
*/
97+
protected Object getTargetPayload(MethodParameter parameter, Message<?> message) {
98+
Class<?> sourceClass = message.getPayload().getClass();
99+
Class<?> targetClass = parameter.getParameterType();
100+
if (ClassUtils.isAssignable(targetClass,sourceClass)) {
101+
return message.getPayload();
102+
}
103+
return this.converter.fromMessage(message, targetClass);
104+
}
105+
106+
/**
107+
* Specify if the given {@code payload} is empty.
108+
* @param payload the payload to check (can be {@code null})
109+
*/
110+
protected boolean isEmptyPayload(Object payload) {
111+
if (payload == null) {
112+
return true;
113+
}
114+
else if (payload instanceof byte[]) {
115+
return ((byte[]) payload).length == 0;
98116
}
99117
else if (payload instanceof String) {
100118
return ((String) payload).trim().equals("");
@@ -128,4 +146,19 @@ else if (this.validator != null) {
128146
}
129147
}
130148

149+
private String createPayloadRequiredExceptionMessage(MethodParameter parameter, Object payload) {
150+
String name = parameter.getParameterName() != null
151+
? parameter.getParameterName() : "arg" + parameter.getParameterIndex();
152+
StringBuilder sb = new StringBuilder("Payload parameter '").append(name)
153+
.append(" at index ").append(parameter.getParameterIndex()).append(" ");
154+
if (payload == null) {
155+
sb.append("could not be converted to '").append(parameter.getParameterType().getName())
156+
.append("' and is required");
157+
}
158+
else {
159+
sb.append("is required");
160+
}
161+
return sb.toString();
162+
}
163+
131164
}

‎spring-messaging/src/test/java/org/springframework/messaging/handler/annotation/support/PayloadArgumentResolverTests.java‎

Lines changed: 98 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -16,77 +16,105 @@
1616

1717
package org.springframework.messaging.handler.annotation.support;
1818

19+
import static org.junit.Assert.*;
20+
1921
import java.lang.reflect.Method;
22+
import java.util.Locale;
2023

2124
import org.junit.Before;
25+
import org.junit.Rule;
2226
import org.junit.Test;
27+
import org.junit.rules.ExpectedException;
28+
2329
import org.springframework.core.LocalVariableTableParameterNameDiscoverer;
2430
import org.springframework.core.MethodParameter;
2531
import org.springframework.messaging.Message;
32+
import org.springframework.messaging.converter.StringMessageConverter;
2633
import org.springframework.messaging.handler.annotation.Payload;
2734
import org.springframework.messaging.support.MessageBuilder;
28-
import org.springframework.messaging.converter.StringMessageConverter;
29-
import org.springframework.util.StringUtils;
35+
import org.springframework.util.Assert;
3036
import org.springframework.validation.Errors;
3137
import org.springframework.validation.Validator;
3238
import org.springframework.validation.annotation.Validated;
3339

34-
import static org.junit.Assert.*;
35-
3640
/**
3741
* Test fixture for {@link PayloadArgumentResolver}.
3842
*
3943
* @author Rossen Stoyanchev
4044
* @author Brian Clozel
45+
* @author Stephane Nicoll
4146
*/
4247
public class PayloadArgumentResolverTests {
4348

49+
@Rule
50+
public final ExpectedException thrown = ExpectedException.none();
51+
4452
private PayloadArgumentResolver resolver;
4553

46-
private MethodParameter param;
47-
private MethodParameter paramNotRequired;
48-
private MethodParameter paramWithSpelExpression;
54+
private Method payloadMethod;
55+
private Method simpleMethod;
4956
private MethodParameter paramValidated;
5057

51-
5258
@Before
5359
public void setup() throws Exception {
54-
5560
this.resolver = new PayloadArgumentResolver(new StringMessageConverter(), testValidator());
5661

57-
Method method = PayloadArgumentResolverTests.class.getDeclaredMethod("handleMessage",
58-
String.class, String.class, String.class, String.class);
62+
payloadMethod = PayloadArgumentResolverTests.class.getDeclaredMethod("handleMessage",
63+
String.class, String.class, Locale.class, String.class, String.class);
64+
simpleMethod = PayloadArgumentResolverTests.class.getDeclaredMethod("handleAnotherMessage",
65+
String.class, String.class);
5966

60-
this.param = new MethodParameter(method , 0);
61-
this.paramNotRequired = new MethodParameter(method , 1);
62-
this.paramWithSpelExpression = new MethodParameter(method , 2);
63-
this.paramValidated = new MethodParameter(method , 3);
67+
this.paramValidated = getMethodParameter(payloadMethod, 4);
6468
this.paramValidated.initParameterNameDiscovery(new LocalVariableTableParameterNameDiscoverer());
6569
}
6670

6771

6872
@Test
6973
public void resolveRequired() throws Exception {
7074
Message<?> message = MessageBuilder.withPayload("ABC".getBytes()).build();
71-
Object actual = this.resolver.resolveArgument(this.param, message);
75+
Object actual = this.resolver.resolveArgument(getMethodParameter(payloadMethod, 0), message);
7276

7377
assertEquals("ABC", actual);
7478
}
7579

80+
@Test
81+
public void resolveRequiredEmpty() throws Exception {
82+
Message<?> message = MessageBuilder.withPayload("").build();
83+
84+
thrown.expect(MethodArgumentNotValidException.class); // Required but empty
85+
this.resolver.resolveArgument(getMethodParameter(payloadMethod, 0), message);
86+
}
87+
7688
@Test
7789
public void resolveNotRequired() throws Exception {
90+
MethodParameter paramNotRequired = getMethodParameter(payloadMethod, 1);
7891

7992
Message<?> emptyByteArrayMessage = MessageBuilder.withPayload(new byte[0]).build();
80-
assertNull(this.resolver.resolveArgument(this.paramNotRequired, emptyByteArrayMessage));
93+
assertNull(this.resolver.resolveArgument(paramNotRequired, emptyByteArrayMessage));
94+
95+
Message<?> emptyStringMessage = MessageBuilder.withPayload("").build();
96+
assertNull(this.resolver.resolveArgument(paramNotRequired, emptyStringMessage));
8197

8298
Message<?> notEmptyMessage = MessageBuilder.withPayload("ABC".getBytes()).build();
83-
assertEquals("ABC", this.resolver.resolveArgument(this.paramNotRequired, notEmptyMessage));
99+
assertEquals("ABC", this.resolver.resolveArgument(paramNotRequired, notEmptyMessage));
84100
}
85101

86-
@Test(expected=IllegalStateException.class)
102+
@Test
103+
public void resolveNonConvertibleParam() throws Exception {
104+
Message<?> notEmptyMessage = MessageBuilder.withPayload(123).build();
105+
106+
// Could not convert from int to Locale so will be "empty" after conversion
107+
thrown.expect(MethodArgumentNotValidException.class);
108+
thrown.expectMessage(Locale.class.getName()); // reference to the type that could not be converted
109+
this.resolver.resolveArgument(getMethodParameter(payloadMethod, 2), notEmptyMessage);
110+
}
111+
112+
@Test
87113
public void resolveSpelExpressionNotSupported() throws Exception {
88114
Message<?> message = MessageBuilder.withPayload("ABC".getBytes()).build();
89-
this.resolver.resolveArgument(this.paramWithSpelExpression, message);
115+
116+
thrown.expect(IllegalStateException.class);
117+
this.resolver.resolveArgument(getMethodParameter(payloadMethod, 3), message);
90118
}
91119

92120
@Test
@@ -95,12 +123,46 @@ public void resolveValidation() throws Exception {
95123
this.resolver.resolveArgument(this.paramValidated, message);
96124
}
97125

98-
@Test(expected=MethodArgumentNotValidException.class)
126+
@Test
99127
public void resolveFailValidation() throws Exception {
100-
Message<?> message = MessageBuilder.withPayload("".getBytes()).build();
128+
// See testValidator()
129+
Message<?> message = MessageBuilder.withPayload("invalidValue".getBytes()).build();
130+
131+
thrown.expect(MethodArgumentNotValidException.class);
101132
this.resolver.resolveArgument(this.paramValidated, message);
102133
}
103134

135+
@Test
136+
public void resolveFailValidationNoConversionNecessary() throws Exception {
137+
Message<?> message = MessageBuilder.withPayload("invalidValue").build();
138+
139+
thrown.expect(MethodArgumentNotValidException.class);
140+
this.resolver.resolveArgument(this.paramValidated, message);
141+
}
142+
143+
@Test
144+
public void resolveNonAnnotatedParameter() throws Exception {
145+
MethodParameter paramNotRequired = getMethodParameter(simpleMethod, 0);
146+
147+
Message<?> emptyByteArrayMessage = MessageBuilder.withPayload(new byte[0]).build();
148+
assertEquals("", this.resolver.resolveArgument(paramNotRequired, emptyByteArrayMessage));
149+
150+
Message<?> emptyStringMessage = MessageBuilder.withPayload("").build();
151+
assertEquals("", this.resolver.resolveArgument(paramNotRequired, emptyStringMessage));
152+
153+
154+
Message<?> notEmptyMessage = MessageBuilder.withPayload("ABC".getBytes()).build();
155+
assertEquals("ABC", this.resolver.resolveArgument(paramNotRequired, notEmptyMessage));
156+
}
157+
158+
@Test
159+
public void resolveNonAnnotatedParameterFailValidation() throws Exception {
160+
// See testValidator()
161+
Message<?> message = MessageBuilder.withPayload("invalidValue".getBytes()).build();
162+
163+
assertEquals("invalidValue", this.resolver.resolveArgument(getMethodParameter(simpleMethod, 1), message));
164+
}
165+
104166
private Validator testValidator() {
105167

106168
return new Validator() {
@@ -111,19 +173,31 @@ public boolean supports(Class<?> clazz) {
111173
@Override
112174
public void validate(Object target, Errors errors) {
113175
String value = (String) target;
114-
if (StringUtils.isEmpty(value.toString())) {
115-
errors.reject("empty value");
176+
if ("invalidValue".equals(value)) {
177+
errors.reject("invalid value");
116178
}
117179
}
118180
};
119181
}
120182

183+
private MethodParameter getMethodParameter(Method method, int index) {
184+
Assert.notNull(method, "Method must be set");
185+
return new MethodParameter(method, index);
186+
}
187+
121188
@SuppressWarnings("unused")
122189
private void handleMessage(
123190
@Payload String param,
124191
@Payload(required=false) String paramNotRequired,
192+
@Payload(required=true) Locale nonConvertibleRequiredParam,
125193
@Payload("foo.bar") String paramWithSpelExpression,
126194
@Validated @Payload String validParam) {
127195
}
128196

197+
@SuppressWarnings("unused")
198+
private void handleAnotherMessage(
199+
String param,
200+
@Validated String validParam) {
201+
}
202+
129203
}

0 commit comments

Comments
 (0)