From ae1ed16cb89cffafb5d6e5ab3a090399d147e721 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Sun, 2 Apr 2017 20:30:24 -0400 Subject: [PATCH] Async return values refactoring in Spring MVC Revise Javadoc on AsyncHandlerMethodReturnValueHandler to clarify its main purpose is to prioritze custom async return value handlers ahead of built-in ones. Also replace the interface from built-in handlers which are prioritized already. Remove DeferredResultAdapter and ResponseBodyEmitterAdapter -- introduced in 4.3 for custom async return value handling, since for 5.0 we will add built-in support for reactive types and the value of these contracts becomes very marginal. Issue: SPR-15365 --- .../AsyncHandlerMethodReturnValueHandler.java | 24 ++- ...dlerMethodReturnValueHandlerComposite.java | 5 +- ...ethodReturnValueHandlerCompositeTests.java | 20 +-- .../AsyncTaskMethodReturnValueHandler.java | 9 +- .../CallableMethodReturnValueHandler.java | 11 +- .../annotation/DeferredResultAdapter.java | 36 ---- ...eferredResultMethodReturnValueHandler.java | 158 ++++++------------ .../ResponseBodyEmitterAdapter.java | 40 ----- ...ResponseBodyEmitterReturnValueHandler.java | 91 +++------- ...DeferredResultReturnValueHandlerTests.java | 140 ++++++---------- 10 files changed, 143 insertions(+), 391 deletions(-) delete mode 100644 spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/DeferredResultAdapter.java delete mode 100644 spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterAdapter.java diff --git a/spring-web/src/main/java/org/springframework/web/method/support/AsyncHandlerMethodReturnValueHandler.java b/spring-web/src/main/java/org/springframework/web/method/support/AsyncHandlerMethodReturnValueHandler.java index cade24384d..1ed8e1055c 100644 --- a/spring-web/src/main/java/org/springframework/web/method/support/AsyncHandlerMethodReturnValueHandler.java +++ b/spring-web/src/main/java/org/springframework/web/method/support/AsyncHandlerMethodReturnValueHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2017 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. @@ -19,20 +19,16 @@ package org.springframework.web.method.support; import org.springframework.core.MethodParameter; /** - * A {@link HandlerMethodReturnValueHandler} that handles return values that - * represent asynchronous computation. Such handlers need to be invoked with - * precedence over other handlers that might otherwise match the return value - * type: e.g. a method that returns a Promise type that is also annotated with - * {@code @ResponseBody}. + * A return value handler that supports async types. Such return value types + * need to be handled with priority so the async value can be "unwrapped". * - *

In {@link #handleReturnValue}, implementations of this class should create - * a {@link org.springframework.web.context.request.async.DeferredResult} or - * adapt to it and then invoke {@code WebAsyncManager} to start async processing. - * For example: - * 

- * DeferredResult deferredResult = (DeferredResult) returnValue;
- * WebAsyncUtils.getAsyncManager(webRequest).startDeferredResultProcessing(deferredResult, mavContainer);
- *
+ *

Note: implementing this contract is not required but it + * should be implemented when the handler needs to be prioritized ahead of others. + * For example custom (async) handlers, by default ordered after built-in + * handlers, should take precedence over {@code @ResponseBody} or + * {@code @ModelAttribute} handling, which should occur once the async value is + * ready. By contrast, built-in (async) handlers are already ordered ahead of + * sync handlers. * * @author Rossen Stoyanchev * @since 4.2 diff --git a/spring-web/src/main/java/org/springframework/web/method/support/HandlerMethodReturnValueHandlerComposite.java b/spring-web/src/main/java/org/springframework/web/method/support/HandlerMethodReturnValueHandlerComposite.java index ad23c2c927..7917e28cd7 100644 --- a/spring-web/src/main/java/org/springframework/web/method/support/HandlerMethodReturnValueHandlerComposite.java +++ b/spring-web/src/main/java/org/springframework/web/method/support/HandlerMethodReturnValueHandlerComposite.java @@ -33,7 +33,7 @@ import org.springframework.web.context.request.NativeWebRequest; * @author Rossen Stoyanchev * @since 3.1 */ -public class HandlerMethodReturnValueHandlerComposite implements AsyncHandlerMethodReturnValueHandler { +public class HandlerMethodReturnValueHandlerComposite implements HandlerMethodReturnValueHandler { protected final Log logger = LogFactory.getLog(getClass()); @@ -94,8 +94,7 @@ public class HandlerMethodReturnValueHandlerComposite implements AsyncHandlerMet return null; } - @Override - public boolean isAsyncReturnValue(Object value, MethodParameter returnType) { + private boolean isAsyncReturnValue(Object value, MethodParameter returnType) { for (HandlerMethodReturnValueHandler handler : this.returnValueHandlers) { if (handler instanceof AsyncHandlerMethodReturnValueHandler) { if (((AsyncHandlerMethodReturnValueHandler) handler).isAsyncReturnValue(value, returnType)) { diff --git a/spring-web/src/test/java/org/springframework/web/method/support/HandlerMethodReturnValueHandlerCompositeTests.java b/spring-web/src/test/java/org/springframework/web/method/support/HandlerMethodReturnValueHandlerCompositeTests.java index 806819dfc9..295469c798 100644 --- a/spring-web/src/test/java/org/springframework/web/method/support/HandlerMethodReturnValueHandlerCompositeTests.java +++ b/spring-web/src/test/java/org/springframework/web/method/support/HandlerMethodReturnValueHandlerCompositeTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2017 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. @@ -21,14 +21,12 @@ import org.junit.Test; import org.springframework.core.MethodParameter; -import static org.junit.Assert.*; -import static org.mockito.Mockito.*; - -import java.lang.annotation.Documented; -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; /** * Test fixture with {@link HandlerMethodReturnValueHandlerComposite}. @@ -86,9 +84,7 @@ public class HandlerMethodReturnValueHandlerCompositeTests { verifyNoMoreInteractions(anotherIntegerHandler); } - // SPR-13083 - - @Test + @Test // SPR-13083 public void handleReturnValueWithAsyncHandler() throws Exception { Promise promise = new Promise<>(); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AsyncTaskMethodReturnValueHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AsyncTaskMethodReturnValueHandler.java index 6691d65cc3..7923d98be1 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AsyncTaskMethodReturnValueHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AsyncTaskMethodReturnValueHandler.java @@ -21,7 +21,7 @@ import org.springframework.core.MethodParameter; import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.context.request.async.WebAsyncTask; import org.springframework.web.context.request.async.WebAsyncUtils; -import org.springframework.web.method.support.AsyncHandlerMethodReturnValueHandler; +import org.springframework.web.method.support.HandlerMethodReturnValueHandler; import org.springframework.web.method.support.ModelAndViewContainer; /** @@ -30,7 +30,7 @@ import org.springframework.web.method.support.ModelAndViewContainer; * @author Rossen Stoyanchev * @since 3.2 */ -public class AsyncTaskMethodReturnValueHandler implements AsyncHandlerMethodReturnValueHandler { +public class AsyncTaskMethodReturnValueHandler implements HandlerMethodReturnValueHandler { private final BeanFactory beanFactory; @@ -45,11 +45,6 @@ public class AsyncTaskMethodReturnValueHandler implements AsyncHandlerMethodRetu return WebAsyncTask.class.isAssignableFrom(returnType.getParameterType()); } - @Override - public boolean isAsyncReturnValue(Object returnValue, MethodParameter returnType) { - return (returnValue != null && returnValue instanceof WebAsyncTask); - } - @Override public void handleReturnValue(Object returnValue, MethodParameter returnType, ModelAndViewContainer mavContainer, NativeWebRequest webRequest) throws Exception { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/CallableMethodReturnValueHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/CallableMethodReturnValueHandler.java index cfd5dade84..1fdb978f7a 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/CallableMethodReturnValueHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/CallableMethodReturnValueHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2017 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. @@ -21,7 +21,7 @@ import java.util.concurrent.Callable; import org.springframework.core.MethodParameter; import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.context.request.async.WebAsyncUtils; -import org.springframework.web.method.support.AsyncHandlerMethodReturnValueHandler; +import org.springframework.web.method.support.HandlerMethodReturnValueHandler; import org.springframework.web.method.support.ModelAndViewContainer; /** @@ -30,18 +30,13 @@ import org.springframework.web.method.support.ModelAndViewContainer; * @author Rossen Stoyanchev * @since 3.2 */ -public class CallableMethodReturnValueHandler implements AsyncHandlerMethodReturnValueHandler { +public class CallableMethodReturnValueHandler implements HandlerMethodReturnValueHandler { @Override public boolean supportsReturnType(MethodParameter returnType) { return Callable.class.isAssignableFrom(returnType.getParameterType()); } - @Override - public boolean isAsyncReturnValue(Object returnValue, MethodParameter returnType) { - return (returnValue != null && returnValue instanceof Callable); - } - @Override public void handleReturnValue(Object returnValue, MethodParameter returnType, ModelAndViewContainer mavContainer, NativeWebRequest webRequest) throws Exception { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/DeferredResultAdapter.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/DeferredResultAdapter.java deleted file mode 100644 index a743b09e41..0000000000 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/DeferredResultAdapter.java +++ /dev/null @@ -1,36 +0,0 @@ -/* - * 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.web.servlet.mvc.method.annotation; - -import org.springframework.web.context.request.async.DeferredResult; - -/** - * Contract to adapt a single-value async return value to {@code DeferredResult}. - * - * @author Rossen Stoyanchev - * @since 4.3 - */ -public interface DeferredResultAdapter { - - /** - * Create a {@code DeferredResult} for the given return value. - * @param returnValue the return value (never {@code null}) - * @return the DeferredResult - */ - DeferredResult adaptToDeferredResult(Object returnValue); - -} diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/DeferredResultMethodReturnValueHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/DeferredResultMethodReturnValueHandler.java index 7337ef980b..15abd90fda 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/DeferredResultMethodReturnValueHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/DeferredResultMethodReturnValueHandler.java @@ -16,70 +16,34 @@ package org.springframework.web.servlet.mvc.method.annotation; -import java.util.HashMap; -import java.util.Map; import java.util.concurrent.CompletionStage; import java.util.function.BiFunction; import org.springframework.core.MethodParameter; -import org.springframework.util.Assert; import org.springframework.util.concurrent.ListenableFuture; import org.springframework.util.concurrent.ListenableFutureCallback; import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.context.request.async.DeferredResult; import org.springframework.web.context.request.async.WebAsyncUtils; -import org.springframework.web.method.support.AsyncHandlerMethodReturnValueHandler; +import org.springframework.web.method.support.HandlerMethodReturnValueHandler; import org.springframework.web.method.support.ModelAndViewContainer; /** - * Handler for return values of type {@link DeferredResult}, {@link ListenableFuture}, - * {@link CompletionStage} and any other async type with a {@link #getAdapterMap() - * registered adapter}. + * Handler for return values of type {@link DeferredResult}, + * {@link ListenableFuture}, and {@link CompletionStage}. * * @author Rossen Stoyanchev * @since 3.2 */ -public class DeferredResultMethodReturnValueHandler implements AsyncHandlerMethodReturnValueHandler { - - private final Map, DeferredResultAdapter> adapterMap; - - - public DeferredResultMethodReturnValueHandler() { - this.adapterMap = new HashMap<>(5); - this.adapterMap.put(DeferredResult.class, new SimpleDeferredResultAdapter()); - this.adapterMap.put(ListenableFuture.class, new ListenableFutureAdapter()); - this.adapterMap.put(CompletionStage.class, new CompletionStageAdapter()); - } - - - /** - * Return the map with {@code DeferredResult} adapters. - *

By default the map contains adapters for {@code DeferredResult}, which - * simply downcasts, {@link ListenableFuture}, and {@link CompletionStage}. - * @return the map of adapters - */ - public Map, DeferredResultAdapter> getAdapterMap() { - return this.adapterMap; - } - - private DeferredResultAdapter getAdapterFor(Class type) { - for (Class adapteeType : getAdapterMap().keySet()) { - if (adapteeType.isAssignableFrom(type)) { - return getAdapterMap().get(adapteeType); - } - } - return null; - } +public class DeferredResultMethodReturnValueHandler implements HandlerMethodReturnValueHandler { @Override public boolean supportsReturnType(MethodParameter returnType) { - return (getAdapterFor(returnType.getParameterType()) != null); - } - - @Override - public boolean isAsyncReturnValue(Object returnValue, MethodParameter returnType) { - return (returnValue != null && (getAdapterFor(returnValue.getClass()) != null)); + Class type = returnType.getParameterType(); + return DeferredResult.class.isAssignableFrom(type) || + ListenableFuture.class.isAssignableFrom(type) || + CompletionStage.class.isAssignableFrom(type); } @Override @@ -91,78 +55,52 @@ public class DeferredResultMethodReturnValueHandler implements AsyncHandlerMetho return; } - DeferredResultAdapter adapter = getAdapterFor(returnValue.getClass()); - if (adapter == null) { - throw new IllegalStateException( - "Could not find DeferredResultAdapter for return value type: " + returnValue.getClass()); + DeferredResult result; + + if (returnValue instanceof DeferredResult) { + result = (DeferredResult) returnValue; } - DeferredResult result = adapter.adaptToDeferredResult(returnValue); + else if (returnValue instanceof ListenableFuture) { + result = adaptListenableFuture((ListenableFuture) returnValue); + } + else if (returnValue instanceof CompletionStage) { + result = adaptCompletionStage((CompletionStage) returnValue); + } + else { + // Should not happen... + throw new IllegalStateException("Unexpected return value type: " + returnValue); + } + WebAsyncUtils.getAsyncManager(webRequest).startDeferredResultProcessing(result, mavContainer); } - - /** - * Adapter for {@code DeferredResult} return values. - */ - private static class SimpleDeferredResultAdapter implements DeferredResultAdapter { - - @Override - public DeferredResult adaptToDeferredResult(Object returnValue) { - Assert.isInstanceOf(DeferredResult.class, returnValue, "DeferredResult expected"); - return (DeferredResult) returnValue; - } + private DeferredResult adaptListenableFuture(ListenableFuture future) { + DeferredResult result = new DeferredResult<>(); + future.addCallback(new ListenableFutureCallback() { + @Override + public void onSuccess(Object value) { + result.setResult(value); + } + @Override + public void onFailure(Throwable ex) { + result.setErrorResult(ex); + } + }); + return result; } - - /** - * Adapter for {@code ListenableFuture} return values. - */ - private static class ListenableFutureAdapter implements DeferredResultAdapter { - - @Override - public DeferredResult adaptToDeferredResult(Object returnValue) { - Assert.isInstanceOf(ListenableFuture.class, returnValue, "ListenableFuture expected"); - final DeferredResult result = new DeferredResult<>(); - ((ListenableFuture) returnValue).addCallback(new ListenableFutureCallback() { - @Override - public void onSuccess(Object value) { - result.setResult(value); - } - @Override - public void onFailure(Throwable ex) { - result.setErrorResult(ex); - } - }); - return result; - } - } - - - /** - * Adapter for {@code CompletionStage} return values. - */ - private static class CompletionStageAdapter implements DeferredResultAdapter { - - @Override - public DeferredResult adaptToDeferredResult(Object returnValue) { - Assert.isInstanceOf(CompletionStage.class, returnValue, "CompletionStage expected"); - final DeferredResult result = new DeferredResult<>(); - @SuppressWarnings("unchecked") - CompletionStage future = (CompletionStage) returnValue; - future.handle(new BiFunction() { - @Override - public Object apply(Object value, Throwable ex) { - if (ex != null) { - result.setErrorResult(ex); - } - else { - result.setResult(value); - } - return null; - } - }); - return result; - } + private DeferredResult adaptCompletionStage(CompletionStage future) { + DeferredResult result = new DeferredResult<>(); + future.handle((BiFunction) (value, ex) -> { + if (ex != null) { + result.setErrorResult(ex); + } + else { + result.setResult(value); + } + return null; + }); + return result; } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterAdapter.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterAdapter.java deleted file mode 100644 index da1e91ca0d..0000000000 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterAdapter.java +++ /dev/null @@ -1,40 +0,0 @@ -/* - * 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.web.servlet.mvc.method.annotation; - -import org.springframework.http.server.ServerHttpResponse; - -/** - * Contract to adapt streaming async types to {@code ResponseBodyEmitter}. - * - * @author Rossen Stoyanchev - * @since 4.3 - */ -@FunctionalInterface -public interface ResponseBodyEmitterAdapter { - - /** - * Obtain a {@code ResponseBodyEmitter} for the given return value. - * If the return is the body {@code ResponseEntity} then the given - * {@code ServerHttpResponse} contains its status and headers. - * @param returnValue the return value (never {@code null}) - * @param response the response - * @return the return value adapted to a {@code ResponseBodyEmitter} - */ - ResponseBodyEmitter adaptToEmitter(Object returnValue, ServerHttpResponse response); - -} diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandler.java index e43b619078..ad461f1e2a 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyEmitterReturnValueHandler.java @@ -18,9 +18,7 @@ package org.springframework.web.servlet.mvc.method.annotation; import java.io.IOException; import java.io.OutputStream; -import java.util.HashMap; import java.util.List; -import java.util.Map; import javax.servlet.ServletRequest; import javax.servlet.http.HttpServletResponse; @@ -41,81 +39,43 @@ import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.context.request.async.DeferredResult; import org.springframework.web.context.request.async.WebAsyncUtils; import org.springframework.web.filter.ShallowEtagHeaderFilter; -import org.springframework.web.method.support.AsyncHandlerMethodReturnValueHandler; +import org.springframework.web.method.support.HandlerMethodReturnValueHandler; import org.springframework.web.method.support.ModelAndViewContainer; /** - * Handler for return values of type {@link ResponseBodyEmitter} (and the - * {@code ResponseEntity} sub-class) as well as any other - * async type with a {@link #getAdapterMap() registered adapter}. + * Handler for return values of type {@link ResponseBodyEmitter} and sub-classes + * such as {@link SseEmitter} including the same types wrapped with + * {@link ResponseEntity}. * * @author Rossen Stoyanchev * @since 4.2 */ -public class ResponseBodyEmitterReturnValueHandler implements AsyncHandlerMethodReturnValueHandler { +public class ResponseBodyEmitterReturnValueHandler implements HandlerMethodReturnValueHandler { private static final Log logger = LogFactory.getLog(ResponseBodyEmitterReturnValueHandler.class); private final List> messageConverters; - private final Map, ResponseBodyEmitterAdapter> adapterMap; - public ResponseBodyEmitterReturnValueHandler(List> messageConverters) { Assert.notEmpty(messageConverters, "HttpMessageConverter List must not be empty"); this.messageConverters = messageConverters; - this.adapterMap = new HashMap<>(4); - this.adapterMap.put(ResponseBodyEmitter.class, new SimpleResponseBodyEmitterAdapter()); - } - - - /** - * Return the map with {@code ResponseBodyEmitter} adapters. - * By default the map contains a single adapter {@code ResponseBodyEmitter} - * that simply downcasts the return value. - * @return the map of adapters - */ - public Map, ResponseBodyEmitterAdapter> getAdapterMap() { - return this.adapterMap; - } - - private ResponseBodyEmitterAdapter getAdapterFor(Class type) { - if (type != null) { - for (Class adapteeType : getAdapterMap().keySet()) { - if (adapteeType.isAssignableFrom(type)) { - return getAdapterMap().get(adapteeType); - } - } - } - return null; } @Override public boolean supportsReturnType(MethodParameter returnType) { - Class bodyType; - if (ResponseEntity.class.isAssignableFrom(returnType.getParameterType())) { - bodyType = ResolvableType.forMethodParameter(returnType).getGeneric(0).resolve(); - } - else { - bodyType = returnType.getParameterType(); - } - return (getAdapterFor(bodyType) != null); + + Class bodyType = ResponseEntity.class.isAssignableFrom(returnType.getParameterType()) ? + ResolvableType.forMethodParameter(returnType).getGeneric(0).resolve() : + returnType.getParameterType(); + + return bodyType != null && supportsBodyType(bodyType); } - @Override - public boolean isAsyncReturnValue(Object returnValue, MethodParameter returnType) { - if (returnValue != null) { - Object adaptFrom = returnValue; - if (returnValue instanceof ResponseEntity) { - adaptFrom = ((ResponseEntity) returnValue).getBody(); - } - if (adaptFrom != null) { - return (getAdapterFor(adaptFrom.getClass()) != null); - } - } - return false; + private boolean supportsBodyType(Class bodyType) { + return ResponseBodyEmitter.class.isAssignableFrom(bodyType); } @Override @@ -145,12 +105,15 @@ public class ResponseBodyEmitterReturnValueHandler implements AsyncHandlerMethod ServletRequest request = webRequest.getNativeRequest(ServletRequest.class); ShallowEtagHeaderFilter.disableContentCaching(request); - ResponseBodyEmitterAdapter adapter = getAdapterFor(returnValue.getClass()); - if (adapter == null) { - throw new IllegalStateException( - "Could not find ResponseBodyEmitterAdapter for return value type: " + returnValue.getClass()); + ResponseBodyEmitter emitter; + + if (returnValue instanceof ResponseBodyEmitter) { + emitter = (ResponseBodyEmitter) returnValue; } - ResponseBodyEmitter emitter = adapter.adaptToEmitter(returnValue, outputMessage); + else { + throw new IllegalStateException("Unexpected return value type: " + returnValue); + } + emitter.extendResponse(outputMessage); // Commit the response and wrap to ignore further header changes @@ -166,18 +129,6 @@ public class ResponseBodyEmitterReturnValueHandler implements AsyncHandlerMethod } - /** - * Adapter for {@code ResponseBodyEmitter} return values. - */ - private static class SimpleResponseBodyEmitterAdapter implements ResponseBodyEmitterAdapter { - - @Override - public ResponseBodyEmitter adaptToEmitter(Object returnValue, ServerHttpResponse response) { - Assert.isInstanceOf(ResponseBodyEmitter.class, returnValue, "ResponseBodyEmitter expected"); - return (ResponseBodyEmitter) returnValue; - } - } - /** * ResponseBodyEmitter.Handler that writes with HttpMessageConverter's. */ diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/DeferredResultReturnValueHandlerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/DeferredResultReturnValueHandlerTests.java index 47ab93dea7..ba7f516328 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/DeferredResultReturnValueHandlerTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/DeferredResultReturnValueHandlerTests.java @@ -15,7 +15,6 @@ */ package org.springframework.web.servlet.mvc.method.annotation; -import java.lang.reflect.Method; import java.util.concurrent.CompletableFuture; import org.junit.Before; @@ -36,8 +35,8 @@ import org.springframework.web.method.support.ModelAndViewContainer; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.springframework.web.method.ResolvableMethod.on; /** * Unit tests for {@link DeferredResultMethodReturnValueHandler}. @@ -68,129 +67,88 @@ public class DeferredResultReturnValueHandlerTests { @Test public void supportsReturnType() throws Exception { - assertTrue(this.handler.supportsReturnType(returnType("handleDeferredResult"))); - assertTrue(this.handler.supportsReturnType(returnType("handleListenableFuture"))); - assertTrue(this.handler.supportsReturnType(returnType("handleCompletableFuture"))); - assertFalse(this.handler.supportsReturnType(returnType("handleString"))); + + assertTrue(this.handler.supportsReturnType( + on(TestController.class).resolveReturnType(DeferredResult.class, String.class))); + + assertTrue(this.handler.supportsReturnType( + on(TestController.class).resolveReturnType(ListenableFuture.class, String.class))); + + assertTrue(this.handler.supportsReturnType( + on(TestController.class).resolveReturnType(CompletableFuture.class, String.class))); + } + + @Test + public void doesNotSupportReturnType() throws Exception { + assertFalse(this.handler.supportsReturnType(on(TestController.class).resolveReturnType(String.class))); } @Test public void deferredResult() throws Exception { - MethodParameter returnType = returnType("handleDeferredResult"); - DeferredResult deferredResult = new DeferredResult<>(); - handleReturnValue(deferredResult, returnType); - - assertTrue(this.request.isAsyncStarted()); - assertFalse(WebAsyncUtils.getAsyncManager(this.webRequest).hasConcurrentResult()); - - deferredResult.setResult("foo"); - assertTrue(WebAsyncUtils.getAsyncManager(this.webRequest).hasConcurrentResult()); - assertEquals("foo", WebAsyncUtils.getAsyncManager(this.webRequest).getConcurrentResult()); - } - - @Test - public void deferredResultWitError() throws Exception { - MethodParameter returnType = returnType("handleDeferredResult"); - DeferredResult deferredResult = new DeferredResult<>(); - handleReturnValue(deferredResult, returnType); - - assertTrue(this.request.isAsyncStarted()); - assertFalse(WebAsyncUtils.getAsyncManager(this.webRequest).hasConcurrentResult()); - + DeferredResult result = new DeferredResult<>(); IllegalStateException ex = new IllegalStateException(); - deferredResult.setErrorResult(ex); - assertTrue(WebAsyncUtils.getAsyncManager(this.webRequest).hasConcurrentResult()); - assertSame(ex, WebAsyncUtils.getAsyncManager(this.webRequest).getConcurrentResult()); + testHandle(result, DeferredResult.class, () -> result.setErrorResult(ex), ex); } @Test public void listenableFuture() throws Exception { - MethodParameter returnType = returnType("handleListenableFuture"); SettableListenableFuture future = new SettableListenableFuture<>(); - handleReturnValue(future, returnType); - - assertTrue(this.request.isAsyncStarted()); - assertFalse(WebAsyncUtils.getAsyncManager(this.webRequest).hasConcurrentResult()); - - future.set("foo"); - assertTrue(WebAsyncUtils.getAsyncManager(this.webRequest).hasConcurrentResult()); - assertEquals("foo", WebAsyncUtils.getAsyncManager(this.webRequest).getConcurrentResult()); - } - - @Test - public void listenableFutureWithError() throws Exception { - MethodParameter returnType = returnType("handleListenableFuture"); - SettableListenableFuture future = new SettableListenableFuture<>(); - handleReturnValue(future, returnType); - - assertTrue(this.request.isAsyncStarted()); - assertFalse(WebAsyncUtils.getAsyncManager(this.webRequest).hasConcurrentResult()); - - IllegalStateException ex = new IllegalStateException(); - future.setException(ex); - assertTrue(WebAsyncUtils.getAsyncManager(this.webRequest).hasConcurrentResult()); - assertSame(ex, WebAsyncUtils.getAsyncManager(this.webRequest).getConcurrentResult()); + testHandle(future, ListenableFuture.class, () -> future.set("foo"), "foo"); } @Test public void completableFuture() throws Exception { - MethodParameter returnType = returnType("handleCompletableFuture"); SettableListenableFuture future = new SettableListenableFuture<>(); - handleReturnValue(future, returnType); + testHandle(future, CompletableFuture.class, () -> future.set("foo"), "foo"); + } - assertTrue(this.request.isAsyncStarted()); - assertFalse(WebAsyncUtils.getAsyncManager(this.webRequest).hasConcurrentResult()); + @Test + public void deferredResultWitError() throws Exception { + DeferredResult result = new DeferredResult<>(); + testHandle(result, DeferredResult.class, () -> result.setResult("foo"), "foo"); + } - future.set("foo"); - assertTrue(WebAsyncUtils.getAsyncManager(this.webRequest).hasConcurrentResult()); - assertEquals("foo", WebAsyncUtils.getAsyncManager(this.webRequest).getConcurrentResult()); + @Test + public void listenableFutureWithError() throws Exception { + SettableListenableFuture future = new SettableListenableFuture<>(); + IllegalStateException ex = new IllegalStateException(); + testHandle(future, ListenableFuture.class, () -> future.setException(ex), ex); } @Test public void completableFutureWithError() throws Exception { - MethodParameter returnType = returnType("handleCompletableFuture"); - CompletableFuture future = new CompletableFuture<>(); - handleReturnValue(future, returnType); + SettableListenableFuture future = new SettableListenableFuture<>(); + IllegalStateException ex = new IllegalStateException(); + testHandle(future, CompletableFuture.class, () -> future.setException(ex), ex); + } + + private void testHandle(Object returnValue, Class asyncType, + Runnable setResultTask, Object expectedValue) throws Exception { + + ModelAndViewContainer mavContainer = new ModelAndViewContainer(); + MethodParameter returnType = on(TestController.class).resolveReturnType(asyncType, String.class); + this.handler.handleReturnValue(returnValue, returnType, mavContainer, this.webRequest); assertTrue(this.request.isAsyncStarted()); assertFalse(WebAsyncUtils.getAsyncManager(this.webRequest).hasConcurrentResult()); - IllegalStateException ex = new IllegalStateException(); - future.completeExceptionally(ex); + setResultTask.run(); + assertTrue(WebAsyncUtils.getAsyncManager(this.webRequest).hasConcurrentResult()); - assertSame(ex, WebAsyncUtils.getAsyncManager(this.webRequest).getConcurrentResult()); - } - - - private void handleReturnValue(Object returnValue, MethodParameter returnType) throws Exception { - ModelAndViewContainer mavContainer = new ModelAndViewContainer(); - this.handler.handleReturnValue(returnValue, returnType, mavContainer, this.webRequest); - } - - private MethodParameter returnType(String methodName) throws NoSuchMethodException { - Method method = TestController.class.getDeclaredMethod(methodName); - return new MethodParameter(method, -1); + assertEquals(expectedValue, WebAsyncUtils.getAsyncManager(this.webRequest).getConcurrentResult()); } @SuppressWarnings("unused") - private static class TestController { + static class TestController { - private String handleString() { - return null; - } + String handleString() { return null; } - private DeferredResult handleDeferredResult() { - return null; - } + DeferredResult handleDeferredResult() { return null; } - private ListenableFuture handleListenableFuture() { - return null; - } + ListenableFuture handleListenableFuture() { return null; } - private CompletableFuture handleCompletableFuture() { - return null; - } + CompletableFuture handleCompletableFuture() { return null; } } }