diff --git a/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/HandlerMethodArgumentResolverComposite.java b/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/HandlerMethodArgumentResolverComposite.java index a92c11ba65..04f5abd9c7 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/HandlerMethodArgumentResolverComposite.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/HandlerMethodArgumentResolverComposite.java @@ -57,9 +57,7 @@ public class HandlerMethodArgumentResolverComposite implements HandlerMethodArgu */ public HandlerMethodArgumentResolverComposite addResolvers(@Nullable HandlerMethodArgumentResolver... resolvers) { if (resolvers != null) { - for (HandlerMethodArgumentResolver resolver : resolvers) { - this.argumentResolvers.add(resolver); - } + Collections.addAll(this.argumentResolvers, resolvers); } return this; } @@ -92,8 +90,8 @@ public class HandlerMethodArgumentResolverComposite implements HandlerMethodArgu /** - * 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) { @@ -101,21 +99,27 @@ public class HandlerMethodArgumentResolverComposite implements HandlerMethodArgu } /** - * 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) { diff --git a/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/InvocableHandlerMethod.java b/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/InvocableHandlerMethod.java index 5e15c24041..ebea311754 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/InvocableHandlerMethod.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/InvocableHandlerMethod.java @@ -20,6 +20,8 @@ import java.lang.reflect.InvocationTargetException; 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; @@ -28,14 +30,14 @@ import org.springframework.core.ResolvableType; 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}. - * - *

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 @@ -43,7 +45,10 @@ import org.springframework.util.ReflectionUtils; */ 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(); @@ -80,7 +85,7 @@ public class InvocableHandlerMethod extends HandlerMethod { * Set {@link HandlerMethodArgumentResolver HandlerMethodArgumentResolvers} to use to use for resolving method argument values. */ public void setMessageMethodArgumentResolvers(HandlerMethodArgumentResolverComposite argumentResolvers) { - this.argumentResolvers = argumentResolvers; + this.resolvers = argumentResolvers; } /** @@ -113,15 +118,9 @@ public class InvocableHandlerMethod extends HandlerMethod { 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); } /** @@ -131,53 +130,55 @@ public class InvocableHandlerMethod extends HandlerMethod { * @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); - } - throw ex; - } + if (!this.resolvers.supportsParameter(parameter)) { + throw new MethodArgumentResolutionException( + message, parameter, formatArgumentError(parameter, "No suitable resolver")); } - if (args[i] == null) { - throw new MethodArgumentResolutionException(message, parameter, - getArgumentResolutionErrorMessage("No suitable resolver for", i)); + 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; } } 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. @@ -191,7 +192,7 @@ public class InvocableHandlerMethod extends HandlerMethod { 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 ... @@ -206,8 +207,7 @@ public class InvocableHandlerMethod extends HandlerMethod { 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); } } } @@ -227,38 +227,23 @@ public class InvocableHandlerMethod extends HandlerMethod { "' 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); diff --git a/spring-messaging/src/test/java/org/springframework/messaging/handler/annotation/support/DefaultMessageHandlerMethodFactoryTests.java b/spring-messaging/src/test/java/org/springframework/messaging/handler/annotation/support/DefaultMessageHandlerMethodFactoryTests.java index 71579f88de..e7f6a05158 100644 --- a/spring-messaging/src/test/java/org/springframework/messaging/handler/annotation/support/DefaultMessageHandlerMethodFactoryTests.java +++ b/spring-messaging/src/test/java/org/springframework/messaging/handler/annotation/support/DefaultMessageHandlerMethodFactoryTests.java @@ -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. @@ -149,7 +149,7 @@ public class DefaultMessageHandlerMethodFactoryTests { createInvocableHandlerMethod(instance, "simpleString", String.class); thrown.expect(MethodArgumentResolutionException.class); - thrown.expectMessage("No suitable resolver for"); + thrown.expectMessage("No suitable resolver"); invocableHandlerMethod2.invoke(message); } diff --git a/spring-messaging/src/test/java/org/springframework/messaging/handler/invocation/InvocableHandlerMethodTests.java b/spring-messaging/src/test/java/org/springframework/messaging/handler/invocation/InvocableHandlerMethodTests.java new file mode 100644 index 0000000000..ef19257319 --- /dev/null +++ b/spring-messaging/src/test/java/org/springframework/messaging/handler/invocation/InvocableHandlerMethodTests.java @@ -0,0 +1,231 @@ +/* + * Copyright 2002-2017 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.messaging.handler.invocation; + +import java.lang.reflect.Method; + +import org.junit.Before; +import org.junit.Test; + +import org.springframework.core.MethodParameter; +import org.springframework.messaging.Message; +import org.springframework.util.ClassUtils; + +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; + +/** + * Unit tests for {@link InvocableHandlerMethod}. + * + * @author Rossen Stoyanchev + */ +public class InvocableHandlerMethodTests { + + private Message message; + + private final HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite(); + + + @Before + public void setUp() { + this.message = null; + } + + + @Test + public void resolveArg() throws Exception { + this.composite.addResolver(new StubArgumentResolver(99)); + this.composite.addResolver(new StubArgumentResolver("value")); + + Object value = getInvocable("handle", Integer.class, String.class).invoke(this.message); + + assertEquals(1, getStubResolver(0).getResolvedParameters().size()); + assertEquals(1, getStubResolver(1).getResolvedParameters().size()); + assertEquals("99-value", value); + assertEquals("intArg", getStubResolver(0).getResolvedParameters().get(0).getParameterName()); + assertEquals("stringArg", getStubResolver(1).getResolvedParameters().get(0).getParameterName()); + } + + @Test + public void resolveNullArg() throws Exception { + this.composite.addResolver(new StubArgumentResolver(Integer.class)); + this.composite.addResolver(new StubArgumentResolver(String.class)); + + Object returnValue = getInvocable("handle", Integer.class, String.class).invoke(this.message); + + assertEquals(1, getStubResolver(0).getResolvedParameters().size()); + assertEquals(1, getStubResolver(1).getResolvedParameters().size()); + assertEquals("null-null", returnValue); + } + + @Test + public void cannotResolveArg() throws Exception { + try { + getInvocable("handle", Integer.class, String.class).invoke(this.message); + fail("Expected exception"); + } + catch (MethodArgumentResolutionException ex) { + assertNotNull(ex.getMessage()); + assertTrue(ex.getMessage().contains("Could not resolve parameter [0]")); + } + } + + @Test + public void resolveProvidedArg() throws Exception { + Object value = getInvocable("handle", Integer.class, String.class).invoke(this.message, 99, "value"); + + assertNotNull(value); + assertEquals(String.class, value.getClass()); + assertEquals("99-value", value); + } + + @Test + public void resolveProvidedArgFirst() throws Exception { + this.composite.addResolver(new StubArgumentResolver(1)); + this.composite.addResolver(new StubArgumentResolver("value1")); + Object value = getInvocable("handle", Integer.class, String.class).invoke(this.message, 2, "value2"); + + assertEquals("2-value2", value); + } + + @Test + public void exceptionInResolvingArg() throws Exception { + this.composite.addResolver(new ExceptionRaisingArgumentResolver()); + try { + getInvocable("handle", Integer.class, String.class).invoke(this.message); + fail("Expected exception"); + } + catch (IllegalArgumentException ex) { + // expected - allow HandlerMethodArgumentResolver exceptions to propagate + } + } + + @Test + public void illegalArgumentException() throws Exception { + this.composite.addResolver(new StubArgumentResolver(Integer.class, "__not_an_int__")); + this.composite.addResolver(new StubArgumentResolver("value")); + try { + getInvocable("handle", Integer.class, String.class).invoke(this.message); + fail("Expected exception"); + } + catch (IllegalStateException ex) { + assertNotNull("Exception not wrapped", ex.getCause()); + assertTrue(ex.getCause() instanceof IllegalArgumentException); + assertTrue(ex.getMessage().contains("Endpoint [")); + assertTrue(ex.getMessage().contains("Method [")); + assertTrue(ex.getMessage().contains("with argument values:")); + assertTrue(ex.getMessage().contains("[0] [type=java.lang.String] [value=__not_an_int__]")); + assertTrue(ex.getMessage().contains("[1] [type=java.lang.String] [value=value")); + } + } + + @Test + public void invocationTargetException() throws Exception { + Throwable expected = new RuntimeException("error"); + try { + getInvocable("handleWithException", Throwable.class).invoke(this.message, expected); + fail("Expected exception"); + } + catch (RuntimeException actual) { + assertSame(expected, actual); + } + + expected = new Error("error"); + try { + getInvocable("handleWithException", Throwable.class).invoke(this.message, expected); + fail("Expected exception"); + } + catch (Error actual) { + assertSame(expected, actual); + } + + expected = new Exception("error"); + try { + getInvocable("handleWithException", Throwable.class).invoke(this.message, expected); + fail("Expected exception"); + } + catch (Exception actual) { + assertSame(expected, actual); + } + + expected = new Throwable("error"); + try { + getInvocable("handleWithException", Throwable.class).invoke(this.message, expected); + fail("Expected exception"); + } + catch (IllegalStateException actual) { + assertNotNull(actual.getCause()); + assertSame(expected, actual.getCause()); + assertTrue(actual.getMessage().contains("Invocation failure")); + } + } + + @Test // Based on SPR-13917 (spring-web) + public void invocationErrorMessage() throws Exception { + this.composite.addResolver(new StubArgumentResolver(double.class)); + try { + getInvocable("handle", double.class).invoke(this.message); + fail(); + } + catch (IllegalStateException ex) { + assertThat(ex.getMessage(), containsString("Illegal argument")); + } + } + + public InvocableHandlerMethod getInvocable(String methodName, Class... argTypes) { + Method method = ClassUtils.getMethod(Handler.class, methodName, argTypes); + InvocableHandlerMethod handlerMethod = new InvocableHandlerMethod(new Handler(), method); + handlerMethod.setMessageMethodArgumentResolvers(this.composite); + return handlerMethod; + } + + private StubArgumentResolver getStubResolver(int index) { + return (StubArgumentResolver) this.composite.getResolvers().get(index); + } + + + + @SuppressWarnings("unused") + private static class Handler { + + public String handle(Integer intArg, String stringArg) { + return intArg + "-" + stringArg; + } + + public void handle(double amount) { + } + + public void handleWithException(Throwable ex) throws Throwable { + throw ex; + } + } + + + private static class ExceptionRaisingArgumentResolver implements HandlerMethodArgumentResolver { + + @Override + public boolean supportsParameter(MethodParameter parameter) { + return true; + } + + @Override + public Object resolveArgument(MethodParameter parameter, Message message) { + throw new IllegalArgumentException("oops, can't read"); + } + } + +} diff --git a/spring-messaging/src/test/java/org/springframework/messaging/handler/invocation/StubArgumentResolver.java b/spring-messaging/src/test/java/org/springframework/messaging/handler/invocation/StubArgumentResolver.java new file mode 100644 index 0000000000..a259f9613f --- /dev/null +++ b/spring-messaging/src/test/java/org/springframework/messaging/handler/invocation/StubArgumentResolver.java @@ -0,0 +1,71 @@ +/* + * 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.messaging.handler.invocation; + +import java.util.ArrayList; +import java.util.List; + +import org.springframework.core.MethodParameter; +import org.springframework.lang.Nullable; +import org.springframework.messaging.Message; + +/** + * Stub resolver for a fixed value type and/or value. + * + * @author Rossen Stoyanchev + */ +public class StubArgumentResolver implements HandlerMethodArgumentResolver { + + private final Class valueType; + + @Nullable + private final Object value; + + private List resolvedParameters = new ArrayList<>(); + + + public StubArgumentResolver(Object value) { + this(value.getClass(), value); + } + + public StubArgumentResolver(Class valueType) { + this(valueType, null); + } + + public StubArgumentResolver(Class valueType, Object value) { + this.valueType = valueType; + this.value = value; + } + + + public List getResolvedParameters() { + return resolvedParameters; + } + + + @Override + public boolean supportsParameter(MethodParameter parameter) { + return parameter.getParameterType().equals(this.valueType); + } + + @Override + public Object resolveArgument(MethodParameter parameter, Message message) { + this.resolvedParameters.add(parameter); + return this.value; + } + +} diff --git a/spring-web/src/main/java/org/springframework/web/method/support/HandlerMethodArgumentResolverComposite.java b/spring-web/src/main/java/org/springframework/web/method/support/HandlerMethodArgumentResolverComposite.java index e5b5a4c132..f3106dcaec 100644 --- a/spring-web/src/main/java/org/springframework/web/method/support/HandlerMethodArgumentResolverComposite.java +++ b/spring-web/src/main/java/org/springframework/web/method/support/HandlerMethodArgumentResolverComposite.java @@ -31,7 +31,8 @@ import org.springframework.web.bind.support.WebDataBinderFactory; import org.springframework.web.context.request.NativeWebRequest; /** - * Resolves method parameters by delegating to a list of registered {@link HandlerMethodArgumentResolver HandlerMethodArgumentResolvers}. + * 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 @@ -62,9 +63,7 @@ public class HandlerMethodArgumentResolverComposite implements HandlerMethodArgu */ public HandlerMethodArgumentResolverComposite addResolvers(@Nullable HandlerMethodArgumentResolver... resolvers) { if (resolvers != null) { - for (HandlerMethodArgumentResolver resolver : resolvers) { - this.argumentResolvers.add(resolver); - } + Collections.addAll(this.argumentResolvers, resolvers); } return this; } @@ -76,9 +75,7 @@ public class HandlerMethodArgumentResolverComposite implements HandlerMethodArgu @Nullable List resolvers) { if (resolvers != null) { - for (HandlerMethodArgumentResolver resolver : resolvers) { - this.argumentResolvers.add(resolver); - } + this.argumentResolvers.addAll(resolvers); } return this; } @@ -100,17 +97,20 @@ public class HandlerMethodArgumentResolverComposite implements HandlerMethodArgu /** - * 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); + 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 @@ -119,13 +119,16 @@ public class HandlerMethodArgumentResolverComposite implements HandlerMethodArgu HandlerMethodArgumentResolver resolver = getArgumentResolver(parameter); if (resolver == null) { - throw new IllegalArgumentException("Unknown parameter type [" + parameter.getParameterType().getName() + "]"); + throw new IllegalArgumentException( + "Unsupported parameter type [" + parameter.getParameterType().getName() + "]." + + " supportsParameter should be called first."); } return resolver.resolveArgument(parameter, mavContainer, webRequest, binderFactory); } /** - * 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) { diff --git a/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java b/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java index 5a21b13801..d3e7c3679f 100644 --- a/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java +++ b/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java @@ -26,6 +26,7 @@ import org.springframework.core.DefaultParameterNameDiscoverer; import org.springframework.core.MethodParameter; import org.springframework.core.ParameterNameDiscoverer; import org.springframework.lang.Nullable; +import org.springframework.util.ObjectUtils; import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; import org.springframework.web.bind.WebDataBinder; @@ -35,14 +36,9 @@ import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.method.HandlerMethod; /** - * Provides a method for invoking the handler method for a given request after resolving its - * method argument values through registered {@link HandlerMethodArgumentResolver HandlerMethodArgumentResolvers}. - * - *

Argument resolution often requires a {@link WebDataBinder} for data binding or for type - * conversion. Use the {@link #setDataBinderFactory(WebDataBinderFactory)} property to supply - * a binder factory to pass to argument resolvers. - * - *

Use {@link #setHandlerMethodArgumentResolvers} 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 @@ -50,10 +46,13 @@ import org.springframework.web.method.HandlerMethod; */ public class InvocableHandlerMethod extends HandlerMethod { + private static final Object[] EMPTY_ARGS = new Object[0]; + + @Nullable private WebDataBinderFactory dataBinderFactory; - private HandlerMethodArgumentResolverComposite argumentResolvers = new HandlerMethodArgumentResolverComposite(); + private HandlerMethodArgumentResolverComposite resolvers = new HandlerMethodArgumentResolverComposite(); private ParameterNameDiscoverer parameterNameDiscoverer = new DefaultParameterNameDiscoverer(); @@ -99,7 +98,7 @@ public class InvocableHandlerMethod extends HandlerMethod { * Set {@link HandlerMethodArgumentResolver HandlerMethodArgumentResolvers} to use to use for resolving method argument values. */ public void setHandlerMethodArgumentResolvers(HandlerMethodArgumentResolverComposite argumentResolvers) { - this.argumentResolvers = argumentResolvers; + this.resolvers = argumentResolvers; } /** @@ -151,64 +150,59 @@ public class InvocableHandlerMethod extends HandlerMethod { protected Object[] getMethodArgumentValues(NativeWebRequest request, @Nullable ModelAndViewContainer mavContainer, 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, mavContainer, request, this.dataBinderFactory); - continue; - } - catch (Exception ex) { - // Leave stack trace for later, e.g. AbstractHandlerExceptionResolver - if (logger.isDebugEnabled()) { - String message = ex.getMessage(); - if (message != null && !message.contains(parameter.getExecutable().toGenericString())) { - logger.debug(formatArgumentError(parameter, message)); - } - } - throw ex; - } - } - if (args[i] == null) { + if (!this.resolvers.supportsParameter(parameter)) { throw new IllegalStateException(formatArgumentError(parameter, "No suitable resolver")); } + try { + args[i] = this.resolvers.resolveArgument(parameter, mavContainer, request, this.dataBinderFactory); + } + 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; + } } return args; } + @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 null; + } + private static String formatArgumentError(MethodParameter param, String message) { return "Could not resolve parameter [" + param.getParameterIndex() + "] in " + param.getExecutable().toGenericString() + (StringUtils.hasText(message) ? ": " + message : ""); } - /** - * Attempt to resolve a method parameter from the list of provided argument values. - */ - @Nullable - private Object resolveProvidedArgument(MethodParameter parameter, @Nullable Object... providedArgs) { - if (providedArgs == null) { - return null; - } - for (Object providedArg : providedArgs) { - if (parameter.getParameterType().isInstance(providedArg)) { - return providedArg; - } - } - return null; - } - - /** * Invoke the handler method with the given argument values. */ + @Nullable protected Object doInvoke(Object... args) throws Exception { ReflectionUtils.makeAccessible(getBridgedMethod()); try { diff --git a/spring-web/src/test/java/org/springframework/web/method/ResolvableMethod.java b/spring-web/src/test/java/org/springframework/web/method/ResolvableMethod.java index c0daebb69c..4d50d03bb1 100644 --- a/spring-web/src/test/java/org/springframework/web/method/ResolvableMethod.java +++ b/spring-web/src/test/java/org/springframework/web/method/ResolvableMethod.java @@ -246,7 +246,7 @@ public class ResolvableMethod { /** - * Main entry point providing access to a {@code ResolvableMethod} builder. + * Create a {@code ResolvableMethod} builder for the given handler class. */ public static Builder on(Class objectClass) { return new Builder<>(objectClass); @@ -281,6 +281,16 @@ public class ResolvableMethod { return this; } + /** + * Filter on methods with the given parameter types. + */ + public Builder argTypes(Class... argTypes) { + addFilter("argTypes=" + Arrays.toString(argTypes), m -> + ObjectUtils.isEmpty(argTypes) ? + m.getParameterTypes().length == 0 : Arrays.equals(m.getParameterTypes(), argTypes)); + return this; + } + /** * Filter on annotated methods. * See {@link MvcAnnotationPredicates}. diff --git a/spring-web/src/test/java/org/springframework/web/method/support/HandlerMethodArgumentResolverCompositeTests.java b/spring-web/src/test/java/org/springframework/web/method/support/HandlerMethodArgumentResolverCompositeTests.java index b2d0a8b6bf..c161a18ad8 100644 --- a/spring-web/src/test/java/org/springframework/web/method/support/HandlerMethodArgumentResolverCompositeTests.java +++ b/spring-web/src/test/java/org/springframework/web/method/support/HandlerMethodArgumentResolverCompositeTests.java @@ -32,56 +32,53 @@ import static org.junit.Assert.*; */ public class HandlerMethodArgumentResolverCompositeTests { - private HandlerMethodArgumentResolverComposite resolvers; + private HandlerMethodArgumentResolverComposite resolverComposite; private MethodParameter paramInt; private MethodParameter paramStr; + @Before public void setUp() throws Exception { - resolvers = new HandlerMethodArgumentResolverComposite(); + this.resolverComposite = new HandlerMethodArgumentResolverComposite(); Method method = getClass().getDeclaredMethod("handle", Integer.class, String.class); paramInt = new MethodParameter(method, 0); paramStr = new MethodParameter(method, 1); } - @Test - public void supportsParameter() throws Exception { - registerResolver(Integer.class, null); - assertTrue(this.resolvers.supportsParameter(paramInt)); - assertFalse(this.resolvers.supportsParameter(paramStr)); + @Test + public void supportsParameter() { + this.resolverComposite.addResolver(new StubArgumentResolver(Integer.class)); + + assertTrue(this.resolverComposite.supportsParameter(paramInt)); + assertFalse(this.resolverComposite.supportsParameter(paramStr)); } @Test public void resolveArgument() throws Exception { - registerResolver(Integer.class, Integer.valueOf(55)); - Object resolvedValue = this.resolvers.resolveArgument(paramInt, null, null, null); + this.resolverComposite.addResolver(new StubArgumentResolver(55)); + Object resolvedValue = this.resolverComposite.resolveArgument(paramInt, null, null, null); - assertEquals(Integer.valueOf(55), resolvedValue); + assertEquals(55, resolvedValue); } @Test public void checkArgumentResolverOrder() throws Exception { - registerResolver(Integer.class, Integer.valueOf(1)); - registerResolver(Integer.class, Integer.valueOf(2)); - Object resolvedValue = this.resolvers.resolveArgument(paramInt, null, null, null); + this.resolverComposite.addResolver(new StubArgumentResolver(1)); + this.resolverComposite.addResolver(new StubArgumentResolver(2)); + Object resolvedValue = this.resolverComposite.resolveArgument(paramInt, null, null, null); - assertEquals("Didn't use the first registered resolver", Integer.valueOf(1), resolvedValue); + assertEquals("Didn't use the first registered resolver", 1, resolvedValue); } @Test(expected = IllegalArgumentException.class) public void noSuitableArgumentResolver() throws Exception { - this.resolvers.resolveArgument(paramStr, null, null, null); + this.resolverComposite.resolveArgument(paramStr, null, null, null); } - protected StubArgumentResolver registerResolver(Class supportedType, Object stubValue) { - StubArgumentResolver resolver = new StubArgumentResolver(supportedType, stubValue); - this.resolvers.addResolver(resolver); - return resolver; - } @SuppressWarnings("unused") private void handle(Integer arg1, String arg2) { diff --git a/spring-web/src/test/java/org/springframework/web/method/support/InvocableHandlerMethodTests.java b/spring-web/src/test/java/org/springframework/web/method/support/InvocableHandlerMethodTests.java index 105766fa80..a12cf274a4 100644 --- a/spring-web/src/test/java/org/springframework/web/method/support/InvocableHandlerMethodTests.java +++ b/spring-web/src/test/java/org/springframework/web/method/support/InvocableHandlerMethodTests.java @@ -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. @@ -22,74 +22,64 @@ import org.junit.Before; import org.junit.Test; import org.springframework.core.MethodParameter; -import org.springframework.http.converter.HttpMessageNotReadableException; import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.mock.web.test.MockHttpServletResponse; import org.springframework.web.bind.support.WebDataBinderFactory; import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.context.request.ServletWebRequest; +import org.springframework.web.method.ResolvableMethod; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; /** - * Test fixture for {@link InvocableHandlerMethod} unit tests. + * Unit tests for {@link InvocableHandlerMethod}. * * @author Rossen Stoyanchev */ public class InvocableHandlerMethodTests { - private InvocableHandlerMethod handlerMethod; + private NativeWebRequest request; - private NativeWebRequest webRequest; + private final HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite(); @Before public void setUp() throws Exception { - Method method = Handler.class.getDeclaredMethod("handle", Integer.class, String.class); - this.handlerMethod = new InvocableHandlerMethod(new Handler(), method); - this.webRequest = new ServletWebRequest(new MockHttpServletRequest(), new MockHttpServletResponse()); + this.request = new ServletWebRequest(new MockHttpServletRequest(), new MockHttpServletResponse()); } @Test public void resolveArg() throws Exception { - StubArgumentResolver intResolver = new StubArgumentResolver(Integer.class, 99); - StubArgumentResolver stringResolver = new StubArgumentResolver(String.class, "value"); + this.composite.addResolver(new StubArgumentResolver(99)); + this.composite.addResolver(new StubArgumentResolver("value")); - HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite(); - composite.addResolver(intResolver); - composite.addResolver(stringResolver); - handlerMethod.setHandlerMethodArgumentResolvers(composite); + Object value = getInvocable(Integer.class, String.class).invokeForRequest(request, null); - Object returnValue = handlerMethod.invokeForRequest(webRequest, null); - assertEquals(1, intResolver.getResolvedParameters().size()); - assertEquals(1, stringResolver.getResolvedParameters().size()); - assertEquals("99-value", returnValue); - assertEquals("intArg", intResolver.getResolvedParameters().get(0).getParameterName()); - assertEquals("stringArg", stringResolver.getResolvedParameters().get(0).getParameterName()); + assertEquals(1, getStubResolver(0).getResolvedParameters().size()); + assertEquals(1, getStubResolver(1).getResolvedParameters().size()); + assertEquals("99-value", value); + assertEquals("intArg", getStubResolver(0).getResolvedParameters().get(0).getParameterName()); + assertEquals("stringArg", getStubResolver(1).getResolvedParameters().get(0).getParameterName()); } @Test public void resolveNullArg() throws Exception { - StubArgumentResolver intResolver = new StubArgumentResolver(Integer.class, null); - StubArgumentResolver stringResolver = new StubArgumentResolver(String.class, null); + this.composite.addResolver(new StubArgumentResolver(Integer.class)); + this.composite.addResolver(new StubArgumentResolver(String.class)); - HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite(); - composite.addResolver(intResolver); - composite.addResolver(stringResolver); - handlerMethod.setHandlerMethodArgumentResolvers(composite); + Object returnValue = getInvocable(Integer.class, String.class).invokeForRequest(request, null); - Object returnValue = handlerMethod.invokeForRequest(webRequest, null); - assertEquals(1, intResolver.getResolvedParameters().size()); - assertEquals(1, stringResolver.getResolvedParameters().size()); + assertEquals(1, getStubResolver(0).getResolvedParameters().size()); + assertEquals(1, getStubResolver(1).getResolvedParameters().size()); assertEquals("null-null", returnValue); } @Test public void cannotResolveArg() throws Exception { try { - handlerMethod.invokeForRequest(webRequest, null); + getInvocable(Integer.class, String.class).invokeForRequest(request, null); fail("Expected exception"); } catch (IllegalStateException ex) { @@ -99,53 +89,40 @@ public class InvocableHandlerMethodTests { @Test public void resolveProvidedArg() throws Exception { - Object returnValue = handlerMethod.invokeForRequest(webRequest, null, 99, "value"); + Object value = getInvocable(Integer.class, String.class).invokeForRequest(request, null, 99, "value"); - assertEquals(String.class, returnValue.getClass()); - assertEquals("99-value", returnValue); + assertNotNull(value); + assertEquals(String.class, value.getClass()); + assertEquals("99-value", value); } @Test public void resolveProvidedArgFirst() throws Exception { - StubArgumentResolver intResolver = new StubArgumentResolver(Integer.class, 1); - StubArgumentResolver stringResolver = new StubArgumentResolver(String.class, "value1"); + this.composite.addResolver(new StubArgumentResolver(1)); + this.composite.addResolver(new StubArgumentResolver("value1")); + Object value = getInvocable(Integer.class, String.class).invokeForRequest(request, null, 2, "value2"); - HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite(); - composite.addResolver(intResolver); - composite.addResolver(stringResolver); - handlerMethod.setHandlerMethodArgumentResolvers(composite); - - Object returnValue = handlerMethod.invokeForRequest(webRequest, null, 2, "value2"); - assertEquals("2-value2", returnValue); + assertEquals("2-value2", value); } @Test public void exceptionInResolvingArg() throws Exception { - HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite(); - composite.addResolver(new ExceptionRaisingArgumentResolver()); - handlerMethod.setHandlerMethodArgumentResolvers(composite); - + this.composite.addResolver(new ExceptionRaisingArgumentResolver()); try { - handlerMethod.invokeForRequest(webRequest, null); + getInvocable(Integer.class, String.class).invokeForRequest(request, null); fail("Expected exception"); } - catch (HttpMessageNotReadableException ex) { + catch (IllegalArgumentException ex) { // expected - allow HandlerMethodArgumentResolver exceptions to propagate } } @Test public void illegalArgumentException() throws Exception { - StubArgumentResolver intResolver = new StubArgumentResolver(Integer.class, "__invalid__"); - StubArgumentResolver stringResolver = new StubArgumentResolver(String.class, "value"); - - HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite(); - composite.addResolver(intResolver); - composite.addResolver(stringResolver); - handlerMethod.setHandlerMethodArgumentResolvers(composite); - + this.composite.addResolver(new StubArgumentResolver(Integer.class, "__not_an_int__")); + this.composite.addResolver(new StubArgumentResolver("value")); try { - handlerMethod.invokeForRequest(webRequest, null); + getInvocable(Integer.class, String.class).invokeForRequest(request, null); fail("Expected exception"); } catch (IllegalStateException ex) { @@ -154,7 +131,7 @@ public class InvocableHandlerMethodTests { assertTrue(ex.getMessage().contains("Controller [")); assertTrue(ex.getMessage().contains("Method [")); assertTrue(ex.getMessage().contains("with argument values:")); - assertTrue(ex.getMessage().contains("[0] [type=java.lang.String] [value=__invalid__]")); + assertTrue(ex.getMessage().contains("[0] [type=java.lang.String] [value=__not_an_int__]")); assertTrue(ex.getMessage().contains("[1] [type=java.lang.String] [value=value")); } } @@ -163,7 +140,8 @@ public class InvocableHandlerMethodTests { public void invocationTargetException() throws Exception { Throwable expected = new RuntimeException("error"); try { - invokeExceptionRaisingHandler(expected); + getInvocable(Throwable.class).invokeForRequest(this.request, null, expected); + fail("Expected exception"); } catch (RuntimeException actual) { assertSame(expected, actual); @@ -171,7 +149,8 @@ public class InvocableHandlerMethodTests { expected = new Error("error"); try { - invokeExceptionRaisingHandler(expected); + getInvocable(Throwable.class).invokeForRequest(this.request, null, expected); + fail("Expected exception"); } catch (Error actual) { assertSame(expected, actual); @@ -179,7 +158,8 @@ public class InvocableHandlerMethodTests { expected = new Exception("error"); try { - invokeExceptionRaisingHandler(expected); + getInvocable(Throwable.class).invokeForRequest(this.request, null, expected); + fail("Expected exception"); } catch (Exception actual) { assertSame(expected, actual); @@ -187,7 +167,8 @@ public class InvocableHandlerMethodTests { expected = new Throwable("error"); try { - invokeExceptionRaisingHandler(expected); + getInvocable(Throwable.class).invokeForRequest(this.request, null, expected); + fail("Expected exception"); } catch (IllegalStateException actual) { assertNotNull(actual.getCause()); @@ -198,16 +179,9 @@ public class InvocableHandlerMethodTests { @Test // SPR-13917 public void invocationErrorMessage() throws Exception { - HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite(); - composite.addResolver(new StubArgumentResolver(double.class, null)); - - Method method = Handler.class.getDeclaredMethod("handle", double.class); - Object handler = new Handler(); - InvocableHandlerMethod hm = new InvocableHandlerMethod(handler, method); - hm.setHandlerMethodArgumentResolvers(composite); - + this.composite.addResolver(new StubArgumentResolver(double.class)); try { - hm.invokeForRequest(this.webRequest, new ModelAndViewContainer()); + getInvocable(double.class).invokeForRequest(this.request, null); fail(); } catch (IllegalStateException ex) { @@ -215,14 +189,18 @@ public class InvocableHandlerMethodTests { } } - - private void invokeExceptionRaisingHandler(Throwable expected) throws Exception { - Method method = ExceptionRaisingHandler.class.getDeclaredMethod("raiseException"); - Object handler = new ExceptionRaisingHandler(expected); - new InvocableHandlerMethod(handler, method).invokeForRequest(webRequest, null); - fail("Expected exception"); + private InvocableHandlerMethod getInvocable(Class... argTypes) { + Method method = ResolvableMethod.on(Handler.class).argTypes(argTypes).resolveMethod(); + InvocableHandlerMethod handlerMethod = new InvocableHandlerMethod(new Handler(), method); + handlerMethod.setHandlerMethodArgumentResolvers(this.composite); + return handlerMethod; } + private StubArgumentResolver getStubResolver(int index) { + return (StubArgumentResolver) this.composite.getResolvers().get(index); + } + + @SuppressWarnings("unused") private static class Handler { @@ -233,20 +211,9 @@ public class InvocableHandlerMethodTests { public void handle(double amount) { } - } - - @SuppressWarnings("unused") - private static class ExceptionRaisingHandler { - - private final Throwable t; - - public ExceptionRaisingHandler(Throwable t) { - this.t = t; - } - - public void raiseException() throws Throwable { - throw t; + public void handleWithException(Throwable ex) throws Throwable { + throw ex; } } @@ -260,8 +227,9 @@ public class InvocableHandlerMethodTests { @Override public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer, - NativeWebRequest webRequest, WebDataBinderFactory binderFactory) throws Exception { - throw new HttpMessageNotReadableException("oops, can't read"); + NativeWebRequest webRequest, WebDataBinderFactory binderFactory) { + + throw new IllegalArgumentException("oops, can't read"); } } diff --git a/spring-web/src/test/java/org/springframework/web/method/support/StubArgumentResolver.java b/spring-web/src/test/java/org/springframework/web/method/support/StubArgumentResolver.java index e423c6d7a7..d511353a17 100644 --- a/spring-web/src/test/java/org/springframework/web/method/support/StubArgumentResolver.java +++ b/spring-web/src/test/java/org/springframework/web/method/support/StubArgumentResolver.java @@ -20,41 +20,55 @@ import java.util.ArrayList; import java.util.List; import org.springframework.core.MethodParameter; +import org.springframework.lang.Nullable; import org.springframework.web.bind.support.WebDataBinderFactory; import org.springframework.web.context.request.NativeWebRequest; /** - * Supports parameters of a given type and resolves them using a stub value. - * Also records the resolved parameter value. + * Stub resolver for a fixed value type and/or value. * * @author Rossen Stoyanchev */ public class StubArgumentResolver implements HandlerMethodArgumentResolver { - private final Class parameterType; + private final Class valueType; - private final Object stubValue; + @Nullable + private final Object value; private List resolvedParameters = new ArrayList<>(); - public StubArgumentResolver(Class supportedParameterType, Object stubValue) { - this.parameterType = supportedParameterType; - this.stubValue = stubValue; + + public StubArgumentResolver(Object value) { + this(value.getClass(), value); } + public StubArgumentResolver(Class valueType) { + this(valueType, null); + } + + public StubArgumentResolver(Class valueType, Object value) { + this.valueType = valueType; + this.value = value; + } + + public List getResolvedParameters() { return resolvedParameters; } + @Override public boolean supportsParameter(MethodParameter parameter) { - return parameter.getParameterType().equals(this.parameterType); + return parameter.getParameterType().equals(this.valueType); } @Override public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer, - NativeWebRequest webRequest, WebDataBinderFactory binderFactory) throws Exception { + NativeWebRequest webRequest, WebDataBinderFactory binderFactory) { + this.resolvedParameters.add(parameter); - return this.stubValue; + return this.value; } + } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/HandlerMethodArgumentResolverComposite.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/HandlerMethodArgumentResolverComposite.java new file mode 100644 index 0000000000..f1ceb1b45f --- /dev/null +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/HandlerMethodArgumentResolverComposite.java @@ -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 argumentResolvers = new LinkedList<>(); + + private final Map 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 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 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 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; + } + +} diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java index 644fb93fc1..d9972c3e40 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java @@ -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 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 resolvers) { - this.resolvers.clear(); - this.resolvers.addAll(resolvers); + this.resolvers.addResolvers(resolvers); } /** * Return the configured argument resolvers. */ public List getResolvers() { - return this.resolvers; + return this.resolvers.getResolvers(); } /** @@ -133,7 +135,7 @@ public class InvocableHandlerMethod extends HandlerMethod { public Mono 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 resolveArguments( + private Mono getMethodArgumentValues( ServerWebExchange exchange, BindingContext bindingContext, Object... providedArgs) { if (ObjectUtils.isEmpty(getMethodParameters())) { return EMPTY_ARGS; } - try { - List> 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> 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 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 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- diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/InvocableHandlerMethodTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/InvocableHandlerMethodTests.java index 81137b8da1..80127fd171 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/InvocableHandlerMethodTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/InvocableHandlerMethodTests.java @@ -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 mono = invoke(new TestController(), method); assertHandlerResultValue(mono, "success"); } @Test - public void invokeMethodWithNoValue() throws Exception { - Mono resolvedValue = Mono.empty(); - Method method = on(TestController.class).mockCall(o -> o.singleArg(null)).method(); - Mono mono = invoke(new TestController(), method, resolverFor(resolvedValue)); + public void invokeMethodWithNoValue() { + Method method = resolveOn().mockCall(o -> o.singleArg(null)).method(); + Mono mono = invoke(new TestController(), method, stubResolver(Mono.empty())); assertHandlerResultValue(mono, "success:null"); } + private ResolvableMethod.Builder resolveOn() { + return ResolvableMethod.on(TestController.class); + } + @Test - public void invokeMethodWithValue() throws Exception { - Mono resolvedValue = Mono.just("value1"); - Method method = on(TestController.class).mockCall(o -> o.singleArg(null)).method(); - Mono mono = invoke(new TestController(), method, resolverFor(resolvedValue)); + public void invokeMethodWithValue() { + Method method = ResolvableMethod.on(TestController.class).mockCall(o -> o.singleArg(null)).method(); + Mono 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 mono = invoke(new TestController(), method); try { @@ -162,10 +159,10 @@ public class InvocableHandlerMethodTests { } @Test - public void resolverThrowsException() throws Exception { - Mono resolvedValue = Mono.error(new UnsupportedMediaTypeStatusException("boo")); - Method method = on(TestController.class).mockCall(o -> o.singleArg(null)).method(); - Mono mono = invoke(new TestController(), method, resolverFor(resolvedValue)); + public void resolverThrowsException() { + Method method = ResolvableMethod.on(TestController.class).mockCall(o -> o.singleArg(null)).method(); + Mono 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 resolvedValue = Mono.just(1); - Method method = on(TestController.class).mockCall(o -> o.singleArg(null)).method(); - Mono mono = invoke(new TestController(), method, resolverFor(resolvedValue)); + public void illegalArgumentException() { + Method method = ResolvableMethod.on(TestController.class).mockCall(o -> o.singleArg(null)).method(); + Mono 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 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 mono = invoke(new TestController(), method); assertHandlerResultValue(mono, "created"); @@ -220,30 +216,31 @@ public class InvocableHandlerMethodTests { } - private Mono 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 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 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 HandlerMethodArgumentResolver resolverFor(Mono resolvedValue) { + private HandlerMethodArgumentResolver stubResolver(Object stubValue) { + return stubResolver(Mono.just(stubValue)); + } + + private HandlerMethodArgumentResolver stubResolver(Mono 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 mono, String expected) { StepVerifier.create(mono) - .consumeNextWith(result -> { - assertEquals(expected, result.getReturnValue()); - }) + .consumeNextWith(result -> assertEquals(expected, result.getReturnValue())) .expectComplete() .verify(); }