diff --git a/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/support/MethodArgumentNotValidException.java b/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/support/MethodArgumentNotValidException.java index 06d0f838a7..e463fcaccb 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/support/MethodArgumentNotValidException.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/support/MethodArgumentNotValidException.java @@ -23,31 +23,39 @@ import org.springframework.validation.BindingResult; import org.springframework.validation.ObjectError; /** - * Exception to be thrown when validation on an method parameter annotated with {@code @Valid} fails. + * Exception to be thrown when a method argument is not valid. For instance, this + * can be issued if a validation on a method parameter annotated with + * {@code @Valid} fails. + * * @author Brian Clozel * @since 4.0.1 */ @SuppressWarnings("serial") public class MethodArgumentNotValidException extends MessagingException { - private final MethodParameter parameter; - - private final BindingResult bindingResult; - + /** + * Create a new message with the given description. + * @see #getMessage() + */ + public MethodArgumentNotValidException(Message message, String description) { + super(message, description); + } - public MethodArgumentNotValidException(Message message, MethodParameter parameter, BindingResult bindingResult) { - super(message); - this.parameter = parameter; - this.bindingResult = bindingResult; + /** + * Create a new instance with a failed validation described by + * the given {@link BindingResult}. + */ + public MethodArgumentNotValidException(Message message, + MethodParameter parameter, BindingResult bindingResult) { + this(message, generateMessage(parameter, bindingResult)); } - @Override - public String getMessage() { + private static String generateMessage(MethodParameter parameter, BindingResult bindingResult) { StringBuilder sb = new StringBuilder("Validation failed for parameter at index ") - .append(this.parameter.getParameterIndex()).append(" in method: ") - .append(this.parameter.getMethod().toGenericString()) - .append(", with ").append(this.bindingResult.getErrorCount()).append(" error(s): "); - for (ObjectError error : this.bindingResult.getAllErrors()) { + .append(parameter.getParameterIndex()).append(" in method: ") + .append(parameter.getMethod().toGenericString()) + .append(", with ").append(bindingResult.getErrorCount()).append(" error(s): "); + for (ObjectError error : bindingResult.getAllErrors()) { sb.append("[").append(error).append("] "); } return sb.toString(); diff --git a/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/support/PayloadArgumentResolver.java b/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/support/PayloadArgumentResolver.java index ba1fa9c8eb..c8cafcd09e 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/support/PayloadArgumentResolver.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/handler/annotation/support/PayloadArgumentResolver.java @@ -42,6 +42,7 @@ import java.lang.annotation.Annotation; * * @author Rossen Stoyanchev * @author Brian Clozel + * @author Stephane Nicoll * @since 4.0 */ public class PayloadArgumentResolver implements HandlerMethodArgumentResolver { @@ -65,36 +66,53 @@ public class PayloadArgumentResolver implements HandlerMethodArgumentResolver { @Override public Object resolveArgument(MethodParameter parameter, Message message) throws Exception { - - Class sourceClass = message.getPayload().getClass(); - Class targetClass = parameter.getParameterType(); - - if (ClassUtils.isAssignable(targetClass,sourceClass)) { - return message.getPayload(); - } - Payload annot = parameter.getParameterAnnotation(Payload.class); + if ((annot != null) && StringUtils.hasText(annot.value())) { + throw new IllegalStateException("@Payload SpEL expressions not supported by this resolver."); + } - if (isEmptyPayload(message)) { - if ((annot != null) && !annot.required()) { + Object target = getTargetPayload(parameter, message); + if (annot != null && isEmptyPayload(target)) { + if (annot.required()) { + throw new MethodArgumentNotValidException(message, createPayloadRequiredExceptionMessage(parameter, target)); + } + else { return null; } } - if ((annot != null) && StringUtils.hasText(annot.value())) { - throw new IllegalStateException("@Payload SpEL expressions not supported by this resolver."); + if (annot != null) { // Only validate @Payload + validate(message, parameter, target); } - - Object target = this.converter.fromMessage(message, targetClass); - validate(message, parameter, target); - return target; } - protected boolean isEmptyPayload(Message message) { - Object payload = message.getPayload(); - if (payload instanceof byte[]) { - return ((byte[]) message.getPayload()).length == 0; + /** + * Return the target payload to handle for the specified message. Can either + * be the payload itself if the parameter type supports it or the converted + * one otherwise. While the payload of a {@link Message} cannot be null by + * design, this method may return a {@code null} payload if the conversion + * result is {@code null}. + */ + protected Object getTargetPayload(MethodParameter parameter, Message message) { + Class sourceClass = message.getPayload().getClass(); + Class targetClass = parameter.getParameterType(); + if (ClassUtils.isAssignable(targetClass,sourceClass)) { + return message.getPayload(); + } + return this.converter.fromMessage(message, targetClass); + } + + /** + * Specify if the given {@code payload} is empty. + * @param payload the payload to check (can be {@code null}) + */ + protected boolean isEmptyPayload(Object payload) { + if (payload == null) { + return true; + } + else if (payload instanceof byte[]) { + return ((byte[]) payload).length == 0; } else if (payload instanceof String) { return ((String) payload).trim().equals(""); @@ -128,4 +146,19 @@ public class PayloadArgumentResolver implements HandlerMethodArgumentResolver { } } + private String createPayloadRequiredExceptionMessage(MethodParameter parameter, Object payload) { + String name = parameter.getParameterName() != null + ? parameter.getParameterName() : "arg" + parameter.getParameterIndex(); + StringBuilder sb = new StringBuilder("Payload parameter '").append(name) + .append(" at index ").append(parameter.getParameterIndex()).append(" "); + if (payload == null) { + sb.append("could not be converted to '").append(parameter.getParameterType().getName()) + .append("' and is required"); + } + else { + sb.append("is required"); + } + return sb.toString(); + } + } diff --git a/spring-messaging/src/test/java/org/springframework/messaging/handler/annotation/support/PayloadArgumentResolverTests.java b/spring-messaging/src/test/java/org/springframework/messaging/handler/annotation/support/PayloadArgumentResolverTests.java index acf88bcf8c..9928988d8a 100644 --- a/spring-messaging/src/test/java/org/springframework/messaging/handler/annotation/support/PayloadArgumentResolverTests.java +++ b/spring-messaging/src/test/java/org/springframework/messaging/handler/annotation/support/PayloadArgumentResolverTests.java @@ -16,51 +16,55 @@ package org.springframework.messaging.handler.annotation.support; +import static org.junit.Assert.*; + import java.lang.reflect.Method; +import java.util.Locale; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; + import org.springframework.core.LocalVariableTableParameterNameDiscoverer; import org.springframework.core.MethodParameter; import org.springframework.messaging.Message; +import org.springframework.messaging.converter.StringMessageConverter; import org.springframework.messaging.handler.annotation.Payload; import org.springframework.messaging.support.MessageBuilder; -import org.springframework.messaging.converter.StringMessageConverter; -import org.springframework.util.StringUtils; +import org.springframework.util.Assert; import org.springframework.validation.Errors; import org.springframework.validation.Validator; import org.springframework.validation.annotation.Validated; -import static org.junit.Assert.*; - /** * Test fixture for {@link PayloadArgumentResolver}. * * @author Rossen Stoyanchev * @author Brian Clozel + * @author Stephane Nicoll */ public class PayloadArgumentResolverTests { + @Rule + public final ExpectedException thrown = ExpectedException.none(); + private PayloadArgumentResolver resolver; - private MethodParameter param; - private MethodParameter paramNotRequired; - private MethodParameter paramWithSpelExpression; + private Method payloadMethod; + private Method simpleMethod; private MethodParameter paramValidated; - @Before public void setup() throws Exception { - this.resolver = new PayloadArgumentResolver(new StringMessageConverter(), testValidator()); - Method method = PayloadArgumentResolverTests.class.getDeclaredMethod("handleMessage", - String.class, String.class, String.class, String.class); + payloadMethod = PayloadArgumentResolverTests.class.getDeclaredMethod("handleMessage", + String.class, String.class, Locale.class, String.class, String.class); + simpleMethod = PayloadArgumentResolverTests.class.getDeclaredMethod("handleAnotherMessage", + String.class, String.class); - this.param = new MethodParameter(method , 0); - this.paramNotRequired = new MethodParameter(method , 1); - this.paramWithSpelExpression = new MethodParameter(method , 2); - this.paramValidated = new MethodParameter(method , 3); + this.paramValidated = getMethodParameter(payloadMethod, 4); this.paramValidated.initParameterNameDiscovery(new LocalVariableTableParameterNameDiscoverer()); } @@ -68,25 +72,49 @@ public class PayloadArgumentResolverTests { @Test public void resolveRequired() throws Exception { Message message = MessageBuilder.withPayload("ABC".getBytes()).build(); - Object actual = this.resolver.resolveArgument(this.param, message); + Object actual = this.resolver.resolveArgument(getMethodParameter(payloadMethod, 0), message); assertEquals("ABC", actual); } + @Test + public void resolveRequiredEmpty() throws Exception { + Message message = MessageBuilder.withPayload("").build(); + + thrown.expect(MethodArgumentNotValidException.class); // Required but empty + this.resolver.resolveArgument(getMethodParameter(payloadMethod, 0), message); + } + @Test public void resolveNotRequired() throws Exception { + MethodParameter paramNotRequired = getMethodParameter(payloadMethod, 1); Message emptyByteArrayMessage = MessageBuilder.withPayload(new byte[0]).build(); - assertNull(this.resolver.resolveArgument(this.paramNotRequired, emptyByteArrayMessage)); + assertNull(this.resolver.resolveArgument(paramNotRequired, emptyByteArrayMessage)); + + Message emptyStringMessage = MessageBuilder.withPayload("").build(); + assertNull(this.resolver.resolveArgument(paramNotRequired, emptyStringMessage)); Message notEmptyMessage = MessageBuilder.withPayload("ABC".getBytes()).build(); - assertEquals("ABC", this.resolver.resolveArgument(this.paramNotRequired, notEmptyMessage)); + assertEquals("ABC", this.resolver.resolveArgument(paramNotRequired, notEmptyMessage)); } - @Test(expected=IllegalStateException.class) + @Test + public void resolveNonConvertibleParam() throws Exception { + Message notEmptyMessage = MessageBuilder.withPayload(123).build(); + + // Could not convert from int to Locale so will be "empty" after conversion + thrown.expect(MethodArgumentNotValidException.class); + thrown.expectMessage(Locale.class.getName()); // reference to the type that could not be converted + this.resolver.resolveArgument(getMethodParameter(payloadMethod, 2), notEmptyMessage); + } + + @Test public void resolveSpelExpressionNotSupported() throws Exception { Message message = MessageBuilder.withPayload("ABC".getBytes()).build(); - this.resolver.resolveArgument(this.paramWithSpelExpression, message); + + thrown.expect(IllegalStateException.class); + this.resolver.resolveArgument(getMethodParameter(payloadMethod, 3), message); } @Test @@ -95,12 +123,46 @@ public class PayloadArgumentResolverTests { this.resolver.resolveArgument(this.paramValidated, message); } - @Test(expected=MethodArgumentNotValidException.class) + @Test public void resolveFailValidation() throws Exception { - Message message = MessageBuilder.withPayload("".getBytes()).build(); + // See testValidator() + Message message = MessageBuilder.withPayload("invalidValue".getBytes()).build(); + + thrown.expect(MethodArgumentNotValidException.class); this.resolver.resolveArgument(this.paramValidated, message); } + @Test + public void resolveFailValidationNoConversionNecessary() throws Exception { + Message message = MessageBuilder.withPayload("invalidValue").build(); + + thrown.expect(MethodArgumentNotValidException.class); + this.resolver.resolveArgument(this.paramValidated, message); + } + + @Test + public void resolveNonAnnotatedParameter() throws Exception { + MethodParameter paramNotRequired = getMethodParameter(simpleMethod, 0); + + Message emptyByteArrayMessage = MessageBuilder.withPayload(new byte[0]).build(); + assertEquals("", this.resolver.resolveArgument(paramNotRequired, emptyByteArrayMessage)); + + Message emptyStringMessage = MessageBuilder.withPayload("").build(); + assertEquals("", this.resolver.resolveArgument(paramNotRequired, emptyStringMessage)); + + + Message notEmptyMessage = MessageBuilder.withPayload("ABC".getBytes()).build(); + assertEquals("ABC", this.resolver.resolveArgument(paramNotRequired, notEmptyMessage)); + } + + @Test + public void resolveNonAnnotatedParameterFailValidation() throws Exception { + // See testValidator() + Message message = MessageBuilder.withPayload("invalidValue".getBytes()).build(); + + assertEquals("invalidValue", this.resolver.resolveArgument(getMethodParameter(simpleMethod, 1), message)); + } + private Validator testValidator() { return new Validator() { @@ -111,19 +173,31 @@ public class PayloadArgumentResolverTests { @Override public void validate(Object target, Errors errors) { String value = (String) target; - if (StringUtils.isEmpty(value.toString())) { - errors.reject("empty value"); + if ("invalidValue".equals(value)) { + errors.reject("invalid value"); } } }; } + private MethodParameter getMethodParameter(Method method, int index) { + Assert.notNull(method, "Method must be set"); + return new MethodParameter(method, index); + } + @SuppressWarnings("unused") private void handleMessage( @Payload String param, @Payload(required=false) String paramNotRequired, + @Payload(required=true) Locale nonConvertibleRequiredParam, @Payload("foo.bar") String paramWithSpelExpression, @Validated @Payload String validParam) { } + @SuppressWarnings("unused") + private void handleAnotherMessage( + String param, + @Validated String validParam) { + } + } diff --git a/spring-messaging/src/test/java/org/springframework/messaging/simp/annotation/support/SimpAnnotationMethodMessageHandlerTests.java b/spring-messaging/src/test/java/org/springframework/messaging/simp/annotation/support/SimpAnnotationMethodMessageHandlerTests.java index b118c6dcbf..559892273c 100644 --- a/spring-messaging/src/test/java/org/springframework/messaging/simp/annotation/support/SimpAnnotationMethodMessageHandlerTests.java +++ b/spring-messaging/src/test/java/org/springframework/messaging/simp/annotation/support/SimpAnnotationMethodMessageHandlerTests.java @@ -35,7 +35,6 @@ import org.springframework.messaging.simp.SimpMessagingTemplate; import org.springframework.messaging.simp.annotation.SubscribeMapping; import org.springframework.messaging.support.MessageBuilder; import org.springframework.stereotype.Controller; -import org.springframework.util.StringUtils; import org.springframework.validation.Errors; import org.springframework.validation.Validator; import org.springframework.validation.annotation.Validated; @@ -51,6 +50,8 @@ import static org.junit.Assert.*; */ public class SimpAnnotationMethodMessageHandlerTests { + private static final String TEST_INVALID_VALUE = "invalidValue"; + private TestSimpAnnotationMethodMessageHandler messageHandler; private TestController testController; @@ -64,7 +65,7 @@ public class SimpAnnotationMethodMessageHandlerTests { this.messageHandler = new TestSimpAnnotationMethodMessageHandler(brokerTemplate, channel, channel); this.messageHandler.setApplicationContext(new StaticApplicationContext()); - this.messageHandler.setValidator(new StringNotEmptyValidator()); + this.messageHandler.setValidator(new StringTestValidator(TEST_INVALID_VALUE)); this.messageHandler.afterPropertiesSet(); testController = new TestController(); @@ -126,7 +127,7 @@ public class SimpAnnotationMethodMessageHandlerTests { public void validationError() { SimpMessageHeaderAccessor headers = SimpMessageHeaderAccessor.create(); headers.setDestination("/pre/validation/payload"); - Message message = MessageBuilder.withPayload(new byte[0]).setHeaders(headers).build(); + Message message = MessageBuilder.withPayload(TEST_INVALID_VALUE.getBytes()).setHeaders(headers).build(); this.messageHandler.handleMessage(message); assertEquals("handleValidationException", this.testController.method); } @@ -196,7 +197,14 @@ public class SimpAnnotationMethodMessageHandlerTests { } } - private static class StringNotEmptyValidator implements Validator { + private static class StringTestValidator implements Validator { + + private final String invalidValue; + + private StringTestValidator(String invalidValue) { + this.invalidValue = invalidValue; + } + @Override public boolean supports(Class clazz) { return String.class.isAssignableFrom(clazz); @@ -204,8 +212,8 @@ public class SimpAnnotationMethodMessageHandlerTests { @Override public void validate(Object target, Errors errors) { String value = (String) target; - if (StringUtils.isEmpty(value.toString())) { - errors.reject("empty value"); + if (invalidValue.equals(value)) { + errors.reject("invalid value '"+invalidValue+"'"); } } }