SPR-7353 Respect 'produces' condition in ContentNegotiatingViewResolver, improve selection of more specific media type in a pair

git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@4340 50f2f4bb-b051-0410-bef5-90022cba6387

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/support/AbstractMessageConverterMethodProcessor.java

@@ -19,15 +19,16 @@ package org.springframework.web.servlet.mvc.method.annotation.support;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
+
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-
 import org.springframework.core.MethodParameter;
 import org.springframework.http.HttpInputMessage;
 import org.springframework.http.HttpOutputMessage;
@@ -70,8 +71,12 @@ public abstract class AbstractMessageConverterMethodProcessor
 		this.allSupportedMediaTypes = getAllSupportedMediaTypes(messageConverters);
 	}
 
+	/**
+	 * Returns the media types supported by all provided message converters preserving their ordering and 
+	 * further sorting by specificity via {@link MediaType#sortBySpecificity(List)}. 
+	 */
 	private static List<MediaType> getAllSupportedMediaTypes(List<HttpMessageConverter<?>> messageConverters) {
-		Set<MediaType> allSupportedMediaTypes = new HashSet<MediaType>();
+		Set<MediaType> allSupportedMediaTypes = new LinkedHashSet<MediaType>();
 		for (HttpMessageConverter<?> messageConverter : messageConverters) {
 			allSupportedMediaTypes.addAll(messageConverter.getSupportedMediaTypes());
 		}
@@ -159,22 +164,24 @@ public abstract class AbstractMessageConverterMethodProcessor
 												  ServletServerHttpResponse outputMessage)
 			throws IOException, HttpMediaTypeNotAcceptableException {
 
-
-		Set<MediaType> acceptableMediaTypes = getAcceptableMediaTypes(inputMessage);
-		Set<MediaType> producibleMediaTypes = getProducibleMediaTypes(inputMessage.getServletRequest());
-
-		List<MediaType> mediaTypes = new ArrayList<MediaType>();
-		for (MediaType acceptableMediaType : acceptableMediaTypes) {
-			for (MediaType producibleMediaType : producibleMediaTypes) {
-				if (acceptableMediaType.isCompatibleWith(producibleMediaType)) {
-					mediaTypes.add(getMostSpecificMediaType(acceptableMediaType, producibleMediaType));
+		List<MediaType> acceptableMediaTypes = getAcceptableMediaTypes(inputMessage);
+		List<MediaType> producibleMediaTypes = getProducibleMediaTypes(inputMessage.getServletRequest());
+		
+		Set<MediaType> compatibleMediaTypes = new LinkedHashSet<MediaType>();
+		for (MediaType a : acceptableMediaTypes) {
+			for (MediaType p : producibleMediaTypes) {
+				if (a.isCompatibleWith(p)) {
+					compatibleMediaTypes.add(getMostSpecificMediaType(a, p));
 				}
 			}
 		}
-		if (mediaTypes.isEmpty()) {
+		if (compatibleMediaTypes.isEmpty()) {
 			throw new HttpMediaTypeNotAcceptableException(allSupportedMediaTypes);
 		}
+		
+		List<MediaType> mediaTypes = new ArrayList<MediaType>(compatibleMediaTypes);
 		MediaType.sortBySpecificity(mediaTypes);
+		
 		MediaType selectedMediaType = null;
 		for (MediaType mediaType : mediaTypes) {
 			if (mediaType.isConcrete()) {
@@ -186,6 +193,7 @@ public abstract class AbstractMessageConverterMethodProcessor
 				break;
 			}
 		}
+		
 		if (selectedMediaType != null) {
 			for (HttpMessageConverter<?> messageConverter : messageConverters) {
 				if (messageConverter.canWrite(returnValue.getClass(), selectedMediaType)) {
@@ -204,36 +212,40 @@ public abstract class AbstractMessageConverterMethodProcessor
 	/**
 	 * Returns the media types that can be produced:
 	 * <ul>
-	 * 	<li>The set of producible media types specified in the request mappings, or
-	 * 	<li>The set of supported media types by all configured message converters, or
+	 * 	<li>The producible media types specified in the request mappings, or
+	 * 	<li>The media types supported by all configured message converters, or
 	 * 	<li>{@link MediaType#ALL}
 	 * </ul>
 	 */
 	@SuppressWarnings("unchecked")
-	protected Set<MediaType> getProducibleMediaTypes(HttpServletRequest request) {
+	protected List<MediaType> getProducibleMediaTypes(HttpServletRequest request) {
 		Set<MediaType> mediaTypes = (Set<MediaType>) request.getAttribute(HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE);
 		if (!CollectionUtils.isEmpty(mediaTypes)) {
-			return mediaTypes;
+			return new ArrayList<MediaType>(mediaTypes);
 		}
 		else if (!allSupportedMediaTypes.isEmpty()) {
-			return new HashSet<MediaType>(allSupportedMediaTypes);
+			return allSupportedMediaTypes;
 		}
 		else {
-			return Collections.singleton(MediaType.ALL);
+			return Collections.singletonList(MediaType.ALL);
 		}
 
 	}
 
-	private Set<MediaType> getAcceptableMediaTypes(HttpInputMessage inputMessage) {
-		Set<MediaType> result = new HashSet<MediaType>(inputMessage.getHeaders().getAccept());
-		if (result.isEmpty()) {
-			result.add(MediaType.ALL);
-		}
-		return result;
+	private List<MediaType> getAcceptableMediaTypes(HttpInputMessage inputMessage) {
+		List<MediaType> result = inputMessage.getHeaders().getAccept();
+		return result.isEmpty() ? Collections.singletonList(MediaType.ALL) : result;
 	}
-	
+
+	/**
+	 * Returns the more specific media type using the q-value of the first media type for both.
+	 */
 	private MediaType getMostSpecificMediaType(MediaType type1, MediaType type2) {
-		return MediaType.SPECIFICITY_COMPARATOR.compare(type1, type2) < 0 ? type1 : type2;
+		double quality = type1.getQualityValue();
+		Map<String, String> params = Collections.singletonMap("q", String.valueOf(quality));
+		MediaType t1 = new MediaType(type1, params);
+		MediaType t2 = new MediaType(type2, params);
+		return MediaType.SPECIFICITY_COMPARATOR.compare(t1, t2) <= 0 ? type1 : type2;
 	}
 
 }

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolver.java

@@ -21,10 +21,12 @@ import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
@@ -50,6 +52,7 @@ import org.springframework.web.context.request.RequestAttributes;
 import org.springframework.web.context.request.RequestContextHolder;
 import org.springframework.web.context.request.ServletRequestAttributes;
 import org.springframework.web.context.support.WebApplicationObjectSupport;
+import org.springframework.web.servlet.HandlerMapping;
 import org.springframework.web.servlet.View;
 import org.springframework.web.servlet.ViewResolver;
 import org.springframework.web.util.UrlPathHelper;
@@ -316,10 +319,24 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport
 		if (!this.ignoreAcceptHeader) {
 			String acceptHeader = request.getHeader(ACCEPT_HEADER);
 			if (StringUtils.hasText(acceptHeader)) {
-				List<MediaType> mediaTypes = MediaType.parseMediaTypes(acceptHeader);
+				List<MediaType> acceptableMediaTypes = MediaType.parseMediaTypes(acceptHeader);
+				List<MediaType> producibleMediaTypes = getProducibleMediaTypes(request);
+
+				Set<MediaType> compatibleMediaTypes = new LinkedHashSet<MediaType>();
+				for (MediaType a : acceptableMediaTypes) {
+					for (MediaType p : producibleMediaTypes) {
+						if (a.isCompatibleWith(p)) {
+							compatibleMediaTypes.add(getMostSpecificMediaType(a, p));
+						}
+					}
+				}
+				
+				List<MediaType> mediaTypes = new ArrayList<MediaType>(compatibleMediaTypes);
 				MediaType.sortByQualityValue(mediaTypes);
+
 				if (logger.isDebugEnabled()) {
-					logger.debug("Requested media types are " + mediaTypes + " (based on Accept header)");
+					logger.debug("Requested media types are " + mediaTypes + " based on Accept header types " + 
+							"and producible media types " + producibleMediaTypes + ")");
 				}
 				return mediaTypes;
 			}
@@ -336,6 +353,28 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport
 		}
 	}
 
+	@SuppressWarnings("unchecked")
+	private List<MediaType> getProducibleMediaTypes(HttpServletRequest request) {
+		Set<MediaType> mediaTypes = (Set<MediaType>) request.getAttribute(HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE);
+		if (!CollectionUtils.isEmpty(mediaTypes)) {
+			return new ArrayList<MediaType>(mediaTypes);
+		}
+		else {
+			return Collections.singletonList(MediaType.ALL);
+		}
+	}
+
+	/**
+	 * Returns the more specific media type using the q-value of the first media type for both.
+	 */
+	private MediaType getMostSpecificMediaType(MediaType type1, MediaType type2) {
+		double quality = type1.getQualityValue();
+		Map<String, String> params = Collections.singletonMap("q", String.valueOf(quality));
+		MediaType t1 = new MediaType(type1, params);
+		MediaType t2 = new MediaType(type2, params);
+		return MediaType.SPECIFICITY_COMPARATOR.compare(t1, t2) <= 0 ? type1 : type2;
+	}
+
 	/**
 	 * Determines the {@link MediaType} for the given filename.
 	 * <p>The default implementation will check the {@linkplain #setMediaTypes(Map) media types}

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolverTests.java

@@ -32,6 +32,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Set;
 
 import org.junit.After;
 import org.junit.Before;
@@ -43,6 +44,7 @@ import org.springframework.mock.web.MockServletContext;
 import org.springframework.web.context.request.RequestContextHolder;
 import org.springframework.web.context.request.ServletRequestAttributes;
 import org.springframework.web.context.support.StaticWebApplicationContext;
+import org.springframework.web.servlet.HandlerMapping;
 import org.springframework.web.servlet.View;
 import org.springframework.web.servlet.ViewResolver;
 
@@ -120,6 +122,16 @@ public class ContentNegotiatingViewResolverTests {
 				result.get(3));
 	}
 
+	@Test
+	public void getMediaTypeAcceptHeaderWithProduces() {
+		Set<MediaType> producibleTypes = Collections.singleton(MediaType.APPLICATION_XHTML_XML);
+		request.setAttribute(HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE, producibleTypes);
+		request.addHeader("Accept", "text/html,application/xml;q=0.9,application/xhtml+xml,*/*;q=0.8");
+		List<MediaType> result = viewResolver.getMediaTypes(request);
+		assertEquals("Invalid amount of media types", 1, result.size());
+		assertEquals("Invalid content type", new MediaType("application", "xhtml+xml"), result.get(0));
+	}
+
 	@Test
 	public void getDefaultContentType() {
 		request.addHeader("Accept", "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8");