diff --git a/spring-web/src/main/java/org/springframework/http/MediaType.java b/spring-web/src/main/java/org/springframework/http/MediaType.java index 6d0f75fe4f..1411040489 100644 --- a/spring-web/src/main/java/org/springframework/http/MediaType.java +++ b/spring-web/src/main/java/org/springframework/http/MediaType.java @@ -33,6 +33,7 @@ import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; import org.springframework.util.LinkedCaseInsensitiveMap; import org.springframework.util.StringUtils; +import org.springframework.util.comparator.CompoundComparator; /** * Represents an Internet Media Type, as defined in the HTTP specification. @@ -43,6 +44,7 @@ import org.springframework.util.StringUtils; * * @author Arjen Poutsma * @author Juergen Hoeller + * @author Rossen Stoyanchev * @since 3.0 * @see HTTP 1.1, section 3.7 */ @@ -102,7 +104,7 @@ public class MediaType implements Comparable { * Public constant media type for {@code application/xhtml+xml}. * */ public final static MediaType APPLICATION_XHTML_XML; - + /** * A String equivalent of {@link MediaType#APPLICATION_XHTML_XML}. */ @@ -112,7 +114,7 @@ public class MediaType implements Comparable { * Public constant media type for {@code application/xml}. */ public final static MediaType APPLICATION_XML; - + /** * A String equivalent of {@link MediaType#APPLICATION_XML}. */ @@ -122,7 +124,7 @@ public class MediaType implements Comparable { * Public constant media type for {@code image/gif}. */ public final static MediaType IMAGE_GIF; - + /** * A String equivalent of {@link MediaType#IMAGE_GIF}. */ @@ -132,7 +134,7 @@ public class MediaType implements Comparable { * Public constant media type for {@code image/jpeg}. */ public final static MediaType IMAGE_JPEG; - + /** * A String equivalent of {@link MediaType#IMAGE_JPEG}. */ @@ -142,7 +144,7 @@ public class MediaType implements Comparable { * Public constant media type for {@code image/png}. */ public final static MediaType IMAGE_PNG; - + /** * A String equivalent of {@link MediaType#IMAGE_PNG}. */ @@ -152,7 +154,7 @@ public class MediaType implements Comparable { * Public constant media type for {@code multipart/form-data}. * */ public final static MediaType MULTIPART_FORM_DATA; - + /** * A String equivalent of {@link MediaType#MULTIPART_FORM_DATA}. */ @@ -162,7 +164,7 @@ public class MediaType implements Comparable { * Public constant media type for {@code text/html}. * */ public final static MediaType TEXT_HTML; - + /** * A String equivalent of {@link MediaType#TEXT_HTML}. */ @@ -172,7 +174,7 @@ public class MediaType implements Comparable { * Public constant media type for {@code text/plain}. * */ public final static MediaType TEXT_PLAIN; - + /** * A String equivalent of {@link MediaType#TEXT_PLAIN}. */ @@ -182,7 +184,7 @@ public class MediaType implements Comparable { * Public constant media type for {@code text/xml}. * */ public final static MediaType TEXT_XML; - + /** * A String equivalent of {@link MediaType#TEXT_XML}. */ @@ -529,6 +531,32 @@ public class MediaType implements Comparable { return false; } + /** + * Return a replica of this instance with the quality value of the given MediaType. + * @return the same instance if the given MediaType doesn't have a quality value, or a new one otherwise + */ + public MediaType copyQualityValue(MediaType mediaType) { + if (!mediaType.parameters.containsKey(PARAM_QUALITY_FACTOR)) { + return this; + } + Map params = new LinkedHashMap(this.parameters); + params.put(PARAM_QUALITY_FACTOR, mediaType.parameters.get(PARAM_QUALITY_FACTOR)); + return new MediaType(this, params); + } + + /** + * Return a replica of this instance with its quality value removed. + * @return the same instance if the media type doesn't contain a quality value, or a new one otherwise + */ + public MediaType removeQualityValue() { + if (!this.parameters.containsKey(PARAM_QUALITY_FACTOR)) { + return this; + } + Map params = new LinkedHashMap(this.parameters); + params.remove(PARAM_QUALITY_FACTOR); + return new MediaType(this, params); + } + /** * Compares this {@code MediaType} to another alphabetically. * @param other media type to compare to @@ -772,6 +800,22 @@ public class MediaType implements Comparable { } } + /** + * Sorts the given list of {@code MediaType} objects by specificity as the + * primary criteria and quality value the secondary. + * @see MediaType#sortBySpecificity(List) + * @see MediaType#sortByQualityValue(List) + */ + public static void sortBySpecificityAndQuality(List mediaTypes) { + Assert.notNull(mediaTypes, "'mediaTypes' must not be null"); + if (mediaTypes.size() > 1) { + Comparator[] comparators = new Comparator[2]; + comparators[0] = MediaType.SPECIFICITY_COMPARATOR; + comparators[1] = MediaType.QUALITY_VALUE_COMPARATOR; + Collections.sort(mediaTypes, new CompoundComparator(comparators)); + } + } + /** * Comparator used by {@link #sortBySpecificity(List)}. diff --git a/spring-web/src/test/java/org/springframework/http/MediaTypeTests.java b/spring-web/src/test/java/org/springframework/http/MediaTypeTests.java index 5ed742a0eb..2e290d0a81 100644 --- a/spring-web/src/test/java/org/springframework/http/MediaTypeTests.java +++ b/spring-web/src/test/java/org/springframework/http/MediaTypeTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 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. @@ -60,7 +60,7 @@ public class MediaTypeTests { assertTrue(applicationWildcardXml.includes(applicationSoapXml)); assertFalse(applicationSoapXml.includes(applicationWildcardXml)); } - + @Test public void isCompatible() throws Exception { MediaType textPlain = MediaType.TEXT_PLAIN; @@ -102,7 +102,7 @@ public class MediaTypeTests { public void slashInSubtype() { new MediaType("text", "/"); } - + @Test public void getDefaultQualityValue() { MediaType mediaType = new MediaType("text", "plain"); @@ -477,7 +477,7 @@ public class MediaTypeTests { } } } - + @Test public void sortByQualityUnrelated() { MediaType audioBasic = new MediaType("audio", "basic"); @@ -504,7 +504,7 @@ public class MediaTypeTests { MediaType mediaType = MediaType.parseMediaType("application/xml"); assertEquals(mediaType, conversionService.convert("application/xml", MediaType.class)); } - + @Test public void isConcrete() { assertTrue("text/plain not concrete", MediaType.TEXT_PLAIN.isConcrete()); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java index a47074fcad..d1aa721bbf 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java @@ -21,7 +21,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; -import java.util.Map; import java.util.Set; import javax.servlet.http.HttpServletRequest; @@ -117,7 +116,7 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe } List mediaTypes = new ArrayList(compatibleMediaTypes); - MediaType.sortBySpecificity(mediaTypes); + MediaType.sortBySpecificityAndQuality(mediaTypes); MediaType selectedMediaType = null; for (MediaType mediaType : mediaTypes) { @@ -131,6 +130,8 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe } } + selectedMediaType = selectedMediaType.removeQualityValue(); + if (selectedMediaType != null) { for (HttpMessageConverter messageConverter : messageConverters) { if (messageConverter.canWrite(returnValueClass, selectedMediaType)) { @@ -188,14 +189,12 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe } /** - * Returns the more specific media type using the q-value of the first media type for both. + * Return the more specific of the acceptable and the producible media types + * with the q-value of the former. */ - 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; + private MediaType getMostSpecificMediaType(MediaType acceptType, MediaType produceType) { + produceType = produceType.copyQualityValue(acceptType); + return MediaType.SPECIFICITY_COMPARATOR.compare(acceptType, produceType) < 0 ? acceptType : produceType; } } \ No newline at end of file diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolver.java index 2738553fea..588489d471 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 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. @@ -101,6 +101,7 @@ import org.springframework.web.util.WebUtils; * * @author Arjen Poutsma * @author Juergen Hoeller + * @author Rossen Stoyanchev * @since 3.0 * @see ViewResolver * @see InternalResourceViewResolver @@ -354,13 +355,13 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport } } } - List mediaTypes = new ArrayList(compatibleMediaTypes); - MediaType.sortByQualityValue(mediaTypes); + List selectedMediaTypes = new ArrayList(compatibleMediaTypes); + MediaType.sortBySpecificityAndQuality(selectedMediaTypes); if (logger.isDebugEnabled()) { - logger.debug("Requested media types are " + mediaTypes + " based on Accept header types " + + logger.debug("Requested media types are " + selectedMediaTypes + " based on Accept header types " + "and producible media types " + producibleMediaTypes + ")"); } - return mediaTypes; + return selectedMediaTypes; } catch (IllegalArgumentException ex) { if (logger.isDebugEnabled()) { @@ -395,14 +396,12 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport } /** - * Returns the more specific media type using the q-value of the first media type for both. + * Return the more specific of the acceptable and the producible media types + * with the q-value of the former. */ - 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; + private MediaType getMostSpecificMediaType(MediaType acceptType, MediaType produceType) { + produceType = produceType.copyQualityValue(acceptType); + return MediaType.SPECIFICITY_COMPARATOR.compare(acceptType, produceType) < 0 ? acceptType : produceType; } /** diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java index a98820751d..8449f51e5e 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorTests.java @@ -48,25 +48,25 @@ import org.springframework.http.MediaType; import org.springframework.http.converter.ByteArrayHttpMessageConverter; import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.converter.StringHttpMessageConverter; +import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean; import org.springframework.web.HttpMediaTypeNotAcceptableException; import org.springframework.web.HttpMediaTypeNotSupportedException; +import org.springframework.web.bind.MethodArgumentNotValidException; import org.springframework.web.bind.WebDataBinder; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.bind.support.WebDataBinderFactory; import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.context.request.ServletWebRequest; -import org.springframework.web.bind.MethodArgumentNotValidException; import org.springframework.web.method.support.ModelAndViewContainer; import org.springframework.web.servlet.HandlerMapping; -import org.springframework.web.servlet.mvc.method.annotation.RequestResponseBodyMethodProcessor; /** * Test fixture with {@link RequestResponseBodyMethodProcessor} and mock {@link HttpMessageConverter}. - * + * * @author Arjen Poutsma * @author Rossen Stoyanchev */ @@ -100,7 +100,7 @@ public class RequestResponseBodyMethodProcessorTests { processor = new RequestResponseBodyMethodProcessor(Collections.>singletonList(messageConverter)); reset(messageConverter); - + Method handle = getClass().getMethod("handle1", String.class, Integer.TYPE); paramRequestBodyString = new MethodParameter(handle, 0); paramInt = new MethodParameter(handle, 1); @@ -110,7 +110,7 @@ public class RequestResponseBodyMethodProcessorTests { paramValidBean = new MethodParameter(getClass().getMethod("handle4", SimpleBean.class), 0); mavContainer = new ModelAndViewContainer(); - + servletRequest = new MockHttpServletRequest(); servletResponse = new MockHttpServletResponse(); webRequest = new ServletWebRequest(servletRequest, servletResponse); @@ -175,10 +175,10 @@ public class RequestResponseBodyMethodProcessorTests { processor = new RequestResponseBodyMethodProcessor(Collections.>singletonList(beanConverter)); processor.resolveArgument(paramValidBean, mavContainer, webRequest, new ValidatingBinderFactory()); - + verify(beanConverter); } - + @Test(expected = HttpMediaTypeNotSupportedException.class) public void resolveArgumentNotReadable() throws Exception { MediaType contentType = MediaType.TEXT_PLAIN; @@ -248,7 +248,7 @@ public class RequestResponseBodyMethodProcessorTests { fail("Expected exception"); } - + @Test(expected = HttpMediaTypeNotAcceptableException.class) public void handleReturnValueNotAcceptableProduces() throws Exception { MediaType accepted = MediaType.TEXT_PLAIN; @@ -264,19 +264,35 @@ public class RequestResponseBodyMethodProcessorTests { fail("Expected exception"); } + // SPR-9160 + @Test - public void handleStringReturnValue() throws Exception { + public void handleReturnValueSortByQuality() throws Exception { + this.servletRequest.addHeader("Accept", "text/plain; q=0.5, application/json"); + + List> converters = new ArrayList>(); + converters.add(new MappingJackson2HttpMessageConverter()); + converters.add(new StringHttpMessageConverter()); + RequestResponseBodyMethodProcessor handler = new RequestResponseBodyMethodProcessor(converters); + + handler.writeWithMessageConverters("Foo", returnTypeStringProduces, webRequest); + + assertEquals("application/json;charset=UTF-8", servletResponse.getHeader("Content-Type")); + } + + @Test + public void handleReturnValueString() throws Exception { List>converters = new ArrayList>(); converters.add(new ByteArrayHttpMessageConverter()); converters.add(new StringHttpMessageConverter()); - + processor = new RequestResponseBodyMethodProcessor(converters); processor.handleReturnValue("Foo", returnTypeString, mavContainer, webRequest); - + assertEquals("text/plain;charset=ISO-8859-1", servletResponse.getHeader("Content-Type")); assertEquals("Foo", servletResponse.getContentAsString()); } - + @ResponseBody public String handle1(@RequestBody String s, int i) { return s; @@ -293,7 +309,7 @@ public class RequestResponseBodyMethodProcessorTests { public void handle4(@Valid @RequestBody SimpleBean b) { } - + private final class ValidatingBinderFactory implements WebDataBinderFactory { public WebDataBinder createBinder(NativeWebRequest webRequest, Object target, String objectName) throws Exception { LocalValidatorFactoryBean validator = new LocalValidatorFactoryBean(); @@ -318,5 +334,5 @@ public class RequestResponseBodyMethodProcessorTests { return name; } } - + } \ No newline at end of file diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolverTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolverTests.java index 14b7199191..5d20f59e79 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolverTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolverTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 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,6 +16,16 @@ package org.springframework.web.servlet.view; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; + import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -28,7 +38,6 @@ import java.util.Set; import org.junit.After; import org.junit.Before; import org.junit.Test; - import org.springframework.http.MediaType; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; @@ -40,9 +49,6 @@ import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.View; import org.springframework.web.servlet.ViewResolver; -import static org.easymock.EasyMock.*; -import static org.junit.Assert.*; - /** * @author Arjen Poutsma */ @@ -148,7 +154,6 @@ public class ContentNegotiatingViewResolverTests { 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)); } @@ -303,6 +308,35 @@ public class ContentNegotiatingViewResolverTests { verify(viewResolverMock1, viewResolverMock2, viewMock1, viewMock2); } + // SPR-9160 + + @Test + public void resolveViewNameAcceptHeaderSortByQuality() throws Exception { + request.addHeader("Accept", "text/plain;q=0.5, application/json"); + + ViewResolver htmlViewResolver = createMock(ViewResolver.class); + ViewResolver jsonViewResolver = createMock(ViewResolver.class); + viewResolver.setViewResolvers(Arrays.asList(htmlViewResolver, jsonViewResolver)); + + View htmlView = createMock("text_html", View.class); + View jsonViewMock = createMock("application_json", View.class); + + String viewName = "view"; + Locale locale = Locale.ENGLISH; + + expect(htmlViewResolver.resolveViewName(viewName, locale)).andReturn(htmlView); + expect(jsonViewResolver.resolveViewName(viewName, locale)).andReturn(jsonViewMock); + expect(htmlView.getContentType()).andReturn("text/html").anyTimes(); + expect(jsonViewMock.getContentType()).andReturn("application/json").anyTimes(); + replay(htmlViewResolver, jsonViewResolver, htmlView, jsonViewMock); + + viewResolver.setFavorPathExtension(false); + View result = viewResolver.resolveViewName(viewName, locale); + assertSame("Invalid view", jsonViewMock, result); + + verify(htmlViewResolver, jsonViewResolver, htmlView, jsonViewMock); + } + @Test public void resolveViewNameAcceptHeaderDefaultView() throws Exception { request.addHeader("Accept", "application/json"); diff --git a/src/dist/changelog.txt b/src/dist/changelog.txt index b528b90857..6c852b29f9 100644 --- a/src/dist/changelog.txt +++ b/src/dist/changelog.txt @@ -18,6 +18,7 @@ Changes in version 3.2 M1 * fix issue with resolving Errors controller method argument * detect controller methods via InitializingBean in RequestMappingHandlerMapping * translate IOException from Jackson to HttpMessageNotReadableException +* fix content negotiation issue when sorting selected media types by quality value Changes in version 3.1.1 (2012-02-16) -------------------------------------