From 956b66bbd466bb7a68e8499a483139a516572b24 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 15 Oct 2014 16:39:03 -0400 Subject: [PATCH] Fix issue with async return value type determination Before this change, the type of asynchronously produced return values (e.g. Callable, DeferredResult, ListenableFuture) could not be properly determined with an actual resulting value of null. Or even with an actual value returned, the generic type could not be properly determined. This change fixes both of those issues. Issue: SPR-12287 --- .../web/method/HandlerMethod.java | 2 +- .../ServletInvocableHandlerMethod.java | 108 ++++++++++++------ .../ServletInvocableHandlerMethodTests.java | 34 ++++++ 3 files changed, 111 insertions(+), 33 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/method/HandlerMethod.java b/spring-web/src/main/java/org/springframework/web/method/HandlerMethod.java index 7c58cae2c9..a0a5063ce2 100644 --- a/spring-web/src/main/java/org/springframework/web/method/HandlerMethod.java +++ b/spring-web/src/main/java/org/springframework/web/method/HandlerMethod.java @@ -247,7 +247,7 @@ public class HandlerMethod { /** * A MethodParameter with HandlerMethod-specific behavior. */ - private class HandlerMethodParameter extends MethodParameter { + protected class HandlerMethodParameter extends MethodParameter { public HandlerMethodParameter(int index) { super(HandlerMethod.this.bridgedMethod, index); 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 a554ebe784..43b6f54acc 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-2012 the original author or authors. + * Copyright 2002-2014 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,8 +19,11 @@ package org.springframework.web.servlet.mvc.method.annotation; import java.io.IOException; import java.lang.annotation.Annotation; import java.lang.reflect.Method; +import java.lang.reflect.Type; import java.util.concurrent.Callable; +import org.springframework.core.MethodParameter; +import org.springframework.core.ResolvableType; import org.springframework.http.HttpStatus; import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; @@ -51,6 +54,9 @@ import org.springframework.web.util.NestedServletException; */ public class ServletInvocableHandlerMethod extends InvocableHandlerMethod { + private static final Method CALLABLE_METHOD = ClassUtils.getMethod(Callable.class, "call"); + + private HttpStatus responseStatus; private String responseReason; @@ -174,46 +180,46 @@ public class ServletInvocableHandlerMethod extends InvocableHandlerMethod { } /** - * Create a ServletInvocableHandlerMethod that will return the given value from an - * async operation instead of invoking the controller method again. The async result - * value is then either processed as if the controller method returned it or an - * exception is raised if the async result value itself is an Exception. + * Create a nested ServletInvocableHandlerMethod sub-class that returns the + * the given value (or raises an Exception if the value is one) rather than + * actually invoking the controller method. This is useful when processing + * async return values (e.g. Callable, DeferredResult, ListenableFuture). */ - ServletInvocableHandlerMethod wrapConcurrentResult(final Object result) { - - return new CallableHandlerMethod(new Callable() { - - @Override - public Object call() throws Exception { - if (result instanceof Exception) { - throw (Exception) result; - } - else if (result instanceof Throwable) { - throw new NestedServletException("Async processing failed", (Throwable) result); - } - return result; - } - }); + ServletInvocableHandlerMethod wrapConcurrentResult(Object result) { + return new ConcurrentResultHandlerMethod(result, new ConcurrentResultMethodParameter(result)); } /** - * A sub-class of {@link HandlerMethod} that invokes the given {@link Callable} - * instead of the target controller method. This is useful for resuming processing - * with the result of an async operation. The goal is to process the value returned - * from the Callable as if it was returned by the target controller method, i.e. - * taking into consideration both method and type-level controller annotations (e.g. - * {@code @ResponseBody}, {@code @ResponseStatus}, etc). + * A nested sub-class of {@code ServletInvocableHandlerMethod} that uses a + * simple {@link Callable} instead of the original controller as the handler in + * order to return the fixed (concurrent) result value given to it. Effectively + * "resumes" processing with the asynchronously produced return value. */ - private class CallableHandlerMethod extends ServletInvocableHandlerMethod { + private class ConcurrentResultHandlerMethod extends ServletInvocableHandlerMethod { - public CallableHandlerMethod(Callable callable) { - super(callable, ClassUtils.getMethod(callable.getClass(), "call")); - this.setHandlerMethodReturnValueHandlers(ServletInvocableHandlerMethod.this.returnValueHandlers); + private final MethodParameter returnType; + + + public ConcurrentResultHandlerMethod(final Object result, ConcurrentResultMethodParameter returnType) { + super(new Callable() { + @Override + public Object call() throws Exception { + if (result instanceof Exception) { + throw (Exception) result; + } + else if (result instanceof Throwable) { + throw new NestedServletException("Async processing failed", (Throwable) result); + } + return result; + } + }, CALLABLE_METHOD); + setHandlerMethodReturnValueHandlers(ServletInvocableHandlerMethod.this.returnValueHandlers); + this.returnType = returnType; } /** - * Bridge to type-level annotations of the target controller method. + * Bridge to actual controller type-level annotations. */ @Override public Class getBeanType() { @@ -221,7 +227,16 @@ public class ServletInvocableHandlerMethod extends InvocableHandlerMethod { } /** - * Bridge to method-level annotations of the target controller method. + * Bridge to actual return value or generic type within the declared + * async return type, e.g. Foo instead of {@code DeferredResult}. + */ + @Override + public MethodParameter getReturnValueType(Object returnValue) { + return this.returnType; + } + + /** + * Bridge to controller method-level annotations. */ @Override public A getMethodAnnotation(Class annotationType) { @@ -229,4 +244,33 @@ public class ServletInvocableHandlerMethod extends InvocableHandlerMethod { } } + /** + * MethodParameter sub-class based on the actual return value type or if + * that's null falling back on the generic type within the declared async + * return type, e.g. Foo instead of {@code DeferredResult}. + */ + private class ConcurrentResultMethodParameter extends HandlerMethodParameter { + + private final Object returnValue; + + private final ResolvableType returnType; + + + public ConcurrentResultMethodParameter(Object returnValue) { + super(-1); + this.returnValue = returnValue; + this.returnType = ResolvableType.forType(super.getGenericParameterType()).getGeneric(0); + } + + @Override + public Class getParameterType() { + return (returnValue != null ? returnValue.getClass() : this.returnType.getRawClass()); + } + + @Override + public Type getGenericParameterType() { + return this.returnType.getType(); + } + } + } 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 ffa35cbc80..188981b6fd 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 @@ -17,6 +17,7 @@ package org.springframework.web.servlet.mvc.method.annotation; import java.lang.reflect.Method; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import javax.servlet.http.HttpServletResponse; @@ -45,6 +46,8 @@ import org.springframework.web.method.support.ModelAndViewContainer; import org.springframework.web.servlet.view.RedirectView; import static org.junit.Assert.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; /** * Test fixture with {@link ServletInvocableHandlerMethod}. @@ -183,6 +186,37 @@ public class ServletInvocableHandlerMethodTests { assertEquals("bar", response.getContentAsString()); } + // SPR-12287 + + @Test + public void wrapConcurrentResult_ResponseEntityNullBody() throws Exception { + List> converters = new ArrayList>(); + converters.add(new StringHttpMessageConverter()); + List advice = Arrays.asList(mock(ResponseBodyAdvice.class)); + HttpEntityMethodProcessor processor = new HttpEntityMethodProcessor(converters, null, advice); + returnValueHandlers.addHandler(processor); + ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new ResponseEntityHandler(), "handle"); + handlerMethod = handlerMethod.wrapConcurrentResult(new ResponseEntity<>(HttpStatus.OK)); + handlerMethod.invokeAndHandle(webRequest, mavContainer); + + assertEquals(200, response.getStatus()); + assertEquals("", response.getContentAsString()); + } + + @Test + public void wrapConcurrentResult_ResponseEntityNullReturnValue() throws Exception { + List> converters = new ArrayList>(); + converters.add(new StringHttpMessageConverter()); + List advice = Arrays.asList(mock(ResponseBodyAdvice.class)); + HttpEntityMethodProcessor processor = new HttpEntityMethodProcessor(converters, null, advice); + returnValueHandlers.addHandler(processor); + ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new ResponseEntityHandler(), "handle"); + handlerMethod = handlerMethod.wrapConcurrentResult(null); + handlerMethod.invokeAndHandle(webRequest, mavContainer); + + assertEquals(200, response.getStatus()); + assertEquals("", response.getContentAsString()); + } private ServletInvocableHandlerMethod getHandlerMethod(Object controller, String methodName, Class... argTypes) throws NoSuchMethodException {