diff --git a/spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolver.java b/spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolver.java index 72672f913eb..130f2bd5cf7 100644 --- a/spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolver.java +++ b/spring-web/src/main/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolver.java @@ -57,12 +57,7 @@ public class MappingMediaTypeFileExtensionResolver implements MediaTypeFileExten public MappingMediaTypeFileExtensionResolver(@Nullable Map mediaTypes) { if (mediaTypes != null) { Set allFileExtensions = CollectionUtils.newHashSet(mediaTypes.size()); - mediaTypes.forEach((extension, mediaType) -> { - String lowerCaseExtension = extension.toLowerCase(Locale.ROOT); - this.mediaTypes.put(lowerCaseExtension, mediaType); - addFileExtension(mediaType, lowerCaseExtension); - allFileExtensions.add(lowerCaseExtension); - }); + mediaTypes.forEach((extension, mediaType) -> addMapping(extension.toLowerCase(Locale.ROOT), mediaType)); this.allFileExtensions.addAll(allFileExtensions); } } @@ -82,16 +77,12 @@ public class MappingMediaTypeFileExtensionResolver implements MediaTypeFileExten protected void addMapping(String extension, MediaType mediaType) { MediaType previous = this.mediaTypes.putIfAbsent(extension, mediaType); if (previous == null) { - addFileExtension(mediaType, extension); + this.fileExtensions.computeIfAbsent( + mediaType, key -> new CopyOnWriteArrayList<>()).add(extension); this.allFileExtensions.add(extension); } } - private void addFileExtension(MediaType mediaType, String extension) { - this.fileExtensions.computeIfAbsent(mediaType, key -> new CopyOnWriteArrayList<>()) - .add(extension); - } - @Override public List resolveFileExtensions(MediaType mediaType) { diff --git a/spring-web/src/test/java/org/springframework/web/accept/MappingContentNegotiationStrategyTests.java b/spring-web/src/test/java/org/springframework/web/accept/MappingContentNegotiationStrategyTests.java index 70770d0ae00..14f9d5d40f3 100644 --- a/spring-web/src/test/java/org/springframework/web/accept/MappingContentNegotiationStrategyTests.java +++ b/spring-web/src/test/java/org/springframework/web/accept/MappingContentNegotiationStrategyTests.java @@ -23,9 +23,11 @@ import java.util.Map; import org.junit.jupiter.api.Test; import org.springframework.http.MediaType; +import org.springframework.web.HttpMediaTypeNotAcceptableException; import org.springframework.web.context.request.NativeWebRequest; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; /** * A test fixture with a test subclass of AbstractMappingContentNegotiationStrategy. @@ -36,7 +38,7 @@ import static org.assertj.core.api.Assertions.assertThat; class MappingContentNegotiationStrategyTests { @Test - void resolveMediaTypes() throws Exception { + void resolveMediaTypesFromRegisteredExtensions() throws Exception { Map mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON); TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy("json", mapping); @@ -47,27 +49,7 @@ class MappingContentNegotiationStrategyTests { } @Test - void resolveMediaTypesNoMatch() throws Exception { - Map mapping = null; - TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy("blah", mapping); - - List mediaTypes = strategy.resolveMediaTypes(null); - - assertThat(mediaTypes).isEqualTo(ContentNegotiationStrategy.MEDIA_TYPE_ALL_LIST); - } - - @Test - void resolveMediaTypesNoKey() throws Exception { - Map mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON); - TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy(null, mapping); - - List mediaTypes = strategy.resolveMediaTypes(null); - - assertThat(mediaTypes).isEqualTo(ContentNegotiationStrategy.MEDIA_TYPE_ALL_LIST); - } - - @Test - void resolveMediaTypesHandleNoMatch() throws Exception { + void resolveMediaTypesViaMediaTypeFactory() throws Exception { Map mapping = null; TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy("xml", mapping); @@ -77,6 +59,25 @@ class MappingContentNegotiationStrategyTests { assertThat(mediaTypes.get(0).toString()).isEqualTo("application/xml"); } + @Test + void resolveMediaTypesUnknownKey() { + Map mapping = null; + TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy("blah", mapping); + + assertThatThrownBy(() -> strategy.resolveMediaTypes(null)) + .isInstanceOf(HttpMediaTypeNotAcceptableException.class); + } + + @Test + void resolveMediaTypesNullKey() throws Exception { + Map mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON); + TestMappingContentNegotiationStrategy strategy = new TestMappingContentNegotiationStrategy(null, mapping); + + List mediaTypes = strategy.resolveMediaTypes(null); + + assertThat(mediaTypes).isEqualTo(ContentNegotiationStrategy.MEDIA_TYPE_ALL_LIST); + } + private static class TestMappingContentNegotiationStrategy extends AbstractMappingContentNegotiationStrategy { @@ -92,10 +93,6 @@ class MappingContentNegotiationStrategyTests { return this.extension; } - @Override - protected MediaType handleNoMatch(NativeWebRequest request, String mappingKey) { - return "xml".equals(mappingKey) ? MediaType.APPLICATION_XML : null; - } } } diff --git a/spring-web/src/test/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolverTests.java b/spring-web/src/test/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolverTests.java index 88c348d0973..b003a9825a7 100644 --- a/spring-web/src/test/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolverTests.java +++ b/spring-web/src/test/java/org/springframework/web/accept/MappingMediaTypeFileExtensionResolverTests.java @@ -68,13 +68,15 @@ class MappingMediaTypeFileExtensionResolverTests { } @Test - void allFileExtensions() { - Map mappings = new HashMap<>(); - mappings.put("json", MediaType.APPLICATION_JSON); - mappings.put("JsOn", MediaType.APPLICATION_JSON); - mappings.put("jSoN", MediaType.APPLICATION_JSON); + void mappingsAreCaseInsensitive() { + Map map = new HashMap<>(); + map.put("json", MediaType.APPLICATION_JSON); + map.put("JsOn", MediaType.APPLICATION_JSON); + map.put("jSoN", MediaType.APPLICATION_JSON); - MappingMediaTypeFileExtensionResolver resolver = new MappingMediaTypeFileExtensionResolver(mappings); + MappingMediaTypeFileExtensionResolver resolver = new MappingMediaTypeFileExtensionResolver(map); assertThat(resolver.getAllFileExtensions()).containsExactly("json"); } + + }