Skip to content

Commit f94aed8

Browse files
rstoyanchevrstoyanchev
committed
Polish ContentNegotiationStrategy support
Issue: SPR-8410, SPR-8417, SPR-8418,SPR-8416, SPR-8419,SPR-7722
1 parent 4623568 commit f94aed8

11 files changed

+80
-61
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,14 @@
3131
* @author Rossen Stoyanchev
3232
* @since 3.2
3333
*/
34-
public abstract class AbstractMappingContentNegotiationStrategy extends MappingMediaTypeExtensionsResolver
35-
implements ContentNegotiationStrategy, MediaTypeExtensionsResolver {
34+
public abstract class AbstractMappingContentNegotiationStrategy extends MappingMediaTypeFileExtensionResolver
35+
implements ContentNegotiationStrategy, MediaTypeFileExtensionResolver {
3636

3737
/**
3838
* Create an instance with the given extension-to-MediaType lookup.
3939
* @throws IllegalArgumentException if a media type string cannot be parsed
4040
*/
41-
public AbstractMappingContentNegotiationStrategy(Map<String, String> mediaTypes) {
41+
public AbstractMappingContentNegotiationStrategy(Map<String, MediaType> mediaTypes) {
4242
super(mediaTypes);
4343
}
4444

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

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -33,28 +33,30 @@
3333
* in a request by delegating to a list of {@link ContentNegotiationStrategy} instances.
3434
*
3535
* <p>It may also be used to determine the extensions associated with a MediaType by
36-
* delegating to a list of {@link MediaTypeExtensionsResolver} instances.
36+
* delegating to a list of {@link MediaTypeFileExtensionResolver} instances.
3737
*
3838
* @author Rossen Stoyanchev
3939
* @since 3.2
4040
*/
41-
public class ContentNegotiationManager implements ContentNegotiationStrategy, MediaTypeExtensionsResolver {
41+
public class ContentNegotiationManager implements ContentNegotiationStrategy, MediaTypeFileExtensionResolver {
4242

43-
private final List<ContentNegotiationStrategy> contentNegotiationStrategies = new ArrayList<ContentNegotiationStrategy>();
43+
private final List<ContentNegotiationStrategy> contentNegotiationStrategies =
44+
new ArrayList<ContentNegotiationStrategy>();
4445

45-
private final Set<MediaTypeExtensionsResolver> extensionResolvers = new LinkedHashSet<MediaTypeExtensionsResolver>();
46+
private final Set<MediaTypeFileExtensionResolver> fileExtensionResolvers =
47+
new LinkedHashSet<MediaTypeFileExtensionResolver>();
4648

4749
/**
4850
* Create an instance with the given ContentNegotiationStrategy instances.
4951
* <p>Each instance is checked to see if it is also an implementation of
50-
* MediaTypeExtensionsResolver, and if so it is registered as such.
52+
* MediaTypeFileExtensionResolver, and if so it is registered as such.
5153
*/
5254
public ContentNegotiationManager(ContentNegotiationStrategy... strategies) {
5355
Assert.notEmpty(strategies, "At least one ContentNegotiationStrategy is expected");
5456
this.contentNegotiationStrategies.addAll(Arrays.asList(strategies));
5557
for (ContentNegotiationStrategy strategy : this.contentNegotiationStrategies) {
56-
if (strategy instanceof MediaTypeExtensionsResolver) {
57-
this.extensionResolvers.add((MediaTypeExtensionsResolver) strategy);
58+
if (strategy instanceof MediaTypeFileExtensionResolver) {
59+
this.fileExtensionResolvers.add((MediaTypeFileExtensionResolver) strategy);
5860
}
5961
}
6062
}
@@ -67,10 +69,10 @@ public ContentNegotiationManager() {
6769
}
6870

6971
/**
70-
* Add MediaTypeExtensionsResolver instances.
72+
* Add MediaTypeFileExtensionResolver instances.
7173
*/
72-
public void addExtensionsResolver(MediaTypeExtensionsResolver... resolvers) {
73-
this.extensionResolvers.addAll(Arrays.asList(resolvers));
74+
public void addFileExtensionResolvers(MediaTypeFileExtensionResolver... resolvers) {
75+
this.fileExtensionResolvers.addAll(Arrays.asList(resolvers));
7476
}
7577

7678
/**
@@ -91,13 +93,13 @@ public List<MediaType> resolveMediaTypes(NativeWebRequest webRequest) throws Htt
9193
}
9294

9395
/**
94-
* Delegate to all configured MediaTypeExtensionsResolver instances and aggregate
95-
* the list of all extensions found.
96+
* Delegate to all configured MediaTypeFileExtensionResolver instances and aggregate
97+
* the list of all file extensions found.
9698
*/
97-
public List<String> resolveExtensions(MediaType mediaType) {
99+
public List<String> resolveFileExtensions(MediaType mediaType) {
98100
Set<String> extensions = new LinkedHashSet<String>();
99-
for (MediaTypeExtensionsResolver resolver : this.extensionResolvers) {
100-
extensions.addAll(resolver.resolveExtensions(mediaType));
101+
for (MediaTypeFileExtensionResolver resolver : this.fileExtensionResolvers) {
102+
extensions.addAll(resolver.resolveFileExtensions(mediaType));
101103
}
102104
return new ArrayList<String>(extensions);
103105
}

‎spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeExtensionsResolver.java renamed to ‎spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolver.java

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*/
1616
package org.springframework.web.accept;
1717

18-
import java.util.ArrayList;
18+
import java.util.Collections;
1919
import java.util.List;
2020
import java.util.Locale;
2121
import java.util.Map;
@@ -24,28 +24,32 @@
2424
import java.util.concurrent.ConcurrentMap;
2525

2626
import org.springframework.http.MediaType;
27+
import org.springframework.util.LinkedMultiValueMap;
28+
import org.springframework.util.MultiValueMap;
2729

2830
/**
29-
* An implementation of {@link MediaTypeExtensionsResolver} that maintains a lookup
31+
* An implementation of {@link MediaTypeFileExtensionResolver} that maintains a lookup
3032
* from extension to MediaType.
3133
*
3234
* @author Rossen Stoyanchev
3335
* @since 3.2
3436
*/
35-
public class MappingMediaTypeExtensionsResolver implements MediaTypeExtensionsResolver {
37+
public class MappingMediaTypeFileExtensionResolver implements MediaTypeFileExtensionResolver {
3638

37-
private ConcurrentMap<String, MediaType> mediaTypes = new ConcurrentHashMap<String, MediaType>();
39+
private final ConcurrentMap<String, MediaType> mediaTypes = new ConcurrentHashMap<String, MediaType>();
40+
41+
private final MultiValueMap<MediaType, String> fileExtensions = new LinkedMultiValueMap<MediaType, String>();
3842

3943
/**
4044
* Create an instance with the given mappings between extensions and media types.
4145
* @throws IllegalArgumentException if a media type string cannot be parsed
4246
*/
43-
public MappingMediaTypeExtensionsResolver(Map<String, String> mediaTypes) {
47+
public MappingMediaTypeFileExtensionResolver(Map<String, MediaType> mediaTypes) {
4448
if (mediaTypes != null) {
45-
for (Map.Entry<String, String> entry : mediaTypes.entrySet()) {
46-
String extension = entry.getKey().toLowerCase(Locale.ENGLISH);
47-
MediaType mediaType = MediaType.parseMediaType(entry.getValue());
48-
this.mediaTypes.put(extension, mediaType);
49+
for (Entry<String, MediaType> entries : mediaTypes.entrySet()) {
50+
String extension = entries.getKey().toLowerCase(Locale.ENGLISH);
51+
MediaType mediaType = entries.getValue();
52+
addMapping(extension, mediaType);
4953
}
5054
}
5155
}
@@ -54,14 +58,9 @@ public MappingMediaTypeExtensionsResolver(Map<String, String> mediaTypes) {
5458
* Find the extensions applicable to the given MediaType.
5559
* @return 0 or more extensions, never {@code null}
5660
*/
57-
public List<String> resolveExtensions(MediaType mediaType) {
58-
List<String> result = new ArrayList<String>();
59-
for (Entry<String, MediaType> entry : this.mediaTypes.entrySet()) {
60-
if (mediaType.includes(entry.getValue())) {
61-
result.add(entry.getKey());
62-
}
63-
}
64-
return result;
61+
public List<String> resolveFileExtensions(MediaType mediaType) {
62+
List<String> fileExtensions = this.fileExtensions.get(mediaType);
63+
return (fileExtensions != null) ? fileExtensions : Collections.<String>emptyList();
6564
}
6665

6766
/**
@@ -76,7 +75,10 @@ public MediaType lookupMediaType(String extension) {
7675
* Map a MediaType to an extension or ignore if the extensions is already mapped.
7776
*/
7877
protected void addMapping(String extension, MediaType mediaType) {
79-
this.mediaTypes.putIfAbsent(extension, mediaType);
78+
MediaType previous = this.mediaTypes.putIfAbsent(extension, mediaType);
79+
if (previous == null) {
80+
this.fileExtensions.add(mediaType, extension);
81+
}
8082
}
8183

8284
}

‎spring-web/src/main/java/org/springframework/web/accept/MediaTypeExtensionsResolver.java renamed to ‎spring-web/src/main/java/org/springframework/web/accept/MediaTypeFileExtensionResolver.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,14 @@
2727
* @author Rossen Stoyanchev
2828
* @since 3.2
2929
*/
30-
public interface MediaTypeExtensionsResolver {
30+
public interface MediaTypeFileExtensionResolver {
3131

3232
/**
3333
* Resolve the given media type to a list of path extensions.
3434
*
3535
* @param mediaType the media type to resolve
3636
* @return a list of extensions or an empty list, never {@code null}
3737
*/
38-
List<String> resolveExtensions(MediaType mediaType);
38+
List<String> resolveFileExtensions(MediaType mediaType);
3939

4040
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public class ParameterContentNegotiationStrategy extends AbstractMappingContentN
4242
* Create an instance with the given extension-to-MediaType lookup.
4343
* @throws IllegalArgumentException if a media type string cannot be parsed
4444
*/
45-
public ParameterContentNegotiationStrategy(Map<String, String> mediaTypes) {
45+
public ParameterContentNegotiationStrategy(Map<String, MediaType> mediaTypes) {
4646
super(mediaTypes);
4747
Assert.notEmpty(mediaTypes, "Cannot look up media types without any mappings");
4848
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public class PathExtensionContentNegotiationStrategy extends AbstractMappingCont
7272
* Create an instance with the given extension-to-MediaType lookup.
7373
* @throws IllegalArgumentException if a media type string cannot be parsed
7474
*/
75-
public PathExtensionContentNegotiationStrategy(Map<String, String> mediaTypes) {
75+
public PathExtensionContentNegotiationStrategy(Map<String, MediaType> mediaTypes) {
7676
super(mediaTypes);
7777
}
7878

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public class AbstractMappingContentNegotiationStrategyTests {
3535

3636
@Test
3737
public void resolveMediaTypes() {
38-
Map<String, String> mapping = Collections.singletonMap("json", "application/json");
38+
Map<String, MediaType> mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON);
3939
TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy("json", mapping);
4040

4141
List<MediaType> mediaTypes = strategy.resolveMediaTypes(null);
@@ -46,7 +46,7 @@ public void resolveMediaTypes() {
4646

4747
@Test
4848
public void resolveMediaTypesNoMatch() {
49-
Map<String, String> mapping = null;
49+
Map<String, MediaType> mapping = null;
5050
TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy("blah", mapping);
5151

5252
List<MediaType> mediaTypes = strategy.resolveMediaTypes(null);
@@ -56,7 +56,7 @@ public void resolveMediaTypesNoMatch() {
5656

5757
@Test
5858
public void resolveMediaTypesNoKey() {
59-
Map<String, String> mapping = Collections.singletonMap("json", "application/json");
59+
Map<String, MediaType> mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON);
6060
TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy(null, mapping);
6161

6262
List<MediaType> mediaTypes = strategy.resolveMediaTypes(null);
@@ -66,7 +66,7 @@ public void resolveMediaTypesNoKey() {
6666

6767
@Test
6868
public void resolveMediaTypesHandleNoMatch() {
69-
Map<String, String> mapping = null;
69+
Map<String, MediaType> mapping = null;
7070
TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy("xml", mapping);
7171

7272
List<MediaType> mediaTypes = strategy.resolveMediaTypes(null);
@@ -80,7 +80,7 @@ private static class TestMappingContentNegotiationStrategy extends AbstractMappi
8080

8181
private final String extension;
8282

83-
public TestMappingContentNegotiationStrategy(String extension, Map<String, String> mapping) {
83+
public TestMappingContentNegotiationStrategy(String extension, Map<String, MediaType> mapping) {
8484
super(mapping);
8585
this.extension = extension;
8686
}

‎spring-web/src/test/java/org/springframework/web/accept/MappingMediaTypeExtensionsResolverTests.java renamed to ‎spring-web/src/test/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolverTests.java

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package org.springframework.web.accept;
1717

1818
import static org.junit.Assert.assertEquals;
19+
import static org.junit.Assert.assertTrue;
1920

2021
import java.util.Collections;
2122
import java.util.List;
@@ -25,20 +26,29 @@
2526
import org.springframework.http.MediaType;
2627

2728
/**
28-
* Test fixture for MappingMediaTypeExtensionsResolver.
29+
* Test fixture for {@link MappingMediaTypeFileExtensionResolver}.
2930
*
3031
* @author Rossen Stoyanchev
3132
*/
32-
public class MappingMediaTypeExtensionsResolverTests {
33+
public class MappingMediaTypeFileExtensionResolverTests {
3334

3435
@Test
3536
public void resolveExtensions() {
36-
Map<String, String> mapping = Collections.singletonMap("json", "application/json");
37-
MappingMediaTypeExtensionsResolver resolver = new MappingMediaTypeExtensionsResolver(mapping);
38-
List<String> extensions = resolver.resolveExtensions(MediaType.APPLICATION_JSON);
37+
Map<String, MediaType> mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON);
38+
MappingMediaTypeFileExtensionResolver resolver = new MappingMediaTypeFileExtensionResolver(mapping);
39+
List<String> extensions = resolver.resolveFileExtensions(MediaType.APPLICATION_JSON);
3940

4041
assertEquals(1, extensions.size());
4142
assertEquals("json", extensions.get(0));
4243
}
4344

45+
@Test
46+
public void resolveExtensionsNoMatch() {
47+
Map<String, MediaType> mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON);
48+
MappingMediaTypeFileExtensionResolver resolver = new MappingMediaTypeFileExtensionResolver(mapping);
49+
List<String> extensions = resolver.resolveFileExtensions(MediaType.TEXT_HTML);
50+
51+
assertTrue(extensions.isEmpty());
52+
}
53+
4454
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public void resolveMediaTypesFromMapping() {
5656

5757
assertEquals(Arrays.asList(new MediaType("text", "html")), mediaTypes);
5858

59-
strategy = new PathExtensionContentNegotiationStrategy(Collections.singletonMap("HTML", "application/xhtml+xml"));
59+
strategy = new PathExtensionContentNegotiationStrategy(Collections.singletonMap("HTML", MediaType.APPLICATION_XHTML_XML));
6060
mediaTypes = strategy.resolveMediaTypes(this.webRequest);
6161

6262
assertEquals(Arrays.asList(new MediaType("application", "xhtml+xml")), mediaTypes);

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ public class ContentNegotiatingViewResolver extends WebApplicationObjectSupport
102102
private boolean favorPathExtension = true;
103103
private boolean favorParameter = false;
104104
private boolean ignoreAcceptHeader = false;
105-
private Map<String, String> mediaTypes = new HashMap<String, String>();
105+
private Map<String, MediaType> mediaTypes = new HashMap<String, MediaType>();
106106
private Boolean useJaf;
107107
private String parameterName;
108108
private MediaType defaultContentType;
@@ -200,7 +200,13 @@ public void setIgnoreAcceptHeader(boolean ignoreAcceptHeader) {
200200
* @deprecated use {@link #setContentNegotiationManager(ContentNegotiationManager)}
201201
*/
202202
public void setMediaTypes(Map<String, String> mediaTypes) {
203-
this.mediaTypes = mediaTypes;
203+
if (mediaTypes != null) {
204+
for (Map.Entry<String, String> entry : mediaTypes.entrySet()) {
205+
String extension = entry.getKey().toLowerCase(Locale.ENGLISH);
206+
MediaType mediaType = MediaType.parseMediaType(entry.getValue());
207+
this.mediaTypes.put(extension, mediaType);
208+
}
209+
}
204210
}
205211

206212
/**
@@ -389,7 +395,7 @@ private List<View> getCandidateViews(String viewName, Locale locale, List<MediaT
389395
candidateViews.add(view);
390396
}
391397
for (MediaType requestedMediaType : requestedMediaTypes) {
392-
List<String> extensions = this.contentNegotiationManager.resolveExtensions(requestedMediaType);
398+
List<String> extensions = this.contentNegotiationManager.resolveFileExtensions(requestedMediaType);
393399
for (String extension : extensions) {
394400
String viewNameWithExtension = viewName + "." + extension;
395401
view = viewResolver.resolveViewName(viewNameWithExtension, locale);

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
import org.springframework.web.accept.ContentNegotiationManager;
4444
import org.springframework.web.accept.FixedContentNegotiationStrategy;
4545
import org.springframework.web.accept.HeaderContentNegotiationStrategy;
46-
import org.springframework.web.accept.MappingMediaTypeExtensionsResolver;
46+
import org.springframework.web.accept.MappingMediaTypeFileExtensionResolver;
4747
import org.springframework.web.accept.ParameterContentNegotiationStrategy;
4848
import org.springframework.web.accept.PathExtensionContentNegotiationStrategy;
4949
import org.springframework.web.context.request.RequestContextHolder;
@@ -117,10 +117,10 @@ public void resolveViewNameWithPathExtension() throws Exception {
117117
public void resolveViewNameWithAcceptHeader() throws Exception {
118118
request.addHeader("Accept", "application/vnd.ms-excel");
119119

120-
Map<String, String> mapping = Collections.singletonMap("xls", "application/vnd.ms-excel");
121-
MappingMediaTypeExtensionsResolver extensionsResolver = new MappingMediaTypeExtensionsResolver(mapping);
120+
Map<String, MediaType> mapping = Collections.singletonMap("xls", MediaType.valueOf("application/vnd.ms-excel"));
121+
MappingMediaTypeFileExtensionResolver extensionsResolver = new MappingMediaTypeFileExtensionResolver(mapping);
122122
ContentNegotiationManager manager = new ContentNegotiationManager(new HeaderContentNegotiationStrategy());
123-
manager.addExtensionsResolver(extensionsResolver);
123+
manager.addFileExtensionResolvers(extensionsResolver);
124124
viewResolver.setContentNegotiationManager(manager);
125125

126126
ViewResolver viewResolverMock = createMock(ViewResolver.class);
@@ -155,7 +155,7 @@ public void resolveViewNameWithInvalidAcceptHeader() throws Exception {
155155
public void resolveViewNameWithRequestParameter() throws Exception {
156156
request.addParameter("format", "xls");
157157

158-
Map<String, String> mapping = Collections.singletonMap("xls", "application/vnd.ms-excel");
158+
Map<String, MediaType> mapping = Collections.singletonMap("xls", MediaType.valueOf("application/vnd.ms-excel"));
159159
ParameterContentNegotiationStrategy paramStrategy = new ParameterContentNegotiationStrategy(mapping);
160160
viewResolver.setContentNegotiationManager(new ContentNegotiationManager(paramStrategy));
161161

@@ -343,8 +343,7 @@ public void resolveViewNameFilename() throws Exception {
343343
public void resolveViewNameFilenameDefaultView() throws Exception {
344344
request.setRequestURI("/test.json");
345345

346-
347-
Map<String, String> mapping = Collections.singletonMap("json", "application/json");
346+
Map<String, MediaType> mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON);
348347
PathExtensionContentNegotiationStrategy pathStrategy = new PathExtensionContentNegotiationStrategy(mapping);
349348
viewResolver.setContentNegotiationManager(new ContentNegotiationManager(pathStrategy));
350349

0 commit comments

Comments
 (0)