Polish ContentNegotiationStrategy support

Issue: SPR-8410, SPR-8417, SPR-8418,SPR-8416, SPR-8419,SPR-7722

spring-web/src/main/java/org/springframework/web/accept/AbstractMappingContentNegotiationStrategy.java

@@ -31,14 +31,14 @@ import org.springframework.web.context.request.NativeWebRequest;
  * @author Rossen Stoyanchev
  * @since 3.2
  */
-public abstract class AbstractMappingContentNegotiationStrategy extends MappingMediaTypeExtensionsResolver
-		implements ContentNegotiationStrategy, MediaTypeExtensionsResolver {
+public abstract class AbstractMappingContentNegotiationStrategy extends MappingMediaTypeFileExtensionResolver
+		implements ContentNegotiationStrategy, MediaTypeFileExtensionResolver {
 
 	/**
 	 * Create an instance with the given extension-to-MediaType lookup.
 	 * @throws IllegalArgumentException if a media type string cannot be parsed
 	 */
-	public AbstractMappingContentNegotiationStrategy(Map<String, String> mediaTypes) {
+	public AbstractMappingContentNegotiationStrategy(Map<String, MediaType> mediaTypes) {
 		super(mediaTypes);
 	}
 

spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManager.java

@@ -33,28 +33,30 @@ import org.springframework.web.context.request.NativeWebRequest;
  * in a request by delegating to a list of {@link ContentNegotiationStrategy} instances.
  *
  * <p>It may also be used to determine the extensions associated with a MediaType by
- * delegating to a list of {@link MediaTypeExtensionsResolver} instances.
+ * delegating to a list of {@link MediaTypeFileExtensionResolver} instances.
  *
  * @author Rossen Stoyanchev
  * @since 3.2
  */
-public class ContentNegotiationManager implements ContentNegotiationStrategy, MediaTypeExtensionsResolver {
+public class ContentNegotiationManager implements ContentNegotiationStrategy, MediaTypeFileExtensionResolver {
 
-	private final List<ContentNegotiationStrategy> contentNegotiationStrategies = new ArrayList<ContentNegotiationStrategy>();
+	private final List<ContentNegotiationStrategy> contentNegotiationStrategies =
+			new ArrayList<ContentNegotiationStrategy>();
 
-	private final Set<MediaTypeExtensionsResolver> extensionResolvers = new LinkedHashSet<MediaTypeExtensionsResolver>();
+	private final Set<MediaTypeFileExtensionResolver> fileExtensionResolvers =
+			new LinkedHashSet<MediaTypeFileExtensionResolver>();
 
 	/**
 	 * Create an instance with the given ContentNegotiationStrategy instances.
 	 * <p>Each instance is checked to see if it is also an implementation of
-	 * MediaTypeExtensionsResolver, and if so it is registered as such.
+	 * MediaTypeFileExtensionResolver, and if so it is registered as such.
 	 */
 	public ContentNegotiationManager(ContentNegotiationStrategy... strategies) {
 		Assert.notEmpty(strategies, "At least one ContentNegotiationStrategy is expected");
 		this.contentNegotiationStrategies.addAll(Arrays.asList(strategies));
 		for (ContentNegotiationStrategy strategy : this.contentNegotiationStrategies) {
-			if (strategy instanceof MediaTypeExtensionsResolver) {
-				this.extensionResolvers.add((MediaTypeExtensionsResolver) strategy);
+			if (strategy instanceof MediaTypeFileExtensionResolver) {
+				this.fileExtensionResolvers.add((MediaTypeFileExtensionResolver) strategy);
 			}
 		}
 	}
@@ -67,10 +69,10 @@ public class ContentNegotiationManager implements ContentNegotiationStrategy, Me
 	}
 
 	/**
-	 * Add MediaTypeExtensionsResolver instances.
+	 * Add MediaTypeFileExtensionResolver instances.
 	 */
-	public void addExtensionsResolver(MediaTypeExtensionsResolver... resolvers) {
-		this.extensionResolvers.addAll(Arrays.asList(resolvers));
+	public void addFileExtensionResolvers(MediaTypeFileExtensionResolver... resolvers) {
+		this.fileExtensionResolvers.addAll(Arrays.asList(resolvers));
 	}
 
 	/**
@@ -91,13 +93,13 @@ public class ContentNegotiationManager implements ContentNegotiationStrategy, Me
 	}
 
 	/**
-	 * Delegate to all configured MediaTypeExtensionsResolver instances and aggregate
-	 * the list of all extensions found.
+	 * Delegate to all configured MediaTypeFileExtensionResolver instances and aggregate
+	 * the list of all file extensions found.
 	 */
-	public List<String> resolveExtensions(MediaType mediaType) {
+	public List<String> resolveFileExtensions(MediaType mediaType) {
 		Set<String> extensions = new LinkedHashSet<String>();
-		for (MediaTypeExtensionsResolver resolver : this.extensionResolvers) {
-			extensions.addAll(resolver.resolveExtensions(mediaType));
+		for (MediaTypeFileExtensionResolver resolver : this.fileExtensionResolvers) {
+			extensions.addAll(resolver.resolveFileExtensions(mediaType));
 		}
 		return new ArrayList<String>(extensions);
 	}

spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeExtensionsResolver.java → spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolver.java

@@ -15,7 +15,7 @@
  */
 package org.springframework.web.accept;
 
-import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -24,28 +24,32 @@ import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
 import org.springframework.http.MediaType;
+import org.springframework.util.LinkedMultiValueMap;
+import org.springframework.util.MultiValueMap;
 
 /**
- * An implementation of {@link MediaTypeExtensionsResolver} that maintains a lookup
+ * An implementation of {@link MediaTypeFileExtensionResolver} that maintains a lookup
  * from extension to MediaType.
  *
  * @author Rossen Stoyanchev
  * @since 3.2
  */
-public class MappingMediaTypeExtensionsResolver implements MediaTypeExtensionsResolver {
+public class MappingMediaTypeFileExtensionResolver implements MediaTypeFileExtensionResolver {
 
-	private ConcurrentMap<String, MediaType> mediaTypes = new ConcurrentHashMap<String, MediaType>();
+	private final ConcurrentMap<String, MediaType> mediaTypes = new ConcurrentHashMap<String, MediaType>();
+
+	private final MultiValueMap<MediaType, String> fileExtensions = new LinkedMultiValueMap<MediaType, String>();
 
 	/**
 	 * Create an instance with the given mappings between extensions and media types.
 	 * @throws IllegalArgumentException if a media type string cannot be parsed
 	 */
-	public MappingMediaTypeExtensionsResolver(Map<String, String> mediaTypes) {
+	public MappingMediaTypeFileExtensionResolver(Map<String, MediaType> mediaTypes) {
 		if (mediaTypes != null) {
-			for (Map.Entry<String, String> entry : mediaTypes.entrySet()) {
-				String extension = entry.getKey().toLowerCase(Locale.ENGLISH);
-				MediaType mediaType = MediaType.parseMediaType(entry.getValue());
-				this.mediaTypes.put(extension, mediaType);
+			for (Entry<String, MediaType> entries : mediaTypes.entrySet()) {
+				String extension = entries.getKey().toLowerCase(Locale.ENGLISH);
+				MediaType mediaType = entries.getValue();
+				addMapping(extension, mediaType);
 			}
 		}
 	}
@@ -54,14 +58,9 @@ public class MappingMediaTypeExtensionsResolver implements MediaTypeExtensionsRe
 	 * Find the extensions applicable to the given MediaType.
 	 * @return 0 or more extensions, never {@code null}
 	 */
-	public List<String> resolveExtensions(MediaType mediaType) {
-		List<String> result = new ArrayList<String>();
-		for (Entry<String, MediaType> entry : this.mediaTypes.entrySet()) {
-			if (mediaType.includes(entry.getValue())) {
-				result.add(entry.getKey());
-			}
-		}
-		return result;
+	public List<String> resolveFileExtensions(MediaType mediaType) {
+		List<String> fileExtensions = this.fileExtensions.get(mediaType);
+		return (fileExtensions != null) ? fileExtensions : Collections.<String>emptyList();
 	}
 
 	/**
@@ -76,7 +75,10 @@ public class MappingMediaTypeExtensionsResolver implements MediaTypeExtensionsRe
 	 * Map a MediaType to an extension or ignore if the extensions is already mapped.
 	 */
 	protected void addMapping(String extension, MediaType mediaType) {
-		this.mediaTypes.putIfAbsent(extension, mediaType);
+		MediaType previous = this.mediaTypes.putIfAbsent(extension, mediaType);
+		if (previous == null) {
+			this.fileExtensions.add(mediaType, extension);
+		}
 	}
 
 }
\ No newline at end of file

spring-web/src/main/java/org/springframework/web/accept/MediaTypeExtensionsResolver.java → spring-web/src/main/java/org/springframework/web/accept/MediaTypeFileExtensionResolver.java

@@ -27,7 +27,7 @@ import org.springframework.http.MediaType;
  * @author Rossen Stoyanchev
  * @since 3.2
  */
-public interface MediaTypeExtensionsResolver {
+public interface MediaTypeFileExtensionResolver {
 
 	/**
 	 * Resolve the given media type to a list of path extensions.
@@ -35,6 +35,6 @@ public interface MediaTypeExtensionsResolver {
 	 * @param mediaType the media type to resolve
 	 * @return a list of extensions or an empty list, never {@code null}
 	 */
-	List<String> resolveExtensions(MediaType mediaType);
+	List<String> resolveFileExtensions(MediaType mediaType);
 
 }

spring-web/src/main/java/org/springframework/web/accept/ParameterContentNegotiationStrategy.java

@@ -42,7 +42,7 @@ public class ParameterContentNegotiationStrategy extends AbstractMappingContentN
 	 * Create an instance with the given extension-to-MediaType lookup.
 	 * @throws IllegalArgumentException if a media type string cannot be parsed
 	 */
-	public ParameterContentNegotiationStrategy(Map<String, String> mediaTypes) {
+	public ParameterContentNegotiationStrategy(Map<String, MediaType> mediaTypes) {
 		super(mediaTypes);
 		Assert.notEmpty(mediaTypes, "Cannot look up media types without any mappings");
 	}

spring-web/src/main/java/org/springframework/web/accept/PathExtensionContentNegotiationStrategy.java

@@ -72,7 +72,7 @@ public class PathExtensionContentNegotiationStrategy extends AbstractMappingCont
 	 * Create an instance with the given extension-to-MediaType lookup.
 	 * @throws IllegalArgumentException if a media type string cannot be parsed
 	 */
-	public PathExtensionContentNegotiationStrategy(Map<String, String> mediaTypes) {
+	public PathExtensionContentNegotiationStrategy(Map<String, MediaType> mediaTypes) {
 		super(mediaTypes);
 	}
 

spring-web/src/test/java/org/springframework/web/accept/AbstractMappingContentNegotiationStrategyTests.java

@@ -35,7 +35,7 @@ public class AbstractMappingContentNegotiationStrategyTests {
 
 	@Test
 	public void resolveMediaTypes() {
-		Map<String, String> mapping = Collections.singletonMap("json", "application/json");
+		Map<String, MediaType> mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON);
 		TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy("json", mapping);
 
 		List<MediaType> mediaTypes = strategy.resolveMediaTypes(null);
@@ -46,7 +46,7 @@ public class AbstractMappingContentNegotiationStrategyTests {
 
 	@Test
 	public void resolveMediaTypesNoMatch() {
-		Map<String, String> mapping = null;
+		Map<String, MediaType> mapping = null;
 		TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy("blah", mapping);
 
 		List<MediaType> mediaTypes = strategy.resolveMediaTypes(null);
@@ -56,7 +56,7 @@ public class AbstractMappingContentNegotiationStrategyTests {
 
 	@Test
 	public void resolveMediaTypesNoKey() {
-		Map<String, String> mapping = Collections.singletonMap("json", "application/json");
+		Map<String, MediaType> mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON);
 		TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy(null, mapping);
 
 		List<MediaType> mediaTypes = strategy.resolveMediaTypes(null);
@@ -66,7 +66,7 @@ public class AbstractMappingContentNegotiationStrategyTests {
 
 	@Test
 	public void resolveMediaTypesHandleNoMatch() {
-		Map<String, String> mapping = null;
+		Map<String, MediaType> mapping = null;
 		TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy("xml", mapping);
 
 		List<MediaType> mediaTypes = strategy.resolveMediaTypes(null);
@@ -80,7 +80,7 @@ public class AbstractMappingContentNegotiationStrategyTests {
 
 		private final String extension;
 
-		public TestMappingContentNegotiationStrategy(String extension, Map<String, String> mapping) {
+		public TestMappingContentNegotiationStrategy(String extension, Map<String, MediaType> mapping) {
 			super(mapping);
 			this.extension = extension;
 		}

spring-web/src/test/java/org/springframework/web/accept/MappingMediaTypeExtensionsResolverTests.java → spring-web/src/test/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolverTests.java

@@ -16,6 +16,7 @@
 package org.springframework.web.accept;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 import java.util.Collections;
 import java.util.List;
@@ -25,20 +26,29 @@ import org.junit.Test;
 import org.springframework.http.MediaType;
 
 /**
- * Test fixture for MappingMediaTypeExtensionsResolver.
+ * Test fixture for {@link MappingMediaTypeFileExtensionResolver}.
  *
  * @author Rossen Stoyanchev
  */
-public class MappingMediaTypeExtensionsResolverTests {
+public class MappingMediaTypeFileExtensionResolverTests {
 
 	@Test
 	public void resolveExtensions() {
-		Map<String, String> mapping = Collections.singletonMap("json", "application/json");
-		MappingMediaTypeExtensionsResolver resolver = new MappingMediaTypeExtensionsResolver(mapping);
-		List<String> extensions = resolver.resolveExtensions(MediaType.APPLICATION_JSON);
+		Map<String, MediaType> mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON);
+		MappingMediaTypeFileExtensionResolver resolver = new MappingMediaTypeFileExtensionResolver(mapping);
+		List<String> extensions = resolver.resolveFileExtensions(MediaType.APPLICATION_JSON);
 
 		assertEquals(1, extensions.size());
 		assertEquals("json", extensions.get(0));
 	}
 
+	@Test
+	public void resolveExtensionsNoMatch() {
+		Map<String, MediaType> mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON);
+		MappingMediaTypeFileExtensionResolver resolver = new MappingMediaTypeFileExtensionResolver(mapping);
+		List<String> extensions = resolver.resolveFileExtensions(MediaType.TEXT_HTML);
+
+		assertTrue(extensions.isEmpty());
+	}
+
 }

spring-web/src/test/java/org/springframework/web/accept/PathExtensionContentNegotiationStrategyTests.java

@@ -56,7 +56,7 @@ public class PathExtensionContentNegotiationStrategyTests {
 
 		assertEquals(Arrays.asList(new MediaType("text", "html")), mediaTypes);
 
-		strategy = new PathExtensionContentNegotiationStrategy(Collections.singletonMap("HTML", "application/xhtml+xml"));
+		strategy = new PathExtensionContentNegotiationStrategy(Collections.singletonMap("HTML", MediaType.APPLICATION_XHTML_XML));
 		mediaTypes = strategy.resolveMediaTypes(this.webRequest);
 
 		assertEquals(Arrays.asList(new MediaType("application", "xhtml+xml")), mediaTypes);

spring-webmvc/src/main/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolver.java

@@ -102,7 +102,7 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport
 	private boolean favorPathExtension = true;
 	private boolean favorParameter = false;
 	private boolean ignoreAcceptHeader = false;
-	private Map<String, String> mediaTypes = new HashMap<String, String>();
+	private Map<String, MediaType> mediaTypes = new HashMap<String, MediaType>();
 	private Boolean useJaf;
 	private String parameterName;
 	private MediaType defaultContentType;
@@ -200,7 +200,13 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport
 	 * @deprecated use {@link #setContentNegotiationManager(ContentNegotiationManager)}
 	 */
 	public void setMediaTypes(Map<String, String> mediaTypes) {
-		this.mediaTypes = mediaTypes;
+		if (mediaTypes != null) {
+			for (Map.Entry<String, String> entry : mediaTypes.entrySet()) {
+				String extension = entry.getKey().toLowerCase(Locale.ENGLISH);
+				MediaType mediaType = MediaType.parseMediaType(entry.getValue());
+				this.mediaTypes.put(extension, mediaType);
+			}
+		}
 	}
 
 	/**
@@ -389,7 +395,7 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport
 				candidateViews.add(view);
 			}
 			for (MediaType requestedMediaType : requestedMediaTypes) {
-				List<String> extensions = this.contentNegotiationManager.resolveExtensions(requestedMediaType);
+				List<String> extensions = this.contentNegotiationManager.resolveFileExtensions(requestedMediaType);
 				for (String extension : extensions) {
 					String viewNameWithExtension = viewName + "." + extension;
 					view = viewResolver.resolveViewName(viewNameWithExtension, locale);

spring-webmvc/src/test/java/org/springframework/web/servlet/view/ContentNegotiatingViewResolverTests.java

@@ -43,7 +43,7 @@ import org.springframework.mock.web.MockServletContext;
 import org.springframework.web.accept.ContentNegotiationManager;
 import org.springframework.web.accept.FixedContentNegotiationStrategy;
 import org.springframework.web.accept.HeaderContentNegotiationStrategy;
-import org.springframework.web.accept.MappingMediaTypeExtensionsResolver;
+import org.springframework.web.accept.MappingMediaTypeFileExtensionResolver;
 import org.springframework.web.accept.ParameterContentNegotiationStrategy;
 import org.springframework.web.accept.PathExtensionContentNegotiationStrategy;
 import org.springframework.web.context.request.RequestContextHolder;
@@ -117,10 +117,10 @@ public class ContentNegotiatingViewResolverTests {
 	public void resolveViewNameWithAcceptHeader() throws Exception {
 		request.addHeader("Accept", "application/vnd.ms-excel");
 
-		Map<String, String> mapping = Collections.singletonMap("xls", "application/vnd.ms-excel");
-		MappingMediaTypeExtensionsResolver extensionsResolver = new MappingMediaTypeExtensionsResolver(mapping);
+		Map<String, MediaType> mapping = Collections.singletonMap("xls", MediaType.valueOf("application/vnd.ms-excel"));
+		MappingMediaTypeFileExtensionResolver extensionsResolver = new MappingMediaTypeFileExtensionResolver(mapping);
 		ContentNegotiationManager manager = new ContentNegotiationManager(new HeaderContentNegotiationStrategy());
-		manager.addExtensionsResolver(extensionsResolver);
+		manager.addFileExtensionResolvers(extensionsResolver);
 		viewResolver.setContentNegotiationManager(manager);
 
 		ViewResolver viewResolverMock = createMock(ViewResolver.class);
@@ -155,7 +155,7 @@ public class ContentNegotiatingViewResolverTests {
 	public void resolveViewNameWithRequestParameter() throws Exception {
 		request.addParameter("format", "xls");
 
-		Map<String, String> mapping = Collections.singletonMap("xls", "application/vnd.ms-excel");
+		Map<String, MediaType> mapping = Collections.singletonMap("xls", MediaType.valueOf("application/vnd.ms-excel"));
 		ParameterContentNegotiationStrategy paramStrategy = new ParameterContentNegotiationStrategy(mapping);
 		viewResolver.setContentNegotiationManager(new ContentNegotiationManager(paramStrategy));
 
@@ -343,8 +343,7 @@ public class ContentNegotiatingViewResolverTests {
 	public void resolveViewNameFilenameDefaultView() throws Exception {
 		request.setRequestURI("/test.json");
 
-
-		Map<String, String> mapping = Collections.singletonMap("json", "application/json");
+		Map<String, MediaType> mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON);
 		PathExtensionContentNegotiationStrategy pathStrategy = new PathExtensionContentNegotiationStrategy(mapping);
 		viewResolver.setContentNegotiationManager(new ContentNegotiationManager(pathStrategy));