From f94aed83860ee46b8fbe5b1f48d19470f54b843b Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Mon, 25 Jun 2012 15:24:05 -0400 Subject: [PATCH] Polish ContentNegotiationStrategy support Issue: SPR-8410, SPR-8417, SPR-8418,SPR-8416, SPR-8419,SPR-7722 --- ...ractMappingContentNegotiationStrategy.java | 6 +-- .../web/accept/ContentNegotiationManager.java | 32 ++++++++-------- ...appingMediaTypeFileExtensionResolver.java} | 38 ++++++++++--------- ...va => MediaTypeFileExtensionResolver.java} | 4 +- .../ParameterContentNegotiationStrategy.java | 2 +- ...thExtensionContentNegotiationStrategy.java | 2 +- ...appingContentNegotiationStrategyTests.java | 10 ++--- ...gMediaTypeFileExtensionResolverTests.java} | 20 +++++++--- ...ensionContentNegotiationStrategyTests.java | 2 +- .../view/ContentNegotiatingViewResolver.java | 12 ++++-- .../ContentNegotiatingViewResolverTests.java | 13 +++---- 11 files changed, 80 insertions(+), 61 deletions(-) rename spring-web/src/main/java/org/springframework/web/accept/{MappingMediaTypeExtensionsResolver.java => MappingMediaTypeFileExtensionResolver.java} (58%) rename spring-web/src/main/java/org/springframework/web/accept/{MediaTypeExtensionsResolver.java => MediaTypeFileExtensionResolver.java} (91%) rename spring-web/src/test/java/org/springframework/web/accept/{MappingMediaTypeExtensionsResolverTests.java => MappingMediaTypeFileExtensionResolverTests.java} (55%) diff --git a/spring-web/src/main/java/org/springframework/web/accept/AbstractMappingContentNegotiationStrategy.java b/spring-web/src/main/java/org/springframework/web/accept/AbstractMappingContentNegotiationStrategy.java index da1dc114ab..edd0b8e22b 100644 --- a/spring-web/src/main/java/org/springframework/web/accept/AbstractMappingContentNegotiationStrategy.java +++ b/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 mediaTypes) { + public AbstractMappingContentNegotiationStrategy(Map mediaTypes) { super(mediaTypes); } diff --git a/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManager.java b/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManager.java index e789bcba50..cb14f7f6b4 100644 --- a/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManager.java +++ b/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. * *

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 contentNegotiationStrategies = new ArrayList(); + private final List contentNegotiationStrategies = + new ArrayList(); - private final Set extensionResolvers = new LinkedHashSet(); + private final Set fileExtensionResolvers = + new LinkedHashSet(); /** * Create an instance with the given ContentNegotiationStrategy instances. *

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 resolveExtensions(MediaType mediaType) { + public List resolveFileExtensions(MediaType mediaType) { Set extensions = new LinkedHashSet(); - for (MediaTypeExtensionsResolver resolver : this.extensionResolvers) { - extensions.addAll(resolver.resolveExtensions(mediaType)); + for (MediaTypeFileExtensionResolver resolver : this.fileExtensionResolvers) { + extensions.addAll(resolver.resolveFileExtensions(mediaType)); } return new ArrayList(extensions); } diff --git a/spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeExtensionsResolver.java b/spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolver.java similarity index 58% rename from spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeExtensionsResolver.java rename to spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolver.java index 0a401a2e73..29ee02a49e 100644 --- a/spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeExtensionsResolver.java +++ b/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 mediaTypes = new ConcurrentHashMap(); + private final ConcurrentMap mediaTypes = new ConcurrentHashMap(); + + private final MultiValueMap fileExtensions = new LinkedMultiValueMap(); /** * 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 mediaTypes) { + public MappingMediaTypeFileExtensionResolver(Map mediaTypes) { if (mediaTypes != null) { - for (Map.Entry entry : mediaTypes.entrySet()) { - String extension = entry.getKey().toLowerCase(Locale.ENGLISH); - MediaType mediaType = MediaType.parseMediaType(entry.getValue()); - this.mediaTypes.put(extension, mediaType); + for (Entry 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 resolveExtensions(MediaType mediaType) { - List result = new ArrayList(); - for (Entry entry : this.mediaTypes.entrySet()) { - if (mediaType.includes(entry.getValue())) { - result.add(entry.getKey()); - } - } - return result; + public List resolveFileExtensions(MediaType mediaType) { + List fileExtensions = this.fileExtensions.get(mediaType); + return (fileExtensions != null) ? fileExtensions : Collections.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 diff --git a/spring-web/src/main/java/org/springframework/web/accept/MediaTypeExtensionsResolver.java b/spring-web/src/main/java/org/springframework/web/accept/MediaTypeFileExtensionResolver.java similarity index 91% rename from spring-web/src/main/java/org/springframework/web/accept/MediaTypeExtensionsResolver.java rename to spring-web/src/main/java/org/springframework/web/accept/MediaTypeFileExtensionResolver.java index 476695b947..73f5882603 100644 --- a/spring-web/src/main/java/org/springframework/web/accept/MediaTypeExtensionsResolver.java +++ b/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 resolveExtensions(MediaType mediaType); + List resolveFileExtensions(MediaType mediaType); } diff --git a/spring-web/src/main/java/org/springframework/web/accept/ParameterContentNegotiationStrategy.java b/spring-web/src/main/java/org/springframework/web/accept/ParameterContentNegotiationStrategy.java index 7e0ce5f9d5..0adaeaab52 100644 --- a/spring-web/src/main/java/org/springframework/web/accept/ParameterContentNegotiationStrategy.java +++ b/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 mediaTypes) { + public ParameterContentNegotiationStrategy(Map mediaTypes) { super(mediaTypes); Assert.notEmpty(mediaTypes, "Cannot look up media types without any mappings"); } diff --git a/spring-web/src/main/java/org/springframework/web/accept/PathExtensionContentNegotiationStrategy.java b/spring-web/src/main/java/org/springframework/web/accept/PathExtensionContentNegotiationStrategy.java index d2ddca7bae..eb76db5161 100644 --- a/spring-web/src/main/java/org/springframework/web/accept/PathExtensionContentNegotiationStrategy.java +++ b/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 mediaTypes) { + public PathExtensionContentNegotiationStrategy(Map mediaTypes) { super(mediaTypes); } diff --git a/spring-web/src/test/java/org/springframework/web/accept/AbstractMappingContentNegotiationStrategyTests.java b/spring-web/src/test/java/org/springframework/web/accept/AbstractMappingContentNegotiationStrategyTests.java index 4088ddebcb..9888177413 100644 --- a/spring-web/src/test/java/org/springframework/web/accept/AbstractMappingContentNegotiationStrategyTests.java +++ b/spring-web/src/test/java/org/springframework/web/accept/AbstractMappingContentNegotiationStrategyTests.java @@ -35,7 +35,7 @@ public class AbstractMappingContentNegotiationStrategyTests { @Test public void resolveMediaTypes() { - Map mapping = Collections.singletonMap("json", "application/json"); + Map mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON); TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy("json", mapping); List mediaTypes = strategy.resolveMediaTypes(null); @@ -46,7 +46,7 @@ public class AbstractMappingContentNegotiationStrategyTests { @Test public void resolveMediaTypesNoMatch() { - Map mapping = null; + Map mapping = null; TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy("blah", mapping); List mediaTypes = strategy.resolveMediaTypes(null); @@ -56,7 +56,7 @@ public class AbstractMappingContentNegotiationStrategyTests { @Test public void resolveMediaTypesNoKey() { - Map mapping = Collections.singletonMap("json", "application/json"); + Map mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON); TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy(null, mapping); List mediaTypes = strategy.resolveMediaTypes(null); @@ -66,7 +66,7 @@ public class AbstractMappingContentNegotiationStrategyTests { @Test public void resolveMediaTypesHandleNoMatch() { - Map mapping = null; + Map mapping = null; TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy("xml", mapping); List mediaTypes = strategy.resolveMediaTypes(null); @@ -80,7 +80,7 @@ public class AbstractMappingContentNegotiationStrategyTests { private final String extension; - public TestMappingContentNegotiationStrategy(String extension, Map mapping) { + public TestMappingContentNegotiationStrategy(String extension, Map mapping) { super(mapping); this.extension = extension; } diff --git a/spring-web/src/test/java/org/springframework/web/accept/MappingMediaTypeExtensionsResolverTests.java b/spring-web/src/test/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolverTests.java similarity index 55% rename from spring-web/src/test/java/org/springframework/web/accept/MappingMediaTypeExtensionsResolverTests.java rename to spring-web/src/test/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolverTests.java index 6aca487535..b00b7c5d0a 100644 --- a/spring-web/src/test/java/org/springframework/web/accept/MappingMediaTypeExtensionsResolverTests.java +++ b/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 mapping = Collections.singletonMap("json", "application/json"); - MappingMediaTypeExtensionsResolver resolver = new MappingMediaTypeExtensionsResolver(mapping); - List extensions = resolver.resolveExtensions(MediaType.APPLICATION_JSON); + Map mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON); + MappingMediaTypeFileExtensionResolver resolver = new MappingMediaTypeFileExtensionResolver(mapping); + List extensions = resolver.resolveFileExtensions(MediaType.APPLICATION_JSON); assertEquals(1, extensions.size()); assertEquals("json", extensions.get(0)); } + @Test + public void resolveExtensionsNoMatch() { + Map mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON); + MappingMediaTypeFileExtensionResolver resolver = new MappingMediaTypeFileExtensionResolver(mapping); + List extensions = resolver.resolveFileExtensions(MediaType.TEXT_HTML); + + assertTrue(extensions.isEmpty()); + } + } diff --git a/spring-web/src/test/java/org/springframework/web/accept/PathExtensionContentNegotiationStrategyTests.java b/spring-web/src/test/java/org/springframework/web/accept/PathExtensionContentNegotiationStrategyTests.java index 47dc26e899..c5922d7a36 100644 --- a/spring-web/src/test/java/org/springframework/web/accept/PathExtensionContentNegotiationStrategyTests.java +++ b/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); 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 7d652aae99..47fa87d7b0 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 @@ -102,7 +102,7 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport private boolean favorPathExtension = true; private boolean favorParameter = false; private boolean ignoreAcceptHeader = false; - private Map mediaTypes = new HashMap(); + private Map mediaTypes = new HashMap(); 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 mediaTypes) { - this.mediaTypes = mediaTypes; + if (mediaTypes != null) { + for (Map.Entry 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 extensions = this.contentNegotiationManager.resolveExtensions(requestedMediaType); + List extensions = this.contentNegotiationManager.resolveFileExtensions(requestedMediaType); for (String extension : extensions) { String viewNameWithExtension = viewName + "." + extension; view = viewResolver.resolveViewName(viewNameWithExtension, locale); 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 6f5ccf6496..d94ebf8d21 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 @@ -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 mapping = Collections.singletonMap("xls", "application/vnd.ms-excel"); - MappingMediaTypeExtensionsResolver extensionsResolver = new MappingMediaTypeExtensionsResolver(mapping); + Map 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 mapping = Collections.singletonMap("xls", "application/vnd.ms-excel"); + Map 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 mapping = Collections.singletonMap("json", "application/json"); + Map mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON); PathExtensionContentNegotiationStrategy pathStrategy = new PathExtensionContentNegotiationStrategy(mapping); viewResolver.setContentNegotiationManager(new ContentNegotiationManager(pathStrategy)); -- GitLab