From 5fa7f24794cc23cd0a803d8d49024d34f7df328b Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 20 May 2011 17:02:20 +0000 Subject: [PATCH] SPR-7353 Respect 'produces' condition in ContentNegotiatingViewResolver, improve selection of more specific media type in a pair --- ...stractMessageConverterMethodProcessor.java | 66 +++++++++++-------- .../view/ContentNegotiatingViewResolver.java | 43 +++++++++++- .../ContentNegotiatingViewResolverTests.java | 12 ++++ 3 files changed, 92 insertions(+), 29 deletions(-) diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/support/AbstractMessageConverterMethodProcessor.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/support/AbstractMessageConverterMethodProcessor.java index 4c1345a154..bcb48a747c 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/support/AbstractMessageConverterMethodProcessor.java +++ b/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 getAllSupportedMediaTypes(List> messageConverters) { - Set allSupportedMediaTypes = new HashSet(); + Set allSupportedMediaTypes = new LinkedHashSet(); for (HttpMessageConverter messageConverter : messageConverters) { allSupportedMediaTypes.addAll(messageConverter.getSupportedMediaTypes()); } @@ -159,22 +164,24 @@ public abstract class AbstractMessageConverterMethodProcessor ServletServerHttpResponse outputMessage) throws IOException, HttpMediaTypeNotAcceptableException { - - Set acceptableMediaTypes = getAcceptableMediaTypes(inputMessage); - Set producibleMediaTypes = getProducibleMediaTypes(inputMessage.getServletRequest()); - - List mediaTypes = new ArrayList(); - for (MediaType acceptableMediaType : acceptableMediaTypes) { - for (MediaType producibleMediaType : producibleMediaTypes) { - if (acceptableMediaType.isCompatibleWith(producibleMediaType)) { - mediaTypes.add(getMostSpecificMediaType(acceptableMediaType, producibleMediaType)); + List acceptableMediaTypes = getAcceptableMediaTypes(inputMessage); + List producibleMediaTypes = getProducibleMediaTypes(inputMessage.getServletRequest()); + + Set compatibleMediaTypes = new LinkedHashSet(); + 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 mediaTypes = new ArrayList(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: *
    - *
  • The set of producible media types specified in the request mappings, or - *
  • The set of supported media types by all configured message converters, or + *
  • The producible media types specified in the request mappings, or + *
  • The media types supported by all configured message converters, or *
  • {@link MediaType#ALL} *
*/ @SuppressWarnings("unchecked") - protected Set getProducibleMediaTypes(HttpServletRequest request) { + protected List getProducibleMediaTypes(HttpServletRequest request) { Set mediaTypes = (Set) request.getAttribute(HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE); if (!CollectionUtils.isEmpty(mediaTypes)) { - return mediaTypes; + return new ArrayList(mediaTypes); } else if (!allSupportedMediaTypes.isEmpty()) { - return new HashSet(allSupportedMediaTypes); + return allSupportedMediaTypes; } else { - return Collections.singleton(MediaType.ALL); + return Collections.singletonList(MediaType.ALL); } } - private Set getAcceptableMediaTypes(HttpInputMessage inputMessage) { - Set result = new HashSet(inputMessage.getHeaders().getAccept()); - if (result.isEmpty()) { - result.add(MediaType.ALL); - } - return result; + private List getAcceptableMediaTypes(HttpInputMessage inputMessage) { + List 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 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; } } \ No newline at end of file diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolver.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolver.java index 9f3ae78c42..e2e4a2ff75 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolver.java +++ b/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 mediaTypes = MediaType.parseMediaTypes(acceptHeader); + List acceptableMediaTypes = MediaType.parseMediaTypes(acceptHeader); + List producibleMediaTypes = getProducibleMediaTypes(request); + + Set compatibleMediaTypes = new LinkedHashSet(); + for (MediaType a : acceptableMediaTypes) { + for (MediaType p : producibleMediaTypes) { + if (a.isCompatibleWith(p)) { + compatibleMediaTypes.add(getMostSpecificMediaType(a, p)); + } + } + } + + List mediaTypes = new ArrayList(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 getProducibleMediaTypes(HttpServletRequest request) { + Set mediaTypes = (Set) request.getAttribute(HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE); + if (!CollectionUtils.isEmpty(mediaTypes)) { + return new ArrayList(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 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. *

The default implementation will check the {@linkplain #setMediaTypes(Map) media types} diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolverTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolverTests.java index 1856e55385..c126f73208 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolverTests.java +++ b/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 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 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");