From d8a7b96b46bffa8ef376f056f57bdafb1881603c Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 18 Oct 2017 19:08:22 -0400 Subject: [PATCH] WebFlux support for "request handled" in controller Issue: SPR-16087 --- .../result/method/InvocableHandlerMethod.java | 62 ++++++++- .../annotation/ControllerMethodResolver.java | 5 + .../method/InvocableHandlerMethodTests.java | 131 +++++++++++++++++- src/docs/asciidoc/web/webflux.adoc | 10 +- src/docs/asciidoc/web/webmvc.adoc | 10 +- 5 files changed, 205 insertions(+), 13 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java index 7f7e119568..362e1c807f 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java @@ -18,6 +18,8 @@ package org.springframework.web.reactive.result.method; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -31,7 +33,10 @@ import reactor.core.publisher.Mono; import org.springframework.core.DefaultParameterNameDiscoverer; import org.springframework.core.MethodParameter; import org.springframework.core.ParameterNameDiscoverer; +import org.springframework.core.ReactiveAdapter; +import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.http.HttpStatus; +import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.lang.Nullable; import org.springframework.util.ClassUtils; import org.springframework.util.ObjectUtils; @@ -61,6 +66,8 @@ public class InvocableHandlerMethod extends HandlerMethod { private ParameterNameDiscoverer parameterNameDiscoverer = new DefaultParameterNameDiscoverer(); + private ReactiveAdapterRegistry reactiveAdapterRegistry = new ReactiveAdapterRegistry(); + public InvocableHandlerMethod(HandlerMethod handlerMethod) { super(handlerMethod); @@ -103,6 +110,18 @@ public class InvocableHandlerMethod extends HandlerMethod { return this.parameterNameDiscoverer; } + /** + * Configure a reactive registry. This is needed for cases where the response + * is fully handled within the controller in combination with an async void + * return value. + *

By default this is an instance of {@link ReactiveAdapterRegistry} with + * default settings. + * @param registry the registry to use + */ + public void setReactiveAdapterRegistry(ReactiveAdapterRegistry registry) { + this.reactiveAdapterRegistry = registry; + } + /** * Invoke the method for the given exchange. @@ -117,11 +136,21 @@ public class InvocableHandlerMethod extends HandlerMethod { return resolveArguments(exchange, bindingContext, providedArgs).flatMap(args -> { try { Object value = doInvoke(args); - HandlerResult result = new HandlerResult(this, value, getReturnType(), bindingContext); + HttpStatus status = getResponseStatus(); if (status != null) { exchange.getResponse().setStatusCode(status); } + + MethodParameter returnType = getReturnType(); + ReactiveAdapter adapter = this.reactiveAdapterRegistry.getAdapter(returnType.getParameterType()); + boolean asyncVoid = isAsyncVoidReturnType(returnType, adapter); + if ((value == null || asyncVoid) && isResponseHandled(args, exchange)) { + logger.debug("Response fully handled in controller method"); + return asyncVoid ? Mono.from(adapter.toPublisher(value)) : Mono.empty(); + } + + HandlerResult result = new HandlerResult(this, value, returnType, bindingContext); return Mono.just(result); } catch (InvocationTargetException ex) { @@ -204,6 +233,7 @@ public class InvocableHandlerMethod extends HandlerMethod { param.getParameterType().getName() + "' on " + getBridgedMethod().toGenericString(); } + @Nullable private Object doInvoke(Object[] args) throws Exception { if (logger.isTraceEnabled()) { logger.trace("Invoking '" + ClassUtils.getQualifiedMethodName(getMethod(), getBeanType()) + @@ -228,4 +258,34 @@ public class InvocableHandlerMethod extends HandlerMethod { "on " + getBridgedMethod().toGenericString(); } + private boolean isAsyncVoidReturnType(MethodParameter returnType, + @Nullable ReactiveAdapter reactiveAdapter) { + + if (reactiveAdapter != null && reactiveAdapter.supportsEmpty()) { + if (reactiveAdapter.isNoValue()) { + return true; + } + Type parameterType = returnType.getGenericParameterType(); + if (parameterType instanceof ParameterizedType) { + ParameterizedType type = (ParameterizedType) parameterType; + if (type.getActualTypeArguments().length == 1) { + return Void.class.equals(type.getActualTypeArguments()[0]); + } + } + } + return false; + } + + private boolean isResponseHandled(Object[] args, ServerWebExchange exchange) { + if (getResponseStatus() != null || exchange.isNotModified()) { + return true; + } + for (Object arg : args) { + if (arg instanceof ServerHttpResponse || arg instanceof ServerWebExchange) { + return true; + } + } + return false; + } + } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolver.java index 985058bae9..d030bcb7ef 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolver.java @@ -82,6 +82,8 @@ class ControllerMethodResolver { private final List exceptionHandlerResolvers; + private final ReactiveAdapterRegistry reactiveAdapterRegistry; + private final Map, Set> initBinderMethodCache = new ConcurrentHashMap<>(64); @@ -127,6 +129,8 @@ class ControllerMethodResolver { addResolversTo(registrar, reactiveRegistry, context); this.exceptionHandlerResolvers = registrar.getResolvers(); + this.reactiveAdapterRegistry = reactiveRegistry; + initControllerAdviceCaches(context); } @@ -214,6 +218,7 @@ class ControllerMethodResolver { public InvocableHandlerMethod getRequestMappingMethod(HandlerMethod handlerMethod) { InvocableHandlerMethod invocable = new InvocableHandlerMethod(handlerMethod); invocable.setArgumentResolvers(this.requestMappingResolvers); + invocable.setReactiveAdapterRegistry(this.reactiveAdapterRegistry); return invocable; } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/InvocableHandlerMethodTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/InvocableHandlerMethodTests.java index 345eb4dc97..41204f70b5 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/InvocableHandlerMethodTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/InvocableHandlerMethodTests.java @@ -16,26 +16,40 @@ package org.springframework.web.reactive.result.method; +import java.io.UnsupportedEncodingException; import java.lang.reflect.Method; +import java.time.Duration; +import java.time.Instant; import java.util.Arrays; import org.junit.Test; +import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; +import org.springframework.core.io.buffer.DataBuffer; +import org.springframework.core.io.buffer.DefaultDataBufferFactory; import org.springframework.http.HttpStatus; +import org.springframework.http.server.reactive.ServerHttpResponse; +import org.springframework.lang.Nullable; import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; import org.springframework.mock.web.test.server.MockServerWebExchange; import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.reactive.BindingContext; import org.springframework.web.reactive.HandlerResult; +import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.UnsupportedMediaTypeStatusException; -import static org.hamcrest.Matchers.*; -import static org.junit.Assert.*; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; import static org.mockito.Mockito.any; -import static org.mockito.Mockito.*; -import static org.springframework.web.method.ResolvableMethod.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.get; +import static org.springframework.web.method.ResolvableMethod.on; /** * Unit tests for {@link InvocableHandlerMethod}. @@ -45,10 +59,72 @@ import static org.springframework.web.method.ResolvableMethod.*; */ public class InvocableHandlerMethodTests { - private final MockServerWebExchange exchange = - MockServerWebExchange.from(MockServerHttpRequest.get("http://localhost:8080/path")); + private final MockServerWebExchange exchange = MockServerWebExchange.from(get("http://localhost:8080/path")); + @Test + public void invokeAndHandle_VoidWithResponseStatus() throws Exception { + Method method = on(VoidController.class).mockCall(VoidController::responseStatus).method(); + HandlerResult result = invoke(new VoidController(), method).block(Duration.ZERO); + + assertNull("Expected no result (i.e. fully handled)", result); + assertEquals(HttpStatus.BAD_REQUEST, this.exchange.getResponse().getStatusCode()); + } + + @Test + public void invokeAndHandle_withResponse() throws Exception { + ServerHttpResponse response = this.exchange.getResponse(); + Method method = on(VoidController.class).mockCall(c -> c.response(response)).method(); + HandlerResult result = invoke(new VoidController(), method, resolverFor(Mono.just(response))) + .block(Duration.ZERO); + + assertNull("Expected no result (i.e. fully handled)", result); + assertEquals("bar", this.exchange.getResponse().getHeaders().getFirst("foo")); + } + + @Test + public void invokeAndHandle_withResponseAndMonoVoid() throws Exception { + ServerHttpResponse response = this.exchange.getResponse(); + Method method = on(VoidController.class).mockCall(c -> c.responseMonoVoid(response)).method(); + HandlerResult result = invoke(new VoidController(), method, resolverFor(Mono.just(response))) + .block(Duration.ZERO); + + assertNull("Expected no result (i.e. fully handled)", result); + assertEquals("body", this.exchange.getResponse().getBodyAsString().block(Duration.ZERO)); + } + + @Test + public void invokeAndHandle_withExchange() throws Exception { + Method method = on(VoidController.class).mockCall(c -> c.exchange(exchange)).method(); + HandlerResult result = invoke(new VoidController(), method, resolverFor(Mono.just(this.exchange))) + .block(Duration.ZERO); + + assertNull("Expected no result (i.e. fully handled)", result); + assertEquals("bar", this.exchange.getResponse().getHeaders().getFirst("foo")); + } + + @Test + public void invokeAndHandle_withExchangeAndMonoVoid() throws Exception { + Method method = on(VoidController.class).mockCall(c -> c.exchangeMonoVoid(exchange)).method(); + HandlerResult result = invoke(new VoidController(), method, resolverFor(Mono.just(this.exchange))) + .block(Duration.ZERO); + + assertNull("Expected no result (i.e. fully handled)", result); + assertEquals("body", this.exchange.getResponse().getBodyAsString().block(Duration.ZERO)); + } + + @Test + public void invokeAndHandle_withNotModified() throws Exception { + ServerWebExchange exchange = MockServerWebExchange.from( + MockServerHttpRequest.get("/").ifModifiedSince(10 * 1000 * 1000)); + + Method method = on(VoidController.class).mockCall(c -> c.notModified(exchange)).method(); + HandlerResult result = invoke(new VoidController(), method, resolverFor(Mono.just(exchange))) + .block(Duration.ZERO); + + assertNull("Expected no result (i.e. fully handled)", result); + } + @Test public void invokeMethodWithNoArguments() throws Exception { Method method = on(TestController.class).mockCall(TestController::noArgs).method(); @@ -146,7 +222,7 @@ public class InvocableHandlerMethodTests { private Mono invoke(Object handler, Method method) { - return this.invoke(handler, method, new HandlerMethodArgumentResolver[0]); + return invoke(handler, method, new HandlerMethodArgumentResolver[0]); } private Mono invoke(Object handler, Method method, @@ -195,4 +271,45 @@ public class InvocableHandlerMethodTests { } } + @SuppressWarnings("unused") + private static class VoidController { + + @ResponseStatus(HttpStatus.BAD_REQUEST) + public void responseStatus() { + } + + public void response(ServerHttpResponse response) { + response.getHeaders().add("foo", "bar"); + } + + public Mono responseMonoVoid(ServerHttpResponse response) { + return response.writeWith(getBody("body")); + } + + public void exchange(ServerWebExchange exchange) { + exchange.getResponse().getHeaders().add("foo", "bar"); + } + + public Mono exchangeMonoVoid(ServerWebExchange exchange) { + return exchange.getResponse().writeWith(getBody("body")); + } + + @Nullable + public String notModified(ServerWebExchange exchange) { + if (exchange.checkNotModified(Instant.ofEpochMilli(1000 * 1000))) { + return null; + } + return "body"; + } + + private Flux getBody(String body) { + try { + return Flux.just(new DefaultDataBufferFactory().wrap(body.getBytes("UTF-8"))); + } + catch (UnsupportedEncodingException ex) { + throw new IllegalStateException(ex); + } + } + } + } diff --git a/src/docs/asciidoc/web/webflux.adoc b/src/docs/asciidoc/web/webflux.adoc index 0a138c8012..4188115256 100644 --- a/src/docs/asciidoc/web/webflux.adoc +++ b/src/docs/asciidoc/web/webflux.adoc @@ -1029,8 +1029,14 @@ from the request path. |An API for model and view rendering scenarios. |`void` -|For use in method that don't write the response body; or methods where the view name is -supposed to be determined implicitly from the request path. +|A method with a `void`, possibly async (e.g. `Mono`), return type (or a `null` return +value) is considered to have fully handled the response if it also has a `ServerHttpResponse`, +or a `ServerWebExchange` argument, or an `@ResponseStatus` annotation. The same is true also +if the controller has made a positive ETag or lastModified timestamp check. +// TODO (see <> for details) + +If none of the above is true, a `void` return type may also indicate "no response body" for +REST controllers, or default view name selection for HTML controllers. |`Flux`, `Observable`, or other reactive type |Emit server-sent events; the `SeverSentEvent` wrapper can be omitted when only data needs diff --git a/src/docs/asciidoc/web/webmvc.adoc b/src/docs/asciidoc/web/webmvc.adoc index 8bad1c59fd..e16e59752a 100644 --- a/src/docs/asciidoc/web/webmvc.adoc +++ b/src/docs/asciidoc/web/webmvc.adoc @@ -1684,9 +1684,13 @@ through a `RequestToViewNameTranslator`. |The view and model attributes to use, and optionally a response status. |`void` -|For use in methods that declare a `ServletResponse` or `OutputStream` argument and write -to the response body; or if the view name is supposed to be implicitly determined through a -`RequestToViewNameTranslator`. +|A method with a `void` return type (or `null` return value) is considered to have fully +handled the response if it also has a `ServletResponse`, or an `OutputStream` argument, or an +`@ResponseStatus` annotation. The same is true also if the controller has made a positive +ETag or lastModified timestamp check (see <> for details). + +If none of the above is true, a `void` return type may also indicate "no response body" for +REST controllers, or default view name selection for HTML controllers. |`Callable` |Produce any of the above return values asynchronously in a Spring MVC managed thread. -- GitLab