From 4ddd9572f70490bd8e1bc86bc89a74ea8391e942 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 (cherry picked from commit cfc560c) --- .../invocation/InvocableHandlerMethod.java | 6 +- .../ServletInvocableHandlerMethod.java | 14 +-- .../ServletInvocableHandlerMethodTests.java | 99 ++++++++++++++----- 3 files changed, 81 insertions(+), 38 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 182e6d9224a3..99fb8592a8e0 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 @@ -277,10 +277,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 05cbba3c0968..f843a58113bf 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -263,16 +263,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 431e4d5ccf4b..a1ba924624f1 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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,6 +56,7 @@ * * @author Rossen Stoyanchev * @author Sam Brannen + * @author Juergen Hoeller */ public class ServletInvocableHandlerMethodTests { @@ -117,19 +118,17 @@ 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); - assertTrue("When a status reason w/ used, the the request is handled", this.mavContainer.isRequestHandled()); + assertTrue("When a status reason w/ used, the request is handled", this.mavContainer.isRequestHandled()); assertEquals(HttpStatus.BAD_REQUEST.value(), this.response.getStatus()); 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()); @@ -160,28 +159,52 @@ 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 { - List> converters = new ArrayList>(); + @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 public void wrapConcurrentResult_ResponseEntity() throws Exception { - List> converters = new ArrayList>(); + List> converters = new ArrayList<>(); converters.add(new StringHttpMessageConverter()); this.returnValueHandlers.addHandler(new HttpEntityMethodProcessor(converters)); ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new ResponseEntityHandler(), "handleDeferred"); @@ -191,11 +214,9 @@ 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>(); + List> converters = new ArrayList<>(); converters.add(new StringHttpMessageConverter()); List advice = Collections.singletonList(mock(ResponseBodyAdvice.class)); HttpEntityMethodProcessor processor = new HttpEntityMethodProcessor(converters, null, advice); @@ -210,7 +231,7 @@ public void wrapConcurrentResult_ResponseEntityNullBody() throws Exception { @Test public void wrapConcurrentResult_ResponseEntityNullReturnValue() throws Exception { - List> converters = new ArrayList>(); + List> converters = new ArrayList<>(); converters.add(new StringHttpMessageConverter()); List advice = Collections.singletonList(mock(ResponseBodyAdvice.class)); HttpEntityMethodProcessor processor = new HttpEntityMethodProcessor(converters, null, advice); @@ -225,7 +246,7 @@ public void wrapConcurrentResult_ResponseEntityNullReturnValue() throws Exceptio @Test public void wrapConcurrentResult_ResponseBodyEmitter() throws Exception { - List> converters = new ArrayList>(); + List> converters = new ArrayList<>(); converters.add(new StringHttpMessageConverter()); this.returnValueHandlers.addHandler(new ResponseBodyEmitterReturnValueHandler(converters)); ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new AsyncHandler(), "handleWithEmitter"); @@ -247,9 +268,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)); @@ -272,6 +291,8 @@ private ServletInvocableHandlerMethod getHandlerMethod(Object controller, return handlerMethod; } + + @SuppressWarnings("unused") @ResponseStatus @Retention(RetentionPolicy.RUNTIME) @interface ComposedResponseStatus { @@ -280,6 +301,7 @@ private ServletInvocableHandlerMethod getHandlerMethod(Object controller, HttpStatus responseStatus() default HttpStatus.INTERNAL_SERVER_ERROR; } + @SuppressWarnings("unused") private static class Handler { @@ -311,6 +333,16 @@ public Object dynamicReturnValue(@RequestParam(required=false) String param) { } } + + @SuppressWarnings("unused") + @ResponseStatus(HttpStatus.BAD_REQUEST) + private static class ResponseStatusHandler { + + public void handle() { + } + } + + private static class MethodLevelResponseBodyHandler { @ResponseBody @@ -319,15 +351,30 @@ public DeferredResult handle() { } } + @SuppressWarnings("unused") @ResponseBody private static class TypeLevelResponseBodyHandler { public DeferredResult handle() { - return new DeferredResult(); + return new DeferredResult<>(); } } + + private static class DeferredResultSubclassHandler { + + @ResponseBody + public CustomDeferredResult handle() { + return new CustomDeferredResult(); + } + } + + + private static class CustomDeferredResult extends DeferredResult { + } + + @SuppressWarnings("unused") private static class ResponseEntityHandler { @@ -340,6 +387,7 @@ public ResponseEntity handleRawType() { } } + private static class ExceptionRaisingReturnValueHandler implements HandlerMethodReturnValueHandler { @Override @@ -354,6 +402,7 @@ public void handleReturnValue(Object returnValue, MethodParameter returnType, } } + @SuppressWarnings("unused") private static class AsyncHandler { @@ -366,4 +415,4 @@ public StreamingResponseBody handleWithStreaming() { } } -} \ No newline at end of file +}