Consistent InvocableHandlerMethod implementations

This commit makes the 3 existing InvocableHandlerMethod types more
consistent and comparable with each other.

1. Use of consistent method names and method order.

2. Consistent error formatting.

3. Explicit for loops for resolving argument values in webflux variant
because that makes it more readable, creates less garabage, and it's
the only way to bring consistency since the other two variants cannot
throw exceptions inside Optional lambdas (vs webflux variant which can
wrap it in a Mono).

4. Use package private HandlerMethodArgumentComposite in webflux
variant in order to pick up the resolver argument caching that the
other two variants have.

5. Polish tests.

6. Add missing tests for messaging variant.
This commit is contained in:
Rossen Stoyanchev
2018-10-30 16:36:01 -04:00
parent 991e9f4269
commit 7c36549e3a
14 changed files with 795 additions and 386 deletions

View File

@@ -0,0 +1,147 @@
/*
* Copyright 2002-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.web.reactive.result.method;
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import reactor.core.publisher.Mono;
import org.springframework.core.MethodParameter;
import org.springframework.lang.Nullable;
import org.springframework.web.reactive.BindingContext;
import org.springframework.web.server.ServerWebExchange;
/**
* Resolves method parameters by delegating to a list of registered
* {@link HandlerMethodArgumentResolver HandlerMethodArgumentResolvers}.
* Previously resolved method parameters are cached for faster lookups.
*
* @author Rossen Stoyanchev
* @since 5.3
*/
class HandlerMethodArgumentResolverComposite implements HandlerMethodArgumentResolver {
protected final Log logger = LogFactory.getLog(getClass());
private final List<HandlerMethodArgumentResolver> argumentResolvers = new LinkedList<>();
private final Map<MethodParameter, HandlerMethodArgumentResolver> argumentResolverCache =
new ConcurrentHashMap<>(256);
/**
* Add the given {@link HandlerMethodArgumentResolver}.
*/
public HandlerMethodArgumentResolverComposite addResolver(HandlerMethodArgumentResolver resolver) {
this.argumentResolvers.add(resolver);
return this;
}
/**
* Add the given {@link HandlerMethodArgumentResolver HandlerMethodArgumentResolvers}.
* @since 4.3
*/
public HandlerMethodArgumentResolverComposite addResolvers(@Nullable HandlerMethodArgumentResolver... resolvers) {
if (resolvers != null) {
Collections.addAll(this.argumentResolvers, resolvers);
}
return this;
}
/**
* Add the given {@link HandlerMethodArgumentResolver HandlerMethodArgumentResolvers}.
*/
public HandlerMethodArgumentResolverComposite addResolvers(
@Nullable List<? extends HandlerMethodArgumentResolver> resolvers) {
if (resolvers != null) {
this.argumentResolvers.addAll(resolvers);
}
return this;
}
/**
* Return a read-only list with the contained resolvers, or an empty list.
*/
public List<HandlerMethodArgumentResolver> getResolvers() {
return Collections.unmodifiableList(this.argumentResolvers);
}
/**
* Clear the list of configured resolvers.
* @since 4.3
*/
public void clear() {
this.argumentResolvers.clear();
}
/**
* Whether the given {@linkplain MethodParameter method parameter} is
* supported by any registered {@link HandlerMethodArgumentResolver}.
*/
@Override
public boolean supportsParameter(MethodParameter parameter) {
return getArgumentResolver(parameter) != null;
}
/**
* Iterate over registered
* {@link HandlerMethodArgumentResolver HandlerMethodArgumentResolvers} and
* invoke the one that supports it.
* @throws IllegalStateException if no suitable
* {@link HandlerMethodArgumentResolver} is found.
*/
@Override
public Mono<Object> resolveArgument(
MethodParameter parameter, BindingContext bindingContext, ServerWebExchange exchange) {
HandlerMethodArgumentResolver resolver = getArgumentResolver(parameter);
if (resolver == null) {
throw new IllegalArgumentException(
"Unsupported parameter type [" + parameter.getParameterType().getName() + "]." +
" supportsParameter should be called first.");
}
return resolver.resolveArgument(parameter, bindingContext, exchange);
}
/**
* Find a registered {@link HandlerMethodArgumentResolver} that supports
* the given method parameter.
*/
@Nullable
private HandlerMethodArgumentResolver getArgumentResolver(MethodParameter parameter) {
HandlerMethodArgumentResolver result = this.argumentResolverCache.get(parameter);
if (result == null) {
for (HandlerMethodArgumentResolver methodArgumentResolver : this.argumentResolvers) {
if (methodArgumentResolver.supportsParameter(parameter)) {
result = methodArgumentResolver;
this.argumentResolverCache.put(parameter, result);
break;
}
}
}
return result;
}
}

