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 ffb61e7901d..fac5088ca2b 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 @@ -74,7 +74,6 @@ import org.springframework.ui.Model; import org.springframework.util.AntPathMatcher; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; -import org.springframework.util.CollectionUtils; import org.springframework.util.ObjectUtils; import org.springframework.util.PathMatcher; import org.springframework.util.StringUtils; @@ -883,7 +882,7 @@ public class AnnotationMethodHandlerAdapter extends WebContentGenerator if (acceptedMediaTypes.isEmpty()) { acceptedMediaTypes = Collections.singletonList(MediaType.ALL); } - MediaType.sortBySpecificity(acceptedMediaTypes); + MediaType.sortByQualityValue(acceptedMediaTypes); Class returnValueType = returnValue.getClass(); List allSupportedMediaTypes = new ArrayList(); if (getMessageConverters() != null) { @@ -1017,6 +1016,8 @@ public class AnnotationMethodHandlerAdapter extends WebContentGenerator private int compareAcceptHeaders(RequestMappingInfo info1, RequestMappingInfo info2) { List requestAccepts = request.getHeaders().getAccept(); + MediaType.sortByQualityValue(requestAccepts); + List info1Accepts = getAcceptHeaderValue(info1); List info2Accepts = getAcceptHeaderValue(info2); diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerExceptionResolver.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerExceptionResolver.java index ef9d81f20f8..f892fe90af1 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerExceptionResolver.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerExceptionResolver.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. @@ -16,11 +16,11 @@ package org.springframework.web.servlet.mvc.annotation; +import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.io.Reader; import java.io.Writer; -import java.io.IOException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.security.Principal; @@ -32,9 +32,9 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; import java.util.Map; +import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; -import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; @@ -42,13 +42,23 @@ import javax.servlet.http.HttpSession; import org.springframework.core.GenericTypeResolver; import org.springframework.core.MethodParameter; import org.springframework.core.annotation.AnnotationUtils; +import org.springframework.http.HttpInputMessage; +import org.springframework.http.HttpOutputMessage; +import org.springframework.http.MediaType; +import org.springframework.http.converter.ByteArrayHttpMessageConverter; +import org.springframework.http.converter.FormHttpMessageConverter; +import org.springframework.http.converter.HttpMessageConverter; +import org.springframework.http.converter.StringHttpMessageConverter; +import org.springframework.http.converter.xml.SourceHttpMessageConverter; +import org.springframework.http.server.ServletServerHttpRequest; +import org.springframework.http.server.ServletServerHttpResponse; import org.springframework.ui.Model; import org.springframework.util.ClassUtils; import org.springframework.util.ObjectUtils; import org.springframework.util.ReflectionUtils; import org.springframework.web.bind.annotation.ExceptionHandler; -import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.bind.annotation.ResponseBody; +import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.bind.support.WebArgumentResolver; import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.context.request.ServletWebRequest; @@ -57,17 +67,6 @@ import org.springframework.web.servlet.ModelAndView; import org.springframework.web.servlet.View; import org.springframework.web.servlet.handler.AbstractHandlerExceptionResolver; import org.springframework.web.servlet.support.RequestContextUtils; -import org.springframework.web.HttpMediaTypeNotAcceptableException; -import org.springframework.http.converter.HttpMessageConverter; -import org.springframework.http.converter.ByteArrayHttpMessageConverter; -import org.springframework.http.converter.StringHttpMessageConverter; -import org.springframework.http.converter.FormHttpMessageConverter; -import org.springframework.http.converter.xml.SourceHttpMessageConverter; -import org.springframework.http.HttpInputMessage; -import org.springframework.http.MediaType; -import org.springframework.http.HttpOutputMessage; -import org.springframework.http.server.ServletServerHttpRequest; -import org.springframework.http.server.ServletServerHttpResponse; /** * Implementation of the {@link org.springframework.web.servlet.HandlerExceptionResolver} interface that handles @@ -396,7 +395,7 @@ public class AnnotationMethodHandlerExceptionResolver extends AbstractHandlerExc if (acceptedMediaTypes.isEmpty()) { acceptedMediaTypes = Collections.singletonList(MediaType.ALL); } - MediaType.sortBySpecificity(acceptedMediaTypes); + MediaType.sortByQualityValue(acceptedMediaTypes); HttpOutputMessage outputMessage = new ServletServerHttpResponse(webRequest.getResponse()); Class returnValueType = returnValue.getClass(); if (messageConverters != null) { 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 76836575bf2..510b12f3bda 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 @@ -23,8 +23,6 @@ import java.util.Collections; import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.SortedMap; -import java.util.TreeMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import javax.activation.FileTypeMap; @@ -292,7 +290,7 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport String acceptHeader = request.getHeader(ACCEPT_HEADER); if (StringUtils.hasText(acceptHeader)) { List mediaTypes = MediaType.parseMediaTypes(acceptHeader); - MediaType.sortBySpecificity(mediaTypes); + MediaType.sortByQualityValue(mediaTypes); if (logger.isDebugEnabled()) { logger.debug("Requested media types are " + mediaTypes + " (based on Accept header)"); } diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/RequestMappingInfoComparatorTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/RequestMappingInfoComparatorTests.java index 57fa7d82e6d..7e095f459cf 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/RequestMappingInfoComparatorTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/RequestMappingInfoComparatorTests.java @@ -120,8 +120,15 @@ public class RequestMappingInfoComparatorTests { assertTrue(comparator.compare(html, xml) == 0); assertTrue(comparator.compare(xml, html) == 0); - } + // See SPR-7000 + request = new MockHttpServletRequest(); + request.addHeader("Accept", "text/html;q=0.9,application/xml"); + comparator = new AnnotationMethodHandlerAdapter.RequestMappingInfoComparator(new MockComparator(), request); + + assertTrue(comparator.compare(html, xml) > 0); + assertTrue(comparator.compare(xml, html) < 0); + } private static class MockComparator implements Comparator { diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java index 74d3663a9cc..2481ef8eb4f 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java @@ -1289,6 +1289,12 @@ public class ServletAnnotationControllerTests { response = new MockHttpServletResponse(); servlet.service(request, response); assertEquals("xml", response.getContentAsString()); + + request = new MockHttpServletRequest("GET", "/something"); + request.addHeader("Accept", "text/html;q=0.9, application/xml"); + response = new MockHttpServletResponse(); + servlet.service(request, response); + assertEquals("xml", response.getContentAsString()); } @Test @@ -2633,5 +2639,4 @@ public class ServletAnnotationControllerTests { } - } 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 44fb24d6a5c..e73a7ab2cbe 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 @@ -82,7 +82,7 @@ public class ContentNegotiatingViewResolverTests { @Test public void getMediaTypeAcceptHeader() { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/test"); - request.addHeader("Accept", "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8"); + 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", 4, result.size()); assertEquals("Invalid content type", new MediaType("text", "html"), result.get(0)); diff --git a/org.springframework.web/src/main/java/org/springframework/http/MediaType.java b/org.springframework.web/src/main/java/org/springframework/http/MediaType.java index 3f973eff3bb..465f21e6aa1 100644 --- a/org.springframework.web/src/main/java/org/springframework/http/MediaType.java +++ b/org.springframework.web/src/main/java/org/springframework/http/MediaType.java @@ -648,6 +648,35 @@ public class MediaType implements Comparable { } } + /** + * Sorts the given list of {@link MediaType} objects by quality value. + * + *

Given two media types: + *

    + *
  1. if the two media types have different {@linkplain #getQualityValue() quality value}, then the media type + * with the highest quality value is ordered before the other.
    2. + *
  2. if either media type has a {@linkplain #isWildcardType() wildcard type}, then the media type without the + * wildcard is ordered before the other.
    3. + *
  3. if the two media types have different {@linkplain #getType() types}, then they are considered equal and + * remain their current order.
    4. + *
  4. if either media type has a {@linkplain #isWildcardSubtype() wildcard subtype}, then the media type without + * the wildcard is sorted before the other.
    5. + *
  5. if the two media types have different {@linkplain #getSubtype() subtypes}, then they are considered equal + * and remain their current order.
    6. + *
  6. if the two media types have a different amount of {@linkplain #getParameter(String) parameters}, then the + * media type with the most parameters is ordered before the other.
    7. + *
+ * + * @param mediaTypes the list of media types to be sorted + * @see #getQualityValue() + */ + public static void sortByQualityValue(List mediaTypes) { + Assert.notNull(mediaTypes, "'mediaTypes' must not be null"); + if (mediaTypes.size() > 1) { + Collections.sort(mediaTypes, QUALITY_VALUE_COMPARATOR); + } + } + static final Comparator SPECIFICITY_COMPARATOR = new Comparator() { public int compare(MediaType mediaType1, MediaType mediaType2) { @@ -686,4 +715,39 @@ public class MediaType implements Comparable { } }; + static final Comparator QUALITY_VALUE_COMPARATOR = new Comparator() { + + public int compare(MediaType mediaType1, MediaType mediaType2) { + double quality1 = mediaType1.getQualityValue(); + double quality2 = mediaType2.getQualityValue(); + int qualityComparison = Double.compare(quality2, quality1); + if (qualityComparison != 0) { + return qualityComparison; // audio/*;q=0.7 < audio/*;q=0.3 + } + else if (mediaType1.isWildcardType() && !mediaType2.isWildcardType()) { // */* < audio/* + return 1; + } + else if (mediaType2.isWildcardType() && !mediaType1.isWildcardType()) { // audio/* > */* + return -1; + } + else if (!mediaType1.getType().equals(mediaType2.getType())) { // audio/basic == text/html + return 0; + } + else { // mediaType1.getType().equals(mediaType2.getType()) + if (mediaType1.isWildcardSubtype() && !mediaType2.isWildcardSubtype()) { // audio/* < audio/basic + return 1; + } + else if (mediaType2.isWildcardSubtype() && !mediaType1.isWildcardSubtype()) { // audio/basic > audio/* + return -1; + } + else if (!mediaType1.getSubtype().equals(mediaType2.getSubtype())) { // audio/basic == audio/wave + return 0; + } else { + int paramsSize1 = mediaType1.parameters.size(); + int paramsSize2 = mediaType2.parameters.size(); + return (paramsSize2 < paramsSize1 ? -1 : (paramsSize2 == paramsSize1 ? 0 : 1)); // audio/basic;level=1 < audio/basic + } + } + } + }; } diff --git a/org.springframework.web/src/test/java/org/springframework/http/MediaTypeTests.java b/org.springframework.web/src/test/java/org/springframework/http/MediaTypeTests.java index 970cf3edf77..d518ddb615a 100644 --- a/org.springframework.web/src/test/java/org/springframework/http/MediaTypeTests.java +++ b/org.springframework.web/src/test/java/org/springframework/http/MediaTypeTests.java @@ -373,4 +373,108 @@ public class MediaTypeTests { } + @Test + public void qualityComparator() throws Exception { + MediaType audioBasic = new MediaType("audio", "basic"); + MediaType audioWave = new MediaType("audio", "wave"); + MediaType audio = new MediaType("audio"); + MediaType audio03 = new MediaType("audio", "*", 0.3); + MediaType audio07 = new MediaType("audio", "*", 0.7); + MediaType audioBasicLevel = new MediaType("audio", "basic", Collections.singletonMap("level", "1")); + MediaType textHtml = new MediaType("text", "html"); + MediaType all = MediaType.ALL; + + Comparator comp = MediaType.QUALITY_VALUE_COMPARATOR; + + // equal + assertEquals("Invalid comparison result", 0, comp.compare(audioBasic,audioBasic)); + assertEquals("Invalid comparison result", 0, comp.compare(audio, audio)); + assertEquals("Invalid comparison result", 0, comp.compare(audio07, audio07)); + assertEquals("Invalid comparison result", 0, comp.compare(audio03, audio03)); + assertEquals("Invalid comparison result", 0, comp.compare(audioBasicLevel, audioBasicLevel)); + + // specific to unspecific + assertTrue("Invalid comparison result", comp.compare(audioBasic, audio) < 0); + assertTrue("Invalid comparison result", comp.compare(audioBasic, all) < 0); + assertTrue("Invalid comparison result", comp.compare(audio, all) < 0); + + // unspecific to specific + assertTrue("Invalid comparison result", comp.compare(audio, audioBasic) > 0); + assertTrue("Invalid comparison result", comp.compare(all, audioBasic) > 0); + assertTrue("Invalid comparison result", comp.compare(all, audio) > 0); + + // qualifiers + assertTrue("Invalid comparison result", comp.compare(audio, audio07) < 0); + assertTrue("Invalid comparison result", comp.compare(audio07, audio) > 0); + assertTrue("Invalid comparison result", comp.compare(audio07, audio03) < 0); + assertTrue("Invalid comparison result", comp.compare(audio03, audio07) > 0); + assertTrue("Invalid comparison result", comp.compare(audio03, all) > 0); + assertTrue("Invalid comparison result", comp.compare(all, audio03) < 0); + + // other parameters + assertTrue("Invalid comparison result", comp.compare(audioBasic, audioBasicLevel) > 0); + assertTrue("Invalid comparison result", comp.compare(audioBasicLevel, audioBasic) < 0); + + // different types + assertEquals("Invalid comparison result", 0, comp.compare(audioBasic, textHtml)); + assertEquals("Invalid comparison result", 0, comp.compare(textHtml, audioBasic)); + + // different subtypes + assertEquals("Invalid comparison result", 0, comp.compare(audioBasic, audioWave)); + assertEquals("Invalid comparison result", 0, comp.compare(audioWave, audioBasic)); + } + + @Test + public void sortByQualityRelated() { + MediaType audioBasic = new MediaType("audio", "basic"); + MediaType audio = new MediaType("audio"); + MediaType audio03 = new MediaType("audio", "*", 0.3); + MediaType audio07 = new MediaType("audio", "*", 0.7); + MediaType audioBasicLevel = new MediaType("audio", "basic", Collections.singletonMap("level", "1")); + MediaType all = MediaType.ALL; + + List expected = new ArrayList(); + expected.add(audioBasicLevel); + expected.add(audioBasic); + expected.add(audio); + expected.add(all); + expected.add(audio07); + expected.add(audio03); + + List result = new ArrayList(expected); + Random rnd = new Random(); + // shuffle & sort 10 times + for (int i = 0; i < 10; i++) { + Collections.shuffle(result, rnd); + MediaType.sortByQualityValue(result); + + for (int j = 0; j < result.size(); j++) { + assertSame("Invalid media type at " + j, expected.get(j), result.get(j)); + } + } + } + + @Test + public void sortByQualityUnrelated() { + MediaType audioBasic = new MediaType("audio", "basic"); + MediaType audioWave = new MediaType("audio", "wave"); + MediaType textHtml = new MediaType("text", "html"); + + List expected = new ArrayList(); + expected.add(textHtml); + expected.add(audioBasic); + expected.add(audioWave); + + List result = new ArrayList(expected); + MediaType.sortBySpecificity(result); + + for (int i = 0; i < result.size(); i++) { + assertSame("Invalid media type at " + i, expected.get(i), result.get(i)); + } + + } + + + + }