From 91d063899bd816cc57815ac0b2317b3ab674868e Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 25 May 2016 17:34:15 -0400 Subject: [PATCH] Polish ResponseBody result handling --- .../reactive/result/SimpleResultHandler.java | 4 +- .../annotation/ResponseBodyResultHandler.java | 138 +++++++++--------- .../view/ViewResolverResultHandler.java | 8 +- .../ResponseBodyResultHandlerTests.java | 32 +++- 4 files changed, 98 insertions(+), 84 deletions(-) diff --git a/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/SimpleResultHandler.java b/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/SimpleResultHandler.java index 447147e7c3..b32a4d0f76 100644 --- a/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/SimpleResultHandler.java +++ b/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/SimpleResultHandler.java @@ -42,10 +42,10 @@ import org.springframework.web.server.ServerWebExchange; */ public class SimpleResultHandler implements Ordered, HandlerResultHandler { - private int order = Ordered.LOWEST_PRECEDENCE; - private ConversionService conversionService; + private int order = Ordered.LOWEST_PRECEDENCE; + public SimpleResultHandler() { } diff --git a/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/method/annotation/ResponseBodyResultHandler.java b/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/method/annotation/ResponseBodyResultHandler.java index 5e5c61a4e9..cea0cfc029 100644 --- a/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/method/annotation/ResponseBodyResultHandler.java +++ b/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/method/annotation/ResponseBodyResultHandler.java @@ -19,10 +19,8 @@ package org.springframework.web.reactive.result.method.annotation; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; -import java.util.HashMap; import java.util.LinkedHashSet; import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -40,7 +38,6 @@ import org.springframework.http.converter.reactive.HttpMessageConverter; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.util.Assert; -import org.springframework.util.MimeType; import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.method.HandlerMethod; import org.springframework.web.reactive.HandlerResult; @@ -50,6 +47,10 @@ import org.springframework.web.server.ServerWebExchange; /** + * {@code HandlerResultHandler} that handles return values from methods annotated + * with {@code @ResponseBody} writing to the body of the request or response with + * an {@link HttpMessageConverter}. + * * @author Rossen Stoyanchev * @author Stephane Maldini * @author Sebastien Deleuze @@ -57,47 +58,48 @@ import org.springframework.web.server.ServerWebExchange; */ public class ResponseBodyResultHandler implements HandlerResultHandler, Ordered { - private static final MediaType MEDIA_TYPE_APPLICATION = new MediaType("application"); + private static final MediaType MEDIA_TYPE_APPLICATION_ALL = new MediaType("application"); private final List> messageConverters; private final ConversionService conversionService; - private final List allMediaTypes; + private final List supportedMediaTypes; - private final Map, List> mediaTypesByEncoder; + private int order = 0; - private int order = 0; // TODO: should be MAX_VALUE + /** + * Constructor with message converters and conversion service. + * @param messageConverters converters for writing the response body with + * @param conversionService for converting to Flux and Mono from other reactive types + */ public ResponseBodyResultHandler(List> messageConverters, - ConversionService service) { + ConversionService conversionService) { + Assert.notEmpty(messageConverters, "At least one message converter is required."); - Assert.notNull(service, "'conversionService' is required."); + Assert.notNull(conversionService, "'conversionService' is required."); this.messageConverters = messageConverters; - this.conversionService = service; - this.allMediaTypes = getAllMediaTypes(messageConverters); - this.mediaTypesByEncoder = getMediaTypesByConverter(messageConverters); + this.conversionService = conversionService; + this.supportedMediaTypes = initSupportedMediaTypes(messageConverters); } - private static List getAllMediaTypes( - List> messageConverters) { + private static List initSupportedMediaTypes(List> converters) { Set set = new LinkedHashSet<>(); - messageConverters.forEach( - converter -> set.addAll(converter.getWritableMediaTypes())); + converters.forEach(converter -> set.addAll(converter.getWritableMediaTypes())); List result = new ArrayList<>(set); MediaType.sortBySpecificity(result); return Collections.unmodifiableList(result); } - private static Map, List> getMediaTypesByConverter( - List> converters) { - Map, List> result = - new HashMap<>(converters.size()); - converters.forEach(converter -> result - .put(converter, converter.getWritableMediaTypes())); - return Collections.unmodifiableMap(result); - } + /** + * Set the order for this result handler relative to others. + *

By default this is set to 0 and is generally save to be ahead of other + * result handlers since it only gets involved if the method (or class) is + * annotated with {@code @ResponseBody}. + * @param order the order + */ public void setOrder(int order) { this.order = order; } @@ -144,27 +146,23 @@ public class ResponseBodyResultHandler implements HandlerResultHandler, Ordered elementType = returnType; } - List compatibleMediaTypes = - getCompatibleMediaTypes(exchange.getRequest(), elementType); + ServerHttpRequest request = exchange.getRequest(); + List compatibleMediaTypes = getCompatibleMediaTypes(request, elementType); if (compatibleMediaTypes.isEmpty()) { - return Mono.error(new NotAcceptableStatusException( - getProducibleMediaTypes(elementType))); + List supported = getProducibleMediaTypes(elementType); + return Mono.error(new NotAcceptableStatusException(supported)); } - Optional selectedMediaType = selectBestMediaType(compatibleMediaTypes); - - if (selectedMediaType.isPresent()) { - HttpMessageConverter converter = - resolveEncoder(elementType, selectedMediaType.get()); + MediaType bestMediaType = selectBestMediaType(compatibleMediaTypes); + if (bestMediaType != null) { + HttpMessageConverter converter = resolveEncoder(elementType, bestMediaType); if (converter != null) { ServerHttpResponse response = exchange.getResponse(); - return converter.write((Publisher) publisher, elementType, - selectedMediaType.get(), - response); + return converter.write((Publisher) publisher, elementType, bestMediaType, response); } } - return Mono.error(new NotAcceptableStatusException(this.allMediaTypes)); + return Mono.error(new NotAcceptableStatusException(this.supportedMediaTypes)); } private List getCompatibleMediaTypes(ServerHttpRequest request, @@ -174,11 +172,12 @@ public class ResponseBodyResultHandler implements HandlerResultHandler, Ordered List producibleMediaTypes = getProducibleMediaTypes(elementType); Set compatibleMediaTypes = new LinkedHashSet<>(); - for (MediaType acceptableMediaType : acceptableMediaTypes) { - compatibleMediaTypes.addAll(producibleMediaTypes.stream(). - filter(acceptableMediaType::isCompatibleWith). - map(producibleType -> getMostSpecificMediaType(acceptableMediaType, - producibleType)).collect(Collectors.toList())); + for (MediaType acceptable : acceptableMediaTypes) { + for (MediaType producible : producibleMediaTypes) { + if (acceptable.isCompatibleWith(producible)) { + compatibleMediaTypes.add(getMostSpecificMediaType(acceptable, producible)); + } + } } List result = new ArrayList<>(compatibleMediaTypes); @@ -191,44 +190,37 @@ public class ResponseBodyResultHandler implements HandlerResultHandler, Ordered return (mediaTypes.isEmpty() ? Collections.singletonList(MediaType.ALL) : mediaTypes); } - private Optional selectBestMediaType( - List compatibleMediaTypes) { - for (MediaType mediaType : compatibleMediaTypes) { - if (mediaType.isConcrete()) { - return Optional.of(mediaType); - } - else if (mediaType.equals(MediaType.ALL) || - mediaType.equals(MEDIA_TYPE_APPLICATION)) { - return Optional.of(MediaType.APPLICATION_OCTET_STREAM); - } - } - return Optional.empty(); - } - private List getProducibleMediaTypes(ResolvableType type) { - List result = this.messageConverters.stream() + return this.messageConverters.stream() .filter(converter -> converter.canWrite(type, null)) - .flatMap(encoder -> this.mediaTypesByEncoder.get(encoder).stream()) - .collect(Collectors.toList()); - if (result.isEmpty()) { - result.add(MediaType.ALL); - } - - return result; + .flatMap(converter -> converter.getWritableMediaTypes().stream()) + .collect(Collectors.collectingAndThen(Collectors.toList(), result -> { + if (result.isEmpty()) { + result.add(MediaType.ALL); + } + return result; + })); } - /** - * Return the more specific of the acceptable and the producible media types - * with the q-value of the former. - */ - private MediaType getMostSpecificMediaType(MediaType acceptType, MediaType produceType) { - produceType = produceType.copyQualityValue(acceptType); + private MediaType getMostSpecificMediaType(MediaType acceptable, MediaType producible) { + producible = producible.copyQualityValue(acceptable); Comparator comparator = MediaType.SPECIFICITY_COMPARATOR; - return (comparator.compare(acceptType, produceType) <= 0 ? acceptType : produceType); + return (comparator.compare(acceptable, producible) <= 0 ? acceptable : producible); + } + + private MediaType selectBestMediaType(List compatibleMediaTypes) { + for (MediaType mediaType : compatibleMediaTypes) { + if (mediaType.isConcrete()) { + return mediaType; + } + else if (mediaType.equals(MediaType.ALL) || mediaType.equals(MEDIA_TYPE_APPLICATION_ALL)) { + return MediaType.APPLICATION_OCTET_STREAM; + } + } + return null; } - private HttpMessageConverter resolveEncoder(ResolvableType type, - MediaType mediaType) { + private HttpMessageConverter resolveEncoder(ResolvableType type, MediaType mediaType) { for (HttpMessageConverter converter : this.messageConverters) { if (converter.canWrite(type, mediaType)) { return converter; diff --git a/spring-web-reactive/src/main/java/org/springframework/web/reactive/view/ViewResolverResultHandler.java b/spring-web-reactive/src/main/java/org/springframework/web/reactive/view/ViewResolverResultHandler.java index bdf89c54c2..d4a22dce13 100644 --- a/spring-web-reactive/src/main/java/org/springframework/web/reactive/view/ViewResolverResultHandler.java +++ b/spring-web-reactive/src/main/java/org/springframework/web/reactive/view/ViewResolverResultHandler.java @@ -53,7 +53,7 @@ public class ViewResolverResultHandler implements HandlerResultHandler, Ordered private final ConversionService conversionService; - private int order = Integer.MAX_VALUE; + private int order = Ordered.LOWEST_PRECEDENCE; public ViewResolverResultHandler(List resolvers, ConversionService service) { @@ -87,17 +87,17 @@ public class ViewResolverResultHandler implements HandlerResultHandler, Ordered @Override public boolean supports(HandlerResult result) { Class clazz = result.getReturnValueType().getRawClass(); - if (isViewNameOrViewReference(clazz)) { + if (isStringOrViewReference(clazz)) { return true; } if (this.conversionService.canConvert(clazz, Mono.class)) { clazz = result.getReturnValueType().getGeneric(0).getRawClass(); - return isViewNameOrViewReference(clazz); + return isStringOrViewReference(clazz); } return false; } - private boolean isViewNameOrViewReference(Class clazz) { + private boolean isStringOrViewReference(Class clazz) { return (CharSequence.class.isAssignableFrom(clazz) || View.class.isAssignableFrom(clazz)); } diff --git a/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/method/annotation/ResponseBodyResultHandlerTests.java b/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/method/annotation/ResponseBodyResultHandlerTests.java index 38106e7465..c439417d1f 100644 --- a/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/method/annotation/ResponseBodyResultHandlerTests.java +++ b/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/method/annotation/ResponseBodyResultHandlerTests.java @@ -16,37 +16,44 @@ package org.springframework.web.reactive.result.method.annotation; -import java.util.Collections; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; import org.junit.Test; import org.reactivestreams.Publisher; import org.springframework.core.ResolvableType; +import org.springframework.core.codec.Encoder; import org.springframework.core.codec.support.StringEncoder; import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.http.converter.reactive.CodecHttpMessageConverter; +import org.springframework.http.converter.reactive.HttpMessageConverter; import org.springframework.ui.ExtendedModelMap; import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.method.HandlerMethod; import org.springframework.web.reactive.HandlerResult; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; + /** + * Unit tests for {@link ResponseBodyResultHandler}. + * * @author Sebastien Deleuze + * @author Rossen Stoyanchev */ public class ResponseBodyResultHandlerTests { @Test public void supports() throws NoSuchMethodException { - ResponseBodyResultHandler handler = new ResponseBodyResultHandler(Collections.singletonList( - new CodecHttpMessageConverter(new StringEncoder(), null)), - new DefaultConversionService()); + ResponseBodyResultHandler handler = createResultHandler(new StringEncoder()); TestController controller = new TestController(); - HandlerMethod hm = new HandlerMethod(controller,TestController.class.getMethod("notAnnotated")); + HandlerMethod hm = new HandlerMethod(controller, TestController.class.getMethod("notAnnotated")); ResolvableType type = ResolvableType.forMethodParameter(hm.getReturnType()); assertFalse(handler.supports(new HandlerResult(hm, null, type, new ExtendedModelMap()))); @@ -59,6 +66,21 @@ public class ResponseBodyResultHandlerTests { assertTrue(handler.supports(new HandlerResult(hm, null, type, new ExtendedModelMap()))); } + @Test + public void defaultOrder() throws Exception { + ResponseBodyResultHandler handler = createResultHandler(new StringEncoder()); + assertEquals(0, handler.getOrder()); + } + + + private ResponseBodyResultHandler createResultHandler(Encoder... encoders) { + List> converters = Arrays.stream(encoders) + .map(encoder -> new CodecHttpMessageConverter<>(encoder, null)) + .collect(Collectors.toList()); + return new ResponseBodyResultHandler(converters, new DefaultConversionService()); + } + + @SuppressWarnings("unused") private static class TestController { -- GitLab