Fix regression with raw ResponseEntity type

This fix addresses a 4.1.1 regression where a raw ResponseEntity return
value (used to return potentially a different kind of body) caused an
exception.

The regression came from the fact we now try to render a null body in
order to give ResponseBodyAdvice a chance to substitute a different
value. That in turn means we have to try to determine the body type
from the method signature.

This change improves the logic for extracting the generic parameter
type to accommodate a raw ResponseEntity class. Also we avoid raising
HttpMediaTypeNotAcceptableException if the value to be rendered is
null.

Issue: SPR-12287

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

@@ -117,10 +117,6 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe
 			throws IOException, HttpMediaTypeNotAcceptableException {
 
 		Class<?> returnValueClass = getReturnValueType(returnValue, returnType);
-		if (returnValue == null && Void.class.equals(returnValueClass)) {
-			return;
-		}
-
 		HttpServletRequest servletRequest = inputMessage.getServletRequest();
 		List<MediaType> requestedMediaTypes = getAcceptableMediaTypes(servletRequest);
 		List<MediaType> producibleMediaTypes = getProducibleMediaTypes(servletRequest, returnValueClass);
@@ -134,7 +130,10 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe
 			}
 		}
 		if (compatibleMediaTypes.isEmpty()) {
-			throw new HttpMediaTypeNotAcceptableException(producibleMediaTypes);
+			if (returnValue != null) {
+				throw new HttpMediaTypeNotAcceptableException(producibleMediaTypes);
+			}
+			return;
 		}
 
 		List<MediaType> mediaTypes = new ArrayList<MediaType>(compatibleMediaTypes);
@@ -169,7 +168,10 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe
 				}
 			}
 		}
