From cfc560c4c460ed0ee465182d715339bd6e81c84b Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Sat, 2 Jul 2016 12:55:30 +0200 Subject: [PATCH] Leniently accept custom DeferredResult etc subclasses for null values Issue: SPR-14423 --- .../invocation/InvocableHandlerMethod.java | 6 +- .../ServletInvocableHandlerMethod.java | 12 +-- .../ServletInvocableHandlerMethodTests.java | 74 ++++++++++++++----- 3 files changed, 63 insertions(+), 29 deletions(-) diff --git a/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/InvocableHandlerMethod.java b/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/InvocableHandlerMethod.java index 03e6fbeab72c..82932ca8a682 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/InvocableHandlerMethod.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/InvocableHandlerMethod.java @@ -283,10 +283,10 @@ public Class getParameterType() { if (this.returnValue != null) { return this.returnValue.getClass(); } - if (ResolvableType.NONE.equals(this.returnType)) { - throw new IllegalArgumentException("Expected Future-like type with generic parameter"); + if (!ResolvableType.NONE.equals(this.returnType)) { + return this.returnType.getRawClass(); } - return this.returnType.getRawClass(); + return super.getParameterType(); } @Override diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java index 362e74961f21..d8c19445759c 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java @@ -281,16 +281,10 @@ public Class getParameterType() { if (this.returnValue != null) { return this.returnValue.getClass(); } - Class parameterType = super.getParameterType(); - if (ResponseBodyEmitter.class.isAssignableFrom(parameterType) || - StreamingResponseBody.class.isAssignableFrom(parameterType)) { - return parameterType; + if (!ResolvableType.NONE.equals(this.returnType)) { + return this.returnType.getRawClass(); } - if (ResolvableType.NONE.equals(this.returnType)) { - throw new IllegalArgumentException("Expected one of Callable, DeferredResult, or ListenableFuture: " + - super.getParameterType()); - } - return this.returnType.getRawClass(); + return super.getParameterType(); } @Override diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethodTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethodTests.java index af00f2ad789b..b2e886a9bc5a 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethodTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethodTests.java @@ -56,6 +56,7 @@ * * @author Rossen Stoyanchev * @author Sam Brannen + * @author Juergen Hoeller */ public class ServletInvocableHandlerMethodTests { @@ -126,9 +127,7 @@ public void invokeAndHandle_VoidRequestNotModified() throws Exception { this.mavContainer.isRequestHandled()); } - // SPR-9159 - - @Test + @Test // SPR-9159 public void invokeAndHandle_NotVoidWithResponseStatusAndReason() throws Exception { ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new Handler(), "responseStatusWithReason"); handlerMethod.invokeAndHandle(this.webRequest, this.mavContainer); @@ -138,7 +137,7 @@ public void invokeAndHandle_NotVoidWithResponseStatusAndReason() throws Exceptio assertEquals("400 Bad Request", this.response.getErrorMessage()); } - @Test(expected=HttpMessageNotWritableException.class) + @Test(expected = HttpMessageNotWritableException.class) public void invokeAndHandle_Exception() throws Exception { this.returnValueHandlers.addHandler(new ExceptionRaisingReturnValueHandler()); @@ -169,23 +168,47 @@ public void invokeAndHandle_DynamicReturnValue() throws Exception { @Test public void wrapConcurrentResult_MethodLevelResponseBody() throws Exception { - wrapConcurrentResult_ResponseBody(new MethodLevelResponseBodyHandler()); + wrapConcurrentResult_ResponseBody(new MethodLevelResponseBodyHandler(), "bar", String.class); + } + + @Test + public void wrapConcurrentResult_MethodLevelResponseBodyEmpty() throws Exception { + wrapConcurrentResult_ResponseBody(new MethodLevelResponseBodyHandler(), null, String.class); } @Test public void wrapConcurrentResult_TypeLevelResponseBody() throws Exception { - wrapConcurrentResult_ResponseBody(new TypeLevelResponseBodyHandler()); + wrapConcurrentResult_ResponseBody(new TypeLevelResponseBodyHandler(), "bar", String.class); + } + + @Test + public void wrapConcurrentResult_TypeLevelResponseBodyEmpty() throws Exception { + wrapConcurrentResult_ResponseBody(new TypeLevelResponseBodyHandler(), null, String.class); + } + + @Test + public void wrapConcurrentResult_DeferredResultSubclass() throws Exception { + wrapConcurrentResult_ResponseBody(new DeferredResultSubclassHandler(), "bar", String.class); } - private void wrapConcurrentResult_ResponseBody(Object handler) throws Exception { + @Test + public void wrapConcurrentResult_DeferredResultSubclassEmpty() throws Exception { + wrapConcurrentResult_ResponseBody(new DeferredResultSubclassHandler(), null, CustomDeferredResult.class); + } + + private void wrapConcurrentResult_ResponseBody(Object handler, Object result, Class expectedReturnType) + throws Exception { + List> converters = new ArrayList<>(); converters.add(new StringHttpMessageConverter()); + this.returnValueHandlers.addHandler(new ModelAndViewMethodReturnValueHandler()); this.returnValueHandlers.addHandler(new RequestResponseBodyMethodProcessor(converters)); ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(handler, "handle"); - handlerMethod = handlerMethod.wrapConcurrentResult("bar"); - handlerMethod.invokeAndHandle(this.webRequest, this.mavContainer); - assertEquals("bar", this.response.getContentAsString()); + handlerMethod = handlerMethod.wrapConcurrentResult(result); + handlerMethod.invokeAndHandle(this.webRequest, this.mavContainer); + assertEquals((result != null ? result.toString() : ""), this.response.getContentAsString()); + assertEquals(expectedReturnType, handlerMethod.getReturnValueType(result).getParameterType()); } @Test @@ -200,9 +223,7 @@ public void wrapConcurrentResult_ResponseEntity() throws Exception { assertEquals("bar", this.response.getContentAsString()); } - // SPR-12287 - - @Test + @Test // SPR-12287 public void wrapConcurrentResult_ResponseEntityNullBody() throws Exception { List> converters = new ArrayList<>(); converters.add(new StringHttpMessageConverter()); @@ -256,9 +277,7 @@ public void wrapConcurrentResult_StreamingResponseBody() throws Exception { assertEquals("", this.response.getContentAsString()); } - // SPR-12287 (16/Oct/14 comments) - - @Test + @Test // SPR-12287 (16/Oct/14 comments) public void responseEntityRawTypeWithNullBody() throws Exception { List> converters = Collections.singletonList(new StringHttpMessageConverter()); List advice = Collections.singletonList(mock(ResponseBodyAdvice.class)); @@ -281,6 +300,7 @@ private ServletInvocableHandlerMethod getHandlerMethod(Object controller, return handlerMethod; } + @SuppressWarnings("unused") @ResponseStatus @Retention(RetentionPolicy.RUNTIME) @@ -290,6 +310,7 @@ private ServletInvocableHandlerMethod getHandlerMethod(Object controller, HttpStatus responseStatus() default HttpStatus.INTERNAL_SERVER_ERROR; } + @SuppressWarnings("unused") private static class Handler { @@ -321,6 +342,7 @@ public Object dynamicReturnValue(@RequestParam(required=false) String param) { } } + @SuppressWarnings("unused") @ResponseStatus(HttpStatus.BAD_REQUEST) private static class ResponseStatusHandler { @@ -329,6 +351,7 @@ public void handle() { } } + private static class MethodLevelResponseBodyHandler { @ResponseBody @@ -337,6 +360,7 @@ public DeferredResult handle() { } } + @SuppressWarnings("unused") @ResponseBody private static class TypeLevelResponseBodyHandler { @@ -346,6 +370,20 @@ public DeferredResult handle() { } } + + private static class DeferredResultSubclassHandler { + + @ResponseBody + public CustomDeferredResult handle() { + return new CustomDeferredResult(); + } + } + + + private static class CustomDeferredResult extends DeferredResult { + } + + @SuppressWarnings("unused") private static class ResponseEntityHandler { @@ -358,6 +396,7 @@ public ResponseEntity handleRawType() { } } + private static class ExceptionRaisingReturnValueHandler implements HandlerMethodReturnValueHandler { @Override @@ -372,6 +411,7 @@ public void handleReturnValue(Object returnValue, MethodParameter returnType, } } + @SuppressWarnings("unused") private static class AsyncHandler { @@ -384,4 +424,4 @@ public StreamingResponseBody handleWithStreaming() { } } -} \ No newline at end of file +}