SPR-6970 - AbstractHttpMessageConverter canWrite logic the wrong way round??

org.springframework.web/src/main/java/org/springframework/http/MediaType.java

@@ -366,17 +366,21 @@ public class MediaType implements Comparable<MediaType> {
 	 * Indicate whether this {@link MediaType} includes the given media type.
 	 *
 	 * <p>For instance, {@code text/*} includes {@code text/plain}, {@code text/html}, and {@code application/*+xml}
-	 * includes {@code application/soap+xml}, etc.
+	 * includes {@code application/soap+xml}, etc. This method is non-symmetic.
 	 *
 	 * @param other the reference media type with which to compare
 	 * @return <code>true</code> if this media type includes the given media type; <code>false</code> otherwise
 	 */
 	public boolean includes(MediaType other) {
-		if (this == other) {
+		if (other == null) {
+			return false;
+		}
+		if (this.isWildcardType()) {
+			// */* includes anything
 			return true;
 		}
-		if (this.type.equals(other.type)) {
-			if (this.subtype.equals(other.subtype) || isWildcardSubtype()) {
+		else if (this.type.equals(other.type)) {
+			if (this.subtype.equals(other.subtype) || this.isWildcardSubtype()) {
 				return true;
 			}
 			// application/*+xml includes application/soap+xml
@@ -392,7 +396,46 @@ public class MediaType implements Comparable<MediaType> {
 				}
 			}
 		}
-		return isWildcardType();
+		return false;
+	}
+
+	/**
+	 * Indicate whether this {@link MediaType} is compatible with the given media type.
+	 *
+	 * <p>For instance, {@code text/*} is compatible with {@code text/plain}, {@code text/html}, and vice versa. In
+	 * effect, this method is similar to {@link #includes(MediaType)}, except that it's symmetric.
+	 *
+	 * @param other the reference media type with which to compare
+	 * @return <code>true</code> if this media type is compatible with the given media type; <code>false</code> otherwise
+	 */
+	public boolean isCompatibleWith(MediaType other) {
+		if (other == null) {
+			return false;
+		}
+		if (isWildcardType() || other.isWildcardType()) {
+			return true;
+		}
+		else if (this.type.equals(other.type)) {
+			if (this.subtype.equals(other.subtype) || this.isWildcardSubtype() || other.isWildcardSubtype()) {
+				return true;
+			}
+			// application/*+xml is compatible with application/soap+xml, and vice-versa
+			int thisPlusIdx = this.subtype.indexOf('+');
+			int otherPlusIdx = other.subtype.indexOf('+');
+			if (thisPlusIdx != -1 && otherPlusIdx != -1) {
+				String thisSubtypeNoSuffix = this.subtype.substring(0, thisPlusIdx);
+				String otherSubtypeNoSuffix = other.subtype.substring(0, otherPlusIdx);
+
+				String thisSubtypeSuffix = this.subtype.substring(thisPlusIdx + 1);
+				String otherSubtypeSuffix = other.subtype.substring(otherPlusIdx + 1);
+
+				if (thisSubtypeSuffix.equals(otherSubtypeSuffix) &&
+						(WILDCARD_TYPE.equals(thisSubtypeNoSuffix) || WILDCARD_TYPE.equals(otherSubtypeNoSuffix))) {
+					return true;
+				}
+			}
+		}
+		return false;
 	}
 
 	/**

org.springframework.web/src/main/java/org/springframework/http/converter/AbstractHttpMessageConverter.java

@@ -100,7 +100,8 @@ public abstract class AbstractHttpMessageConverter<T> implements HttpMessageConv
 	/**
 	 * Returns true if any of the {@linkplain #setSupportedMediaTypes(List) supported media types}
 	 * include the given media type.
-	 * @param mediaType the media type
+	 * @param mediaType the media type to read, can be {@code null} if not specified. Typically the value of a
+	 *                  {@code Content-Type} header.
 	 * @return true if the supported media types include the media type, or if the media type is {@code null}
 	 */
 	protected boolean canRead(MediaType mediaType) {
@@ -123,20 +124,21 @@ public abstract class AbstractHttpMessageConverter<T> implements HttpMessageConv
 	 */
 	public boolean canWrite(Class<?> clazz, MediaType mediaType) {
 		return supports(clazz) && canWrite(mediaType);
-	}
+		}
 
 	/**
 	 * 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}
+	 * @param mediaType the media type to write, can be {@code null} if not specified. Typically the value of an
+	 * 		  			{@code Accept} header.
+	 * @return true if the supported media types are compatible with the media type, or if the media type is {@code null}
 	 */
 	protected boolean canWrite(MediaType mediaType) {
-		if (mediaType == null) {
+		if (mediaType == null || MediaType.ALL.equals(mediaType)) {
 			return true;
 		}
 		for (MediaType supportedMediaType : getSupportedMediaTypes()) {
-			if (mediaType.includes(supportedMediaType)) {
+			if (supportedMediaType.isCompatibleWith(mediaType)) {
 				return true;
 			}
 		}

org.springframework.web/src/main/java/org/springframework/http/converter/FormHttpMessageConverter.java

@@ -138,8 +138,8 @@ public class FormHttpMessageConverter implements HttpMessageConverter<MultiValue
 			return false;
 		}
 		if (mediaType != null) {
-			return mediaType.includes(MediaType.APPLICATION_FORM_URLENCODED) ||
-					mediaType.includes(MediaType.MULTIPART_FORM_DATA);
+			return mediaType.isCompatibleWith(MediaType.APPLICATION_FORM_URLENCODED) ||
+					mediaType.isCompatibleWith(MediaType.MULTIPART_FORM_DATA);
 		}
 		else {
 			return true;

org.springframework.web/src/main/java/org/springframework/http/converter/HttpMessageConverter.java

@@ -35,16 +35,18 @@ public interface HttpMessageConverter<T> {
 	/**
 	 * Indicates whether the given class can be read by this converter.
 	 * @param clazz the class to test for readability
-	 * @param mediaType the media type to read, can be {@code null} if not specified
-	 * @return <code>true</code> if readable; <code>false</code> otherwise
+	 * @param mediaType the media type to read, can be {@code null} if not specified. Typically the value of a
+	 *                  {@code Content-Type} header.
+	 * @return {@code true} if readable; {@code false} otherwise
 	 */
 	boolean canRead(Class<?> clazz, MediaType mediaType);
 
 	/**
 	 * Indicates whether the given class can be written by this converter.
 	 * @param clazz the class to test for writability
-	 * @param mediaType the media type to write, can be {@code null} if not specified
-	 * @return <code>true</code> if writable; <code>false</code> otherwise
+	 * @param mediaType the media type to write, can be {@code null} if not specified. Typically the value of an
+	 * 		  			{@code Accept} header.
+	 * @return {@code true} if writable; {@code false} otherwise
 	 */
 	boolean canWrite(Class<?> clazz, MediaType mediaType);
 

org.springframework.web/src/test/java/org/springframework/http/MediaTypeTests.java

@@ -34,10 +34,15 @@ public class MediaTypeTests {
 		MediaType textPlain = MediaType.TEXT_PLAIN;
 		assertTrue("Equal types is not inclusive", textPlain.includes(textPlain));
 		MediaType allText = new MediaType("text");
+
 		assertTrue("All subtypes is not inclusive", allText.includes(textPlain));
-		assertFalse("All subtypes is not inclusive", textPlain.includes(allText));
+		assertFalse("All subtypes is inclusive", textPlain.includes(allText));
+
 		assertTrue("All types is not inclusive", MediaType.ALL.includes(textPlain));
-		assertFalse("All types is not inclusive", textPlain.includes(MediaType.ALL));
+		assertFalse("All types is inclusive", textPlain.includes(MediaType.ALL));
+
+		assertTrue("All types is not inclusive", MediaType.ALL.includes(textPlain));
+		assertFalse("All types is inclusive", textPlain.includes(MediaType.ALL));
 
 		MediaType applicationSoapXml = new MediaType("application", "soap+xml");
 		MediaType applicationWildcardXml = new MediaType("application", "*+xml");
@@ -49,6 +54,31 @@ public class MediaTypeTests {
 		assertFalse(applicationSoapXml.includes(applicationWildcardXml));
 	}
 	
+	@Test
+	public void isCompatible() throws Exception {
+		MediaType textPlain = MediaType.TEXT_PLAIN;
+		assertTrue("Equal types is not compatible", textPlain.isCompatibleWith(textPlain));
+		MediaType allText = new MediaType("text");
+
+		assertTrue("All subtypes is not compatible", allText.isCompatibleWith(textPlain));
+		assertTrue("All subtypes is not compatible", textPlain.isCompatibleWith(allText));
+
+		assertTrue("All types is not compatible", MediaType.ALL.isCompatibleWith(textPlain));
+		assertTrue("All types is not compatible", textPlain.isCompatibleWith(MediaType.ALL));
+
+		assertTrue("All types is not compatible", MediaType.ALL.isCompatibleWith(textPlain));
+		assertTrue("All types is compatible", textPlain.isCompatibleWith(MediaType.ALL));
+
+		MediaType applicationSoapXml = new MediaType("application", "soap+xml");
+		MediaType applicationWildcardXml = new MediaType("application", "*+xml");
+
+		assertTrue(applicationSoapXml.isCompatibleWith(applicationSoapXml));
+		assertTrue(applicationWildcardXml.isCompatibleWith(applicationWildcardXml));
+
+		assertTrue(applicationWildcardXml.isCompatibleWith(applicationSoapXml));
+		assertTrue(applicationSoapXml.isCompatibleWith(applicationWildcardXml));
+	}
+
 	@Test
 	public void testToString() throws Exception {
 		MediaType mediaType = new MediaType("text", "plain", 0.7);

org.springframework.web/src/test/java/org/springframework/http/converter/xml/SourceHttpMessageConverterTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2009 the original author or authors.
+ * 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.
@@ -25,7 +25,7 @@ import javax.xml.transform.sax.SAXSource;
 import javax.xml.transform.stream.StreamSource;
 
 import static org.custommonkey.xmlunit.XMLAssert.*;
-import static org.junit.Assert.assertEquals;
+import org.junit.Before;
 import org.junit.Test;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
@@ -37,26 +37,45 @@ import org.springframework.http.MockHttpOutputMessage;
 import org.springframework.util.FileCopyUtils;
 
 /** @author Arjen Poutsma */
+@SuppressWarnings("unchecked")
 public class SourceHttpMessageConverterTests {
 
+	private SourceHttpMessageConverter<Source> converter;
+
+	@Before
+	public void setUp() {
+		converter = new SourceHttpMessageConverter<Source>();
+	}
+
+	@Test
+	public void canRead() {
+		assertTrue(converter.canRead(Source.class, new MediaType("application", "xml")));
+		assertTrue(converter.canRead(Source.class, new MediaType("application", "soap+xml")));
+	}
+
+	@Test
+	public void canWrite() {
+		assertTrue(converter.canWrite(Source.class, new MediaType("application", "xml")));
+		assertTrue(converter.canWrite(Source.class, new MediaType("application", "soap+xml")));
+		assertTrue(converter.canWrite(Source.class, MediaType.ALL));
+	}
+
 	@Test
 	public void readDOMSource() throws Exception {
-		SourceHttpMessageConverter<DOMSource> converter = new SourceHttpMessageConverter<DOMSource>();
 		String body = "<root>Hello World</root>";
 		MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes("UTF-8"));
 		inputMessage.getHeaders().setContentType(new MediaType("application", "xml"));
-		DOMSource result = converter.read(DOMSource.class, inputMessage);
+		DOMSource result = (DOMSource) converter.read(DOMSource.class, inputMessage);
 		Document document = (Document) result.getNode();
 		assertEquals("Invalid result", "root", document.getDocumentElement().getLocalName());
 	}
 
 	@Test
 	public void readSAXSource() throws Exception {
-		SourceHttpMessageConverter<SAXSource> converter = new SourceHttpMessageConverter<SAXSource>();
 		String body = "<root>Hello World</root>";
 		MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes("UTF-8"));
 		inputMessage.getHeaders().setContentType(new MediaType("application", "xml"));
-		SAXSource result = converter.read(SAXSource.class, inputMessage);
+		SAXSource result = (SAXSource) converter.read(SAXSource.class, inputMessage);
 		InputSource inputSource = result.getInputSource();
 		String s = FileCopyUtils.copyToString(new InputStreamReader(inputSource.getByteStream()));
 		assertXMLEqual("Invalid result", body, s);
@@ -64,18 +83,16 @@ public class SourceHttpMessageConverterTests {
 
 	@Test
 	public void readStreamSource() throws Exception {
-		SourceHttpMessageConverter<StreamSource> converter = new SourceHttpMessageConverter<StreamSource>();
 		String body = "<root>Hello World</root>";
 		MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes("UTF-8"));
 		inputMessage.getHeaders().setContentType(new MediaType("application", "xml"));
-		StreamSource result = converter.read(StreamSource.class, inputMessage);
+		StreamSource result = (StreamSource) converter.read(StreamSource.class, inputMessage);
 		String s = FileCopyUtils.copyToString(new InputStreamReader(result.getInputStream()));
 		assertXMLEqual("Invalid result", body, s);
 	}
 
 	@Test
 	public void readSource() throws Exception {
-		SourceHttpMessageConverter<Source> converter = new SourceHttpMessageConverter<Source>();
 		String body = "<root>Hello World</root>";
 		MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes("UTF-8"));
 		inputMessage.getHeaders().setContentType(new MediaType("application", "xml"));
@@ -92,7 +109,6 @@ public class SourceHttpMessageConverterTests {
 		rootElement.setTextContent("Hello World");
 		DOMSource domSource = new DOMSource(document);
 
-		SourceHttpMessageConverter<Source> converter = new SourceHttpMessageConverter<Source>();
 		MockHttpOutputMessage outputMessage = new MockHttpOutputMessage();
 		converter.write(domSource, null, outputMessage);
 		assertXMLEqual("Invalid result", "<root>Hello World</root>",