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

Author: rstoyanchev

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,148 +15,68 @@
  */
 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<String, MediaType> mediaTypeLookup = new ConcurrentHashMap<>(64);
 
-	/** Reverse lookup for keys associated with a media type */
-	private final MultiValueMap<MediaType, String> keyLookup = new LinkedMultiValueMap<>(64);
 
-
-	/**
-	 * Create an instance with the given map of file extensions and media types.
-	 */
-	public AbstractMappingContentTypeResolver(@Nullable Map<String, MediaType> mediaTypes) {
-		if (mediaTypes != null) {
-			for (Map.Entry<String, MediaType> 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 Map<String, MediaType> 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<MediaType> getAllMediaTypes() {
-		return new ArrayList<>(this.mediaTypeLookup.values());
+	public AbstractMappingContentTypeResolver(Map<String, MediaType> mediaTypes) {
+		mediaTypes.forEach((key, mediaType) ->
+				this.mediaTypeLookup.put(key.toLowerCase(Locale.ENGLISH), mediaType));
 	}
 
 
-	// RequestedContentTypeResolver implementation
-
 	@Override
-	public List<MediaType> 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<MediaType> resolveMediaTypes(@Nullable String key) throws NotAcceptableStatusException {
+	public List<MediaType> 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);
 			}
 		}
 		return Collections.emptyList();
 	}
 
 	/**
-	 * 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);
-
-	/**
-	 * Override to provide handling when a key is successfully resolved via
-	 * {@link #getMediaType(String)}.
-	 */
-	@SuppressWarnings("UnusedParameters")
-	protected void handleMatch(String key, MediaType mediaType) {
-	}
+	protected abstract String getKey(ServerWebExchange exchange);
 
 	/**
-	 * 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.
+	 * Get the MediaType for the given key.
 	 */
-	@SuppressWarnings("UnusedParameters")
 	@Nullable
-	protected MediaType handleNoMatch(String key) throws NotAcceptableStatusException {
-		return null;
-	}
-
-	// MappingContentTypeResolver implementation
-
-	@Override
-	public Set<String> getKeysFor(MediaType mediaType) {
-		List<String> keys = this.keyLookup.get(mediaType);
-		return (keys != null ? new HashSet<>(keys) : Collections.emptySet());
-	}
-
-	@Override
-	public Set<String> 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;
 	}
 
 }

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 @@
 
 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.
- *
- * <p>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<RequestedContentTypeResolver> resolvers = new ArrayList<>();
 
@@ -49,32 +40,8 @@ public CompositeContentTypeResolver(List<RequestedContentTypeResolver> resolvers
 	}
 
 
-	/**
-	 * Return a read-only list of the configured resolvers.
-	 */
-	public List<RequestedContentTypeResolver> 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 extends RequestedContentTypeResolver> T findResolver(Class<T> resolverType) {
-		for (RequestedContentTypeResolver resolver : this.resolvers) {
-			if (resolverType.isInstance(resolver)) {
-				return (T) resolver;
-			}
-		}
-		return null;
-	}
-
-
 	@Override
-	public List<MediaType> resolveMediaTypes(ServerWebExchange exchange) throws NotAcceptableStatusException {
+	public List<MediaType> resolveMediaTypes(ServerWebExchange exchange) {
 		for (RequestedContentTypeResolver resolver : this.resolvers) {
 			List<MediaType> mediaTypes = resolver.resolveMediaTypes(exchange);
 			if (mediaTypes.isEmpty() || (mediaTypes.size() == 1 && mediaTypes.contains(MediaType.ALL))) {
@@ -85,24 +52,4 @@ public List<MediaType> resolveMediaTypes(ServerWebExchange exchange) throws NotA
 		return Collections.emptyList();
 	}
 
-	@Override
-	public Set<String> getKeysFor(MediaType mediaType) {
-		Set<String> result = new LinkedHashSet<>();
-		for (RequestedContentTypeResolver resolver : this.resolvers) {
-			if (resolver instanceof MappingContentTypeResolver)
-			result.addAll(((MappingContentTypeResolver) resolver).getKeysFor(mediaType));
-		}
-		return result;
-	}
-
-	@Override
-	public Set<String> getKeys() {
-		Set<String> result = new LinkedHashSet<>();
-		for (RequestedContentTypeResolver resolver : this.resolvers) {
-			if (resolver instanceof MappingContentTypeResolver)
-				result.addAll(((MappingContentTypeResolver) resolver).getKeys());
-		}
-		return result;
-	}
-
 }

spring-webflux/src/main/java/org/springframework/web/reactive/accept/MappingContentTypeResolver.java

@@ -18,31 +18,21 @@
 
 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<String, MediaType> mediaTypes) {
 		super(mediaTypes);
 	}
@@ -53,7 +43,7 @@ public ParameterContentTypeResolver(Map<String, MediaType> mediaTypes) {
 	 * <p>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 String getParameterName() {
 
 
 	@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());
-	}
-
 }