From 00069572740906bf6ce5b83525753edc89e2db42 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Sun, 23 Oct 2016 13:58:16 +0200 Subject: [PATCH] Support ResponseStatus on reactive controllers This commit adds support for `@ResponseStatus` annotations on reactive controller methods. `HandlerResultHandler`s implementations now set the status on the `ServerWebExchange`, if and only if the invocation of the controller method succeeded. Issue: SPR-14830 --- ...java => AbstractHandlerResultHandler.java} | 22 +++++++++++++++--- .../AbstractMessageWriterResultHandler.java | 12 ++++++---- .../view/ViewResolutionResultHandler.java | 5 ++-- ...ts.java => HandlerResultHandlerTests.java} | 6 ++--- .../ResponseBodyResultHandlerTests.java | 23 +++++++++++++++++++ .../ViewResolutionResultHandlerTests.java | 10 ++++++++ 6 files changed, 65 insertions(+), 13 deletions(-) rename spring-web-reactive/src/main/java/org/springframework/web/reactive/result/{ContentNegotiatingResultHandlerSupport.java => AbstractHandlerResultHandler.java} (82%) rename spring-web-reactive/src/test/java/org/springframework/web/reactive/result/{ContentNegotiatingResultHandlerSupportTests.java => HandlerResultHandlerTests.java} (95%) diff --git a/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/ContentNegotiatingResultHandlerSupport.java b/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/AbstractHandlerResultHandler.java similarity index 82% rename from spring-web-reactive/src/main/java/org/springframework/web/reactive/result/ContentNegotiatingResultHandlerSupport.java rename to spring-web-reactive/src/main/java/org/springframework/web/reactive/result/AbstractHandlerResultHandler.java index 5fd865ab37..beff3a38b9 100644 --- a/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/ContentNegotiatingResultHandlerSupport.java +++ b/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/AbstractHandlerResultHandler.java @@ -23,10 +23,13 @@ import java.util.List; import java.util.Optional; import java.util.Set; +import org.springframework.core.MethodParameter; import org.springframework.core.Ordered; import org.springframework.core.ReactiveAdapterRegistry; +import org.springframework.core.annotation.AnnotationUtils; import org.springframework.http.MediaType; import org.springframework.util.Assert; +import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.reactive.HandlerMapping; import org.springframework.web.reactive.accept.RequestedContentTypeResolver; import org.springframework.web.server.ServerWebExchange; @@ -38,7 +41,7 @@ import org.springframework.web.server.ServerWebExchange; * @author Rossen Stoyanchev * @since 5.0 */ -public abstract class ContentNegotiatingResultHandlerSupport implements Ordered { +public abstract class AbstractHandlerResultHandler implements Ordered { private static final MediaType MEDIA_TYPE_APPLICATION_ALL = new MediaType("application"); @@ -50,11 +53,11 @@ public abstract class ContentNegotiatingResultHandlerSupport implements Ordered private int order = LOWEST_PRECEDENCE; - protected ContentNegotiatingResultHandlerSupport(RequestedContentTypeResolver contentTypeResolver) { + protected AbstractHandlerResultHandler(RequestedContentTypeResolver contentTypeResolver) { this(contentTypeResolver, new ReactiveAdapterRegistry()); } - protected ContentNegotiatingResultHandlerSupport(RequestedContentTypeResolver contentTypeResolver, + protected AbstractHandlerResultHandler(RequestedContentTypeResolver contentTypeResolver, ReactiveAdapterRegistry adapterRegistry) { Assert.notNull(contentTypeResolver, "'contentTypeResolver' is required."); @@ -151,4 +154,17 @@ public abstract class ContentNegotiatingResultHandlerSupport implements Ordered return (comparator.compare(acceptable, producible) <= 0 ? acceptable : producible); } + /** + * Optionally set the response status using the information provided by {@code @ResponseStatus}. + * @param methodParameter the controller method return parameter + * @param exchange the server exchange being handled + */ + protected void updateResponseStatus(MethodParameter methodParameter, ServerWebExchange exchange) { + ResponseStatus annotation = methodParameter.getMethodAnnotation(ResponseStatus.class); + if (annotation != null) { + annotation = AnnotationUtils.synthesizeAnnotation(annotation, methodParameter.getMethod()); + exchange.getResponse().setStatusCode(annotation.code()); + } + } + } diff --git a/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageWriterResultHandler.java b/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageWriterResultHandler.java index 713481ec09..0031d5978d 100644 --- a/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageWriterResultHandler.java +++ b/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageWriterResultHandler.java @@ -33,7 +33,7 @@ import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.util.Assert; import org.springframework.web.reactive.accept.RequestedContentTypeResolver; -import org.springframework.web.reactive.result.ContentNegotiatingResultHandlerSupport; +import org.springframework.web.reactive.result.AbstractHandlerResultHandler; import org.springframework.web.server.NotAcceptableStatusException; import org.springframework.web.server.ServerWebExchange; @@ -44,7 +44,7 @@ import org.springframework.web.server.ServerWebExchange; * @author Rossen Stoyanchev * @since 5.0 */ -public abstract class AbstractMessageWriterResultHandler extends ContentNegotiatingResultHandlerSupport { +public abstract class AbstractMessageWriterResultHandler extends AbstractHandlerResultHandler { private final List> messageWriters; @@ -110,7 +110,8 @@ public abstract class AbstractMessageWriterResultHandler extends ContentNegotiat } if (void.class == elementType.getRawClass() || Void.class == elementType.getRawClass()) { - return Mono.from((Publisher) publisher); + return Mono.from((Publisher) publisher) + .doOnSubscribe(sub -> updateResponseStatus(bodyParameter, exchange)); } List producibleTypes = getProducibleMediaTypes(elementType); @@ -125,11 +126,12 @@ public abstract class AbstractMessageWriterResultHandler extends ContentNegotiat if (bestMediaType != null) { for (HttpMessageWriter messageWriter : getMessageWriters()) { if (messageWriter.canWrite(elementType, bestMediaType)) { - return (messageWriter instanceof ServerHttpMessageWriter ? - ((ServerHttpMessageWriter)messageWriter).write((Publisher) publisher, + Mono bodyWriter = (messageWriter instanceof ServerHttpMessageWriter ? + ((ServerHttpMessageWriter) messageWriter).write((Publisher) publisher, bodyType, elementType, bestMediaType, request, response, Collections.emptyMap()) : messageWriter.write((Publisher) publisher, elementType, bestMediaType, response, Collections.emptyMap())); + return bodyWriter.doOnSubscribe(sub -> updateResponseStatus(bodyParameter, exchange)); } } } diff --git a/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java b/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java index 0dcb351830..03b3b75788 100644 --- a/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java +++ b/spring-web-reactive/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java @@ -43,7 +43,7 @@ import org.springframework.web.bind.annotation.ModelAttribute; import org.springframework.web.reactive.HandlerResult; import org.springframework.web.reactive.HandlerResultHandler; import org.springframework.web.reactive.accept.RequestedContentTypeResolver; -import org.springframework.web.reactive.result.ContentNegotiatingResultHandlerSupport; +import org.springframework.web.reactive.result.AbstractHandlerResultHandler; import org.springframework.web.server.NotAcceptableStatusException; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.util.HttpRequestPathHelper; @@ -74,7 +74,7 @@ import org.springframework.web.util.HttpRequestPathHelper; * @author Rossen Stoyanchev * @since 5.0 */ -public class ViewResolutionResultHandler extends ContentNegotiatingResultHandlerSupport +public class ViewResolutionResultHandler extends AbstractHandlerResultHandler implements HandlerResultHandler, Ordered { private final List viewResolvers = new ArrayList<>(4); @@ -208,6 +208,7 @@ public class ViewResolutionResultHandler extends ContentNegotiatingResultHandler } Map model = result.getModel(); return viewMono.then(view -> { + updateResponseStatus(result.getReturnTypeSource(), exchange); if (view instanceof View) { return ((View) view).render(model, null, exchange); } diff --git a/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/ContentNegotiatingResultHandlerSupportTests.java b/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/HandlerResultHandlerTests.java similarity index 95% rename from spring-web-reactive/src/test/java/org/springframework/web/reactive/result/ContentNegotiatingResultHandlerSupportTests.java rename to spring-web-reactive/src/test/java/org/springframework/web/reactive/result/HandlerResultHandlerTests.java index 7c1ddbc885..1cdcb6c94c 100644 --- a/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/ContentNegotiatingResultHandlerSupportTests.java +++ b/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/HandlerResultHandlerTests.java @@ -46,10 +46,10 @@ import static org.springframework.http.MediaType.TEXT_PLAIN; import static org.springframework.web.reactive.HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE; /** - * Unit tests for {@link ContentNegotiatingResultHandlerSupport}. + * Unit tests for {@link AbstractHandlerResultHandler}. * @author Rossen Stoyanchev */ -public class ContentNegotiatingResultHandlerSupportTests { +public class HandlerResultHandlerTests { private TestResultHandler resultHandler; @@ -117,7 +117,7 @@ public class ContentNegotiatingResultHandlerSupportTests { @SuppressWarnings("WeakerAccess") - private static class TestResultHandler extends ContentNegotiatingResultHandlerSupport { + private static class TestResultHandler extends AbstractHandlerResultHandler { protected TestResultHandler() { this(new HeaderContentTypeResolver()); 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 084734a093..fa83247de2 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 @@ -23,12 +23,14 @@ import java.util.List; import org.junit.Before; import org.junit.Test; import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; import rx.Completable; import rx.Single; import org.springframework.core.codec.ByteBufferEncoder; import org.springframework.core.codec.CharSequenceEncoder; import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.http.codec.EncoderHttpMessageWriter; import org.springframework.http.codec.HttpMessageWriter; @@ -42,6 +44,7 @@ import org.springframework.stereotype.Controller; import org.springframework.ui.ExtendedModelMap; import org.springframework.util.ObjectUtils; import org.springframework.web.bind.annotation.ResponseBody; +import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.method.HandlerMethod; import org.springframework.web.reactive.HandlerResult; @@ -114,6 +117,22 @@ public class ResponseBodyResultHandlerTests { testSupports(controller, "handleToMonoResponseEntity", false); } + @Test + public void writeResponseStatus() throws NoSuchMethodException { + Object controller = new TestRestController(); + HandlerMethod hm = handlerMethod(controller, "handleToString"); + HandlerResult handlerResult = new HandlerResult(hm, null, hm.getReturnType(), new ExtendedModelMap()); + + StepVerifier.create(this.resultHandler.handleResult(this.exchange, handlerResult)).expectComplete().verify(); + assertEquals(HttpStatus.NO_CONTENT, this.response.getStatusCode()); + + hm = handlerMethod(controller, "handleToMonoVoid"); + handlerResult = new HandlerResult(hm, null, hm.getReturnType(), new ExtendedModelMap()); + + StepVerifier.create(this.resultHandler.handleResult(this.exchange, handlerResult)).expectComplete().verify(); + assertEquals(HttpStatus.CREATED, this.response.getStatusCode()); + } + private void testSupports(Object controller, String method, boolean result) throws NoSuchMethodException { HandlerMethod hm = handlerMethod(controller, method); HandlerResult handlerResult = new HandlerResult(hm, null, hm.getReturnType(), new ExtendedModelMap()); @@ -134,6 +153,10 @@ public class ResponseBodyResultHandlerTests { @RestController @SuppressWarnings("unused") private static class TestRestController { + @ResponseStatus(code = HttpStatus.CREATED) + public Mono handleToMonoVoid() { return null;} + + @ResponseStatus(code = HttpStatus.NO_CONTENT) public String handleToString() { return null; } diff --git a/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java b/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java index 8f466cd651..735009e47e 100644 --- a/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java +++ b/spring-web-reactive/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java @@ -42,6 +42,7 @@ import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DefaultDataBufferFactory; import org.springframework.core.io.buffer.support.DataBufferTestUtils; import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; @@ -50,6 +51,7 @@ import org.springframework.ui.ExtendedModelMap; import org.springframework.ui.Model; import org.springframework.ui.ModelMap; import org.springframework.web.bind.annotation.ModelAttribute; +import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.reactive.HandlerResult; import org.springframework.web.reactive.accept.HeaderContentTypeResolver; import org.springframework.web.reactive.accept.RequestedContentTypeResolver; @@ -128,18 +130,22 @@ public class ViewResolutionResultHandlerTests { returnType = ResolvableType.forClass(View.class); returnValue = new TestView("account"); testHandle("/path", returnType, returnValue, "account: {id=123}"); + assertEquals(HttpStatus.NO_CONTENT, this.exchange.getResponse().getStatusCode()); returnType = ResolvableType.forClassWithGenerics(Mono.class, View.class); returnValue = Mono.just(new TestView("account")); testHandle("/path", returnType, returnValue, "account: {id=123}"); + assertEquals(HttpStatus.SEE_OTHER, this.exchange.getResponse().getStatusCode()); returnType = ResolvableType.forClass(String.class); returnValue = "account"; testHandle("/path", returnType, returnValue, "account: {id=123}", resolver); + assertEquals(HttpStatus.CREATED, this.exchange.getResponse().getStatusCode()); returnType = ResolvableType.forClassWithGenerics(Mono.class, String.class); returnValue = Mono.just("account"); testHandle("/path", returnType, returnValue, "account: {id=123}", resolver); + assertEquals(HttpStatus.PARTIAL_CONTENT, this.exchange.getResponse().getStatusCode()); returnType = ResolvableType.forClass(Model.class); returnValue = new ExtendedModelMap().addAttribute("name", "Joe"); @@ -392,12 +398,16 @@ public class ViewResolutionResultHandlerTests { @SuppressWarnings("unused") private static class TestController { + @ResponseStatus(code = HttpStatus.CREATED) String string() { return null; } + @ResponseStatus(HttpStatus.NO_CONTENT) View view() { return null; } + @ResponseStatus(HttpStatus.PARTIAL_CONTENT) Mono monoString() { return null; } + @ResponseStatus(code = HttpStatus.SEE_OTHER) Mono monoView() { return null; } Mono monoVoid() { return null; }