Fix content negotiation issue with sort by q-value
Before this fix the q-value of media types in the Accept header were ignored when using the new RequestMappingHandlerAdapter in combination with @ResponseBody and HttpMessageConverters. Issue: SPR-9160
This commit is contained in:
Rossen Stoyanchev
parent
75578d4e88
commit
982cb2f258
|
|
@ -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 <a href="http://tools.ietf.org/html/rfc2616#section-3.7">HTTP 1.1, section 3.7</a>
|
||||
*/
|
||||
|
|
@ -529,6 +531,32 @@ public class MediaType implements Comparable<MediaType> {
|
|||
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<String, String> params = new LinkedHashMap<String, String>(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<String, String> params = new LinkedHashMap<String, String>(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<MediaType> {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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<MediaType> 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<MediaType>(comparators));
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Comparator used by {@link #sortBySpecificity(List)}.
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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<MediaType> mediaTypes = new ArrayList<MediaType>(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<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;
|
||||
private MediaType getMostSpecificMediaType(MediaType acceptType, MediaType produceType) {
|
||||
produceType = produceType.copyQualityValue(acceptType);
|
||||
return MediaType.SPECIFICITY_COMPARATOR.compare(acceptType, produceType) < 0 ? acceptType : produceType;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
@ -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<MediaType> mediaTypes = new ArrayList<MediaType>(compatibleMediaTypes);
|
||||
MediaType.sortByQualityValue(mediaTypes);
|
||||
List<MediaType> selectedMediaTypes = new ArrayList<MediaType>(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<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;
|
||||
private MediaType getMostSpecificMediaType(MediaType acceptType, MediaType produceType) {
|
||||
produceType = produceType.copyQualityValue(acceptType);
|
||||
return MediaType.SPECIFICITY_COMPARATOR.compare(acceptType, produceType) < 0 ? acceptType : produceType;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -48,21 +48,21 @@ 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}.
|
||||
|
|
@ -264,8 +264,24 @@ 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<HttpMessageConverter<?>> converters = new ArrayList<HttpMessageConverter<?>>();
|
||||
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<HttpMessageConverter<?>>converters = new ArrayList<HttpMessageConverter<?>>();
|
||||
converters.add(new ByteArrayHttpMessageConverter());
|
||||
converters.add(new StringHttpMessageConverter());
|
||||
|
|
|
|||
|
|
@ -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<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));
|
||||
}
|
||||
|
||||
|
|
@ -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");
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
-------------------------------------
|
||||
|
|
|
|||
Loading…
Reference in New Issue