From befacf4a3577a4e7a023ed348a8e9ed142c30a48 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 11 Jul 2017 17:50:34 +0200 Subject: [PATCH] ParameterContentNegotiationStrategy uses MediaTypeFactory Issue: SPR-15649 --- ...ractMappingContentNegotiationStrategy.java | 54 ++++++++++++++++++- .../web/accept/ContentNegotiationManager.java | 2 +- .../ContentNegotiationManagerFactoryBean.java | 16 ++++-- .../ParameterContentNegotiationStrategy.java | 32 +++-------- ...thExtensionContentNegotiationStrategy.java | 49 +---------------- ...entNegotiationManagerFactoryBeanTests.java | 2 +- .../RequestedContentTypeResolverBuilder.java | 30 ++++++----- 7 files changed, 92 insertions(+), 93 deletions(-) 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 cbf20e91537..1ebfe6bae02 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 @@ -19,8 +19,13 @@ package org.springframework.web.accept; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Optional; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.http.MediaType; +import org.springframework.http.MediaTypeFactory; import org.springframework.lang.Nullable; import org.springframework.util.StringUtils; import org.springframework.web.HttpMediaTypeNotAcceptableException; @@ -47,6 +52,13 @@ import org.springframework.web.context.request.NativeWebRequest; public abstract class AbstractMappingContentNegotiationStrategy extends MappingMediaTypeFileExtensionResolver implements ContentNegotiationStrategy { + protected final Log logger = LogFactory.getLog(getClass()); + + private boolean useRegisteredExtensionsOnly = false; + + private boolean ignoreUnknownExtensions = false; + + /** * Create an instance with the given map of file extensions and media types. */ @@ -55,6 +67,34 @@ public abstract class AbstractMappingContentNegotiationStrategy extends MappingM } + /** + * Whether to only use the registered mappings to look up file extensions, + * or also to use dynamic resolution (e.g. via {@link MediaTypeFactory}. + *

By default this is set to {@code false}. + */ + public void setUseRegisteredExtensionsOnly(boolean useRegisteredExtensionsOnly) { + this.useRegisteredExtensionsOnly = useRegisteredExtensionsOnly; + } + + public boolean isUseRegisteredExtensionsOnly() { + return this.useRegisteredExtensionsOnly; + } + + /** + * Whether to ignore requests with unknown file extension. Setting this to + * {@code false} results in {@code HttpMediaTypeNotAcceptableException}. + *

By default this is set to {@literal false} but is overridden in + * {@link PathExtensionContentNegotiationStrategy} to {@literal true}. + */ + public void setIgnoreUnknownExtensions(boolean ignoreUnknownExtensions) { + this.ignoreUnknownExtensions = ignoreUnknownExtensions; + } + + public boolean isIgnoreUnknownExtensions() { + return this.ignoreUnknownExtensions; + } + + @Override public List resolveMediaTypes(NativeWebRequest webRequest) throws HttpMediaTypeNotAcceptableException { @@ -98,6 +138,9 @@ public abstract class AbstractMappingContentNegotiationStrategy extends MappingM * {@link #lookupMediaType}. */ protected void handleMatch(String key, MediaType mediaType) { + if (logger.isTraceEnabled()) { + logger.trace("Requested MediaType='" + mediaType + "' based on key='" + key + "'."); + } } /** @@ -110,7 +153,16 @@ public abstract class AbstractMappingContentNegotiationStrategy extends MappingM protected MediaType handleNoMatch(NativeWebRequest request, String key) throws HttpMediaTypeNotAcceptableException { - return null; + if (!isUseRegisteredExtensionsOnly()) { + Optional mediaType = MediaTypeFactory.getMediaType("file." + key); + if (mediaType.isPresent()) { + return mediaType.get(); + } + } + if (isIgnoreUnknownExtensions()) { + return null; + } + throw new HttpMediaTypeNotAcceptableException(getAllMediaTypes()); } } 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 92621006955..7b727401dda 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 @@ -44,7 +44,7 @@ import org.springframework.web.context.request.NativeWebRequest; */ public class ContentNegotiationManager implements ContentNegotiationStrategy, MediaTypeFileExtensionResolver { - private static final List MEDIA_TYPE_ALL = Collections.singletonList(MediaType.ALL); + private static final List MEDIA_TYPE_ALL = Collections.singletonList(MediaType.ALL); private final List strategies = new ArrayList<>(); diff --git a/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBean.java b/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBean.java index 7a1a25978e5..6e4faa3dc38 100644 --- a/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBean.java +++ b/spring-web/src/main/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBean.java @@ -191,11 +191,11 @@ public class ContentNegotiationManagerFactoryBean } /** - * When {@link #setFavorPathExtension 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. + * When {@link #setFavorPathExtension favorPathExtension} or + * {@link #setFavorParameter(boolean)} is set, this property determines + * whether to use only registered {@code MediaType} mappings or to allow + * dynamic resolution, e.g. via {@link MediaTypeFactory}. + *

By default this is not set in which case dynamic resolution is on. */ public void setUseRegisteredExtensionsOnly(boolean useRegisteredExtensionsOnly) { this.useRegisteredExtensionsOnly = useRegisteredExtensionsOnly; @@ -300,6 +300,12 @@ public class ContentNegotiationManagerFactoryBean ParameterContentNegotiationStrategy strategy = new ParameterContentNegotiationStrategy(this.mediaTypes); strategy.setParameterName(this.parameterName); + if (this.useRegisteredExtensionsOnly != null) { + strategy.setUseRegisteredExtensionsOnly(this.useRegisteredExtensionsOnly); + } + else { + strategy.setUseRegisteredExtensionsOnly(true); // backwards compatibility + } strategies.add(strategy); } 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 f6a9cb4eaa1..5f4c8c2acdf 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-201 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,25 +18,24 @@ package org.springframework.web.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.HttpMediaTypeNotAcceptableException; import org.springframework.web.context.request.NativeWebRequest; /** - * A {@code ContentNegotiationStrategy} that resolves a query parameter to a key - * to be used to look up a media type. The default parameter name is {@code format}. + * Strategy that resolves the requested content type from a query parameter. + * The default query parameter name is {@literal "format"}. + * + *

You can register static mappings between keys (i.e. the expected value of + * the query parameter) and MediaType's via {@link #addMapping(String, MediaType)}. + * As of 5.0 this strategy also supports dynamic lookups of keys via + * {@link org.springframework.http.MediaTypeFactory#getMediaType}. * * @author Rossen Stoyanchev * @since 3.2 */ public class ParameterContentNegotiationStrategy extends AbstractMappingContentNegotiationStrategy { - private static final Log logger = LogFactory.getLog(ParameterContentNegotiationStrategy.class); - private String parameterName = "format"; @@ -67,19 +66,4 @@ public class ParameterContentNegotiationStrategy extends AbstractMappingContentN return request.getParameter(getParameterName()); } - @Override - protected void handleMatch(String mediaTypeKey, MediaType mediaType) { - if (logger.isDebugEnabled()) { - logger.debug("Requested media type: '" + mediaType + "' based on '" + - getParameterName() + "'='" + mediaTypeKey + "'"); - } - } - - @Override - protected MediaType handleNoMatch(NativeWebRequest request, String key) - throws HttpMediaTypeNotAcceptableException { - - throw new HttpMediaTypeNotAcceptableException(getAllMediaTypes()); - } - } 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 b5d97f71ac0..2c627898566 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 @@ -18,20 +18,14 @@ package org.springframework.web.accept; import java.util.Locale; import java.util.Map; -import java.util.Optional; - import javax.servlet.http.HttpServletRequest; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; - import org.springframework.core.io.Resource; import org.springframework.http.MediaType; import org.springframework.http.MediaTypeFactory; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.StringUtils; -import org.springframework.web.HttpMediaTypeNotAcceptableException; import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.util.UriUtils; import org.springframework.web.util.UrlPathHelper; @@ -49,14 +43,8 @@ import org.springframework.web.util.UrlPathHelper; */ public class PathExtensionContentNegotiationStrategy extends AbstractMappingContentNegotiationStrategy { - private static final Log logger = LogFactory.getLog(PathExtensionContentNegotiationStrategy.class); - private UrlPathHelper urlPathHelper = new UrlPathHelper(); - private boolean useRegisteredExtensionsOnly = false; - - private boolean ignoreUnknownExtensions = true; - /** * Create an instance without any mappings to start with. Mappings may be added @@ -71,6 +59,8 @@ public class PathExtensionContentNegotiationStrategy extends AbstractMappingCont */ public PathExtensionContentNegotiationStrategy(@Nullable Map mediaTypes) { super(mediaTypes); + setUseRegisteredExtensionsOnly(false); + setIgnoreUnknownExtensions(true); this.urlPathHelper.setUrlDecode(false); } @@ -92,25 +82,6 @@ public class PathExtensionContentNegotiationStrategy extends AbstractMappingCont setUseRegisteredExtensionsOnly(!useJaf); } - /** - * 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 getMediaTypeKey(NativeWebRequest webRequest) { HttpServletRequest request = webRequest.getNativeRequest(HttpServletRequest.class); @@ -123,22 +94,6 @@ public class PathExtensionContentNegotiationStrategy extends AbstractMappingCont return (StringUtils.hasText(extension) ? extension.toLowerCase(Locale.ENGLISH) : null); } - @Override - protected MediaType handleNoMatch(NativeWebRequest webRequest, String extension) - throws HttpMediaTypeNotAcceptableException { - - if (!this.useRegisteredExtensionsOnly) { - Optional mediaType = MediaTypeFactory.getMediaType("file." + extension); - if (mediaType.isPresent()) { - return mediaType.get(); - } - } - if (this.ignoreUnknownExtensions) { - return null; - } - throw new HttpMediaTypeNotAcceptableException(getAllMediaTypes()); - } - /** * A public method exposing the knowledge of the path extension strategy to * resolve file extensions to a {@link MediaType} in this case for a given diff --git a/spring-web/src/test/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBeanTests.java b/spring-web/src/test/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBeanTests.java index 10a83ffab83..e4bdd553237 100644 --- a/spring-web/src/test/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBeanTests.java +++ b/spring-web/src/test/java/org/springframework/web/accept/ContentNegotiationManagerFactoryBeanTests.java @@ -146,7 +146,7 @@ public class ContentNegotiationManagerFactoryBeanTests { ContentNegotiationManager manager = this.factoryBean.getObject(); this.servletRequest.setRequestURI("/flower"); - this.servletRequest.setParameter("format", "xyz"); + this.servletRequest.setParameter("format", "invalid"); manager.resolveMediaTypes(this.webRequest); } 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 3d40e84de7c..8b0d7cd584c 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 @@ -30,16 +30,15 @@ import org.springframework.lang.Nullable; /** * Builder for a composite {@link RequestedContentTypeResolver} that delegates - * to one or more other resolvers each implementing a different strategy to - * determine the requested content type(s), e.g. from the Accept header, - * through a query parameter, or other custom strategy. + * to other resolvers each implementing a different strategy to determine the + * requested content type -- e.g. Accept header, query parameter, or other. * - *

Use methods of this builder to add resolvers in the desired order. - * The result of the first resolver to return a non-empty list of media types - * is used. + *

Use builder methods to add resolvers in the desired order. For a given + * request he first resolver to return a list that is not empty and does not + * consist of just {@link MediaType#ALL}, will be used. * - *

If no resolvers are configured, by default the builder will configure - * {@link HeaderContentTypeResolver} only. + *

By default, if no resolvers are explicitly configured, the builder will + * add {@link HeaderContentTypeResolver}. * * @author Rossen Stoyanchev * @since 5.0 @@ -50,8 +49,8 @@ public class RequestedContentTypeResolverBuilder { /** - * Add resolver extracting the requested content type from a query parameter. - * By default the expected query parameter name is {@code "format"}. + * Add a resolver to get the requested content type from a query parameter. + * By default the query parameter name is {@code "format"}. */ public ParameterResolverConfigurer parameterResolver() { ParameterResolverConfigurer parameterBuilder = new ParameterResolverConfigurer(); @@ -60,7 +59,7 @@ public class RequestedContentTypeResolverBuilder { } /** - * Add resolver extracting the requested content type from the + * Add resolver to get the requested content type from the * {@literal "Accept"} header. */ public void headerResolver() { @@ -68,7 +67,7 @@ public class RequestedContentTypeResolverBuilder { } /** - * Add resolver that always returns a fixed set of media types. + * Add resolver that returns a fixed set of media types. * @param mediaTypes the media types to use */ public void fixedResolver(MediaType... mediaTypes) { @@ -108,7 +107,7 @@ public class RequestedContentTypeResolverBuilder { /** - * Helps to create a {@link ParameterContentTypeResolver}. + * Helper to create and configure {@link ParameterContentTypeResolver}. */ public static class ParameterResolverConfigurer { @@ -146,7 +145,10 @@ public class RequestedContentTypeResolverBuilder { return this; } - RequestedContentTypeResolver createResolver() { + /** + * Private factory method to create the resolver. + */ + private RequestedContentTypeResolver createResolver() { ParameterContentTypeResolver resolver = new ParameterContentTypeResolver(this.mediaTypes); if (this.parameterName != null) { resolver.setParameterName(this.parameterName);