From b0e8e7f536e021836ec7737d6d97395f5736f2ff Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 7 Jun 2017 12:17:49 -0400 Subject: [PATCH] Refactor MappingContentTypeResolver implementations After the removal of suffix pattern matches, there is no longer a need to expose the list of registered file extensions. Also polish, refactor, and simplify the abstract base class AbstractMappingContentTypeResolver and its sub-classes. Issue: SPR-15639 --- .../AbstractMappingContentTypeResolver.java | 124 ++++-------------- .../accept/CompositeContentTypeResolver.java | 61 +-------- .../accept/MappingContentTypeResolver.java | 46 ------- .../accept/ParameterContentTypeResolver.java | 29 +--- .../PathExtensionContentTypeResolver.java | 68 +--------- .../accept/RequestedContentTypeResolver.java | 2 +- .../RequestedContentTypeResolverBuilder.java | 35 +---- .../config/WebFluxConfigurationSupport.java | 1 - .../result/method/RequestMappingInfo.java | 7 - ...positeContentTypeResolverBuilderTests.java | 22 ---- .../MappingContentTypeResolverTests.java | 61 +++------ ...PathExtensionContentTypeResolverTests.java | 33 ++--- .../RequestMappingHandlerMappingTests.java | 51 +------ 13 files changed, 73 insertions(+), 467 deletions(-) delete mode 100644 spring-webflux/src/main/java/org/springframework/web/reactive/accept/MappingContentTypeResolver.java diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/AbstractMappingContentTypeResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/accept/AbstractMappingContentTypeResolver.java index a60e1ceeb4..c987bcce54 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/AbstractMappingContentTypeResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/accept/AbstractMappingContentTypeResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,104 +15,45 @@ */ package org.springframework.web.reactive.accept; -import java.util.ArrayList; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import org.springframework.http.MediaType; +import org.springframework.http.MediaTypeFactory; import org.springframework.lang.Nullable; -import org.springframework.util.LinkedMultiValueMap; -import org.springframework.util.MultiValueMap; import org.springframework.util.StringUtils; -import org.springframework.web.server.NotAcceptableStatusException; import org.springframework.web.server.ServerWebExchange; /** - * Abstract base class for {@link MappingContentTypeResolver} implementations. - * Maintains the actual mappings and pre-implements the overall algorithm with - * sub-classes left to provide a way to extract the lookup key (e.g. file - * extension, query parameter, etc) for a given exchange. + * Base class for resolvers that extract a key from the request and look up a + * mapping to a MediaType. The use case is URI-based content negotiation for + * example based on query parameter or file extension in the request path. * * @author Rossen Stoyanchev * @since 5.0 */ -public abstract class AbstractMappingContentTypeResolver implements MappingContentTypeResolver { +public abstract class AbstractMappingContentTypeResolver implements RequestedContentTypeResolver { /** Primary lookup for media types by key (e.g. "json" -> "application/json") */ private final Map mediaTypeLookup = new ConcurrentHashMap<>(64); - /** Reverse lookup for keys associated with a media type */ - private final MultiValueMap keyLookup = new LinkedMultiValueMap<>(64); - - /** - * Create an instance with the given map of file extensions and media types. - */ - public AbstractMappingContentTypeResolver(@Nullable Map mediaTypes) { - if (mediaTypes != null) { - for (Map.Entry entry : mediaTypes.entrySet()) { - String extension = entry.getKey().toLowerCase(Locale.ENGLISH); - MediaType mediaType = entry.getValue(); - this.mediaTypeLookup.put(extension, mediaType); - this.keyLookup.add(mediaType, extension); - } - } + public AbstractMappingContentTypeResolver(Map mediaTypes) { + mediaTypes.forEach((key, mediaType) -> + this.mediaTypeLookup.put(key.toLowerCase(Locale.ENGLISH), mediaType)); } - public Map getMediaTypes() { - return this.mediaTypeLookup; - } - - /** - * Sub-classes can use this method to look up a MediaType by key. - * @param key the key converted to lower case - * @return a MediaType or {@code null} - */ - @Nullable - protected MediaType getMediaType(String key) { - return this.mediaTypeLookup.get(key.toLowerCase(Locale.ENGLISH)); - } - - /** - * Sub-classes can use this method get all mapped media types. - */ - protected List getAllMediaTypes() { - return new ArrayList<>(this.mediaTypeLookup.values()); - } - - - // RequestedContentTypeResolver implementation - @Override - public List resolveMediaTypes(ServerWebExchange exchange) throws NotAcceptableStatusException { - return resolveMediaTypes(extractKey(exchange)); - } - - /** - * An overloaded resolve method with a pre-resolved lookup key. - * @param key the key for looking up media types - * @return a list of resolved media types or an empty list - * @throws NotAcceptableStatusException - */ - public List resolveMediaTypes(@Nullable String key) throws NotAcceptableStatusException { + public List resolveMediaTypes(ServerWebExchange exchange) { + String key = getKey(exchange); if (StringUtils.hasText(key)) { MediaType mediaType = getMediaType(key); if (mediaType != null) { - handleMatch(key, mediaType); - return Collections.singletonList(mediaType); - } - mediaType = handleNoMatch(key); - if (mediaType != null) { - MediaType previous = this.mediaTypeLookup.putIfAbsent(key, mediaType); - if (previous == null) { - this.keyLookup.add(mediaType, key); - } + this.mediaTypeLookup.putIfAbsent(key, mediaType); return Collections.singletonList(mediaType); } } @@ -120,43 +61,22 @@ public abstract class AbstractMappingContentTypeResolver implements MappingConte } /** - * Extract the key to use to look up a media type from the given exchange, - * e.g. file extension, query parameter, etc. - * @return the key or {@code null} + * Get the key to look up a MediaType with. */ @Nullable - protected abstract String extractKey(ServerWebExchange exchange); + protected abstract String getKey(ServerWebExchange exchange); /** - * Override to provide handling when a key is successfully resolved via - * {@link #getMediaType(String)}. + * Get the MediaType for the given key. */ - @SuppressWarnings("UnusedParameters") - protected void handleMatch(String key, MediaType mediaType) { - } - - /** - * Override to provide handling when a key is not resolved via. - * {@link #getMediaType(String)}. If a MediaType is returned from - * this method it will be added to the mappings. - */ - @SuppressWarnings("UnusedParameters") @Nullable - protected MediaType handleNoMatch(String key) throws NotAcceptableStatusException { - return null; - } - - // MappingContentTypeResolver implementation - - @Override - public Set getKeysFor(MediaType mediaType) { - List keys = this.keyLookup.get(mediaType); - return (keys != null ? new HashSet<>(keys) : Collections.emptySet()); - } - - @Override - public Set getKeys() { - return new HashSet<>(this.mediaTypeLookup.keySet()); + protected MediaType getMediaType(String key) { + key = key.toLowerCase(Locale.ENGLISH); + MediaType mediaType = this.mediaTypeLookup.get(key); + if (mediaType == null) { + mediaType = MediaTypeFactory.getMediaType("filename." + key).orElse(null); + } + return mediaType; } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/CompositeContentTypeResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/accept/CompositeContentTypeResolver.java index 423653438d..bc2f8c0e60 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/CompositeContentTypeResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/accept/CompositeContentTypeResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,28 +17,19 @@ package org.springframework.web.reactive.accept; import java.util.ArrayList; import java.util.Collections; -import java.util.LinkedHashSet; import java.util.List; -import java.util.Set; import org.springframework.http.MediaType; -import org.springframework.lang.Nullable; import org.springframework.util.Assert; -import org.springframework.web.server.NotAcceptableStatusException; import org.springframework.web.server.ServerWebExchange; /** - * A {@link RequestedContentTypeResolver} that contains and delegates to a list of other - * resolvers. - * - *

Also an implementation of {@link MappingContentTypeResolver} that delegates - * to those resolvers from the list that are also of type - * {@code MappingContentTypeResolver}. + * Contains and delegates to other {@link RequestedContentTypeResolver}. * * @author Rossen Stoyanchev * @since 5.0 */ -public class CompositeContentTypeResolver implements MappingContentTypeResolver { +public class CompositeContentTypeResolver implements RequestedContentTypeResolver { private final List resolvers = new ArrayList<>(); @@ -49,32 +40,8 @@ public class CompositeContentTypeResolver implements MappingContentTypeResolver } - /** - * Return a read-only list of the configured resolvers. - */ - public List getResolvers() { - return Collections.unmodifiableList(this.resolvers); - } - - /** - * Return the first {@link RequestedContentTypeResolver} of the given type. - * @param resolverType the resolver type - * @return the first matching resolver or {@code null}. - */ - @SuppressWarnings("unchecked") - @Nullable - public T findResolver(Class resolverType) { - for (RequestedContentTypeResolver resolver : this.resolvers) { - if (resolverType.isInstance(resolver)) { - return (T) resolver; - } - } - return null; - } - - @Override - public List resolveMediaTypes(ServerWebExchange exchange) throws NotAcceptableStatusException { + public List resolveMediaTypes(ServerWebExchange exchange) { for (RequestedContentTypeResolver resolver : this.resolvers) { List mediaTypes = resolver.resolveMediaTypes(exchange); if (mediaTypes.isEmpty() || (mediaTypes.size() == 1 && mediaTypes.contains(MediaType.ALL))) { @@ -85,24 +52,4 @@ public class CompositeContentTypeResolver implements MappingContentTypeResolver return Collections.emptyList(); } - @Override - public Set getKeysFor(MediaType mediaType) { - Set result = new LinkedHashSet<>(); - for (RequestedContentTypeResolver resolver : this.resolvers) { - if (resolver instanceof MappingContentTypeResolver) - result.addAll(((MappingContentTypeResolver) resolver).getKeysFor(mediaType)); - } - return result; - } - - @Override - public Set getKeys() { - Set result = new LinkedHashSet<>(); - for (RequestedContentTypeResolver resolver : this.resolvers) { - if (resolver instanceof MappingContentTypeResolver) - result.addAll(((MappingContentTypeResolver) resolver).getKeys()); - } - return result; - } - } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/MappingContentTypeResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/accept/MappingContentTypeResolver.java deleted file mode 100644 index 55c53123bb..0000000000 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/MappingContentTypeResolver.java +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Copyright 2002-2016 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.web.reactive.accept; - -import java.util.Set; - -import org.springframework.http.MediaType; - -/** - * An extension of {@link RequestedContentTypeResolver} that maintains a mapping - * between keys (e.g. file extension, query parameter) and media types. - * - * @author Rossen Stoyanchev - * @since 5.0 - */ -public interface MappingContentTypeResolver extends RequestedContentTypeResolver { - - /** - * Resolve the given media type to a list of path extensions. - * - * @param mediaType the media type to resolve - * @return a list of extensions or an empty list, never {@code null} - */ - Set getKeysFor(MediaType mediaType); - - /** - * Return all registered keys (e.g. "json", "xml"). - * @return a list of keys or an empty list, never {@code null} - */ - Set getKeys(); - -} diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/ParameterContentTypeResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/accept/ParameterContentTypeResolver.java index aa57177dd7..6c294ef691 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/ParameterContentTypeResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/accept/ParameterContentTypeResolver.java @@ -18,31 +18,21 @@ package org.springframework.web.reactive.accept; import java.util.Map; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; - import org.springframework.http.MediaType; import org.springframework.util.Assert; -import org.springframework.web.server.NotAcceptableStatusException; import org.springframework.web.server.ServerWebExchange; /** - * A {@link RequestedContentTypeResolver} that extracts the media type lookup - * key from a known query parameter named "format" by default. + * Query parameter based {@link AbstractMappingContentTypeResolver}. * * @author Rossen Stoyanchev * @since 5.0 */ public class ParameterContentTypeResolver extends AbstractMappingContentTypeResolver { - private static final Log logger = LogFactory.getLog(ParameterContentTypeResolver.class); - private String parameterName = "format"; - /** - * Create an instance with the given map of file extensions and media types. - */ public ParameterContentTypeResolver(Map mediaTypes) { super(mediaTypes); } @@ -53,7 +43,7 @@ public class ParameterContentTypeResolver extends AbstractMappingContentTypeReso *

By default this is set to {@code "format"}. */ public void setParameterName(String parameterName) { - Assert.notNull(parameterName, "parameterName is required"); + Assert.notNull(parameterName, "'parameterName' is required"); this.parameterName = parameterName; } @@ -63,21 +53,8 @@ public class ParameterContentTypeResolver extends AbstractMappingContentTypeReso @Override - protected String extractKey(ServerWebExchange exchange) { + protected String getKey(ServerWebExchange exchange) { return exchange.getRequest().getQueryParams().getFirst(getParameterName()); } - @Override - protected void handleMatch(String mediaTypeKey, MediaType mediaType) { - if (logger.isDebugEnabled()) { - logger.debug("Requested media type is '" + mediaType + - "' based on '" + getParameterName() + "'='" + mediaTypeKey + "'."); - } - } - - @Override - protected MediaType handleNoMatch(String key) throws NotAcceptableStatusException { - throw new NotAcceptableStatusException(getAllMediaTypes()); - } - } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/PathExtensionContentTypeResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/accept/PathExtensionContentTypeResolver.java index 1d58d0d221..41b0359eb7 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/PathExtensionContentTypeResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/accept/PathExtensionContentTypeResolver.java @@ -16,90 +16,30 @@ package org.springframework.web.reactive.accept; -import java.util.Locale; import java.util.Map; -import java.util.Optional; import org.springframework.http.MediaType; -import org.springframework.http.MediaTypeFactory; -import org.springframework.lang.Nullable; -import org.springframework.util.StringUtils; -import org.springframework.web.server.NotAcceptableStatusException; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.util.UriUtils; /** - * A {@link RequestedContentTypeResolver} that extracts the file extension from - * the request path and uses that as the media type lookup key. - * - *

If the file extension is not found in the explicit registrations provided - * to the constructor, the {@link MediaTypeFactory} is used as a fallback - * mechanism. + * Path file extension sub-class of {@link AbstractMappingContentTypeResolver}. * * @author Rossen Stoyanchev * @since 5.0 */ public class PathExtensionContentTypeResolver extends AbstractMappingContentTypeResolver { - private boolean useRegisteredExtensionsOnly = false; - private boolean ignoreUnknownExtensions = true; - - - /** - * Create an instance without any mappings to start with. Mappings may be added - * later on if any extensions are resolved through the Java Activation framework. - */ - public PathExtensionContentTypeResolver() { - super(null); - } - - /** - * Create an instance with the given map of file extensions and media types. - */ - public PathExtensionContentTypeResolver(@Nullable Map mediaTypes) { + public PathExtensionContentTypeResolver(Map mediaTypes) { super(mediaTypes); } - /** - * Whether to only use the registered mappings to look up file extensions, or also refer to - * defaults. - *

By default this is set to {@code false}, meaning that defaults are used. - */ - public void setUseRegisteredExtensionsOnly(boolean useRegisteredExtensionsOnly) { - this.useRegisteredExtensionsOnly = useRegisteredExtensionsOnly; - } - - /** - * Whether to ignore requests with unknown file extension. Setting this to - * {@code false} results in {@code HttpMediaTypeNotAcceptableException}. - *

By default this is set to {@code true}. - */ - public void setIgnoreUnknownExtensions(boolean ignoreUnknownExtensions) { - this.ignoreUnknownExtensions = ignoreUnknownExtensions; - } - - @Override - protected String extractKey(ServerWebExchange exchange) { + protected String getKey(ServerWebExchange exchange) { String path = exchange.getRequest().getURI().getRawPath(); - String extension = UriUtils.extractFileExtension(path); - return (StringUtils.hasText(extension)) ? extension.toLowerCase(Locale.ENGLISH) : null; - } - - @Override - protected MediaType handleNoMatch(String key) throws NotAcceptableStatusException { - if (!this.useRegisteredExtensionsOnly) { - Optional mediaType = MediaTypeFactory.getMediaType("file." + key); - if (mediaType.isPresent()) { - return mediaType.get(); - } - } - if (this.ignoreUnknownExtensions) { - return null; - } - throw new NotAcceptableStatusException(getAllMediaTypes()); + return UriUtils.extractFileExtension(path); } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/RequestedContentTypeResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/accept/RequestedContentTypeResolver.java index b7ca986173..87e8ce2372 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/RequestedContentTypeResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/accept/RequestedContentTypeResolver.java @@ -38,6 +38,6 @@ public interface RequestedContentTypeResolver { * @return the requested media types or an empty list * @throws NotAcceptableStatusException if the requested media types is invalid */ - List resolveMediaTypes(ServerWebExchange exchange) throws NotAcceptableStatusException; + List resolveMediaTypes(ServerWebExchange exchange); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/RequestedContentTypeResolverBuilder.java b/spring-webflux/src/main/java/org/springframework/web/reactive/accept/RequestedContentTypeResolverBuilder.java index 264ce1fa29..09eff60a04 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/RequestedContentTypeResolverBuilder.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/accept/RequestedContentTypeResolverBuilder.java @@ -92,10 +92,6 @@ public class RequestedContentTypeResolverBuilder { private Map mediaTypes = new HashMap<>(); - private boolean ignoreUnknownPathExtensions = true; - - private Boolean useRegisteredExtensionsOnly; - private String parameterName = "format"; private RequestedContentTypeResolver contentTypeResolver; @@ -143,30 +139,6 @@ public class RequestedContentTypeResolverBuilder { return this; } - /** - * Whether to ignore requests with path extension that cannot be resolved - * to any media type. Setting this to {@code false} will result in an - * {@link org.springframework.web.HttpMediaTypeNotAcceptableException} if - * there is no match. - *

By default this is set to {@code true}. - */ - public RequestedContentTypeResolverBuilder ignoreUnknownPathExtensions(boolean ignore) { - this.ignoreUnknownPathExtensions = ignore; - return this; - } - - /** - * When {@link #favorPathExtension favorPathExtension} is set, this - * property determines whether to use only registered {@code MediaType} mappings - * to resolve a path extension to a specific MediaType. - *

By default this is not set in which case - * {@code PathExtensionContentNegotiationStrategy} will use defaults if available. - */ - public RequestedContentTypeResolverBuilder useRegisteredExtensionsOnly(boolean useRegisteredExtensionsOnly) { - this.useRegisteredExtensionsOnly = useRegisteredExtensionsOnly; - return this; - } - /** * Whether a request parameter ("format" by default) should be used to * determine the requested media type. For this option to work you must @@ -230,12 +202,7 @@ public class RequestedContentTypeResolverBuilder { List resolvers = new ArrayList<>(); if (this.favorPathExtension) { - PathExtensionContentTypeResolver resolver = new PathExtensionContentTypeResolver(this.mediaTypes); - resolver.setIgnoreUnknownExtensions(this.ignoreUnknownPathExtensions); - if (this.useRegisteredExtensionsOnly != null) { - resolver.setUseRegisteredExtensionsOnly(this.useRegisteredExtensionsOnly); - } - resolvers.add(resolver); + resolvers.add(new PathExtensionContentTypeResolver(this.mediaTypes)); } if (this.favorParameter) { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurationSupport.java b/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurationSupport.java index 6c48f69720..294d49c446 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurationSupport.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurationSupport.java @@ -64,7 +64,6 @@ import org.springframework.web.reactive.result.view.ViewResolver; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebExceptionHandler; import org.springframework.web.server.handler.ResponseStatusExceptionHandler; -import org.springframework.web.server.support.HttpRequestPathHelper; /** * The main class for Spring WebFlux configuration. diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfo.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfo.java index f345a0053d..ca31429113 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfo.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfo.java @@ -22,7 +22,6 @@ import org.springframework.lang.Nullable; import org.springframework.util.PathMatcher; import org.springframework.util.StringUtils; import org.springframework.web.bind.annotation.RequestMethod; -import org.springframework.web.reactive.accept.MappingContentTypeResolver; import org.springframework.web.reactive.accept.RequestedContentTypeResolver; import org.springframework.web.reactive.result.condition.ConsumesRequestCondition; import org.springframework.web.reactive.result.condition.HeadersRequestCondition; @@ -566,12 +565,6 @@ public final class RequestMappingInfo implements RequestCondition getFileExtensions() { - RequestedContentTypeResolver resolver = getContentTypeResolver(); - if (useRegisteredSuffixPatternMatch() && resolver != null) { - if (resolver instanceof MappingContentTypeResolver) { - return ((MappingContentTypeResolver) resolver).getKeys(); - } - } return null; } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/accept/CompositeContentTypeResolverBuilderTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/accept/CompositeContentTypeResolverBuilderTests.java index 4441c1b43f..9996fb3668 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/accept/CompositeContentTypeResolverBuilderTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/accept/CompositeContentTypeResolverBuilderTests.java @@ -23,7 +23,6 @@ import org.junit.Test; import org.springframework.http.MediaType; import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; import org.springframework.mock.http.server.reactive.test.MockServerWebExchange; -import org.springframework.web.server.NotAcceptableStatusException; import org.springframework.web.server.ServerWebExchange; import static org.junit.Assert.assertEquals; @@ -80,17 +79,6 @@ public class CompositeContentTypeResolverBuilderTests { assertEquals(Collections.singletonList(MediaType.IMAGE_GIF), resolver.resolveMediaTypes(exchange)); } - @Test(expected = NotAcceptableStatusException.class) // SPR-10170 - public void favorPathWithIgnoreUnknownPathExtensionTurnedOff() throws Exception { - RequestedContentTypeResolver resolver = new RequestedContentTypeResolverBuilder() - .favorPathExtension(true) - .ignoreUnknownPathExtensions(false) - .build(); - - ServerWebExchange exchange = MockServerHttpRequest.get("/flower.foobar?format=json").toExchange(); - resolver.resolveMediaTypes(exchange); - } - @Test public void favorParameter() throws Exception { RequestedContentTypeResolver resolver = new RequestedContentTypeResolverBuilder() @@ -103,16 +91,6 @@ public class CompositeContentTypeResolverBuilderTests { assertEquals(Collections.singletonList(MediaType.APPLICATION_JSON), resolver.resolveMediaTypes(exchange)); } - @Test(expected = NotAcceptableStatusException.class) // SPR-10170 - public void favorParameterWithUnknownMediaType() throws Exception { - RequestedContentTypeResolver resolver = new RequestedContentTypeResolverBuilder() - .favorParameter(true) - .build(); - - ServerWebExchange exchange = MockServerHttpRequest.get("/flower?format=xyz").toExchange(); - resolver.resolveMediaTypes(exchange); - } - @Test public void ignoreAcceptHeader() throws Exception { RequestedContentTypeResolver resolver = new RequestedContentTypeResolverBuilder() diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/accept/MappingContentTypeResolverTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/accept/MappingContentTypeResolverTests.java index 3b7684353b..241d30dc05 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/accept/MappingContentTypeResolverTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/accept/MappingContentTypeResolverTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,15 +18,16 @@ package org.springframework.web.reactive.accept; import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Set; import org.junit.Test; import org.springframework.http.MediaType; +import org.springframework.lang.Nullable; +import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; +import org.springframework.web.server.NotAcceptableStatusException; import org.springframework.web.server.ServerWebExchange; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; /** * Unit tests for {@link AbstractMappingContentTypeResolver}. @@ -34,65 +35,46 @@ import static org.junit.Assert.assertTrue; */ public class MappingContentTypeResolverTests { - @Test - public void resolveExtensions() { - Map mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON); - TestMappingContentTypeResolver resolver = new TestMappingContentTypeResolver("", mapping); - Set keys = resolver.getKeysFor(MediaType.APPLICATION_JSON); - - assertEquals(1, keys.size()); - assertEquals("json", keys.iterator().next()); - } - - @Test - public void resolveExtensionsNoMatch() { - Map mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON); - TestMappingContentTypeResolver resolver = new TestMappingContentTypeResolver("", mapping); - Set keys = resolver.getKeysFor(MediaType.TEXT_HTML); - - assertTrue(keys.isEmpty()); - } - @Test // SPR-13747 - public void lookupMediaTypeCaseInsensitive() { + public void resolveCaseInsensitive() { Map mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON); - TestMappingContentTypeResolver resolver = new TestMappingContentTypeResolver("", mapping); - MediaType mediaType = resolver.getMediaType("JSoN"); + TestMappingContentTypeResolver resolver = new TestMappingContentTypeResolver("JSoN", mapping); + List mediaTypes = resolver.resolve(); - assertEquals(mediaType, MediaType.APPLICATION_JSON); + assertEquals(Collections.singletonList(MediaType.APPLICATION_JSON), mediaTypes); } @Test public void resolveMediaTypes() throws Exception { Map mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON); TestMappingContentTypeResolver resolver = new TestMappingContentTypeResolver("json", mapping); - List mediaTypes = resolver.resolveMediaTypes((ServerWebExchange) null); + List mediaTypes = resolver.resolve(); assertEquals(1, mediaTypes.size()); assertEquals("application/json", mediaTypes.get(0).toString()); } @Test - public void resolveMediaTypesNoMatch() throws Exception { - TestMappingContentTypeResolver resolver = new TestMappingContentTypeResolver("blah", null); - List mediaTypes = resolver.resolveMediaTypes((ServerWebExchange) null); + public void resolveNoMatch() throws Exception { + TestMappingContentTypeResolver resolver = new TestMappingContentTypeResolver("blah", Collections.emptyMap()); + List mediaTypes = resolver.resolve(); assertEquals(0, mediaTypes.size()); } @Test - public void resolveMediaTypesNoKey() throws Exception { + public void resolveNoKey() throws Exception { Map mapping = Collections.singletonMap("json", MediaType.APPLICATION_JSON); TestMappingContentTypeResolver resolver = new TestMappingContentTypeResolver(null, mapping); - List mediaTypes = resolver.resolveMediaTypes((ServerWebExchange) null); + List mediaTypes = resolver.resolve(); assertEquals(0, mediaTypes.size()); } @Test public void resolveMediaTypesHandleNoMatch() throws Exception { - TestMappingContentTypeResolver resolver = new TestMappingContentTypeResolver("xml", null); - List mediaTypes = resolver.resolveMediaTypes((ServerWebExchange) null); + TestMappingContentTypeResolver resolver = new TestMappingContentTypeResolver("xml", Collections.emptyMap()); + List mediaTypes = resolver.resolve(); assertEquals(1, mediaTypes.size()); assertEquals("application/xml", mediaTypes.get(0).toString()); @@ -103,19 +85,18 @@ public class MappingContentTypeResolverTests { private final String key; - public TestMappingContentTypeResolver(String key, Map mapping) { + TestMappingContentTypeResolver(@Nullable String key, Map mapping) { super(mapping); this.key = key; } - @Override - protected String extractKey(ServerWebExchange exchange) { - return this.key; + public List resolve() throws NotAcceptableStatusException { + return super.resolveMediaTypes(MockServerHttpRequest.get("/").toExchange()); } @Override - protected MediaType handleNoMatch(String mappingKey) { - return "xml".equals(mappingKey) ? MediaType.APPLICATION_XML : null; + protected String getKey(ServerWebExchange exchange) { + return this.key; } } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/accept/PathExtensionContentTypeResolverTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/accept/PathExtensionContentTypeResolverTests.java index 0148839354..db2c5a474b 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/accept/PathExtensionContentTypeResolverTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/accept/PathExtensionContentTypeResolverTests.java @@ -23,7 +23,6 @@ import org.junit.Test; import org.springframework.http.MediaType; import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; -import org.springframework.web.server.NotAcceptableStatusException; import org.springframework.web.server.ServerWebExchange; import static org.junit.Assert.assertEquals; @@ -36,9 +35,9 @@ import static org.junit.Assert.assertEquals; public class PathExtensionContentTypeResolverTests { @Test - public void resolveMediaTypesFromMapping() throws Exception { + public void resolveFromRegistrations() throws Exception { ServerWebExchange exchange = MockServerHttpRequest.get("/test.html").toExchange(); - PathExtensionContentTypeResolver resolver = new PathExtensionContentTypeResolver(); + PathExtensionContentTypeResolver resolver = createResolver(); List mediaTypes = resolver.resolveMediaTypes(exchange); assertEquals(Collections.singletonList(new MediaType("text", "html")), mediaTypes); @@ -51,42 +50,34 @@ public class PathExtensionContentTypeResolverTests { } @Test - public void resolveMediaTypesFromJaf() throws Exception { + public void resolveFromMediaTypeFactory() throws Exception { ServerWebExchange exchange = MockServerHttpRequest.get("test.xls").toExchange(); - PathExtensionContentTypeResolver resolver = new PathExtensionContentTypeResolver(); + PathExtensionContentTypeResolver resolver = createResolver(); List mediaTypes = resolver.resolveMediaTypes(exchange); assertEquals(Collections.singletonList(new MediaType("application", "vnd.ms-excel")), mediaTypes); } - // SPR-9390 - - @Test - public void getMediaTypeFilenameWithEncodedURI() throws Exception { + @Test // SPR-9390 + public void resolveFromFilenameWithEncodedURI() throws Exception { ServerWebExchange exchange = MockServerHttpRequest.get("/quo%20vadis%3f.html").toExchange(); - PathExtensionContentTypeResolver resolver = new PathExtensionContentTypeResolver(); + PathExtensionContentTypeResolver resolver = createResolver(); List result = resolver.resolveMediaTypes(exchange); assertEquals("Invalid content type", Collections.singletonList(new MediaType("text", "html")), result); } - // SPR-10170 - - @Test - public void resolveMediaTypesIgnoreUnknownExtension() throws Exception { + @Test // SPR-10170 + public void resolveAndIgnoreUnknownExtension() throws Exception { ServerWebExchange exchange = MockServerHttpRequest.get("test.foobar").toExchange(); - PathExtensionContentTypeResolver resolver = new PathExtensionContentTypeResolver(); + PathExtensionContentTypeResolver resolver = createResolver(); List mediaTypes = resolver.resolveMediaTypes(exchange); assertEquals(Collections.emptyList(), mediaTypes); } - @Test(expected = NotAcceptableStatusException.class) - public void resolveMediaTypesDoNotIgnoreUnknownExtension() throws Exception { - ServerWebExchange exchange = MockServerHttpRequest.get("test.foobar").toExchange(); - PathExtensionContentTypeResolver resolver = new PathExtensionContentTypeResolver(); - resolver.setIgnoreUnknownExtensions(false); - resolver.resolveMediaTypes(exchange); + private PathExtensionContentTypeResolver createResolver() { + return new PathExtensionContentTypeResolver(Collections.emptyMap()); } } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMappingTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMappingTests.java index b925a6e0ca..946519dcaa 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMappingTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMappingTests.java @@ -22,8 +22,6 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.lang.reflect.Method; import java.util.ArrayList; -import java.util.Collections; -import java.util.HashSet; import java.util.Set; import org.junit.Before; @@ -40,11 +38,13 @@ import org.springframework.web.bind.annotation.PutMapping; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.context.support.StaticWebApplicationContext; -import org.springframework.web.reactive.accept.MappingContentTypeResolver; import org.springframework.web.reactive.result.method.RequestMappingInfo; -import static org.junit.Assert.*; -import static org.mockito.Mockito.*; +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; /** * Unit tests for {@link RequestMappingHandlerMapping}. @@ -63,47 +63,6 @@ public class RequestMappingHandlerMappingTests { this.handlerMapping.setApplicationContext(wac); } - - @Test - public void useRegisteredSuffixPatternMatch() { - assertTrue(this.handlerMapping.useSuffixPatternMatch()); - assertTrue(this.handlerMapping.useRegisteredSuffixPatternMatch()); - - MappingContentTypeResolver contentTypeResolver = mock(MappingContentTypeResolver.class); - when(contentTypeResolver.getKeys()).thenReturn(Collections.singleton("json")); - - this.handlerMapping.setContentTypeResolver(contentTypeResolver); - this.handlerMapping.afterPropertiesSet(); - - assertTrue(this.handlerMapping.useSuffixPatternMatch()); - assertTrue(this.handlerMapping.useRegisteredSuffixPatternMatch()); - assertEquals(Collections.singleton("json"), this.handlerMapping.getFileExtensions()); - } - - @Test - public void useRegisteredSuffixPatternMatchInitialization() { - MappingContentTypeResolver contentTypeResolver = mock(MappingContentTypeResolver.class); - when(contentTypeResolver.getKeys()).thenReturn(Collections.singleton("json")); - - final Set actualExtensions = new HashSet<>(); - RequestMappingHandlerMapping localHandlerMapping = new RequestMappingHandlerMapping() { - @Override - protected RequestMappingInfo getMappingForMethod(Method method, Class handlerType) { - actualExtensions.addAll(getFileExtensions()); - return super.getMappingForMethod(method, handlerType); - } - }; - this.wac.registerSingleton("testController", ComposedAnnotationController.class); - this.wac.refresh(); - - localHandlerMapping.setContentTypeResolver(contentTypeResolver); - localHandlerMapping.setUseRegisteredSuffixPatternMatch(true); - localHandlerMapping.setApplicationContext(this.wac); - localHandlerMapping.afterPropertiesSet(); - - assertEquals(Collections.singleton("json"), actualExtensions); - } - @Test public void useSuffixPatternMatch() { assertTrue(this.handlerMapping.useSuffixPatternMatch());