From 8d5fc2bf919d809a12d5572406235ac769bfe57e Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Fri, 15 Jan 2010 10:23:59 +0000 Subject: [PATCH] SPR-6686 - @ResponseBody throws HttpMediaTypeNotAcceptableException if client accepts "*/*" --- .../AnnotationMethodHandlerAdapter.java | 5 +- .../AbstractHttpMessageConverter.java | 33 +++++-- .../MappingJacksonHttpMessageConverter.java | 4 +- .../Jaxb2RootElementHttpMessageConverter.java | 4 +- .../ByteArrayHttpMessageConverterTests.java | 11 +++ .../FormHttpMessageConverterTests.java | 13 +++ .../converter/HttpMessageConverterTests.java | 98 +++++++++++++++++++ .../StringHttpMessageConverterTests.java | 11 +++ 8 files changed, 167 insertions(+), 12 deletions(-) create mode 100644 org.springframework.web/src/test/java/org/springframework/http/converter/HttpMessageConverterTests.java diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerAdapter.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerAdapter.java index fea02c54fb..a3b0605118 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerAdapter.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerAdapter.java @@ -864,6 +864,9 @@ public class AnnotationMethodHandlerAdapter extends WebContentGenerator if (acceptedMediaTypes.isEmpty()) { acceptedMediaTypes = Collections.singletonList(MediaType.ALL); } + else { + Collections.sort(acceptedMediaTypes); + } HttpOutputMessage outputMessage = new ServletServerHttpResponse(webRequest.getResponse()); Class returnValueType = returnValue.getClass(); List allSupportedMediaTypes = new ArrayList(); @@ -872,7 +875,7 @@ public class AnnotationMethodHandlerAdapter extends WebContentGenerator allSupportedMediaTypes.addAll(messageConverter.getSupportedMediaTypes()); for (MediaType acceptedMediaType : acceptedMediaTypes) { if (messageConverter.canWrite(returnValueType, acceptedMediaType)) { - messageConverter.write(returnValue, null, outputMessage); + messageConverter.write(returnValue, acceptedMediaType, outputMessage); this.responseArgumentUsed = true; return; } diff --git a/org.springframework.web/src/main/java/org/springframework/http/converter/AbstractHttpMessageConverter.java b/org.springframework.web/src/main/java/org/springframework/http/converter/AbstractHttpMessageConverter.java index 555631e2bd..19a56319ad 100644 --- a/org.springframework.web/src/main/java/org/springframework/http/converter/AbstractHttpMessageConverter.java +++ b/org.springframework.web/src/main/java/org/springframework/http/converter/AbstractHttpMessageConverter.java @@ -92,7 +92,26 @@ public abstract class AbstractHttpMessageConverter implements HttpMessageConv * type. */ public boolean canRead(Class clazz, MediaType mediaType) { - return supports(clazz) && isSupported(mediaType); + return supports(clazz) && canRead(mediaType); + } + + /** + * Returns true if any of the {@linkplain #setSupportedMediaTypes(List) supported media types} include the given media + * type. + * + * @param mediaType the media type + * @return true if the supported media types include the media type, or if the media type is {@code null} + */ + protected boolean canRead(MediaType mediaType) { + if (mediaType == null) { + return true; + } + for (MediaType supportedMediaType : getSupportedMediaTypes()) { + if (supportedMediaType.includes(mediaType)) { + return true; + } + } + return false; } /** @@ -103,22 +122,22 @@ public abstract class AbstractHttpMessageConverter implements HttpMessageConv * type. */ public boolean canWrite(Class clazz, MediaType mediaType) { - return supports(clazz) && isSupported(mediaType); + return supports(clazz) && canWrite(mediaType); } /** - * Returns true if any of the {@linkplain #setSupportedMediaTypes(List) supported media types} include the given media - * type. + * Returns true if the given media type includes any of the + * {@linkplain #setSupportedMediaTypes(List) supported media types}. * * @param mediaType the media type * @return true if the supported media types include the media type, or if the media type is {@code null} */ - protected boolean isSupported(MediaType mediaType) { + protected boolean canWrite(MediaType mediaType) { if (mediaType == null) { return true; } for (MediaType supportedMediaType : getSupportedMediaTypes()) { - if (supportedMediaType.includes(mediaType)) { + if (mediaType.includes(supportedMediaType)) { return true; } } @@ -165,7 +184,7 @@ public abstract class AbstractHttpMessageConverter implements HttpMessageConv public final void write(T t, MediaType contentType, HttpOutputMessage outputMessage) throws IOException, HttpMessageNotWritableException { HttpHeaders headers = outputMessage.getHeaders(); - if (contentType == null) { + if (contentType == null || contentType.isWildcardType() || contentType.isWildcardSubtype()) { contentType = getDefaultContentType(t); } if (contentType != null) { diff --git a/org.springframework.web/src/main/java/org/springframework/http/converter/json/MappingJacksonHttpMessageConverter.java b/org.springframework.web/src/main/java/org/springframework/http/converter/json/MappingJacksonHttpMessageConverter.java index 7e1e78a22d..c8ce393d90 100644 --- a/org.springframework.web/src/main/java/org/springframework/http/converter/json/MappingJacksonHttpMessageConverter.java +++ b/org.springframework.web/src/main/java/org/springframework/http/converter/json/MappingJacksonHttpMessageConverter.java @@ -89,12 +89,12 @@ public class MappingJacksonHttpMessageConverter extends AbstractHttpMessageConve @Override public boolean canRead(Class clazz, MediaType mediaType) { JavaType javaType = TypeFactory.fromClass(clazz); - return objectMapper.canDeserialize(javaType) && isSupported(mediaType); + return objectMapper.canDeserialize(javaType) && canRead(mediaType); } @Override public boolean canWrite(Class clazz, MediaType mediaType) { - return objectMapper.canSerialize(clazz) && isSupported(mediaType); + return objectMapper.canSerialize(clazz) && canWrite(mediaType); } @Override diff --git a/org.springframework.web/src/main/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverter.java b/org.springframework.web/src/main/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverter.java index 10b3294be1..8eb0fbfb4c 100644 --- a/org.springframework.web/src/main/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverter.java +++ b/org.springframework.web/src/main/java/org/springframework/http/converter/xml/Jaxb2RootElementHttpMessageConverter.java @@ -52,12 +52,12 @@ public class Jaxb2RootElementHttpMessageConverter extends AbstractJaxb2HttpMessa @Override public boolean canRead(Class clazz, MediaType mediaType) { return (clazz.isAnnotationPresent(XmlRootElement.class) || clazz.isAnnotationPresent(XmlType.class)) && - isSupported(mediaType); + canRead(mediaType); } @Override public boolean canWrite(Class clazz, MediaType mediaType) { - return AnnotationUtils.findAnnotation(clazz, XmlRootElement.class) != null && isSupported(mediaType); + return AnnotationUtils.findAnnotation(clazz, XmlRootElement.class) != null && canWrite(mediaType); } @Override diff --git a/org.springframework.web/src/test/java/org/springframework/http/converter/ByteArrayHttpMessageConverterTests.java b/org.springframework.web/src/test/java/org/springframework/http/converter/ByteArrayHttpMessageConverterTests.java index 3a0c22cdb1..4b8eda8400 100644 --- a/org.springframework.web/src/test/java/org/springframework/http/converter/ByteArrayHttpMessageConverterTests.java +++ b/org.springframework.web/src/test/java/org/springframework/http/converter/ByteArrayHttpMessageConverterTests.java @@ -36,6 +36,17 @@ public class ByteArrayHttpMessageConverterTests { converter = new ByteArrayHttpMessageConverter(); } + @Test + public void canRead() { + assertTrue(converter.canRead(byte[].class, new MediaType("application", "octet-stream"))); + } + + @Test + public void canWrite() { + assertTrue(converter.canWrite(byte[].class, new MediaType("application", "octet-stream"))); + assertTrue(converter.canWrite(byte[].class, MediaType.ALL)); + } + @Test public void read() throws IOException { byte[] body = new byte[]{0x1, 0x2}; diff --git a/org.springframework.web/src/test/java/org/springframework/http/converter/FormHttpMessageConverterTests.java b/org.springframework.web/src/test/java/org/springframework/http/converter/FormHttpMessageConverterTests.java index f795bf2e57..fedc3bbae6 100644 --- a/org.springframework.web/src/test/java/org/springframework/http/converter/FormHttpMessageConverterTests.java +++ b/org.springframework.web/src/test/java/org/springframework/http/converter/FormHttpMessageConverterTests.java @@ -40,6 +40,19 @@ public class FormHttpMessageConverterTests { converter = new FormHttpMessageConverter(); } + @Test + @SuppressWarnings("unchecked") + public void canRead() { + assertTrue(converter.canRead((Class>) MultiValueMap.class, new MediaType("application", "x-www-form-urlencoded"))); + } + + @Test + @SuppressWarnings("unchecked") + public void canWrite() { + assertTrue(converter.canWrite((Class>) MultiValueMap.class, new MediaType("application", "x-www-form-urlencoded"))); + assertTrue(converter.canWrite((Class>) MultiValueMap.class, MediaType.ALL)); + } + @SuppressWarnings("unchecked") @Test public void read() throws Exception { diff --git a/org.springframework.web/src/test/java/org/springframework/http/converter/HttpMessageConverterTests.java b/org.springframework.web/src/test/java/org/springframework/http/converter/HttpMessageConverterTests.java new file mode 100644 index 0000000000..7768680892 --- /dev/null +++ b/org.springframework.web/src/test/java/org/springframework/http/converter/HttpMessageConverterTests.java @@ -0,0 +1,98 @@ +/* + * Copyright 2002-2010 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.http.converter; + +import java.io.IOException; + +import org.junit.Before; +import org.junit.Test; +import static org.junit.Assert.*; + +import org.springframework.http.HttpInputMessage; +import org.springframework.http.HttpOutputMessage; +import org.springframework.http.MediaType; + +/** + * Test-case for AbstractHttpMessageConverter. + * + * @author Arjen Poutsma + */ +public class HttpMessageConverterTests { + + private static final MediaType MEDIA_TYPE = new MediaType("foo", "bar"); + + @Test + public void canRead() { + AbstractHttpMessageConverter converter = new MyHttpMessageConverter(MEDIA_TYPE) { + @Override + protected boolean supports(Class clazz) { + return MyType.class.equals(clazz); + } + + }; + + assertTrue(converter.canRead(MyType.class, MEDIA_TYPE)); + assertFalse(converter.canRead(MyType.class, new MediaType("foo", "*"))); + assertFalse(converter.canRead(MyType.class, MediaType.ALL)); + } + + @Test + public void canWrite() { + AbstractHttpMessageConverter converter = new MyHttpMessageConverter(MEDIA_TYPE) { + @Override + protected boolean supports(Class clazz) { + return MyType.class.equals(clazz); + } + + }; + + assertTrue(converter.canWrite(MyType.class, MEDIA_TYPE)); + assertTrue(converter.canWrite(MyType.class, new MediaType("foo", "*"))); + assertTrue(converter.canWrite(MyType.class, MediaType.ALL)); + } + + + private static class MyHttpMessageConverter extends AbstractHttpMessageConverter { + + private MyHttpMessageConverter(MediaType supportedMediaType) { + super(supportedMediaType); + } + + @Override + protected boolean supports(Class clazz) { + fail("Not expected"); + return false; + } + + @Override + protected T readInternal(Class clazz, HttpInputMessage inputMessage) + throws IOException, HttpMessageNotReadableException { + fail("Not expected"); + return null; + } + + @Override + protected void writeInternal(T t, HttpOutputMessage outputMessage) + throws IOException, HttpMessageNotWritableException { + fail("Not expected"); + } + } + + private static class MyType { + + } +} diff --git a/org.springframework.web/src/test/java/org/springframework/http/converter/StringHttpMessageConverterTests.java b/org.springframework.web/src/test/java/org/springframework/http/converter/StringHttpMessageConverterTests.java index 502edadebe..f605b2c1fe 100644 --- a/org.springframework.web/src/test/java/org/springframework/http/converter/StringHttpMessageConverterTests.java +++ b/org.springframework.web/src/test/java/org/springframework/http/converter/StringHttpMessageConverterTests.java @@ -37,6 +37,17 @@ public class StringHttpMessageConverterTests { converter = new StringHttpMessageConverter(); } + @Test + public void canRead() { + assertTrue(converter.canRead(String.class, new MediaType("text", "plain"))); + } + + @Test + public void canWrite() { + assertTrue(converter.canWrite(String.class, new MediaType("text", "plain"))); + assertTrue(converter.canWrite(String.class, MediaType.ALL)); + } + @Test public void read() throws IOException { String body = "Hello World";