ParameterContentNegotiationStrategy uses MediaTypeFactory

Issue: SPR-15649

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}.
+	 * <p>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}.
+	 * <p>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<MediaType> 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> mediaType = MediaTypeFactory.getMediaType("file." + key);
+			if (mediaType.isPresent()) {
+				return mediaType.get();
+			}
+		}
+		if (isIgnoreUnknownExtensions()) {
+			return null;
+		}
+		throw new HttpMediaTypeNotAcceptableException(getAllMediaTypes());
 	}
 
 }

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<MediaType> MEDIA_TYPE_ALL = Collections.<MediaType>singletonList(MediaType.ALL);
+	private static final List<MediaType> MEDIA_TYPE_ALL = Collections.singletonList(MediaType.ALL);
 
 
 	private final List<ContentNegotiationStrategy> strategies = new ArrayList<>();

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.
-	 * <p>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}.
+	 * <p>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);
 		}
 

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"}.
+ *
+ * <p>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());
-	}
-
 }

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<String, MediaType> 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.
-	 * <p>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}.
-	 * <p>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> 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

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);
 	}

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.
  *
- * <p>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.
+ * <p>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.
  *
- * <p>If no resolvers are configured, by default the builder will configure
- * {@link HeaderContentTypeResolver} only.
+ * <p>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);