-		throw new HttpMediaTypeNotAcceptableException(this.allSupportedMediaTypes);
+
+		if (returnValue != null) {
+			throw new HttpMediaTypeNotAcceptableException(this.allSupportedMediaTypes);
+		}
 	}
 
 	/**

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

@@ -102,12 +102,17 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro
 		Type parameterType = parameter.getGenericParameterType();
 		if (parameterType instanceof ParameterizedType) {
 			ParameterizedType type = (ParameterizedType) parameterType;
-			if (type.getActualTypeArguments().length == 1) {
-				return type.getActualTypeArguments()[0];
+			if (type.getActualTypeArguments().length != 1) {
+				throw new IllegalArgumentException("Expected single generic parameter on '" +
+						parameter.getParameterName() + "' in method " + parameter.getMethod());
 			}
+			return type.getActualTypeArguments()[0];
+		}
+		else if (parameterType instanceof Class) {
+			return Object.class;
 		}
 		throw new IllegalArgumentException("HttpEntity parameter '" + parameter.getParameterName() +
-				"' in method " + parameter.getMethod() + " is not parameterized or has more than one parameter");
+				"' in method " + parameter.getMethod() + " is not parameterized");
 	}
 
 	@Override
@@ -134,9 +139,10 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro
 		}
 
 		Object body = responseEntity.getBody();
-		if (body != null || getAdviceChain().hasAdvice()) {
-			writeWithMessageConverters(body, returnType, inputMessage, outputMessage);
-		}
+
+		// Try even with null body. ResponseBodyAdvice could get involved.
+		writeWithMessageConverters(body, returnType, inputMessage, outputMessage);
+
 		// Ensure headers are flushed even if no body was written
 		outputMessage.getBody();
 	}

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

@@ -194,9 +194,9 @@ public class RequestResponseBodyMethodProcessor extends AbstractMessageConverter
 			throws IOException, HttpMediaTypeNotAcceptableException {
 
 		mavContainer.setRequestHandled(true);
-		if (returnValue != null || getAdviceChain().hasAdvice()) {
-			writeWithMessageConverters(returnValue, returnType, webRequest);
-		}
+
+		// Try even with null return value. ResponseBodyAdvice could get involved.
+		writeWithMessageConverters(returnValue, returnType, webRequest);
 	}
 
 }

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethodTests.java

@@ -180,7 +180,7 @@ public class ServletInvocableHandlerMethodTests {
 		List<HttpMessageConverter<?>> converters = new ArrayList<HttpMessageConverter<?>>();
 		converters.add(new StringHttpMessageConverter());
 		this.returnValueHandlers.addHandler(new HttpEntityMethodProcessor(converters));
-		ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new ResponseEntityHandler(), "handle");
+		ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new ResponseEntityHandler(), "handleDeferred");
 		handlerMethod = handlerMethod.wrapConcurrentResult(new ResponseEntity<>("bar", HttpStatus.OK));
 		handlerMethod.invokeAndHandle(this.webRequest, this.mavContainer);
 
@@ -196,7 +196,7 @@ public class ServletInvocableHandlerMethodTests {
 		List<Object> advice = Arrays.asList(mock(ResponseBodyAdvice.class));
 		HttpEntityMethodProcessor processor = new HttpEntityMethodProcessor(converters, null, advice);
 		this.returnValueHandlers.addHandler(processor);
-		ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new ResponseEntityHandler(), "handle");
+		ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new ResponseEntityHandler(), "handleDeferred");
 		handlerMethod = handlerMethod.wrapConcurrentResult(new ResponseEntity<>(HttpStatus.OK));
 		handlerMethod.invokeAndHandle(this.webRequest, this.mavContainer);
 
@@ -211,7 +211,7 @@ public class ServletInvocableHandlerMethodTests {
 		List<Object> advice = Arrays.asList(mock(ResponseBodyAdvice.class));
 		HttpEntityMethodProcessor processor = new HttpEntityMethodProcessor(converters, null, advice);
 		this.returnValueHandlers.addHandler(processor);
-		ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new ResponseEntityHandler(), "handle");
+		ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new ResponseEntityHandler(), "handleDeferred");
 		handlerMethod = handlerMethod.wrapConcurrentResult(null);
 		handlerMethod.invokeAndHandle(this.webRequest, this.mavContainer);
 
@@ -219,13 +219,28 @@ public class ServletInvocableHandlerMethodTests {
 		assertEquals("", this.response.getContentAsString());
 	}
 
+	// SPR-12287 (16/Oct/14 comments)
+
+	@Test
+	public void responseEntityRawTypeWithNullBody() throws Exception {
+		List<HttpMessageConverter<?>> converters = Arrays.asList(new StringHttpMessageConverter());
+		List<Object> advice = Arrays.asList(mock(ResponseBodyAdvice.class));
+		HttpEntityMethodProcessor processor = new HttpEntityMethodProcessor(converters, null, advice);
+		this.returnValueHandlers.addHandler(processor);
+		ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new ResponseEntityHandler(), "handleRawType");
+		handlerMethod.invokeAndHandle(this.webRequest, this.mavContainer);
+
+		assertEquals(200, this.response.getStatus());
+		assertEquals("", this.response.getContentAsString());
+	}
+
 	private ServletInvocableHandlerMethod getHandlerMethod(Object controller,
 			String methodName, Class<?>... argTypes) throws NoSuchMethodException {
 
 		Method method = controller.getClass().getDeclaredMethod(methodName, argTypes);
 		ServletInvocableHandlerMethod handlerMethod = new ServletInvocableHandlerMethod(controller, method);
-		handlerMethod.setHandlerMethodArgumentResolvers(argumentResolvers);
-		handlerMethod.setHandlerMethodReturnValueHandlers(returnValueHandlers);
+		handlerMethod.setHandlerMethodArgumentResolvers(this.argumentResolvers);
+		handlerMethod.setHandlerMethodReturnValueHandlers(this.returnValueHandlers);
 		return handlerMethod;
 	}
 
@@ -277,9 +292,14 @@ public class ServletInvocableHandlerMethodTests {
 	private static class ResponseEntityHandler {
 
 		@SuppressWarnings("unused")
-		public DeferredResult<ResponseEntity<String>> handle() {
+		public DeferredResult<ResponseEntity<String>> handleDeferred() {
 			return new DeferredResult<>();
 		}
+
+		@SuppressWarnings("unused")
+		public ResponseEntity handleRawType() {
+			return ResponseEntity.ok().build();
+		}
 	}
 
 	private static class ExceptionRaisingReturnValueHandler implements HandlerMethodReturnValueHandler {