From 8d7812b1b6c47c3edef1332639cb57b92327ec46 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 17 Jun 2015 17:00:21 -0400 Subject: [PATCH] Raise IAE when no converter matches return type Before this change the AbstractMessageConverterMethodProcessor always raised a 406 if it couldn't find a converter. However if the reason for not finding it is because there is simply no converter for the return value type (i.e. programming error) and doesn't have anything to do with content negotiation, then we should raise a 500 instead and make it easier to figure out what's wrong. Issue: SPR-13135 --- ...stractMessageConverterMethodProcessor.java | 4 ++ ...questResponseBodyMethodProcessorTests.java | 69 +++++++++++-------- .../ServletInvocableHandlerMethodTests.java | 16 ++--- 3 files changed, 52 insertions(+), 37 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 c20d47ea90..ddd6931c28 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 @@ -32,6 +32,7 @@ import org.springframework.http.MediaType; import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.server.ServletServerHttpRequest; import org.springframework.http.server.ServletServerHttpResponse; +import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; import org.springframework.web.HttpMediaTypeNotAcceptableException; import org.springframework.web.accept.ContentNegotiationManager; @@ -116,6 +117,9 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe List requestedMediaTypes = getAcceptableMediaTypes(servletRequest); List producibleMediaTypes = getProducibleMediaTypes(servletRequest, returnValueClass); + Assert.isTrue(returnValue == null || !producibleMediaTypes.isEmpty(), + "No converter found for return value of type: " + returnValueClass); + Set compatibleMediaTypes = new LinkedHashSet(); for (MediaType requestedType : requestedMediaTypes) { for (MediaType producibleType : producibleMediaTypes) { diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java index 5463fb877b..7414a63e4a 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java @@ -16,11 +16,15 @@ package org.springframework.web.servlet.mvc.method.annotation; +import static org.junit.Assert.*; + +import java.io.ByteArrayOutputStream; +import java.io.OutputStream; import java.io.Serializable; import java.lang.reflect.Method; import java.lang.reflect.Type; import java.util.ArrayList; -import java.util.Arrays; +import java.util.Collections; import java.util.List; import com.fasterxml.jackson.annotation.JsonView; @@ -62,8 +66,6 @@ import org.springframework.web.method.support.ModelAndViewContainer; import org.springframework.web.servlet.ModelAndView; import org.springframework.web.servlet.view.json.MappingJackson2JsonView; -import static org.junit.Assert.*; - /** * Test fixture for a {@link RequestResponseBodyMethodProcessor} with * actual delegation to {@link HttpMessageConverter} instances. @@ -73,6 +75,7 @@ import static org.junit.Assert.*; * @author Rossen Stoyanchev * @author Sebastien Deleuze */ +@SuppressWarnings("unused") public class RequestResponseBodyMethodProcessorTests { private MethodParameter paramGenericList; @@ -94,7 +97,9 @@ public class RequestResponseBodyMethodProcessorTests { @Before public void setUp() throws Exception { - Method method = getClass().getMethod("handle", List.class, SimpleBean.class, MultiValueMap.class, String.class); + + Method method = getClass().getDeclaredMethod("handle", List.class, + SimpleBean.class, MultiValueMap.class, String.class); paramGenericList = new MethodParameter(method, 0); paramSimpleBean = new MethodParameter(method, 1); @@ -197,8 +202,8 @@ public class RequestResponseBodyMethodProcessorTests { public void resolveArgumentRequiredNoContentDefaultValue() throws Exception { this.servletRequest.setContent(new byte[0]); this.servletRequest.setContentType("text/plain"); - List> converters = Arrays.asList(new StringHttpMessageConverter()); - List advice = Arrays.asList(new EmptyRequestBodyAdvice()); + List> converters = Collections.singletonList(new StringHttpMessageConverter()); + List advice = Collections.singletonList(new EmptyRequestBodyAdvice()); RequestResponseBodyMethodProcessor processor = new RequestResponseBodyMethodProcessor(converters, advice); String arg = (String) processor.resolveArgument(paramString, mavContainer, webRequest, binderFactory); assertNotNull(arg); @@ -294,10 +299,10 @@ public class RequestResponseBodyMethodProcessorTests { public void handleReturnValueImage() throws Exception { this.servletRequest.addHeader("Accept", "*/*"); - Method method = getClass().getMethod("getImage"); + Method method = getClass().getDeclaredMethod("getImage"); MethodParameter returnType = new MethodParameter(method, -1); - List> converters = Arrays.asList(new ResourceHttpMessageConverter()); + List> converters = Collections.singletonList(new ResourceHttpMessageConverter()); RequestResponseBodyMethodProcessor processor = new RequestResponseBodyMethodProcessor(converters); ClassPathResource resource = new ClassPathResource("logo.jpg", getClass()); @@ -306,6 +311,16 @@ public class RequestResponseBodyMethodProcessorTests { assertEquals("image/jpeg", this.servletResponse.getHeader("Content-Type")); } + // SPR-13135 + + @Test(expected = IllegalArgumentException.class) + public void handleReturnValueWithInvalidReturnType() throws Exception { + Method method = getClass().getDeclaredMethod("handleAndReturnOutputStream"); + MethodParameter returnType = new MethodParameter(method, -1); + RequestResponseBodyMethodProcessor processor = new RequestResponseBodyMethodProcessor(new ArrayList<>()); + processor.writeWithMessageConverters(new ByteArrayOutputStream(), returnType, this.webRequest); + } + @Test public void supportsReturnTypeResponseBodyOnType() throws Exception { Method method = ResponseBodyController.class.getMethod("handle"); @@ -342,7 +357,7 @@ public class RequestResponseBodyMethodProcessorTests { converters.add(new MappingJackson2HttpMessageConverter()); RequestResponseBodyMethodProcessor processor = new RequestResponseBodyMethodProcessor( - converters, null, Arrays.asList(new JsonViewResponseBodyAdvice())); + converters, null, Collections.singletonList(new JsonViewResponseBodyAdvice())); Object returnValue = new JacksonViewController().handleResponseBody(); processor.handleReturnValue(returnValue, methodReturnType, this.mavContainer, this.webRequest); @@ -363,7 +378,7 @@ public class RequestResponseBodyMethodProcessorTests { converters.add(new MappingJackson2HttpMessageConverter()); HttpEntityMethodProcessor processor = new HttpEntityMethodProcessor( - converters, null, Arrays.asList(new JsonViewResponseBodyAdvice())); + converters, null, Collections.singletonList(new JsonViewResponseBodyAdvice())); Object returnValue = new JacksonViewController().handleResponseEntity(); processor.handleReturnValue(returnValue, methodReturnType, this.mavContainer, this.webRequest); @@ -384,7 +399,7 @@ public class RequestResponseBodyMethodProcessorTests { converters.add(new MappingJackson2XmlHttpMessageConverter()); RequestResponseBodyMethodProcessor processor = new RequestResponseBodyMethodProcessor( - converters, null, Arrays.asList(new JsonViewResponseBodyAdvice())); + converters, null, Collections.singletonList(new JsonViewResponseBodyAdvice())); Object returnValue = new JacksonViewController().handleResponseBody(); processor.handleReturnValue(returnValue, methodReturnType, this.mavContainer, this.webRequest); @@ -405,7 +420,7 @@ public class RequestResponseBodyMethodProcessorTests { converters.add(new MappingJackson2XmlHttpMessageConverter()); HttpEntityMethodProcessor processor = new HttpEntityMethodProcessor( - converters, null, Arrays.asList(new JsonViewResponseBodyAdvice())); + converters, null, Collections.singletonList(new JsonViewResponseBodyAdvice())); Object returnValue = new JacksonViewController().handleResponseEntity(); processor.handleReturnValue(returnValue, methodReturnType, this.mavContainer, this.webRequest); @@ -430,7 +445,7 @@ public class RequestResponseBodyMethodProcessorTests { converters.add(new MappingJackson2HttpMessageConverter()); RequestResponseBodyMethodProcessor processor = new RequestResponseBodyMethodProcessor( - converters, null, Arrays.asList(new JsonViewRequestBodyAdvice())); + converters, null, Collections.singletonList(new JsonViewRequestBodyAdvice())); @SuppressWarnings("unchecked") JacksonViewBean result = (JacksonViewBean)processor.resolveArgument(methodParameter, @@ -456,7 +471,7 @@ public class RequestResponseBodyMethodProcessorTests { converters.add(new MappingJackson2HttpMessageConverter()); HttpEntityMethodProcessor processor = new HttpEntityMethodProcessor( - converters, null, Arrays.asList(new JsonViewRequestBodyAdvice())); + converters, null, Collections.singletonList(new JsonViewRequestBodyAdvice())); @SuppressWarnings("unchecked") HttpEntity result = (HttpEntity)processor.resolveArgument(methodParameter, @@ -483,7 +498,7 @@ public class RequestResponseBodyMethodProcessorTests { converters.add(new MappingJackson2XmlHttpMessageConverter()); RequestResponseBodyMethodProcessor processor = new RequestResponseBodyMethodProcessor( - converters, null, Arrays.asList(new JsonViewRequestBodyAdvice())); + converters, null, Collections.singletonList(new JsonViewRequestBodyAdvice())); @SuppressWarnings("unchecked") JacksonViewBean result = (JacksonViewBean)processor.resolveArgument(methodParameter, @@ -509,7 +524,7 @@ public class RequestResponseBodyMethodProcessorTests { converters.add(new MappingJackson2XmlHttpMessageConverter()); HttpEntityMethodProcessor processor = new HttpEntityMethodProcessor( - converters, null, Arrays.asList(new JsonViewRequestBodyAdvice())); + converters, null, Collections.singletonList(new JsonViewRequestBodyAdvice())); @SuppressWarnings("unchecked") HttpEntity result = (HttpEntity)processor.resolveArgument(methodParameter, @@ -523,8 +538,7 @@ public class RequestResponseBodyMethodProcessorTests { } - @SuppressWarnings("unused") - public String handle( + String handle( @RequestBody List list, @RequestBody SimpleBean simpleBean, @RequestBody MultiValueMap multiValueMap, @@ -533,29 +547,29 @@ public class RequestResponseBodyMethodProcessorTests { return null; } - @SuppressWarnings("unused") - public Resource getImage() { + Resource getImage() { + return null; + } + + @RequestMapping + OutputStream handleAndReturnOutputStream() { return null; } - @SuppressWarnings("unused") private static abstract class MyParameterizedController { @SuppressWarnings("unused") public void handleDto(@RequestBody DTO dto) {} } - - @SuppressWarnings("unused") private static class MySimpleParameterizedController extends MyParameterizedController { } - private interface Identifiable extends Serializable { - public Long getId(); + Long getId(); - public void setId(Long id); + void setId(Long id); } @@ -580,7 +594,6 @@ public class RequestResponseBodyMethodProcessorTests { return name; } - @SuppressWarnings("unused") public void setName(String name) { this.name = name; } @@ -622,7 +635,6 @@ public class RequestResponseBodyMethodProcessorTests { private interface MyJacksonView1 {} private interface MyJacksonView2 {} - @SuppressWarnings("unused") private static class JacksonViewBean { @JsonView(MyJacksonView1.class) @@ -694,7 +706,6 @@ public class RequestResponseBodyMethodProcessorTests { public JacksonViewBean handleHttpEntity(@JsonView(MyJacksonView1.class) HttpEntity entity) { return entity.getBody(); } - } private static class EmptyRequestBodyAdvice implements RequestBodyAdvice { 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 59f8f0ac9b..1f8c7c6523 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 @@ -16,9 +16,12 @@ package org.springframework.web.servlet.mvc.method.annotation; +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; + import java.lang.reflect.Method; import java.util.ArrayList; -import java.util.Arrays; +import java.util.Collections; import java.util.List; import javax.servlet.http.HttpServletResponse; @@ -46,9 +49,6 @@ import org.springframework.web.method.support.HandlerMethodReturnValueHandlerCom import org.springframework.web.method.support.ModelAndViewContainer; import org.springframework.web.servlet.view.RedirectView; -import static org.junit.Assert.*; -import static org.mockito.Mockito.*; - /** * Test fixture with {@link ServletInvocableHandlerMethod}. * @@ -185,7 +185,7 @@ public class ServletInvocableHandlerMethodTests { public void wrapConcurrentResult_ResponseEntityNullBody() throws Exception { List> converters = new ArrayList>(); converters.add(new StringHttpMessageConverter()); - List advice = Arrays.asList(mock(ResponseBodyAdvice.class)); + List advice = Collections.singletonList(mock(ResponseBodyAdvice.class)); HttpEntityMethodProcessor processor = new HttpEntityMethodProcessor(converters, null, advice); this.returnValueHandlers.addHandler(processor); ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new ResponseEntityHandler(), "handleDeferred"); @@ -200,7 +200,7 @@ public class ServletInvocableHandlerMethodTests { public void wrapConcurrentResult_ResponseEntityNullReturnValue() throws Exception { List> converters = new ArrayList>(); converters.add(new StringHttpMessageConverter()); - List advice = Arrays.asList(mock(ResponseBodyAdvice.class)); + List advice = Collections.singletonList(mock(ResponseBodyAdvice.class)); HttpEntityMethodProcessor processor = new HttpEntityMethodProcessor(converters, null, advice); this.returnValueHandlers.addHandler(processor); ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new ResponseEntityHandler(), "handleDeferred"); @@ -239,8 +239,8 @@ public class ServletInvocableHandlerMethodTests { @Test public void responseEntityRawTypeWithNullBody() throws Exception { - List> converters = Arrays.asList(new StringHttpMessageConverter()); - List advice = Arrays.asList(mock(ResponseBodyAdvice.class)); + List> converters = Collections.singletonList(new StringHttpMessageConverter()); + List advice = Collections.singletonList(mock(ResponseBodyAdvice.class)); HttpEntityMethodProcessor processor = new HttpEntityMethodProcessor(converters, null, advice); this.returnValueHandlers.addHandler(processor); ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new ResponseEntityHandler(), "handleRawType");