From a9b2a12491aa2259851b93824b7e560b3dbd1902 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 19 Sep 2014 14:57:56 -0400 Subject: [PATCH] Allow ResponseBodyAdvice to modify null return values This change defers determination of whether to invoke a message converter in case of a null @ResponseBody value (or ResponseEntity with a null body) until after the invocation of the ResponseBodyAdvice chain. This allows a ResponseBodyAdvice to handle null values potentially turning them into non-null value.s Issue: SPR-12152 --- ...stractMessageConverterMethodProcessor.java | 26 +++++++++++++++---- .../annotation/HttpEntityMethodProcessor.java | 17 +++++++++--- .../RequestResponseBodyMethodProcessor.java | 2 +- .../annotation/ResponseBodyAdviceChain.java | 6 +++++ .../HttpEntityMethodProcessorMockTests.java | 26 +++++++++++++++++++ 5 files changed, 67 insertions(+), 10 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java index ad0e6daf16..995d53e4c9 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java @@ -75,6 +75,10 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe } + protected ResponseBodyAdviceChain getAdviceChain() { + return this.adviceChain; + } + /** * Creates a new {@link HttpOutputMessage} from the given {@link NativeWebRequest}. * @param webRequest the web request to create an output message from @@ -112,7 +116,7 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe ServletServerHttpRequest inputMessage, ServletServerHttpResponse outputMessage) throws IOException, HttpMediaTypeNotAcceptableException { - Class returnValueClass = returnValue.getClass(); + Class returnValueClass = getReturnValueType(returnValue, returnType); HttpServletRequest servletRequest = inputMessage.getServletRequest(); List requestedMediaTypes = getAcceptableMediaTypes(servletRequest); List producibleMediaTypes = getProducibleMediaTypes(servletRequest, returnValueClass); @@ -150,10 +154,12 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe if (messageConverter.canWrite(returnValueClass, selectedMediaType)) { returnValue = this.adviceChain.invoke(returnValue, returnType, selectedMediaType, (Class>) messageConverter.getClass(), inputMessage, outputMessage); - ((HttpMessageConverter) messageConverter).write(returnValue, selectedMediaType, outputMessage); - if (logger.isDebugEnabled()) { - logger.debug("Written [" + returnValue + "] as \"" + selectedMediaType + "\" using [" + - messageConverter + "]"); + if (returnValue != null) { + ((HttpMessageConverter) messageConverter).write(returnValue, selectedMediaType, outputMessage); + if (logger.isDebugEnabled()) { + logger.debug("Written [" + returnValue + "] as \"" + selectedMediaType + "\" using [" + + messageConverter + "]"); + } } return; } @@ -162,6 +168,16 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe throw new HttpMediaTypeNotAcceptableException(this.allSupportedMediaTypes); } + /** + * Return the type of the value to be written to the response. Typically this + * is a simple check via getClass on the returnValue but if the returnValue is + * null, then the returnType needs to be examined possibly including generic + * type determination (e.g. {@code ResponseEntity}). + */ + protected Class getReturnValueType(Object returnValue, MethodParameter returnType) { + return (returnValue != null ? returnValue.getClass() : returnType.getParameterType()); + } + /** * Returns the media types that can be produced: *
    diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java index 3b01e1ca80..ec799a96c6 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java @@ -22,6 +22,7 @@ import java.lang.reflect.Type; import java.util.List; import org.springframework.core.MethodParameter; +import org.springframework.core.ResolvableType; import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; import org.springframework.http.RequestEntity; @@ -134,13 +135,21 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro } Object body = responseEntity.getBody(); - if (body != null) { + if (body != null || getAdviceChain().hasAdvice()) { writeWithMessageConverters(body, returnType, inputMessage, outputMessage); } + // Ensure headers are flushed even if no body was written + outputMessage.getBody(); + } + + @Override + protected Class getReturnValueType(Object returnValue, MethodParameter returnType) { + if (returnValue != null) { + return returnValue.getClass(); + } else { - // Flush headers to the HttpServletResponse - outputMessage.getBody(); + Type type = getHttpEntityType(returnType); + return ResolvableType.forMethodParameter(returnType, type).resolve(Object.class); } } - } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessor.java index 7001e7a619..53ac9473c9 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessor.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessor.java @@ -198,7 +198,7 @@ public class RequestResponseBodyMethodProcessor extends AbstractMessageConverter throws IOException, HttpMediaTypeNotAcceptableException { mavContainer.setRequestHandled(true); - if (returnValue != null) { + if (returnValue != null || getAdviceChain().hasAdvice()) { writeWithMessageConverters(returnValue, returnType, webRequest); } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyAdviceChain.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyAdviceChain.java index 2368d969f3..894f50e69e 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyAdviceChain.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseBodyAdviceChain.java @@ -23,8 +23,10 @@ import org.springframework.http.MediaType; import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.server.ServerHttpRequest; import org.springframework.http.server.ServerHttpResponse; +import org.springframework.util.CollectionUtils; import org.springframework.web.method.ControllerAdviceBean; +import java.util.Collections; import java.util.List; /** @@ -45,6 +47,10 @@ class ResponseBodyAdviceChain { } + public boolean hasAdvice() { + return !CollectionUtils.isEmpty(this.advice); + } + @SuppressWarnings("unchecked") public T invoke(T body, MethodParameter returnType, MediaType selectedContentType, Class> selectedConverterType, diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java index 9af77258db..163649f02f 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessorMockTests.java @@ -20,6 +20,7 @@ import java.lang.reflect.Method; import java.net.URI; import java.util.Arrays; import java.util.Collections; +import java.util.List; import org.junit.Before; import org.junit.Test; @@ -40,11 +41,13 @@ import org.springframework.mock.web.test.MockHttpServletResponse; import org.springframework.web.HttpMediaTypeNotAcceptableException; import org.springframework.web.HttpMediaTypeNotSupportedException; import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.context.request.ServletWebRequest; import org.springframework.web.method.support.ModelAndViewContainer; import static org.junit.Assert.*; import static org.mockito.BDDMockito.*; +import static org.mockito.Matchers.eq; import static org.springframework.web.servlet.HandlerMapping.*; /** @@ -219,6 +222,29 @@ public class HttpEntityMethodProcessorMockTests { verify(messageConverter).write(eq(body), eq(MediaType.TEXT_HTML), isA(HttpOutputMessage.class)); } + @Test + public void handleReturnValueWithResponseBodyAdvice() throws Exception { + ResponseEntity returnValue = new ResponseEntity<>(HttpStatus.OK); + + servletRequest.addHeader("Accept", "text/*"); + servletRequest.setAttribute(PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE, Collections.singleton(MediaType.TEXT_HTML)); + + ResponseBodyAdvice advice = mock(ResponseBodyAdvice.class); + given(advice.supports(any(), any())).willReturn(true); + given(advice.beforeBodyWrite(any(), any(), any(), any(), any(), any())).willReturn("Foo"); + + HttpEntityMethodProcessor processor = new HttpEntityMethodProcessor( + Collections.singletonList(messageConverter), null, Collections.singletonList(advice)); + + reset(messageConverter); + given(messageConverter.canWrite(String.class, MediaType.TEXT_HTML)).willReturn(true); + + processor.handleReturnValue(returnValue, returnTypeResponseEntity, mavContainer, webRequest); + + assertTrue(mavContainer.isRequestHandled()); + verify(messageConverter).write(eq("Foo"), eq(MediaType.TEXT_HTML), isA(HttpOutputMessage.class)); + } + @Test(expected = HttpMediaTypeNotAcceptableException.class) public void handleReturnValueNotAcceptable() throws Exception { String body = "Foo";