fixed ContentNegotiatingViewResolver to work with the combination of ignoreAcceptHeader=true plus defaultContentType as well (SPR-6163)

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

@@ -91,66 +91,66 @@ import org.springframework.web.util.WebUtils;
  * text/html} request {@code Accept} header has the same result.
  *
  * @author Arjen Poutsma
- * @author Rostislav Hristov
+ * @author Juergen Hoeller
+ * @since 3.0
  * @see ViewResolver
  * @see InternalResourceViewResolver
  * @see BeanNameViewResolver
- * @since 3.0
  */
 public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport implements ViewResolver, Ordered {
 
+	private static final String ACCEPT_HEADER = "Accept";
+
 	private static final boolean jafPresent =
 			ClassUtils.isPresent("javax.activation.FileTypeMap", ContentNegotiatingViewResolver.class.getClassLoader());
 
-	private static final String ACCEPT_HEADER = "Accept";
+	private static final UrlPathHelper urlPathHelper = new UrlPathHelper();
 
-	private UrlPathHelper urlPathHelper = new UrlPathHelper();
+
+	private int order = Ordered.HIGHEST_PRECEDENCE;
 
 	private boolean favorPathExtension = true;
 
 	private boolean favorParameter = false;
 
-	private boolean ignoreAcceptHeader = false;
-
 	private String parameterName = "format";
 
-	private int order = Ordered.HIGHEST_PRECEDENCE;
+	private boolean ignoreAcceptHeader = false;
 
 	private ConcurrentMap<String, MediaType> mediaTypes = new ConcurrentHashMap<String, MediaType>();
 
 	private List<View> defaultViews;
 
+	private MediaType defaultContentType;
+
 	private List<ViewResolver> viewResolvers;
 
-	private MediaType defaultContentType;
 
 	public void setOrder(int order) {
 		this.order = order;
 	}
 
 	public int getOrder() {
-		return order;
+		return this.order;
 	}
 
 	/**
-	 * Indicates whether the extension of the request path should be used to determine the requested media type, in favor
+	 * Indicates whether the extension of the request path should be used to determine the requested media type,
-	 * of looking at the {@code Accept} header. The default value is {@code true}.
+	 * in favor of looking at the {@code Accept} header. The default value is {@code true}.
-	 *
+	 * <p>For instance, when this flag is <code>true</code> (the default), a request for {@code /hotels.pdf}
-	 * <p>For instance, when this flag is <code>true</code> (the default), a request for {@code /hotels.pdf} will result in
+	 * will result in an {@code AbstractPdfView} being resolved, while the {@code Accept} header can be the
-	 * an {@code AbstractPdfView} being resolved, while the {@code Accept} header can be the browser-defined {@code
+	 * browser-defined {@code text/html,application/xhtml+xml}.
-	 * text/html,application/xhtml+xml}.
 	 */
 	public void setFavorPathExtension(boolean favorPathExtension) {
 		this.favorPathExtension = favorPathExtension;
 	}
 
 	/**
-	 * Indicates whether a request parameter should be used to determine the requested media type, in favor of looking at
+	 * Indicates whether a request parameter should be used to determine the requested media type,
-	 * the {@code Accept} header. The default value is {@code false}.
+	 * in favor of looking at the {@code Accept} header. The default value is {@code false}.
-	 *
+	 * <p>For instance, when this flag is <code>true</code>, a request for {@code /hotels?format=pdf} will result
-	 * <p>For instance, when this flag is <code>true</code>, a request for {@code /hotels?format=pdf} will result in an
+	 * in an {@code AbstractPdfView} being resolved, while the {@code Accept} header can be the browser-defined
-	 * {@code AbstractPdfView} being resolved, while the {@code Accept} header can be the browser-defined {@code
+	 * {@code text/html,application/xhtml+xml}.
-	 * text/html,application/xhtml+xml}.
 	 */
 	public void setFavorParameter(boolean favorParameter) {
 		this.favorParameter = favorParameter;
@@ -166,10 +166,9 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport
 
 	/**
 	 * Indicates whether the HTTP {@code Accept} header should be ignored. Default is {@code false}.
-	 *
+	 * If set to {@code true}, this view resolver will only refer to the file extension and/or paramter,
-	 * If set to {@code true}, this view resolver will only refer to the file extension and/or paramter, as indicated by
+	 * as indicated by the {@link #setFavorPathExtension(boolean) favorPathExtension} and
-	 * the {@link #setFavorPathExtension(boolean) favorPathExtension} and {@link #setFavorParameter(boolean)
+	 * {@link #setFavorParameter(boolean) favorParameter} properties.
-	 * favorParameter} properties.
 	 */
 	public void setIgnoreAcceptHeader(boolean ignoreAcceptHeader) {
 		this.ignoreAcceptHeader = ignoreAcceptHeader;
@@ -177,9 +176,8 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport
 
 	/**
 	 * Sets the mapping from file extensions to media types.
-	 *
+	 * <p>When this mapping is not set or when an extension is not present, this view resolver
-	 * <p>When this mapping is not set or when an extension is not present, this view resolver will fall back to using a
+	 * will fall back to using a {@link FileTypeMap} when the Java Action Framework is available.
-	 * {@link FileTypeMap} when the Java Action Framework is available.
 	 */
 	public void setMediaTypes(Map<String, String> mediaTypes) {
 		Assert.notNull(mediaTypes, "'mediaTypes' must not be null");
@@ -190,13 +188,17 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport
 		}
 	}
 
-	/** Sets the default views to use when a more specific view can not be obtained from the {@link ViewResolver} chain. */
+	/**
+	 * Sets the default views to use when a more specific view can not be obtained
+	 * from the {@link ViewResolver} chain.
+	 */
 	public void setDefaultViews(List<View> defaultViews) {
 		this.defaultViews = defaultViews;
 	}
 
 	/**
-	 * Sets the default content type. This content type will be used when file extension, parameter, nor {@code Accept}
+	 * Sets the default content type.
+	 * <p>This content type will be used when file extension, parameter, nor {@code Accept}
 	 * header define a content-type, either through being disabled or empty.
 	 */
 	public void setDefaultContentType(MediaType defaultContentType) {
@@ -205,7 +207,6 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport
 
 	/**
 	 * Sets the view resolvers to be wrapped by this view resolver.
-	 *
 	 * <p>If this property is not set, view resolvers will be detected automatically.
 	 */
 	public void setViewResolvers(List<ViewResolver> viewResolvers) {
@@ -214,9 +215,9 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport
 
 	@Override
 	protected void initServletContext(ServletContext servletContext) {
-		if (viewResolvers == null) {
+		if (this.viewResolvers == null) {
-			Map<String, ViewResolver> matchingBeans = BeanFactoryUtils
+			Map<String, ViewResolver> matchingBeans =
-					.beansOfTypeIncludingAncestors(getApplicationContext(), ViewResolver.class, true, false);
+					BeanFactoryUtils.beansOfTypeIncludingAncestors(getApplicationContext(), ViewResolver.class);
 			this.viewResolvers = new ArrayList<ViewResolver>(matchingBeans.size());
 			for (ViewResolver viewResolver : matchingBeans.values()) {
 				if (this != viewResolver) {
@@ -233,19 +234,16 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport
 
 	/**
 	 * Determines the list of {@link MediaType} for the given {@link HttpServletRequest}.
-	 *
 	 * <p>The default implementation invokes {@link #getMediaTypeFromFilename(String)} if {@linkplain
 	 * #setFavorPathExtension(boolean) favorPathExtension} property is <code>true</code>. If the property is
-	 * <code>false</code>, or when a media type cannot be determined from the request path, this method will inspect the
+	 * <code>false</code>, or when a media type cannot be determined from the request path, this method will
-	 * {@code Accept} header of the request.
+	 * inspect the {@code Accept} header of the request.
-	 *
 	 * <p>This method can be overriden to provide a different algorithm.
-	 *
 	 * @param request the current servlet request
 	 * @return the list of media types requested, if any
 	 */
 	protected List<MediaType> getMediaTypes(HttpServletRequest request) {
-		if (favorPathExtension) {
+		if (this.favorPathExtension) {
 			String requestUri = urlPathHelper.getRequestUri(request);
 			String filename = WebUtils.extractFullFilenameFromUrlPath(requestUri);
 			MediaType mediaType = getMediaTypeFromFilename(filename);
@@ -258,15 +256,14 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport
 				return mediaTypes;
 			}
 		}
-		if (favorParameter) {
+		if (this.favorParameter) {
-			if (request.getParameter(parameterName) != null) {
+			if (request.getParameter(this.parameterName) != null) {
-				String parameterValue = request.getParameter(parameterName);
+				String parameterValue = request.getParameter(this.parameterName);
 				MediaType mediaType = getMediaTypeFromParameter(parameterValue);
 				if (mediaType != null) {
 					if (logger.isDebugEnabled()) {
-						logger.debug(
+						logger.debug("Requested media type is '" + mediaType + "' (based on parameter '" +
-								"Requested media type is '" + mediaType + "' (based on parameter '" + parameterName +
+								this.parameterName + "'='" + parameterValue + "')");
-										"'='" + parameterValue + "')");
 					}
 					List<MediaType> mediaTypes = new ArrayList<MediaType>();
 					mediaTypes.add(mediaType);
@@ -274,7 +271,7 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport
 				}
 			}
 		}
-		if (!ignoreAcceptHeader) {
+		if (!this.ignoreAcceptHeader) {
 			String acceptHeader = request.getHeader(ACCEPT_HEADER);
 			if (StringUtils.hasText(acceptHeader)) {
 				List<MediaType> mediaTypes = MediaType.parseMediaTypes(acceptHeader);
@@ -284,8 +281,8 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport
 				return mediaTypes;
 			}
 		}
-		if (defaultContentType != null) {
+		if (this.defaultContentType != null) {
-			return Collections.singletonList(defaultContentType);
+			return Collections.singletonList(this.defaultContentType);
 		}
 		else {
 			return Collections.emptyList();
@@ -294,13 +291,10 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport
 
 	/**
 	 * Determines the {@link MediaType} for the given filename.
-	 *
 	 * <p>The default implementation will check the {@linkplain #setMediaTypes(Map) media types} property first for a
 	 * defined mapping. If not present, and if the Java Activation Framework can be found on the class path, it will call
 	 * {@link FileTypeMap#getContentType(String)}
-	 *
 	 * <p>This method can be overriden to provide a different algorithm.
-	 *
 	 * @param filename the current request file name (i.e. {@code hotels.html})
 	 * @return the media type, if any
 	 */
@@ -310,11 +304,11 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport
 			return null;
 		}
 		extension = extension.toLowerCase(Locale.ENGLISH);
-		MediaType mediaType = mediaTypes.get(extension);
+		MediaType mediaType = this.mediaTypes.get(extension);
 		if (mediaType == null && jafPresent) {
 			mediaType = ActivationMediaTypeFactory.getMediaType(filename);
 			if (mediaType != null) {
-				mediaTypes.putIfAbsent(extension, mediaType);
+				this.mediaTypes.putIfAbsent(extension, mediaType);
 			}
 		}
 		return mediaType;
@@ -322,38 +316,39 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport
 
 	/**
 	 * Determines the {@link MediaType} for the given parameter value.
-	 *
+	 * <p>The default implementation will check the {@linkplain #setMediaTypes(Map) media types}
-	 * <p>The default implementation will check the {@linkplain #setMediaTypes(Map) media types} property for a defined
+	 * property for a defined mapping.
-	 * mapping.
-	 *
 	 * <p>This method can be overriden to provide a different algorithm.
-	 *
 	 * @param parameterValue the parameter value (i.e. {@code pdf}).
 	 * @return the media type, if any
 	 */
 	protected MediaType getMediaTypeFromParameter(String parameterValue) {
-		parameterValue = parameterValue.toLowerCase(Locale.ENGLISH);
+		return this.mediaTypes.get(parameterValue.toLowerCase(Locale.ENGLISH));
-		return mediaTypes.get(parameterValue);
 	}
 
 	public View resolveViewName(String viewName, Locale locale) throws Exception {
 		RequestAttributes attrs = RequestContextHolder.getRequestAttributes();
 		Assert.isInstanceOf(ServletRequestAttributes.class, attrs);
 		ServletRequestAttributes servletAttrs = (ServletRequestAttributes) attrs;
+
 		List<MediaType> requestedMediaTypes = getMediaTypes(servletAttrs.getRequest());
-		Collections.sort(requestedMediaTypes);
+		if (requestedMediaTypes.size() > 1) {
+			// avoid sorting attempt for empty list and singleton list
+			Collections.sort(requestedMediaTypes);
+		}
 
 		SortedMap<MediaType, View> views = new TreeMap<MediaType, View>();
 		List<View> candidateViews = new ArrayList<View>();
-		for (ViewResolver viewResolver : viewResolvers) {
+		for (ViewResolver viewResolver : this.viewResolvers) {
 			View view = viewResolver.resolveViewName(viewName, locale);
 			if (view != null) {
 				candidateViews.add(view);
 			}
 		}
-		if (!CollectionUtils.isEmpty(defaultViews)) {
+		if (!CollectionUtils.isEmpty(this.defaultViews)) {
-			candidateViews.addAll(defaultViews);
+			candidateViews.addAll(this.defaultViews);
 		}
+
 		for (View candidateView : candidateViews) {
 			MediaType viewMediaType = MediaType.parseMediaType(candidateView.getContentType());
 			for (MediaType requestedMediaType : requestedMediaTypes) {
@@ -365,6 +360,7 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport
 				}
 			}
 		}
+
 		if (!views.isEmpty()) {
 			MediaType mediaType = views.firstKey();
 			View view = views.get(mediaType);
@@ -378,7 +374,10 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport
 		}
 	}
 
-	/** Inner class to avoid hard-coded JAF dependency. */
+
+	/**
+	 * Inner class to avoid hard-coded JAF dependency.
+	 */
 	private static class ActivationMediaTypeFactory {
 
 		private static final FileTypeMap fileTypeMap;

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

@@ -93,16 +93,51 @@ public class ContentNegotiatingViewResolverTests {
 	}
 
 	@Test
-	public void getDefaultMediaType() {
+	public void getDefaultContentType() {
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/test");
 		request.addHeader("Accept", "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8");
-		viewResolver.setDefaultContentType(new MediaType("application", "pdf"));
 		viewResolver.setIgnoreAcceptHeader(true);
+		viewResolver.setDefaultContentType(new MediaType("application", "pdf"));
 		List<MediaType> result = viewResolver.getMediaTypes(request);
 		assertEquals("Invalid amount of media types", 1, result.size());
 		assertEquals("Invalid content type", new MediaType("application", "pdf"), result.get(0));
 	}
 
+	@Test
+	public void resolveViewNameWithDefaultContentType() throws Exception {
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/test");
+		request.addHeader("Accept", "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8");
+		RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(request));
+
+		viewResolver.setIgnoreAcceptHeader(true);
+		viewResolver.setDefaultContentType(new MediaType("application", "xml"));
+
+		ViewResolver viewResolverMock1 = createMock(ViewResolver.class);
+		ViewResolver viewResolverMock2 = createMock(ViewResolver.class);
+		List<ViewResolver> viewResolverMocks = new ArrayList<ViewResolver>();
+		viewResolverMocks.add(viewResolverMock1);
+		viewResolverMocks.add(viewResolverMock2);
+		viewResolver.setViewResolvers(viewResolverMocks);
+
+		View viewMock1 = createMock("application_xml", View.class);
+		View viewMock2 = createMock("text_html", View.class);
+
+		String viewName = "view";
+		Locale locale = Locale.ENGLISH;
+
+		expect(viewResolverMock1.resolveViewName(viewName, locale)).andReturn(viewMock1);
+		expect(viewResolverMock2.resolveViewName(viewName, locale)).andReturn(viewMock2);
+		expect(viewMock1.getContentType()).andReturn("application/xml");
+		expect(viewMock2.getContentType()).andReturn("text/html;charset=ISO-8859-1");
+
+		replay(viewResolverMock1, viewResolverMock2, viewMock1, viewMock2);
+
+		View result = viewResolver.resolveViewName(viewName, locale);
+		assertSame("Invalid view", viewMock1, result);
+
+		verify(viewResolverMock1, viewResolverMock2, viewMock1, viewMock2);
+	}
+
 	@Test
 	public void resolveViewNameAcceptHeader() throws Exception {
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/test");