From 224d52e03286e868c5361bb205f641918a7a7e33 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 27 Mar 2018 16:53:16 -0400 Subject: [PATCH] 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 --- .../accept/FixedContentTypeResolver.java | 16 +++++++++------- .../accept/HeaderContentTypeResolver.java | 5 +++-- .../accept/ParameterContentTypeResolver.java | 2 +- .../accept/RequestedContentTypeResolver.java | 14 ++++++++++++-- .../RequestedContentTypeResolverBuilder.java | 8 ++++---- .../result/HandlerResultHandlerSupport.java | 6 ++---- .../condition/ProducesRequestCondition.java | 7 ++----- .../ParameterContentTypeResolverTests.java | 12 ++++++------ 8 files changed, 39 insertions(+), 31 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/FixedContentTypeResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/accept/FixedContentTypeResolver.java index e834c14288f..95019daf0fa 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/FixedContentTypeResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/accept/FixedContentTypeResolver.java @@ -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"); * 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.springframework.http.MediaType; +import org.springframework.util.Assert; 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 final List mediaTypes; + private final List contentTypes; /** @@ -54,8 +55,9 @@ public class FixedContentTypeResolver implements RequestedContentTypeResolver { *

Consider appending {@link MediaType#ALL} at the end if destinations * are present which do not support any of the other default media types. */ - public FixedContentTypeResolver(List mediaTypes) { - this.mediaTypes = Collections.unmodifiableList(mediaTypes); + public FixedContentTypeResolver(List contentTypes) { + 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. */ public List getContentTypes() { - return this.mediaTypes; + return this.contentTypes; } @Override public List resolveMediaTypes(ServerWebExchange exchange) { if (logger.isDebugEnabled()) { - logger.debug("Requested media types: " + this.mediaTypes); + logger.debug("Requested media types: " + this.contentTypes); } - return this.mediaTypes; + return this.contentTypes; } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/HeaderContentTypeResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/accept/HeaderContentTypeResolver.java index 58d10e85a8b..25ea29048af 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/accept/HeaderContentTypeResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/accept/HeaderContentTypeResolver.java @@ -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"); * 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.MediaType; +import org.springframework.util.CollectionUtils; import org.springframework.web.server.NotAcceptableStatusException; import org.springframework.web.server.ServerWebExchange; @@ -35,7 +36,7 @@ public class HeaderContentTypeResolver implements RequestedContentTypeResolver { try { List mediaTypes = exchange.getRequest().getHeaders().getAccept(); MediaType.sortBySpecificityAndQuality(mediaTypes); - return mediaTypes; + return !CollectionUtils.isEmpty(mediaTypes) ? mediaTypes : MEDIA_TYPE_ALL_LIST; } catch (InvalidMediaTypeException ex) { String value = exchange.getRequest().getHeaders().getFirst("Accept"); 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 29a71761f38..82e34ed9cfb 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 @@ -72,7 +72,7 @@ public class ParameterContentTypeResolver implements RequestedContentTypeResolve public List resolveMediaTypes(ServerWebExchange exchange) throws NotAcceptableStatusException { String key = exchange.getRequest().getQueryParams().getFirst(getParameterName()); if (!StringUtils.hasText(key)) { - return Collections.emptyList(); + return MEDIA_TYPE_ALL_LIST; } key = formatKey(key); MediaType match = this.mediaTypes.get(key); 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 d4479708c64..0cfeb1ca965 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 @@ -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"); * you may not use this file except in compliance with the License. @@ -16,6 +16,7 @@ package org.springframework.web.reactive.accept; +import java.util.Collections; import java.util.List; import org.springframework.http.MediaType; @@ -33,11 +34,20 @@ import org.springframework.web.server.ServerWebExchange; */ 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 MEDIA_TYPE_ALL_LIST = Collections.singletonList(MediaType.ALL); + + /** * Resolve the given request to a list of requested media types. The returned * list is ordered by specificity first and by quality parameter second. * @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 */ 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 8b0d7cd584c..aa27e928a44 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 @@ -95,13 +95,13 @@ public class RequestedContentTypeResolverBuilder { return exchange -> { for (RequestedContentTypeResolver resolver : resolvers) { - List type = resolver.resolveMediaTypes(exchange); - if (type.isEmpty() || (type.size() == 1 && type.contains(MediaType.ALL))) { + List mediaTypes = resolver.resolveMediaTypes(exchange); + if (mediaTypes.equals(RequestedContentTypeResolver.MEDIA_TYPE_ALL_LIST)) { continue; } - return type; + return mediaTypes; } - return Collections.emptyList(); + return RequestedContentTypeResolver.MEDIA_TYPE_ALL_LIST; }; } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/HandlerResultHandlerSupport.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/HandlerResultHandlerSupport.java index 5f36840ffbe..506a1ff201a 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/HandlerResultHandlerSupport.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/HandlerResultHandlerSupport.java @@ -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"); * you may not use this file except in compliance with the License. @@ -17,7 +17,6 @@ package org.springframework.web.reactive.result; import java.util.ArrayList; -import java.util.Collections; import java.util.Comparator; import java.util.LinkedHashSet; import java.util.List; @@ -149,8 +148,7 @@ public abstract class HandlerResultHandlerSupport implements Ordered { } private List getAcceptableTypes(ServerWebExchange exchange) { - List mediaTypes = getContentTypeResolver().resolveMediaTypes(exchange); - return (mediaTypes.isEmpty() ? Collections.singletonList(MediaType.ALL) : mediaTypes); + return getContentTypeResolver().resolveMediaTypes(exchange); } @SuppressWarnings("unchecked") diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java index b9a799a1896..147d83eacc3 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java @@ -238,11 +238,8 @@ public final class ProducesRequestCondition extends AbstractRequestCondition getAcceptedMediaTypes(ServerWebExchange exchange) - throws NotAcceptableStatusException { - - List mediaTypes = this.contentTypeResolver.resolveMediaTypes(exchange); - return mediaTypes.isEmpty() ? Collections.singletonList(MediaType.ALL) : mediaTypes; + private List getAcceptedMediaTypes(ServerWebExchange exchange) throws NotAcceptableStatusException { + return this.contentTypeResolver.resolveMediaTypes(exchange); } private int indexOfEqualMediaType(MediaType mediaType) { diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/accept/ParameterContentTypeResolverTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/accept/ParameterContentTypeResolverTests.java index 8c9c3ec73ca..6998af27dd6 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/accept/ParameterContentTypeResolverTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/accept/ParameterContentTypeResolverTests.java @@ -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"); * 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 { @Test - public void noKey() throws Exception { + public void noKey() { ParameterContentTypeResolver resolver = new ParameterContentTypeResolver(Collections.emptyMap()); ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/")); List mediaTypes = resolver.resolveMediaTypes(exchange); - assertEquals(0, mediaTypes.size()); + assertEquals(RequestedContentTypeResolver.MEDIA_TYPE_ALL_LIST, mediaTypes); } @Test(expected = NotAcceptableStatusException.class) - public void noMatchForKey() throws Exception { + public void noMatchForKey() { ParameterContentTypeResolver resolver = new ParameterContentTypeResolver(Collections.emptyMap()); List mediaTypes = resolver.resolveMediaTypes(createExchange("blah")); @@ -53,7 +53,7 @@ public class ParameterContentTypeResolverTests { } @Test - public void resolveKeyFromRegistrations() throws Exception { + public void resolveKeyFromRegistrations() { ServerWebExchange exchange = createExchange("html"); Map mapping = Collections.emptyMap(); @@ -68,7 +68,7 @@ public class ParameterContentTypeResolverTests { } @Test - public void resolveKeyThroughMediaTypeFactory() throws Exception { + public void resolveKeyThroughMediaTypeFactory() { ServerWebExchange exchange = createExchange("xls"); RequestedContentTypeResolver resolver = new ParameterContentTypeResolver(Collections.emptyMap()); List mediaTypes = resolver.resolveMediaTypes(exchange);