diff --git a/spring-web-reactive/src/main/java/org/springframework/reactive/web/dispatch/method/HandlerMethodArgumentResolver.java b/spring-web-reactive/src/main/java/org/springframework/reactive/web/dispatch/method/HandlerMethodArgumentResolver.java index f09fa42830..a7b5b16f95 100644 --- a/spring-web-reactive/src/main/java/org/springframework/reactive/web/dispatch/method/HandlerMethodArgumentResolver.java +++ b/spring-web-reactive/src/main/java/org/springframework/reactive/web/dispatch/method/HandlerMethodArgumentResolver.java @@ -16,6 +16,8 @@ package org.springframework.reactive.web.dispatch.method; +import org.reactivestreams.Publisher; + import org.springframework.core.MethodParameter; import org.springframework.http.server.ReactiveServerHttpRequest; @@ -27,6 +29,12 @@ public interface HandlerMethodArgumentResolver { boolean supportsParameter(MethodParameter parameter); - Object resolveArgument(MethodParameter parameter, ReactiveServerHttpRequest request); + /** + * The returned Publisher must produce a single value. As Reactive Streams + * does not allow publishing null values, if the value may be {@code null} + * use {@link java.util.Optional#ofNullable(Object)} to wrap it. + */ + Publisher resolveArgument(MethodParameter parameter, ReactiveServerHttpRequest request); + } diff --git a/spring-web-reactive/src/main/java/org/springframework/reactive/web/dispatch/method/InvocableHandlerMethod.java b/spring-web-reactive/src/main/java/org/springframework/reactive/web/dispatch/method/InvocableHandlerMethod.java index 7507677180..c409425f7b 100644 --- a/spring-web-reactive/src/main/java/org/springframework/reactive/web/dispatch/method/InvocableHandlerMethod.java +++ b/spring-web-reactive/src/main/java/org/springframework/reactive/web/dispatch/method/InvocableHandlerMethod.java @@ -19,8 +19,14 @@ package org.springframework.reactive.web.dispatch.method; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.ArrayList; -import java.util.Arrays; +import java.util.Collections; import java.util.List; +import java.util.Optional; + +import org.reactivestreams.Publisher; +import reactor.Publishers; +import reactor.fn.tuple.Tuple; +import reactor.rx.Streams; import org.springframework.core.DefaultParameterNameDiscoverer; import org.springframework.core.GenericTypeResolver; @@ -32,9 +38,6 @@ import org.springframework.web.method.HandlerMethod; /** - * 90% overlap with the existing one in spring-web except for the different - * HandlerMethodArgumentResolver contract. - * * @author Rossen Stoyanchev */ public class InvocableHandlerMethod extends HandlerMethod { @@ -55,53 +58,74 @@ public class InvocableHandlerMethod extends HandlerMethod { } - public Object invokeForRequest(ReactiveServerHttpRequest request, Object... providedArgs) throws Exception { - Object[] args = getMethodArgumentValues(request, providedArgs); - if (logger.isTraceEnabled()) { - logger.trace("Invoking [" + getBeanType().getSimpleName() + "." + - getMethod().getName() + "] method with arguments " + Arrays.asList(args)); - } - Object returnValue = doInvoke(args); - if (logger.isTraceEnabled()) { - logger.trace("Method [" + getMethod().getName() + "] returned [" + returnValue + "]"); - } - return returnValue; + public Publisher invokeForRequest(ReactiveServerHttpRequest request, + Object... providedArgs) { + + List> argPublishers = getMethodArguments(request, providedArgs); + + Publisher argValues = (!argPublishers.isEmpty() ? + Streams.zip(argPublishers, this::unwrapOptionalArgValues) : Publishers.just(new Object[0])); + + return Publishers.map(argValues, args -> { + if (logger.isTraceEnabled()) { + logger.trace("Invoking [" + getBeanType().getSimpleName() + "." + + getMethod().getName() + "] method with arguments " + + Collections.singletonList(argPublishers)); + } + Object returnValue = null; + try { + returnValue = doInvoke(args); + if (logger.isTraceEnabled()) { + logger.trace("Method [" + getMethod().getName() + "] returned [" + returnValue + "]"); + } + } + catch (Exception ex) { + // TODO: how to best handle error inside map? (also wrapping hides original ex) + throw new IllegalStateException(ex); + } + return returnValue; + }); } - private Object[] getMethodArgumentValues(ReactiveServerHttpRequest request, Object... providedArgs) throws Exception { + private List> getMethodArguments(ReactiveServerHttpRequest request, + Object... providedArgs) { + MethodParameter[] parameters = getMethodParameters(); - Object[] args = new Object[parameters.length]; + List> valuePublishers = new ArrayList<>(parameters.length); for (int i = 0; i < parameters.length; i++) { MethodParameter parameter = parameters[i]; parameter.initParameterNameDiscovery(this.parameterNameDiscoverer); GenericTypeResolver.resolveParameterType(parameter, getBean().getClass()); - args[i] = resolveProvidedArgument(parameter, providedArgs); - if (args[i] != null) { + Object value = resolveProvidedArgument(parameter, providedArgs); + if (value != null) { + valuePublishers.add(Publishers.just(value)); continue; } + boolean resolved = false; for (HandlerMethodArgumentResolver resolver : this.argumentResolvers) { if (resolver.supportsParameter(parameter)) { try { - args[i] = resolver.resolveArgument(parameter, request); + valuePublishers.add(resolver.resolveArgument(parameter, request)); + resolved = true; break; } catch (Exception ex) { - if (logger.isDebugEnabled()) { - logger.debug(getArgumentResolutionErrorMessage("Error resolving argument", i), ex); - } - throw ex; + String msg = buildArgErrorMessage("Error resolving argument", i); + valuePublishers.add(Publishers.error(new IllegalStateException(msg, ex))); + break; } } } - if (args[i] == null) { - String msg = getArgumentResolutionErrorMessage("No suitable resolver for argument", i); - throw new IllegalStateException(msg); + if (!resolved) { + String msg = buildArgErrorMessage("No suitable resolver for argument", i); + valuePublishers.add(Publishers.error(new IllegalStateException(msg))); + break; } } - return args; + return valuePublishers; } - private String getArgumentResolutionErrorMessage(String message, int index) { + private String buildArgErrorMessage(String message, int index) { MethodParameter param = getMethodParameters()[index]; message += " [" + index + "] [type=" + param.getParameterType().getName() + "]"; return getDetailedErrorMessage(message); @@ -125,6 +149,27 @@ public class InvocableHandlerMethod extends HandlerMethod { return null; } + private void unwrapOptionalArgValues(Object[] args) { + for (int i = 0; i < args.length; i++) { + if (args[i] instanceof Optional) { + Optional optional = (Optional) args[i]; + args[i] = optional.isPresent() ? optional.get() : null; + } + } + } + + private Object[] unwrapOptionalArgValues(Tuple tuple) { + Object[] args = new Object[tuple.size()]; + for (int i = 0; i < tuple.size(); i++) { + args[i] = tuple.get(i); + if (args[i] instanceof Optional) { + Optional optional = (Optional) args[i]; + args[i] = optional.isPresent() ? optional.get() : null; + } + } + return args; + } + protected Object doInvoke(Object... args) throws Exception { ReflectionUtils.makeAccessible(getBridgedMethod()); try { diff --git a/spring-web-reactive/src/main/java/org/springframework/reactive/web/dispatch/method/annotation/RequestBodyArgumentResolver.java b/spring-web-reactive/src/main/java/org/springframework/reactive/web/dispatch/method/annotation/RequestBodyArgumentResolver.java index e14be9ee21..8a0119c569 100644 --- a/spring-web-reactive/src/main/java/org/springframework/reactive/web/dispatch/method/annotation/RequestBodyArgumentResolver.java +++ b/spring-web-reactive/src/main/java/org/springframework/reactive/web/dispatch/method/annotation/RequestBodyArgumentResolver.java @@ -23,6 +23,7 @@ import java.util.Collections; import java.util.List; import org.reactivestreams.Publisher; +import reactor.Publishers; import org.springframework.core.MethodParameter; import org.springframework.core.ResolvableType; @@ -66,9 +67,7 @@ public class RequestBodyArgumentResolver implements HandlerMethodArgumentResolve } @Override - @SuppressWarnings("unchecked") - public Object resolveArgument(MethodParameter parameter, ReactiveServerHttpRequest request) { - + public Publisher resolveArgument(MethodParameter parameter, ReactiveServerHttpRequest request) { MediaType mediaType = resolveMediaType(request); ResolvableType type = ResolvableType.forMethodParameter(parameter); List hints = new ArrayList<>(); @@ -85,12 +84,10 @@ public class RequestBodyArgumentResolver implements HandlerMethodArgumentResolve } elementStream = deserializer.decode(inputStream, elementType, mediaType, hints.toArray()); } - if (conversionService.canConvert(Publisher.class, type.getRawClass())) { - return conversionService.convert(elementStream, type.getRawClass()); - } - else { - return elementStream; + if (this.conversionService.canConvert(Publisher.class, type.getRawClass())) { + return Publishers.just(this.conversionService.convert(elementStream, type.getRawClass())); } + return Publishers.map(elementStream, element -> element); } private MediaType resolveMediaType(ReactiveServerHttpRequest request) { diff --git a/spring-web-reactive/src/main/java/org/springframework/reactive/web/dispatch/method/annotation/RequestMappingHandlerAdapter.java b/spring-web-reactive/src/main/java/org/springframework/reactive/web/dispatch/method/annotation/RequestMappingHandlerAdapter.java index e4b1f98579..2b89f9f7ed 100644 --- a/spring-web-reactive/src/main/java/org/springframework/reactive/web/dispatch/method/annotation/RequestMappingHandlerAdapter.java +++ b/spring-web-reactive/src/main/java/org/springframework/reactive/web/dispatch/method/annotation/RequestMappingHandlerAdapter.java @@ -79,14 +79,8 @@ public class RequestMappingHandlerAdapter implements HandlerAdapter, Initializin InvocableHandlerMethod handlerMethod = new InvocableHandlerMethod((HandlerMethod) handler); handlerMethod.setHandlerMethodArgumentResolvers(this.argumentResolvers); - try { - Object result = handlerMethod.invokeForRequest(request); - return Publishers.just(new HandlerResult(handlerMethod, result)); - } - catch (Exception e) { - // TODO: remove throws declaration from InvocableHandlerMethod - return Publishers.error(e); - } + Publisher resultPublisher = handlerMethod.invokeForRequest(request); + return Publishers.map(resultPublisher, result -> new HandlerResult(handlerMethod, result)); } } \ No newline at end of file diff --git a/spring-web-reactive/src/main/java/org/springframework/reactive/web/dispatch/method/annotation/RequestParamArgumentResolver.java b/spring-web-reactive/src/main/java/org/springframework/reactive/web/dispatch/method/annotation/RequestParamArgumentResolver.java index ab7fa92d85..124f840b13 100644 --- a/spring-web-reactive/src/main/java/org/springframework/reactive/web/dispatch/method/annotation/RequestParamArgumentResolver.java +++ b/spring-web-reactive/src/main/java/org/springframework/reactive/web/dispatch/method/annotation/RequestParamArgumentResolver.java @@ -17,6 +17,11 @@ package org.springframework.reactive.web.dispatch.method.annotation; +import java.util.Optional; + +import org.reactivestreams.Publisher; +import reactor.Publishers; + import org.springframework.core.MethodParameter; import org.springframework.http.server.ReactiveServerHttpRequest; import org.springframework.reactive.web.dispatch.method.HandlerMethodArgumentResolver; @@ -40,11 +45,12 @@ public class RequestParamArgumentResolver implements HandlerMethodArgumentResolv @Override - public Object resolveArgument(MethodParameter param, ReactiveServerHttpRequest request) { + public Publisher resolveArgument(MethodParameter param, ReactiveServerHttpRequest request) { RequestParam annotation = param.getParameterAnnotation(RequestParam.class); String name = (annotation.value().length() != 0 ? annotation.value() : param.getParameterName()); UriComponents uriComponents = UriComponentsBuilder.fromUri(request.getURI()).build(); - return uriComponents.getQueryParams().getFirst(name); + String value = uriComponents.getQueryParams().getFirst(name); + return Publishers.just(Optional.ofNullable(value)); } } diff --git a/spring-web-reactive/src/test/java/org/springframework/reactive/web/dispatch/method/annotation/RequestMappingIntegrationTests.java b/spring-web-reactive/src/test/java/org/springframework/reactive/web/dispatch/method/annotation/RequestMappingIntegrationTests.java index e46dc4ff46..0b3544740c 100644 --- a/spring-web-reactive/src/test/java/org/springframework/reactive/web/dispatch/method/annotation/RequestMappingIntegrationTests.java +++ b/spring-web-reactive/src/test/java/org/springframework/reactive/web/dispatch/method/annotation/RequestMappingIntegrationTests.java @@ -180,6 +180,11 @@ public class RequestMappingIntegrationTests extends AbstractHttpHandlerIntegrati capitalizeCollection("http://localhost:" + port + "/stream-capitalize"); } + @Test + public void personCapitalize() throws Exception { + capitalizePojo("http://localhost:" + port + "/person-capitalize"); + } + @Test public void completableFutureCapitalize() throws Exception { capitalizePojo("http://localhost:" + port + "/completable-future-capitalize"); @@ -376,6 +381,13 @@ public class RequestMappingIntegrationTests extends AbstractHttpHandlerIntegrati }); } + @RequestMapping("/person-capitalize") + @ResponseBody + public Person personCapitalize(@RequestBody Person person) { + person.setName(person.getName().toUpperCase()); + return person; + } + @RequestMapping("/completable-future-capitalize") @ResponseBody public CompletableFuture completableFutureCapitalize(@RequestBody CompletableFuture personFuture) {