Skip to content

Commit

Permalink
Consistent InvocableHandlerMethod implementations
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@rstoyanchev
rstoyanchev committed Oct 30, 2018
1 parent 991e9f4 commit 7c36549
Show file tree
Hide file tree
Showing 14 changed files with 790 additions and 381 deletions.
22 changes: 13 additions & 9 deletions .../springframework/messaging/handler/invocation/HandlerMethodArgumentResolverComposite.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,7 @@ public HandlerMethodArgumentResolverComposite addResolver(HandlerMethodArgumentR
*/
public HandlerMethodArgumentResolverComposite addResolvers(@Nullable HandlerMethodArgumentResolver... resolvers) {
if (resolvers != null) {
for (HandlerMethodArgumentResolver resolver : resolvers) {
this.argumentResolvers.add(resolver);
}
Collections.addAll(this.argumentResolvers, resolvers);
}
return this;
}
Expand Down Expand Up @@ -92,30 +90,36 @@ public void clear() {


/**
* Whether the given {@linkplain MethodParameter method parameter} is supported by any registered
* {@link HandlerMethodArgumentResolver}.
* 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.
* Iterate over registered
* {@link HandlerMethodArgumentResolver HandlerMethodArgumentResolvers}
* and invoke the one that supports it.
* @throws IllegalStateException if no suitable
* {@link HandlerMethodArgumentResolver} is found.
*/
@Override
@Nullable
public Object resolveArgument(MethodParameter parameter, Message<?> message) throws Exception {
HandlerMethodArgumentResolver resolver = getArgumentResolver(parameter);
if (resolver == null) {
throw new IllegalStateException("Unknown parameter type [" + parameter.getParameterType().getName() + "]");
throw new IllegalStateException(
"Unsupported parameter type [" + parameter.getParameterType().getName() + "]." +
" supportsParameter should be called first.");
}
return resolver.resolveArgument(parameter, message);
}

/**
* Find a registered {@link HandlerMethodArgumentResolver} that supports the given method parameter.
* Find a registered {@link HandlerMethodArgumentResolver} that supports
* the given method parameter.
*/
@Nullable
private HandlerMethodArgumentResolver getArgumentResolver(MethodParameter parameter) {
Expand Down
127 changes: 56 additions & 71 deletions ...rc/main/java/org/springframework/messaging/handler/invocation/InvocableHandlerMethod.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import java.lang.reflect.Method;
import java.lang.reflect.Type;
import java.util.Arrays;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

import org.springframework.core.DefaultParameterNameDiscoverer;
import org.springframework.core.MethodParameter;
Expand All @@ -28,22 +30,25 @@
import org.springframework.lang.Nullable;
import org.springframework.messaging.Message;
import org.springframework.messaging.handler.HandlerMethod;
import org.springframework.util.ClassUtils;
import org.springframework.util.ObjectUtils;
import org.springframework.util.ReflectionUtils;
import org.springframework.util.StringUtils;

/**
* Provides a method for invoking the handler method for a given message after resolving its
* method argument values through registered {@link HandlerMethodArgumentResolver HandlerMethodArgumentResolvers}.
*
* <p>Use {@link #setMessageMethodArgumentResolvers} to customize the list of argument resolvers.
* Extension of {@link HandlerMethod} that invokes the underlying method with
* argument values resolved from the current HTTP request through a list of
* {@link HandlerMethodArgumentResolver}.
*
* @author Rossen Stoyanchev
* @author Juergen Hoeller
* @since 4.0
*/
public class InvocableHandlerMethod extends HandlerMethod {

private HandlerMethodArgumentResolverComposite argumentResolvers = new HandlerMethodArgumentResolverComposite();
private static final Object[] EMPTY_ARGS = new Object[0];


private HandlerMethodArgumentResolverComposite resolvers = new HandlerMethodArgumentResolverComposite();

private ParameterNameDiscoverer parameterNameDiscoverer = new DefaultParameterNameDiscoverer();

Expand Down Expand Up @@ -80,7 +85,7 @@ public InvocableHandlerMethod(Object bean, String methodName, Class<?>... parame
* Set {@link HandlerMethodArgumentResolver HandlerMethodArgumentResolvers} to use to use for resolving method argument values.
*/
public void setMessageMethodArgumentResolvers(HandlerMethodArgumentResolverComposite argumentResolvers) {
this.argumentResolvers = argumentResolvers;
this.resolvers = argumentResolvers;
}

/**
Expand Down Expand Up @@ -113,15 +118,9 @@ public void setParameterNameDiscoverer(ParameterNameDiscoverer parameterNameDisc
public Object invoke(Message<?> message, Object... providedArgs) throws Exception {
Object[] args = getMethodArgumentValues(message, providedArgs);
if (logger.isTraceEnabled()) {
logger.trace("Invoking '" + ClassUtils.getQualifiedMethodName(getMethod(), getBeanType()) +
"' with arguments " + Arrays.toString(args));
logger.trace("Arguments: " + Arrays.toString(args));
}
Object returnValue = doInvoke(args);
if (logger.isTraceEnabled()) {
logger.trace("Method [" + ClassUtils.getQualifiedMethodName(getMethod(), getBeanType()) +
"] returned [" + returnValue + "]");
}
return returnValue;
return doInvoke(args);
}

/**
Expand All @@ -131,53 +130,55 @@ public Object invoke(Message<?> message, Object... providedArgs) throws Exceptio
* @since 5.1.2
*/
protected Object[] getMethodArgumentValues(Message<?> message, Object... providedArgs) throws Exception {
if (ObjectUtils.isEmpty(getMethodParameters())) {
return EMPTY_ARGS;
}
MethodParameter[] parameters = getMethodParameters();
Object[] args = new Object[parameters.length];
for (int i = 0; i < parameters.length; i++) {
MethodParameter parameter = parameters[i];
parameter.initParameterNameDiscovery(this.parameterNameDiscoverer);
args[i] = resolveProvidedArgument(parameter, providedArgs);
args[i] = findProvidedArgument(parameter, providedArgs);
if (args[i] != null) {
continue;
}
if (this.argumentResolvers.supportsParameter(parameter)) {
try {
args[i] = this.argumentResolvers.resolveArgument(parameter, message);
continue;
}
catch (Exception ex) {
if (logger.isDebugEnabled()) {
logger.debug(getArgumentResolutionErrorMessage("Failed to resolve", i), ex);
if (!this.resolvers.supportsParameter(parameter)) {
throw new MethodArgumentResolutionException(
message, parameter, formatArgumentError(parameter, "No suitable resolver"));
}
try {
args[i] = this.resolvers.resolveArgument(parameter, message);
}
catch (Exception ex) {
// Leave stack trace for later, exception may actually be resolved and handled..
if (logger.isDebugEnabled()) {
String error = ex.getMessage();
if (error != null && !error.contains(parameter.getExecutable().toGenericString())) {
logger.debug(formatArgumentError(parameter, error));
}
throw ex;
}
}
if (args[i] == null) {
throw new MethodArgumentResolutionException(message, parameter,
getArgumentResolutionErrorMessage("No suitable resolver for", i));
throw ex;
}
}
return args;
}

private String getArgumentResolutionErrorMessage(String text, int index) {
Class<?> paramType = getMethodParameters()[index].getParameterType();
return text + " argument " + index + " of type '" + paramType.getName() + "'";
}

/**
* Attempt to resolve a method parameter from the list of provided argument values.
*/
@Nullable
private Object resolveProvidedArgument(MethodParameter parameter, Object... providedArgs) {
for (Object providedArg : providedArgs) {
if (parameter.getParameterType().isInstance(providedArg)) {
return providedArg;
private Object findProvidedArgument(MethodParameter parameter, @Nullable Object... providedArgs) {
if (!ObjectUtils.isEmpty(providedArgs)) {
for (Object providedArg : providedArgs) {
if (parameter.getParameterType().isInstance(providedArg)) {
return providedArg;
}
}
}
return null;
}

private static String formatArgumentError(MethodParameter param, String message) {
return "Could not resolve parameter [" + param.getParameterIndex() + "] in " +
param.getExecutable().toGenericString() + (StringUtils.hasText(message) ? ": " + message : "");
}

/**
* Invoke the handler method with the given argument values.
Expand All @@ -191,7 +192,7 @@ protected Object doInvoke(Object... args) throws Exception {
catch (IllegalArgumentException ex) {
assertTargetBean(getBridgedMethod(), getBean(), args);
String text = (ex.getMessage() != null ? ex.getMessage() : "Illegal argument");
throw new IllegalStateException(getInvocationErrorMessage(text, args), ex);
throw new IllegalStateException(formatInvokeError(text, args), ex);
}
catch (InvocationTargetException ex) {
// Unwrap for HandlerExceptionResolvers ...
Expand All @@ -206,8 +207,7 @@ else if (targetException instanceof Exception) {
throw (Exception) targetException;
}
else {
String text = getInvocationErrorMessage("Failed to invoke handler method", args);
throw new IllegalStateException(text, targetException);
throw new IllegalStateException(formatInvokeError("Invocation failure", args), targetException);
}
}
}
Expand All @@ -227,38 +227,23 @@ private void assertTargetBean(Method method, Object targetBean, Object[] args) {
"' is not an instance of the actual endpoint bean class '" +
targetBeanClass.getName() + "'. If the endpoint requires proxying " +
"(e.g. due to @Transactional), please use class-based proxying.";
throw new IllegalStateException(getInvocationErrorMessage(text, args));
throw new IllegalStateException(formatInvokeError(text, args));
}
}

private String getInvocationErrorMessage(String text, Object[] resolvedArgs) {
StringBuilder sb = new StringBuilder(getDetailedErrorMessage(text));
sb.append("Resolved arguments: \n");
for (int i = 0; i < resolvedArgs.length; i++) {
sb.append("[").append(i).append("] ");
if (resolvedArgs[i] == null) {
sb.append("[null] \n");
}
else {
sb.append("[type=").append(resolvedArgs[i].getClass().getName()).append("] ");
sb.append("[value=").append(resolvedArgs[i]).append("]\n");
}
}
return sb.toString();
}
private String formatInvokeError(String text, Object[] args) {

/**
* Adds HandlerMethod details such as the bean type and method signature to the message.
* @param text error message to append the HandlerMethod details to
*/
protected String getDetailedErrorMessage(String text) {
StringBuilder sb = new StringBuilder(text).append("\n");
sb.append("HandlerMethod details: \n");
sb.append("Endpoint [").append(getBeanType().getName()).append("]\n");
sb.append("Method [").append(getBridgedMethod().toGenericString()).append("]\n");
return sb.toString();
}
String formattedArgs = IntStream.range(0, args.length)
.mapToObj(i -> (args[i] != null ?
"[" + i + "] [type=" + args[i].getClass().getName() + "] [value=" + args[i] + "]" :
"[" + i + "] [null]"))
.collect(Collectors.joining(",\n", " ", " "));

return text + "\n" +
"Endpoint [" + getBeanType().getName() + "]\n" +
"Method [" + getBridgedMethod().toGenericString() + "] " +
"with argument values:\n" + formattedArgs;
}

MethodParameter getAsyncReturnValueType(@Nullable Object returnValue) {
return new AsyncResultMethodParameter(returnValue);
Expand Down
4 changes: 2 additions & 2 deletions ...amework/messaging/handler/annotation/support/DefaultMessageHandlerMethodFactoryTests.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2017 the original author or authors.
* 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.
Expand Down Expand Up @@ -149,7 +149,7 @@ public void overrideArgumentResolvers() throws Exception {
createInvocableHandlerMethod(instance, "simpleString", String.class);

thrown.expect(MethodArgumentResolutionException.class);
thrown.expectMessage("No suitable resolver for");
thrown.expectMessage("No suitable resolver");
invocableHandlerMethod2.invoke(message);
}

Expand Down
Loading

0 comments on commit 7c36549

Please sign in to comment.