View File

@@ -21,9 +21,7 @@ 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;
import java.util.Optional;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import java.util.stream.Stream;
@@ -62,17 +60,22 @@ public class InvocableHandlerMethod extends HandlerMethod {
private static final Object NO_ARG_VALUE = new Object();
private List<HandlerMethodArgumentResolver> resolvers = new ArrayList<>();
private HandlerMethodArgumentResolverComposite resolvers = new HandlerMethodArgumentResolverComposite();
private ParameterNameDiscoverer parameterNameDiscoverer = new DefaultParameterNameDiscoverer();
private ReactiveAdapterRegistry reactiveAdapterRegistry = ReactiveAdapterRegistry.getSharedInstance();
/**
* Create an instance from a {@code HandlerMethod}.
*/
public InvocableHandlerMethod(HandlerMethod handlerMethod) {
super(handlerMethod);
}
/**
* Create an instance from a bean instance and a method.
*/
public InvocableHandlerMethod(Object bean, Method method) {
super(bean, method);
}
@@ -83,15 +86,14 @@ public class InvocableHandlerMethod extends HandlerMethod {
* argument values against a {@code ServerWebExchange}.
*/
public void setArgumentResolvers(List<HandlerMethodArgumentResolver> resolvers) {
this.resolvers.clear();
this.resolvers.addAll(resolvers);
this.resolvers.addResolvers(resolvers);
}
/**
* Return the configured argument resolvers.
*/
public List<HandlerMethodArgumentResolver> getResolvers() {
return this.resolvers;
return this.resolvers.getResolvers();
}
/**
@@ -133,7 +135,7 @@ public class InvocableHandlerMethod extends HandlerMethod {
public Mono<HandlerResult> invoke(
ServerWebExchange exchange, BindingContext bindingContext, Object... providedArgs) {
return resolveArguments(exchange, bindingContext, providedArgs).flatMap(args -> {
return getMethodArgumentValues(exchange, bindingContext, providedArgs).flatMap(args -> {
Object value;
try {
ReflectionUtils.makeAccessible(getBridgedMethod());
@@ -169,63 +171,54 @@ public class InvocableHandlerMethod extends HandlerMethod {
});
}
private Mono<Object[]> resolveArguments(
private Mono<Object[]> getMethodArgumentValues(
ServerWebExchange exchange, BindingContext bindingContext, Object... providedArgs) {
if (ObjectUtils.isEmpty(getMethodParameters())) {
return EMPTY_ARGS;
}
try {
List<Mono<Object>> argMonos = Stream.of(getMethodParameters())
.map(param -> {
param.initParameterNameDiscovery(this.parameterNameDiscoverer);
return findProvidedArgument(param, providedArgs)
.map(Mono::just)
.orElseGet(() -> {
HandlerMethodArgumentResolver resolver = findResolver(exchange, param);
return resolveArg(resolver, param, bindingContext, exchange);
});
})
.collect(Collectors.toList());
// Create Mono with array of resolved values...
return Mono.zip(argMonos, argValues ->
Stream.of(argValues).map(o -> o != NO_ARG_VALUE ? o : null).toArray());
}
catch (Throwable ex) {
return Mono.error(ex);
MethodParameter[] parameters = getMethodParameters();
List<Mono<Object>> argMonos = new ArrayList<>(parameters.length);
for (MethodParameter parameter : parameters) {
parameter.initParameterNameDiscovery(this.parameterNameDiscoverer);
Object providedArg = findProvidedArgument(parameter, providedArgs);
if (providedArg != null) {
argMonos.add(Mono.just(providedArg));
continue;
}
if (!this.resolvers.supportsParameter(parameter)) {
return Mono.error(new IllegalStateException(
formatArgumentError(parameter, "No suitable resolver")));
}
try {
argMonos.add(this.resolvers.resolveArgument(parameter, bindingContext, exchange)
.defaultIfEmpty(NO_ARG_VALUE)
.doOnError(cause -> logArgumentErrorIfNecessary(exchange, parameter, cause)));
}
catch (Exception ex) {
logArgumentErrorIfNecessary(exchange, parameter, ex);
argMonos.add(Mono.error(ex));
}
}
return Mono.zip(argMonos, values ->
Stream.of(values).map(o -> o != NO_ARG_VALUE ? o : null).toArray());
}
private Optional<Object> findProvidedArgument(MethodParameter parameter, Object... providedArgs) {
if (ObjectUtils.isEmpty(providedArgs)) {
return Optional.empty();
@Nullable
private Object findProvidedArgument(MethodParameter parameter, @Nullable Object... providedArgs) {
if (!ObjectUtils.isEmpty(providedArgs)) {
for (Object providedArg : providedArgs) {
if (parameter.getParameterType().isInstance(providedArg)) {
return providedArg;
}
}
}
return Arrays.stream(providedArgs)
.filter(arg -> parameter.getParameterType().isInstance(arg))
.findFirst();
return null;
}
private HandlerMethodArgumentResolver findResolver(ServerWebExchange exchange, MethodParameter param) {
return this.resolvers.stream()
.filter(r -> r.supportsParameter(param))
.findFirst().orElseThrow(() ->
new IllegalStateException(formatArgumentError(param, "No suitable resolver")));
}
private Mono<Object> resolveArg(HandlerMethodArgumentResolver resolver, MethodParameter parameter,
BindingContext bindingContext, ServerWebExchange exchange) {
try {
return resolver.resolveArgument(parameter, bindingContext, exchange)
.defaultIfEmpty(NO_ARG_VALUE)
.doOnError(cause -> logArgumentErrorIfNecessary(exchange, parameter, cause));
}
catch (Exception ex) {
logArgumentErrorIfNecessary(exchange, parameter, ex);
return Mono.error(ex);
}
private static String formatArgumentError(MethodParameter param, String message) {
return "Could not resolve parameter [" + param.getParameterIndex() + "] in " +
param.getExecutable().toGenericString() + (StringUtils.hasText(message) ? ": " + message : "");
}
private void logArgumentErrorIfNecessary(
@@ -240,11 +233,6 @@ public class InvocableHandlerMethod extends HandlerMethod {
}
}
private static String formatArgumentError(MethodParameter param, String message) {
return "Could not resolve parameter [" + param.getParameterIndex() + "] in " +
param.getExecutable().toGenericString() + (StringUtils.hasText(message) ? ": " + message : "");
}
/**
* Assert that the target bean class is an instance of the class where the given
* method is declared. In some cases the actual controller instance at request-

View File

@@ -35,6 +35,7 @@ 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.method.ResolvableMethod;
import org.springframework.web.reactive.BindingContext;
import org.springframework.web.reactive.HandlerResult;
import org.springframework.web.server.ServerWebExchange;
@@ -45,7 +46,6 @@ import static org.junit.Assert.*;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.*;
import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.*;
import static org.springframework.web.method.ResolvableMethod.*;
/**
* Unit tests for {@link InvocableHandlerMethod}.
@@ -59,96 +59,93 @@ public class InvocableHandlerMethodTests {
@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);
public void invokeAndHandle_VoidWithResponseStatus() {
Method method = ResolvableMethod.on(VoidController.class).mockCall(VoidController::responseStatus).method();
HandlerResult result = invokeForResult(new VoidController(), method);
assertNull("Expected no result (i.e. fully handled)", result);
assertEquals(HttpStatus.BAD_REQUEST, this.exchange.getResponse().getStatusCode());
}
@Test
public void invokeAndHandle_withResponse() throws Exception {
public void invokeAndHandle_withResponse() {
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);
Method method = ResolvableMethod.on(VoidController.class).mockCall(c -> c.response(response)).method();
HandlerResult result = invokeForResult(new VoidController(), method, stubResolver(response));
assertNull("Expected no result (i.e. fully handled)", result);
assertEquals("bar", this.exchange.getResponse().getHeaders().getFirst("foo"));
}
@Test
public void invokeAndHandle_withResponseAndMonoVoid() throws Exception {
public void invokeAndHandle_withResponseAndMonoVoid() {
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);
Method method = ResolvableMethod.on(VoidController.class).mockCall(c -> c.responseMonoVoid(response)).method();
HandlerResult result = invokeForResult(new VoidController(), method, stubResolver(response));
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);
public void invokeAndHandle_withExchange() {
Method method = ResolvableMethod.on(VoidController.class).mockCall(c -> c.exchange(exchange)).method();
HandlerResult result = invokeForResult(new VoidController(), method, stubResolver(this.exchange));
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);
public void invokeAndHandle_withExchangeAndMonoVoid() {
Method method = ResolvableMethod.on(VoidController.class).mockCall(c -> c.exchangeMonoVoid(exchange)).method();
HandlerResult result = invokeForResult(new VoidController(), method, stubResolver(this.exchange));
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 {
public void invokeAndHandle_withNotModified() {
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);
Method method = ResolvableMethod.on(VoidController.class).mockCall(c -> c.notModified(exchange)).method();
HandlerResult result = invokeForResult(new VoidController(), method, stubResolver(exchange));
assertNull("Expected no result (i.e. fully handled)", result);
}
@Test
public void invokeMethodWithNoArguments() throws Exception {
Method method = on(TestController.class).mockCall(TestController::noArgs).method();
public void invokeMethodWithNoArguments() {
Method method = ResolvableMethod.on(TestController.class).mockCall(TestController::noArgs).method();
Mono<HandlerResult> mono = invoke(new TestController(), method);
assertHandlerResultValue(mono, "success");
}
@Test
public void invokeMethodWithNoValue() throws Exception {
Mono<Object> resolvedValue = Mono.empty();
Method method = on(TestController.class).mockCall(o -> o.singleArg(null)).method();
Mono<HandlerResult> mono = invoke(new TestController(), method, resolverFor(resolvedValue));
public void invokeMethodWithNoValue() {
Method method = resolveOn().mockCall(o -> o.singleArg(null)).method();
Mono<HandlerResult> mono = invoke(new TestController(), method, stubResolver(Mono.empty()));
assertHandlerResultValue(mono, "success:null");
}
private ResolvableMethod.Builder<TestController> resolveOn() {
return ResolvableMethod.on(TestController.class);
}
@Test
public void invokeMethodWithValue() throws Exception {
Mono<Object> resolvedValue = Mono.just("value1");
Method method = on(TestController.class).mockCall(o -> o.singleArg(null)).method();
Mono<HandlerResult> mono = invoke(new TestController(), method, resolverFor(resolvedValue));
public void invokeMethodWithValue() {
Method method = ResolvableMethod.on(TestController.class).mockCall(o -> o.singleArg(null)).method();
Mono<HandlerResult> mono = invoke(new TestController(), method, stubResolver("value1"));
assertHandlerResultValue(mono, "success:value1");
}
@Test
public void noMatchingResolver() throws Exception {
Method method = on(TestController.class).mockCall(o -> o.singleArg(null)).method();
public void noMatchingResolver() {
Method method = ResolvableMethod.on(TestController.class).mockCall(o -> o.singleArg(null)).method();
Mono<HandlerResult> mono = invoke(new TestController(), method);
try {
@@ -162,10 +159,10 @@ public class InvocableHandlerMethodTests {
}
@Test
public void resolverThrowsException() throws Exception {
Mono<Object> resolvedValue = Mono.error(new UnsupportedMediaTypeStatusException("boo"));
Method method = on(TestController.class).mockCall(o -> o.singleArg(null)).method();
Mono<HandlerResult> mono = invoke(new TestController(), method, resolverFor(resolvedValue));
public void resolverThrowsException() {
Method method = ResolvableMethod.on(TestController.class).mockCall(o -> o.singleArg(null)).method();
Mono<HandlerResult> mono = invoke(new TestController(), method,
stubResolver(Mono.error(new UnsupportedMediaTypeStatusException("boo"))));
try {
mono.block();
@@ -177,10 +174,9 @@ public class InvocableHandlerMethodTests {
}
@Test
public void illegalArgumentException() throws Exception {
Mono<Object> resolvedValue = Mono.just(1);
Method method = on(TestController.class).mockCall(o -> o.singleArg(null)).method();
Mono<HandlerResult> mono = invoke(new TestController(), method, resolverFor(resolvedValue));
public void illegalArgumentException() {
Method method = ResolvableMethod.on(TestController.class).mockCall(o -> o.singleArg(null)).method();
Mono<HandlerResult> mono = invoke(new TestController(), method, stubResolver(1));
try {
mono.block();
@@ -197,8 +193,8 @@ public class InvocableHandlerMethodTests {
}
@Test
public void invocationTargetExceptionIsUnwrapped() throws Exception {
Method method = on(TestController.class).mockCall(TestController::exceptionMethod).method();
public void invocationTargetExceptionIsUnwrapped() {
Method method = ResolvableMethod.on(TestController.class).mockCall(TestController::exceptionMethod).method();
Mono<HandlerResult> mono = invoke(new TestController(), method);
try {
@@ -211,8 +207,8 @@ public class InvocableHandlerMethodTests {
}
@Test
public void invokeMethodWithResponseStatus() throws Exception {
Method method = on(TestController.class).annotPresent(ResponseStatus.class).resolveMethod();
public void invokeMethodWithResponseStatus() {
Method method = ResolvableMethod.on(TestController.class).annotPresent(ResponseStatus.class).resolveMethod();
Mono<HandlerResult> mono = invoke(new TestController(), method);
assertHandlerResultValue(mono, "created");
@@ -220,30 +216,31 @@ public class InvocableHandlerMethodTests {
}
private Mono<HandlerResult> invoke(Object handler, Method method) {
return invoke(handler, method, new HandlerMethodArgumentResolver[0]);
@Nullable
private HandlerResult invokeForResult(Object handler, Method method, HandlerMethodArgumentResolver... resolvers) {
return invoke(handler, method, resolvers).block(Duration.ZERO);
}
private Mono<HandlerResult> invoke(Object handler, Method method,
HandlerMethodArgumentResolver... resolver) {
InvocableHandlerMethod hm = new InvocableHandlerMethod(handler, method);
hm.setArgumentResolvers(Arrays.asList(resolver));
return hm.invoke(this.exchange, new BindingContext());
private Mono<HandlerResult> invoke(Object handler, Method method, HandlerMethodArgumentResolver... resolvers) {
InvocableHandlerMethod invocable = new InvocableHandlerMethod(handler, method);
invocable.setArgumentResolvers(Arrays.asList(resolvers));
return invocable.invoke(this.exchange, new BindingContext());
}
private <T> HandlerMethodArgumentResolver resolverFor(Mono<Object> resolvedValue) {
private <T> HandlerMethodArgumentResolver stubResolver(Object stubValue) {
return stubResolver(Mono.just(stubValue));
}
private <T> HandlerMethodArgumentResolver stubResolver(Mono<Object> stubValue) {
HandlerMethodArgumentResolver resolver = mock(HandlerMethodArgumentResolver.class);
when(resolver.supportsParameter(any())).thenReturn(true);
when(resolver.resolveArgument(any(), any(), any())).thenReturn(resolvedValue);
when(resolver.resolveArgument(any(), any(), any())).thenReturn(stubValue);
return resolver;
}
private void assertHandlerResultValue(Mono<HandlerResult> mono, String expected) {
StepVerifier.create(mono)
.consumeNextWith(result -> {
assertEquals(expected, result.getReturnValue());
})
.consumeNextWith(result -> assertEquals(expected, result.getReturnValue()))
.expectComplete()
.verify();
}