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

Author: rstoyanchev

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 @@
 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".
  *
- * <p>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:
- * <pre>
- * DeferredResult<?> deferredResult = (DeferredResult<?>) returnValue;
- * WebAsyncUtils.getAsyncManager(webRequest).startDeferredResultProcessing(deferredResult, mavContainer);
- * </pre>
+ * <p><strong>Note: </strong> 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

spring-web/src/main/java/org/springframework/web/method/support/HandlerMethodReturnValueHandlerComposite.java

@@ -33,7 +33,7 @@
  * @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 @@ private HandlerMethodReturnValueHandler selectHandler(Object value, MethodParame
 		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)) {

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.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 void handleReturnValueWithMultipleHandlers() throws Exception {
 		verifyNoMoreInteractions(anotherIntegerHandler);
 	}
 
-	// SPR-13083
-
-	@Test
+	@Test // SPR-13083
 	public void handleReturnValueWithAsyncHandler() throws Exception {
 
 		Promise<Integer> promise = new Promise<>();

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AsyncTaskMethodReturnValueHandler.java

@@ -21,7 +21,7 @@
 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 @@
  * @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 boolean supportsReturnType(MethodParameter returnType) {
 		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 {

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 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 @@
  * @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 {

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/DeferredResultAdapter.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<Class<?>, 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.
-	 * <p>By default the map contains adapters for {@code DeferredResult}, which
-	 * simply downcasts, {@link ListenableFuture}, and {@link CompletionStage}.
-	 * @return the map of adapters
-	 */
-	public Map<Class<?>, 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 void handleReturnValue(Object returnValue, MethodParameter returnType,
 			return;
 		}
 
-		DeferredResultAdapter adapter = getAdapterFor(returnValue.getClass());
-		if (adapter == null) {
-			throw new IllegalStateException(
-					"Could not find DeferredResultAdapter for return value type: " + returnValue.getClass());
-		}
-		DeferredResult<?> result = adapter.adaptToDeferredResult(returnValue);
-		WebAsyncUtils.getAsyncManager(webRequest).startDeferredResultProcessing(result, mavContainer);
-	}
-
-
-	/**
-	 * Adapter for {@code DeferredResult} return values.
-	 */
-	private static class SimpleDeferredResultAdapter implements DeferredResultAdapter {
+		DeferredResult<?> result;
 
-		@Override
-		public DeferredResult<?> adaptToDeferredResult(Object returnValue) {
-			Assert.isInstanceOf(DeferredResult.class, returnValue, "DeferredResult expected");
-			return (DeferredResult<?>) returnValue;
+		if (returnValue instanceof DeferredResult) {
+			result = (DeferredResult<?>) returnValue;
 		}
-	}
-
-
-	/**
-	 * 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<Object> result = new DeferredResult<>();
-			((ListenableFuture<?>) returnValue).addCallback(new ListenableFutureCallback<Object>() {
-				@Override
-				public void onSuccess(Object value) {
-					result.setResult(value);
-				}
-				@Override
-				public void onFailure(Throwable ex) {
-					result.setErrorResult(ex);
-				}
-			});
-			return result;
+		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 CompletionStage} return values.
-	 */
-	private static class CompletionStageAdapter implements DeferredResultAdapter {
+	private DeferredResult<Object> adaptListenableFuture(ListenableFuture<?> future) {
+		DeferredResult<Object> result = new DeferredResult<>();
+		future.addCallback(new ListenableFutureCallback<Object>() {
+			@Override
+			public void onSuccess(Object value) {
+				result.setResult(value);
+			}
+			@Override
+			public void onFailure(Throwable ex) {
+				result.setErrorResult(ex);
+			}
+		});
+		return result;
+	}
 
-		@Override
-		public DeferredResult<?> adaptToDeferredResult(Object returnValue) {
-			Assert.isInstanceOf(CompletionStage.class, returnValue, "CompletionStage expected");
-			final DeferredResult<Object> result = new DeferredResult<>();
-			@SuppressWarnings("unchecked")
-			CompletionStage<?> future = (CompletionStage<?>) returnValue;
-			future.handle(new BiFunction<Object, Throwable, Object>() {
-				@Override
-				public Object apply(Object value, Throwable ex) {
-					if (ex != null) {
-						result.setErrorResult(ex);
-					}
-					else {
-						result.setResult(value);
-					}
-					return null;
-				}
-			});
-			return result;
-		}
+	private DeferredResult<Object> adaptCompletionStage(CompletionStage<?> future) {
+		DeferredResult<Object> result = new DeferredResult<>();
+		future.handle((BiFunction<Object, Throwable, Object>) (value, ex) -> {
+			if (ex != null) {
+				result.setErrorResult(ex);
+			}
+			else {
+				result.setResult(value);
+			}
+			return null;
+		});
+		return result;
 	}
 
 }