Refine RequestedContentTypeResolver contract

Consistently return "*/*" if no media types were requested rather than
an empty list. Existing code has to check for both in any case to see
if nothing was requested.

Issue: SPR-16624
This commit is contained in:
Rossen Stoyanchev 2018-03-27 16:53:16 -04:00
parent f3994467c4
commit 224d52e032
8 changed files with 39 additions and 31 deletions

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2017 the original author or authors. * Copyright 2002-2018 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -23,6 +23,7 @@ import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory; import org.apache.commons.logging.LogFactory;
import org.springframework.http.MediaType; import org.springframework.http.MediaType;
import org.springframework.util.Assert;
import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.ServerWebExchange;
/** /**
@ -38,7 +39,7 @@ public class FixedContentTypeResolver implements RequestedContentTypeResolver {
private static final Log logger = LogFactory.getLog(FixedContentTypeResolver.class); private static final Log logger = LogFactory.getLog(FixedContentTypeResolver.class);
private final List<MediaType> mediaTypes; private final List<MediaType> contentTypes;
/** /**
@ -54,8 +55,9 @@ public class FixedContentTypeResolver implements RequestedContentTypeResolver {
* <p>Consider appending {@link MediaType#ALL} at the end if destinations * <p>Consider appending {@link MediaType#ALL} at the end if destinations
* are present which do not support any of the other default media types. * are present which do not support any of the other default media types.
*/ */
public FixedContentTypeResolver(List<MediaType> mediaTypes) { public FixedContentTypeResolver(List<MediaType> contentTypes) {
this.mediaTypes = Collections.unmodifiableList(mediaTypes); Assert.notNull(contentTypes, "'contentTypes' must not be null");
this.contentTypes = Collections.unmodifiableList(contentTypes);
} }
@ -63,16 +65,16 @@ public class FixedContentTypeResolver implements RequestedContentTypeResolver {
* Return the configured list of media types. * Return the configured list of media types.
*/ */
public List<MediaType> getContentTypes() { public List<MediaType> getContentTypes() {
return this.mediaTypes; return this.contentTypes;
} }
@Override @Override
public List<MediaType> resolveMediaTypes(ServerWebExchange exchange) { public List<MediaType> resolveMediaTypes(ServerWebExchange exchange) {
if (logger.isDebugEnabled()) { if (logger.isDebugEnabled()) {
logger.debug("Requested media types: " + this.mediaTypes); logger.debug("Requested media types: " + this.contentTypes);
} }
return this.mediaTypes; return this.contentTypes;
} }
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2016 the original author or authors. * Copyright 2002-2018 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -19,6 +19,7 @@ import java.util.List;
import org.springframework.http.InvalidMediaTypeException; import org.springframework.http.InvalidMediaTypeException;
import org.springframework.http.MediaType; import org.springframework.http.MediaType;
import org.springframework.util.CollectionUtils;
import org.springframework.web.server.NotAcceptableStatusException; import org.springframework.web.server.NotAcceptableStatusException;
import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.ServerWebExchange;
@ -35,7 +36,7 @@ public class HeaderContentTypeResolver implements RequestedContentTypeResolver {
try { try {
List<MediaType> mediaTypes = exchange.getRequest().getHeaders().getAccept(); List<MediaType> mediaTypes = exchange.getRequest().getHeaders().getAccept();
MediaType.sortBySpecificityAndQuality(mediaTypes); MediaType.sortBySpecificityAndQuality(mediaTypes);
return mediaTypes; return !CollectionUtils.isEmpty(mediaTypes) ? mediaTypes : MEDIA_TYPE_ALL_LIST;
} }
catch (InvalidMediaTypeException ex) { catch (InvalidMediaTypeException ex) {
String value = exchange.getRequest().getHeaders().getFirst("Accept"); String value = exchange.getRequest().getHeaders().getFirst("Accept");

View File

@ -72,7 +72,7 @@ public class ParameterContentTypeResolver implements RequestedContentTypeResolve
public List<MediaType> resolveMediaTypes(ServerWebExchange exchange) throws NotAcceptableStatusException { public List<MediaType> resolveMediaTypes(ServerWebExchange exchange) throws NotAcceptableStatusException {
String key = exchange.getRequest().getQueryParams().getFirst(getParameterName()); String key = exchange.getRequest().getQueryParams().getFirst(getParameterName());
if (!StringUtils.hasText(key)) { if (!StringUtils.hasText(key)) {
return Collections.emptyList(); return MEDIA_TYPE_ALL_LIST;
} }
key = formatKey(key); key = formatKey(key);
MediaType match = this.mediaTypes.get(key); MediaType match = this.mediaTypes.get(key);

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2017 the original author or authors. * Copyright 2002-2018 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -16,6 +16,7 @@
package org.springframework.web.reactive.accept; package org.springframework.web.reactive.accept;
import java.util.Collections;
import java.util.List; import java.util.List;
import org.springframework.http.MediaType; import org.springframework.http.MediaType;
@ -33,11 +34,20 @@ import org.springframework.web.server.ServerWebExchange;
*/ */
public interface RequestedContentTypeResolver { public interface RequestedContentTypeResolver {
/**
* A singleton list with {@link MediaType#ALL} that is returned from
* {@link #resolveMediaTypes} when no specific media types are requested.
* @since 5.0.5
*/
List<MediaType> MEDIA_TYPE_ALL_LIST = Collections.singletonList(MediaType.ALL);
/** /**
* Resolve the given request to a list of requested media types. The returned * Resolve the given request to a list of requested media types. The returned
* list is ordered by specificity first and by quality parameter second. * list is ordered by specificity first and by quality parameter second.
* @param exchange the current exchange * @param exchange the current exchange
* @return the requested media types or an empty list * @return the requested media types, or {@link #MEDIA_TYPE_ALL_LIST} if none
* were requested.
* @throws NotAcceptableStatusException if the requested media type is invalid * @throws NotAcceptableStatusException if the requested media type is invalid
*/ */
List<MediaType> resolveMediaTypes(ServerWebExchange exchange); List<MediaType> resolveMediaTypes(ServerWebExchange exchange);

View File

@ -95,13 +95,13 @@ public class RequestedContentTypeResolverBuilder {
return exchange -> { return exchange -> {
for (RequestedContentTypeResolver resolver : resolvers) { for (RequestedContentTypeResolver resolver : resolvers) {
List<MediaType> type = resolver.resolveMediaTypes(exchange); List<MediaType> mediaTypes = resolver.resolveMediaTypes(exchange);
if (type.isEmpty() || (type.size() == 1 && type.contains(MediaType.ALL))) { if (mediaTypes.equals(RequestedContentTypeResolver.MEDIA_TYPE_ALL_LIST)) {
continue; continue;
} }
return type; return mediaTypes;
} }
return Collections.emptyList(); return RequestedContentTypeResolver.MEDIA_TYPE_ALL_LIST;
}; };
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2017 the original author or authors. * Copyright 2002-2018 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -17,7 +17,6 @@
package org.springframework.web.reactive.result; package org.springframework.web.reactive.result;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator; import java.util.Comparator;
import java.util.LinkedHashSet; import java.util.LinkedHashSet;
import java.util.List; import java.util.List;
@ -149,8 +148,7 @@ public abstract class HandlerResultHandlerSupport implements Ordered {
} }
private List<MediaType> getAcceptableTypes(ServerWebExchange exchange) { private List<MediaType> getAcceptableTypes(ServerWebExchange exchange) {
List<MediaType> mediaTypes = getContentTypeResolver().resolveMediaTypes(exchange); return getContentTypeResolver().resolveMediaTypes(exchange);
return (mediaTypes.isEmpty() ? Collections.singletonList(MediaType.ALL) : mediaTypes);
} }
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")

View File

@ -238,11 +238,8 @@ public final class ProducesRequestCondition extends AbstractRequestCondition<Pro
} }
} }
private List<MediaType> getAcceptedMediaTypes(ServerWebExchange exchange) private List<MediaType> getAcceptedMediaTypes(ServerWebExchange exchange) throws NotAcceptableStatusException {
throws NotAcceptableStatusException { return this.contentTypeResolver.resolveMediaTypes(exchange);
List<MediaType> mediaTypes = this.contentTypeResolver.resolveMediaTypes(exchange);
return mediaTypes.isEmpty() ? Collections.singletonList(MediaType.ALL) : mediaTypes;
} }
private int indexOfEqualMediaType(MediaType mediaType) { private int indexOfEqualMediaType(MediaType mediaType) {

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2017 the original author or authors. * Copyright 2002-2018 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -36,16 +36,16 @@ import static org.junit.Assert.assertEquals;
public class ParameterContentTypeResolverTests { public class ParameterContentTypeResolverTests {
@Test @Test
public void noKey() throws Exception { public void noKey() {
ParameterContentTypeResolver resolver = new ParameterContentTypeResolver(Collections.emptyMap()); ParameterContentTypeResolver resolver = new ParameterContentTypeResolver(Collections.emptyMap());
ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/")); ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/"));
List<MediaType> mediaTypes = resolver.resolveMediaTypes(exchange); List<MediaType> mediaTypes = resolver.resolveMediaTypes(exchange);
assertEquals(0, mediaTypes.size()); assertEquals(RequestedContentTypeResolver.MEDIA_TYPE_ALL_LIST, mediaTypes);
} }
@Test(expected = NotAcceptableStatusException.class) @Test(expected = NotAcceptableStatusException.class)
public void noMatchForKey() throws Exception { public void noMatchForKey() {
ParameterContentTypeResolver resolver = new ParameterContentTypeResolver(Collections.emptyMap()); ParameterContentTypeResolver resolver = new ParameterContentTypeResolver(Collections.emptyMap());
List<MediaType> mediaTypes = resolver.resolveMediaTypes(createExchange("blah")); List<MediaType> mediaTypes = resolver.resolveMediaTypes(createExchange("blah"));
@ -53,7 +53,7 @@ public class ParameterContentTypeResolverTests {
} }
@Test @Test
public void resolveKeyFromRegistrations() throws Exception { public void resolveKeyFromRegistrations() {
ServerWebExchange exchange = createExchange("html"); ServerWebExchange exchange = createExchange("html");
Map<String, MediaType> mapping = Collections.emptyMap(); Map<String, MediaType> mapping = Collections.emptyMap();
@ -68,7 +68,7 @@ public class ParameterContentTypeResolverTests {
} }
@Test @Test
public void resolveKeyThroughMediaTypeFactory() throws Exception { public void resolveKeyThroughMediaTypeFactory() {
ServerWebExchange exchange = createExchange("xls"); ServerWebExchange exchange = createExchange("xls");
RequestedContentTypeResolver resolver = new ParameterContentTypeResolver(Collections.emptyMap()); RequestedContentTypeResolver resolver = new ParameterContentTypeResolver(Collections.emptyMap());
List<MediaType> mediaTypes = resolver.resolveMediaTypes(exchange); List<MediaType> mediaTypes = resolver.resolveMediaTypes(exchange);