Improve @RequestAttribute WebFlux resolver

The resolver now takes into account the possibility the attribute
itself may be a reactive type.

Issue: SPR-16158
This commit is contained in:
Rossen Stoyanchev 2017-11-06 21:44:45 -05:00
parent 14f02d7192
commit 9786750b5a
3 changed files with 107 additions and 34 deletions

View File

@ -24,6 +24,7 @@ import org.springframework.core.annotation.AnnotatedElementUtils;
import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatus;
import org.springframework.web.bind.annotation.MatrixVariable; import org.springframework.web.bind.annotation.MatrixVariable;
import org.springframework.web.bind.annotation.ModelAttribute; 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.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.RequestMethod;
@ -62,6 +63,10 @@ public class MvcAnnotationPredicates {
return new RequestPartPredicate(); return new RequestPartPredicate();
} }
public static RequestAttributePredicate requestAttribute() {
return new RequestAttributePredicate();
}
public static MatrixVariablePredicate matrixAttribute() { public static MatrixVariablePredicate matrixAttribute() {
return new MatrixVariablePredicate(); return new MatrixVariablePredicate();
} }
@ -256,6 +261,38 @@ public class MvcAnnotationPredicates {
} }
} }
public static class RequestAttributePredicate implements Predicate<MethodParameter> {
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<Method> { public static class ResponseStatusPredicate implements Predicate<Method> {
private HttpStatus code = HttpStatus.INTERNAL_SERVER_ERROR; private HttpStatus code = HttpStatus.INTERNAL_SERVER_ERROR;

View File

@ -16,8 +16,11 @@
package org.springframework.web.reactive.result.method.annotation; package org.springframework.web.reactive.result.method.annotation;
import reactor.core.publisher.Mono;
import org.springframework.beans.factory.config.ConfigurableBeanFactory; import org.springframework.beans.factory.config.ConfigurableBeanFactory;
import org.springframework.core.MethodParameter; import org.springframework.core.MethodParameter;
import org.springframework.core.ReactiveAdapter;
import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.core.ReactiveAdapterRegistry;
import org.springframework.lang.Nullable; import org.springframework.lang.Nullable;
import org.springframework.util.Assert; import org.springframework.util.Assert;
@ -51,7 +54,7 @@ public class RequestAttributeMethodArgumentResolver extends AbstractNamedValueSy
@Override @Override
public boolean supportsParameter(MethodParameter param) { 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 @Override
protected Object resolveNamedValue(String name, MethodParameter parameter, ServerWebExchange exchange) { 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 @Override

View File

@ -16,26 +16,24 @@
package org.springframework.web.reactive.result.method.annotation; package org.springframework.web.reactive.result.method.annotation;
import java.lang.reflect.Method; import java.time.Duration;
import java.util.Optional; import java.util.Optional;
import io.reactivex.Single;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
import reactor.core.publisher.Mono; import reactor.core.publisher.Mono;
import reactor.test.StepVerifier; import reactor.test.StepVerifier;
import org.springframework.context.annotation.AnnotationConfigApplicationContext; 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.MethodParameter;
import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.core.ReactiveAdapterRegistry;
import org.springframework.core.annotation.SynthesizingMethodParameter;
import org.springframework.format.support.DefaultFormattingConversionService; import org.springframework.format.support.DefaultFormattingConversionService;
import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
import org.springframework.mock.web.test.server.MockServerWebExchange; 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.annotation.RequestAttribute;
import org.springframework.web.bind.support.ConfigurableWebBindingInitializer; import org.springframework.web.bind.support.ConfigurableWebBindingInitializer;
import org.springframework.web.method.ResolvableMethod;
import org.springframework.web.reactive.BindingContext; import org.springframework.web.reactive.BindingContext;
import org.springframework.web.server.ServerWebInputException; 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.assertNull;
import static org.junit.Assert.assertSame; import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue; 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}. * Unit tests for {@link RequestAttributeMethodArgumentResolver}.
@ -58,37 +56,36 @@ public class RequestAttributeMethodArgumentResolverTests {
private final MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/")); private final MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/"));
private Method handleMethod; private final ResolvableMethod testMethod = ResolvableMethod.on(getClass())
.named("handleWithRequestAttribute").build();
@Before @Before
public void setup() throws Exception { public void setup() throws Exception {
AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext();
context.refresh(); context.refresh();
ReactiveAdapterRegistry adapterRegistry = new ReactiveAdapterRegistry(); ReactiveAdapterRegistry registry = new ReactiveAdapterRegistry();
this.resolver = new RequestAttributeMethodArgumentResolver(context.getBeanFactory(), adapterRegistry); this.resolver = new RequestAttributeMethodArgumentResolver(context.getBeanFactory(), registry);
this.handleMethod = ReflectionUtils.findMethod(getClass(), "handleWithRequestAttribute", (Class<?>[]) null);
} }
@Test @Test
public void supportsParameter() throws Exception { public void supportsParameter() throws Exception {
assertTrue(this.resolver.supportsParameter(new MethodParameter(this.handleMethod, 0)));
assertFalse(this.resolver.supportsParameter(new MethodParameter(this.handleMethod, 4))); assertTrue(this.resolver.supportsParameter(
try { this.testMethod.annot(requestAttribute().noName()).arg(Foo.class)));
this.resolver.supportsParameter(new MethodParameter(this.handleMethod, 5));
fail(); // SPR-16158
} assertTrue(this.resolver.supportsParameter(
catch (IllegalStateException ex) { this.testMethod.annotPresent(RequestAttribute.class).arg(Mono.class, Foo.class)));
assertTrue("Unexpected error message:\n" + ex.getMessage(),
ex.getMessage().startsWith( assertFalse(this.resolver.supportsParameter(
"RequestAttributeMethodArgumentResolver doesn't support reactive type wrapper")); this.testMethod.annotNotPresent(RequestAttribute.class).arg()));
}
} }
@Test @Test
public void resolve() throws Exception { public void resolve() throws Exception {
MethodParameter param = initMethodParameter(0); MethodParameter param = this.testMethod.annot(requestAttribute().noName()).arg(Foo.class);
Mono<Object> mono = this.resolver.resolveArgument(param, new BindingContext(), this.exchange); Mono<Object> mono = this.resolver.resolveArgument(param, new BindingContext(), this.exchange);
StepVerifier.create(mono) StepVerifier.create(mono)
.expectNextCount(0) .expectNextCount(0)
@ -103,7 +100,7 @@ public class RequestAttributeMethodArgumentResolverTests {
@Test @Test
public void resolveWithName() throws Exception { public void resolveWithName() throws Exception {
MethodParameter param = initMethodParameter(1); MethodParameter param = this.testMethod.annot(requestAttribute().name("specialFoo")).arg();
Foo foo = new Foo(); Foo foo = new Foo();
this.exchange.getAttributes().put("specialFoo", foo); this.exchange.getAttributes().put("specialFoo", foo);
Mono<Object> mono = this.resolver.resolveArgument(param, new BindingContext(), this.exchange); Mono<Object> mono = this.resolver.resolveArgument(param, new BindingContext(), this.exchange);
@ -112,7 +109,7 @@ public class RequestAttributeMethodArgumentResolverTests {
@Test @Test
public void resolveNotRequired() throws Exception { public void resolveNotRequired() throws Exception {
MethodParameter param = initMethodParameter(2); MethodParameter param = this.testMethod.annot(requestAttribute().name("foo").notRequired()).arg();
Mono<Object> mono = this.resolver.resolveArgument(param, new BindingContext(), this.exchange); Mono<Object> mono = this.resolver.resolveArgument(param, new BindingContext(), this.exchange);
assertNull(mono.block()); assertNull(mono.block());
@ -124,7 +121,7 @@ public class RequestAttributeMethodArgumentResolverTests {
@Test @Test
public void resolveOptional() throws Exception { public void resolveOptional() throws Exception {
MethodParameter param = initMethodParameter(3); MethodParameter param = this.testMethod.annot(requestAttribute().name("foo")).arg(Optional.class, Foo.class);
Mono<Object> mono = this.resolver.resolveArgument(param, new BindingContext(), this.exchange); Mono<Object> mono = this.resolver.resolveArgument(param, new BindingContext(), this.exchange);
assertNotNull(mono.block()); assertNotNull(mono.block());
@ -146,12 +143,30 @@ public class RequestAttributeMethodArgumentResolverTests {
assertSame(foo, optional.get()); 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) { // Mono attribute
MethodParameter param = new SynthesizingMethodParameter(this.handleMethod, parameterIndex); Foo foo = new Foo();
param.initParameterNameDiscovery(new DefaultParameterNameDiscoverer()); Mono<Foo> fooMono = Mono.just(foo);
GenericTypeResolver.resolveParameterType(param, this.resolver.getClass()); this.exchange.getAttributes().put("fooMono", fooMono);
return param; Mono<Object> mono = this.resolver.resolveArgument(param, new BindingContext(), this.exchange);
assertSame(fooMono, mono.block(Duration.ZERO));
// RxJava Single attribute
Single<Foo> 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("specialFoo") Foo namedFoo,
@RequestAttribute(name="foo", required = false) Foo notRequiredFoo, @RequestAttribute(name="foo", required = false) Foo notRequiredFoo,
@RequestAttribute(name="foo") Optional<Foo> optionalFoo, @RequestAttribute(name="foo") Optional<Foo> optionalFoo,
String notSupported, @RequestAttribute Mono<Foo> fooMono,
@RequestAttribute Mono<Foo> alsoNotSupported) { String notSupported) {
} }