From 9786750b5acb151fcc845abfb2c8b8679ec44599 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Mon, 6 Nov 2017 21:44:45 -0500 Subject: [PATCH] Improve @RequestAttribute WebFlux resolver The resolver now takes into account the possibility the attribute itself may be a reactive type. Issue: SPR-16158 --- .../web/method/MvcAnnotationPredicates.java | 37 +++++++++ ...equestAttributeMethodArgumentResolver.java | 25 +++++- ...tAttributeMethodArgumentResolverTests.java | 79 +++++++++++-------- 3 files changed, 107 insertions(+), 34 deletions(-) diff --git a/spring-web/src/test/java/org/springframework/web/method/MvcAnnotationPredicates.java b/spring-web/src/test/java/org/springframework/web/method/MvcAnnotationPredicates.java index 7c875d70b4f..7157781a7f7 100644 --- a/spring-web/src/test/java/org/springframework/web/method/MvcAnnotationPredicates.java +++ b/spring-web/src/test/java/org/springframework/web/method/MvcAnnotationPredicates.java @@ -24,6 +24,7 @@ import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.http.HttpStatus; import org.springframework.web.bind.annotation.MatrixVariable; import org.springframework.web.bind.annotation.ModelAttribute; +import org.springframework.web.bind.annotation.RequestAttribute; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; @@ -62,6 +63,10 @@ public class MvcAnnotationPredicates { return new RequestPartPredicate(); } + public static RequestAttributePredicate requestAttribute() { + return new RequestAttributePredicate(); + } + public static MatrixVariablePredicate matrixAttribute() { return new MatrixVariablePredicate(); } @@ -256,6 +261,38 @@ public class MvcAnnotationPredicates { } } + public static class RequestAttributePredicate implements Predicate { + + private String name; + + private boolean required = true; + + + public RequestAttributePredicate name(String name) { + this.name = name; + return this; + } + + public RequestAttributePredicate noName() { + this.name = ""; + return this; + } + + public RequestAttributePredicate notRequired() { + this.required = false; + return this; + } + + + @Override + public boolean test(MethodParameter parameter) { + RequestAttribute annotation = parameter.getParameterAnnotation(RequestAttribute.class); + return annotation != null && + (this.name == null || annotation.name().equals(this.name)) && + annotation.required() == this.required; + } + } + public static class ResponseStatusPredicate implements Predicate { private HttpStatus code = HttpStatus.INTERNAL_SERVER_ERROR; diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestAttributeMethodArgumentResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestAttributeMethodArgumentResolver.java index 18faa1247cf..5fffa69ea7d 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestAttributeMethodArgumentResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestAttributeMethodArgumentResolver.java @@ -16,8 +16,11 @@ package org.springframework.web.reactive.result.method.annotation; +import reactor.core.publisher.Mono; + import org.springframework.beans.factory.config.ConfigurableBeanFactory; import org.springframework.core.MethodParameter; +import org.springframework.core.ReactiveAdapter; import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -51,7 +54,7 @@ public class RequestAttributeMethodArgumentResolver extends AbstractNamedValueSy @Override public boolean supportsParameter(MethodParameter param) { - return checkAnnotatedParamNoReactiveWrapper(param, RequestAttribute.class, (annot, type) -> true); + return param.hasParameterAnnotation(RequestAttribute.class); } @@ -64,7 +67,25 @@ public class RequestAttributeMethodArgumentResolver extends AbstractNamedValueSy @Override protected Object resolveNamedValue(String name, MethodParameter parameter, ServerWebExchange exchange) { - return exchange.getAttribute(name); + Object value = exchange.getAttribute(name); + ReactiveAdapter toAdapter = getAdapterRegistry().getAdapter(parameter.getParameterType()); + if (toAdapter != null) { + if (value == null) { + Assert.isTrue(toAdapter.supportsEmpty(), + () -> "No request attribute '" + name + "' and target type " + + parameter.getGenericParameterType() + " doesn't support empty values."); + return toAdapter.fromPublisher(Mono.empty()); + } + if (parameter.getParameterType().isAssignableFrom(value.getClass())) { + return value; + } + ReactiveAdapter fromAdapter = getAdapterRegistry().getAdapter(value.getClass()); + Assert.isTrue(fromAdapter != null, + () -> getClass().getSimpleName() + " doesn't support " + + "reactive type wrapper: " + parameter.getGenericParameterType()); + return toAdapter.fromPublisher(fromAdapter.toPublisher(value)); + } + return value; } @Override diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestAttributeMethodArgumentResolverTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestAttributeMethodArgumentResolverTests.java index 72b9d29e644..774a4c070c8 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestAttributeMethodArgumentResolverTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestAttributeMethodArgumentResolverTests.java @@ -16,26 +16,24 @@ package org.springframework.web.reactive.result.method.annotation; -import java.lang.reflect.Method; +import java.time.Duration; import java.util.Optional; +import io.reactivex.Single; import org.junit.Before; import org.junit.Test; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; import org.springframework.context.annotation.AnnotationConfigApplicationContext; -import org.springframework.core.DefaultParameterNameDiscoverer; -import org.springframework.core.GenericTypeResolver; import org.springframework.core.MethodParameter; import org.springframework.core.ReactiveAdapterRegistry; -import org.springframework.core.annotation.SynthesizingMethodParameter; import org.springframework.format.support.DefaultFormattingConversionService; import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; import org.springframework.mock.web.test.server.MockServerWebExchange; -import org.springframework.util.ReflectionUtils; import org.springframework.web.bind.annotation.RequestAttribute; import org.springframework.web.bind.support.ConfigurableWebBindingInitializer; +import org.springframework.web.method.ResolvableMethod; import org.springframework.web.reactive.BindingContext; import org.springframework.web.server.ServerWebInputException; @@ -45,7 +43,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import static org.springframework.web.method.MvcAnnotationPredicates.requestAttribute; /** * Unit tests for {@link RequestAttributeMethodArgumentResolver}. @@ -58,37 +56,36 @@ public class RequestAttributeMethodArgumentResolverTests { private final MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/")); - private Method handleMethod; + private final ResolvableMethod testMethod = ResolvableMethod.on(getClass()) + .named("handleWithRequestAttribute").build(); @Before public void setup() throws Exception { AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); context.refresh(); - ReactiveAdapterRegistry adapterRegistry = new ReactiveAdapterRegistry(); - this.resolver = new RequestAttributeMethodArgumentResolver(context.getBeanFactory(), adapterRegistry); - this.handleMethod = ReflectionUtils.findMethod(getClass(), "handleWithRequestAttribute", (Class[]) null); + ReactiveAdapterRegistry registry = new ReactiveAdapterRegistry(); + this.resolver = new RequestAttributeMethodArgumentResolver(context.getBeanFactory(), registry); } @Test public void supportsParameter() throws Exception { - assertTrue(this.resolver.supportsParameter(new MethodParameter(this.handleMethod, 0))); - assertFalse(this.resolver.supportsParameter(new MethodParameter(this.handleMethod, 4))); - try { - this.resolver.supportsParameter(new MethodParameter(this.handleMethod, 5)); - fail(); - } - catch (IllegalStateException ex) { - assertTrue("Unexpected error message:\n" + ex.getMessage(), - ex.getMessage().startsWith( - "RequestAttributeMethodArgumentResolver doesn't support reactive type wrapper")); - } + + assertTrue(this.resolver.supportsParameter( + this.testMethod.annot(requestAttribute().noName()).arg(Foo.class))); + + // SPR-16158 + assertTrue(this.resolver.supportsParameter( + this.testMethod.annotPresent(RequestAttribute.class).arg(Mono.class, Foo.class))); + + assertFalse(this.resolver.supportsParameter( + this.testMethod.annotNotPresent(RequestAttribute.class).arg())); } @Test public void resolve() throws Exception { - MethodParameter param = initMethodParameter(0); + MethodParameter param = this.testMethod.annot(requestAttribute().noName()).arg(Foo.class); Mono mono = this.resolver.resolveArgument(param, new BindingContext(), this.exchange); StepVerifier.create(mono) .expectNextCount(0) @@ -103,7 +100,7 @@ public class RequestAttributeMethodArgumentResolverTests { @Test public void resolveWithName() throws Exception { - MethodParameter param = initMethodParameter(1); + MethodParameter param = this.testMethod.annot(requestAttribute().name("specialFoo")).arg(); Foo foo = new Foo(); this.exchange.getAttributes().put("specialFoo", foo); Mono mono = this.resolver.resolveArgument(param, new BindingContext(), this.exchange); @@ -112,7 +109,7 @@ public class RequestAttributeMethodArgumentResolverTests { @Test public void resolveNotRequired() throws Exception { - MethodParameter param = initMethodParameter(2); + MethodParameter param = this.testMethod.annot(requestAttribute().name("foo").notRequired()).arg(); Mono mono = this.resolver.resolveArgument(param, new BindingContext(), this.exchange); assertNull(mono.block()); @@ -124,7 +121,7 @@ public class RequestAttributeMethodArgumentResolverTests { @Test public void resolveOptional() throws Exception { - MethodParameter param = initMethodParameter(3); + MethodParameter param = this.testMethod.annot(requestAttribute().name("foo")).arg(Optional.class, Foo.class); Mono mono = this.resolver.resolveArgument(param, new BindingContext(), this.exchange); assertNotNull(mono.block()); @@ -146,12 +143,30 @@ public class RequestAttributeMethodArgumentResolverTests { assertSame(foo, optional.get()); } + @Test // SPR-16158 + public void resolveMonoParameter() throws Exception { + MethodParameter param = this.testMethod.annot(requestAttribute().noName()).arg(Mono.class, Foo.class); - private MethodParameter initMethodParameter(int parameterIndex) { - MethodParameter param = new SynthesizingMethodParameter(this.handleMethod, parameterIndex); - param.initParameterNameDiscovery(new DefaultParameterNameDiscoverer()); - GenericTypeResolver.resolveParameterType(param, this.resolver.getClass()); - return param; + // Mono attribute + Foo foo = new Foo(); + Mono fooMono = Mono.just(foo); + this.exchange.getAttributes().put("fooMono", fooMono); + Mono mono = this.resolver.resolveArgument(param, new BindingContext(), this.exchange); + assertSame(fooMono, mono.block(Duration.ZERO)); + + // RxJava Single attribute + Single singleMono = Single.just(foo); + this.exchange.getAttributes().clear(); + this.exchange.getAttributes().put("fooMono", singleMono); + mono = this.resolver.resolveArgument(param, new BindingContext(), this.exchange); + Object value = mono.block(Duration.ZERO); + assertTrue(value instanceof Mono); + assertSame(foo, ((Mono) value).block(Duration.ZERO)); + + // No attribute --> Mono.empty + this.exchange.getAttributes().clear(); + mono = this.resolver.resolveArgument(param, new BindingContext(), this.exchange); + assertSame(Mono.empty(), mono.block(Duration.ZERO)); } @@ -161,8 +176,8 @@ public class RequestAttributeMethodArgumentResolverTests { @RequestAttribute("specialFoo") Foo namedFoo, @RequestAttribute(name="foo", required = false) Foo notRequiredFoo, @RequestAttribute(name="foo") Optional optionalFoo, - String notSupported, - @RequestAttribute Mono alsoNotSupported) { + @RequestAttribute Mono fooMono, + String notSupported) { }