diff --git a/spring-integration-core/src/main/java/org/springframework/integration/util/MessagingMethodInvokerHelper.java b/spring-integration-core/src/main/java/org/springframework/integration/util/MessagingMethodInvokerHelper.java index db7e563b7e..114159e8af 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/util/MessagingMethodInvokerHelper.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/util/MessagingMethodInvokerHelper.java @@ -108,6 +108,25 @@ public class MessagingMethodInvokerHelper extends AbstractExpressionEvaluator private static final Log logger = LogFactory.getLog(MessagingMethodInvokerHelper.class); + // Number of times to try an InvocableHandlerMethod before giving up in favor of an expression. + private static final int FAILED_ATTEMPTS_THRESHOLD = 100; + + private static final SpelExpressionParser EXPRESSION_PARSER = new SpelExpressionParser(); + + private static final ParameterNameDiscoverer PARAMETER_NAME_DISCOVERER = + new LocalVariableTableParameterNameDiscoverer(); + + private static final TypeDescriptor messageTypeDescriptor = TypeDescriptor.valueOf(Message.class); + + @SuppressWarnings("unused") + private static final Collection> dummyMessages = Collections.emptyList(); + + private static final TypeDescriptor messageListTypeDescriptor = new TypeDescriptor( + ReflectionUtils.findField(MessagingMethodInvokerHelper.class, "dummyMessages")); + + private static final TypeDescriptor messageArrayTypeDescriptor = TypeDescriptor.valueOf(Message[].class); + + private final DefaultMessageHandlerMethodFactory messageHandlerMethodFactory = new DefaultMessageHandlerMethodFactory(); @@ -390,37 +409,13 @@ public class MessagingMethodInvokerHelper extends AbstractExpressionEvaluator Expression expression = candidate.expression; Assert.notNull(candidate, "No candidate methods found for messages."); - T result = null; - try { - if (this.useSpelInvoker || candidate.spelOnly) { - result = invokeExpression(expression, parameters); - } - else { - result = candidate.invoke(parameters); - } - } - catch (MethodArgumentResolutionException | MessageConversionException | IllegalStateException e) { - if (e instanceof MessageConversionException) { - if (e.getCause() instanceof ConversionFailedException && - !(e.getCause().getCause() instanceof ConverterNotFoundException)) { - throw e; - } - } - else if (e instanceof IllegalStateException) { - if (e.getCause() instanceof IllegalArgumentException - && !"argument type mismatch".equals(e.getCause().getMessage())) { - throw e; - } - } - if (logger.isInfoEnabled()) { - logger.info("Failed to invoke [ " + candidate.invocableHandlerMethod + - "] with provided arguments [ " + parameters + " ]. \n" + - "Falling back to SpEL invocation for expression [ " + - expression.getExpressionString() + " ]"); - } - + T result; + if (this.useSpelInvoker || candidate.spelOnly) { result = invokeExpression(expression, parameters); } + else { + result = invokeHandlerMethod(candidate, parameters); + } if (result != null && this.expectedType != null) { return (T) getEvaluationContext(true) @@ -432,6 +427,44 @@ public class MessagingMethodInvokerHelper extends AbstractExpressionEvaluator } } + @SuppressWarnings("unchecked") + private T invokeHandlerMethod(HandlerMethod handlerMethod, ParametersWrapper parameters) throws Exception { + try { + return (T) handlerMethod.invoke(parameters); + } + catch (MethodArgumentResolutionException | MessageConversionException | IllegalStateException e) { + if (e instanceof MessageConversionException) { + if (e.getCause() instanceof ConversionFailedException && + !(e.getCause().getCause() instanceof ConverterNotFoundException)) { + throw e; + } + } + else if (e instanceof IllegalStateException) { + if (!(e.getCause() instanceof IllegalArgumentException) || + !e.getStackTrace()[0].getClassName().equals(InvocableHandlerMethod.class.getName()) || + (!"argument type mismatch".equals(e.getCause().getMessage()) && + // JVM generates GeneratedMethodAccessor### after several calls with less error checking + !e.getCause().getMessage().startsWith("java.lang.ClassCastException@"))) { + throw e; + } + } + + Expression expression = handlerMethod.expression; + + if (++handlerMethod.failedAttempts >= FAILED_ATTEMPTS_THRESHOLD) { + handlerMethod.spelOnly = true; + if (logger.isInfoEnabled()) { + logger.info("Failed to invoke [ " + handlerMethod.invocableHandlerMethod + + "] with provided arguments [ " + parameters + " ]. \n" + + "Falling back to SpEL invocation for expression [ " + + expression.getExpressionString() + " ]"); + } + } + + return invokeExpression(expression, parameters); + } + } + @SuppressWarnings("unchecked") private T invokeExpression(Expression expression, ParametersWrapper parameters) throws Exception { try { @@ -745,21 +778,6 @@ public class MessagingMethodInvokerHelper extends AbstractExpressionEvaluator */ private static class HandlerMethod { - private static final SpelExpressionParser EXPRESSION_PARSER = new SpelExpressionParser(); - - private static final ParameterNameDiscoverer PARAMETER_NAME_DISCOVERER = - new LocalVariableTableParameterNameDiscoverer(); - - private static final TypeDescriptor messageTypeDescriptor = TypeDescriptor.valueOf(Message.class); - - private static final TypeDescriptor messageListTypeDescriptor = new TypeDescriptor( - ReflectionUtils.findField(HandlerMethod.class, "dummyMessages")); - - private static final TypeDescriptor messageArrayTypeDescriptor = TypeDescriptor.valueOf(Message[].class); - - @SuppressWarnings("unused") - private static final Collection> dummyMessages = Collections.emptyList(); - private final Expression expression; private final InvocableHandlerMethod invocableHandlerMethod; @@ -774,6 +792,11 @@ public class MessagingMethodInvokerHelper extends AbstractExpressionEvaluator private volatile boolean spelOnly; + // The number of times InvocableHandlerMethod was attempted and failed - enables us to eventually + // give up trying to call it when it just doesn't seem to be possible. + // Switching to spelOnly afterwards forever. + private volatile int failedAttempts = 0; + HandlerMethod(InvocableHandlerMethod invocableHandlerMethod, boolean canProcessMessageList) { this.invocableHandlerMethod = invocableHandlerMethod; this.canProcessMessageList = canProcessMessageList; @@ -1050,6 +1073,20 @@ public class MessagingMethodInvokerHelper extends AbstractExpressionEvaluator return this.messages.getClass(); } + @Override + public String toString() { + final StringBuilder sb = new StringBuilder("ParametersWrapper{"); + if (this.messages != null) { + sb.append("messages=").append(this.messages) + .append(", headers=").append(this.headers); + } + else { + sb.append("message=").append(this.message); + } + return sb.append('}') + .toString(); + } + } @SuppressWarnings("serial") diff --git a/spring-integration-core/src/test/java/org/springframework/integration/handler/MethodInvokingMessageProcessorAnnotationTests.java b/spring-integration-core/src/test/java/org/springframework/integration/handler/MethodInvokingMessageProcessorAnnotationTests.java index 5a45cd1c74..55d46a3d81 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/handler/MethodInvokingMessageProcessorAnnotationTests.java +++ b/spring-integration-core/src/test/java/org/springframework/integration/handler/MethodInvokingMessageProcessorAnnotationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * 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. @@ -17,6 +17,7 @@ package org.springframework.integration.handler; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -38,6 +39,7 @@ import org.junit.Assert; import org.junit.Test; import org.springframework.integration.support.MessageBuilder; +import org.springframework.integration.test.util.TestUtils; import org.springframework.messaging.Message; import org.springframework.messaging.MessageHandlingException; import org.springframework.messaging.MessageHeaders; @@ -52,6 +54,7 @@ import org.springframework.messaging.support.GenericMessage; * @author Iwein Fuld * @author Oleg Zhurakousky * @author Gary Russell + * @author Artem Bilan */ @SuppressWarnings({ "rawtypes", "unchecked" }) public class MethodInvokingMessageProcessorAnnotationTests { @@ -102,7 +105,7 @@ public class MethodInvokingMessageProcessorAnnotationTests { Method method = TestService.class.getMethod("requiredHeader", Integer.class); MethodInvokingMessageProcessor processor = new MethodInvokingMessageProcessor(testService, method); Message messageWithHeader = MessageBuilder.withPayload("foo") - .setHeader("num", new Integer(123)).build(); + .setHeader("num", 123).build(); GenericMessage messageWithoutHeader = new GenericMessage("foo"); processor.processMessage(messageWithHeader); @@ -113,7 +116,7 @@ public class MethodInvokingMessageProcessorAnnotationTests { public void fromMessageWithRequiredHeaderProvided() throws Exception { Method method = TestService.class.getMethod("requiredHeader", Integer.class); Message message = MessageBuilder.withPayload("foo") - .setHeader("num", new Integer(123)).build(); + .setHeader("num", 123).build(); MethodInvokingMessageProcessor processor = new MethodInvokingMessageProcessor(testService, method); Object result = processor.processMessage(message); assertEquals(123, result); @@ -133,7 +136,7 @@ public class MethodInvokingMessageProcessorAnnotationTests { Method method = TestService.class.getMethod("optionalAndRequiredHeader", String.class, Integer.class); MethodInvokingMessageProcessor processor = new MethodInvokingMessageProcessor(testService, method); Message message = MessageBuilder.withPayload("foo") - .setHeader("num", new Integer(123)).build(); + .setHeader("num", 123).build(); Object result = processor.processMessage(message); assertEquals("null123", result); } @@ -143,7 +146,7 @@ public class MethodInvokingMessageProcessorAnnotationTests { Method method = TestService.class.getMethod("optionalAndRequiredHeader", String.class, Integer.class); MethodInvokingMessageProcessor processor = new MethodInvokingMessageProcessor(testService, method); Message message = MessageBuilder.withPayload("foo") - .setHeader("num", new Integer(123)) + .setHeader("num", 123) .setHeader("prop", "bar") .build(); Object result = processor.processMessage(message); @@ -156,10 +159,21 @@ public class MethodInvokingMessageProcessorAnnotationTests { MethodInvokingMessageProcessor processor = new MethodInvokingMessageProcessor(testService, method); Message message = MessageBuilder.withPayload("test") .setHeader("prop1", "foo").setHeader("prop2", "bar").build(); + assertFalse(TestUtils.getPropertyValue(processor, "delegate.handlerMethod.spelOnly", Boolean.class)); + for (int i = 0; i < 99; i++) { + Object result = processor.processMessage(message); + Properties props = (Properties) result; + assertEquals("foo", props.getProperty("prop1")); + assertEquals("bar", props.getProperty("prop2")); + assertFalse(TestUtils.getPropertyValue(processor, "delegate.handlerMethod.spelOnly", Boolean.class)); + } + Object result = processor.processMessage(message); Properties props = (Properties) result; assertEquals("foo", props.getProperty("prop1")); assertEquals("bar", props.getProperty("prop2")); + + assertTrue(TestUtils.getPropertyValue(processor, "delegate.handlerMethod.spelOnly", Boolean.class)); } @Test @@ -210,8 +224,8 @@ public class MethodInvokingMessageProcessorAnnotationTests { Method method = TestService.class.getMethod("mapHeaders", Map.class); MethodInvokingMessageProcessor processor = new MethodInvokingMessageProcessor(testService, method); Message message = MessageBuilder.withPayload("test") - .setHeader("attrib1", new Integer(123)) - .setHeader("attrib2", new Integer(456)).build(); + .setHeader("attrib1", 123) + .setHeader("attrib2", 456).build(); Map result = (Map) processor.processMessage(message); assertEquals(123, result.get("attrib1")); assertEquals(456, result.get("attrib2")); @@ -225,8 +239,8 @@ public class MethodInvokingMessageProcessorAnnotationTests { payload.put("attrib1", 88); payload.put("attrib2", 99); Message> message = MessageBuilder.withPayload(payload) - .setHeader("attrib1", new Integer(123)) - .setHeader("attrib2", new Integer(456)).build(); + .setHeader("attrib1", 123) + .setHeader("attrib2", 456).build(); Map result = (Map) processor.processMessage(message); assertEquals(2, result.size()); assertEquals(new Integer(88), result.get("attrib1")); @@ -337,11 +351,19 @@ public class MethodInvokingMessageProcessorAnnotationTests { } + private Message getMessage() { + MessageBuilder builder = MessageBuilder.withPayload(employee); + builder.setHeader("day", "monday"); + builder.setHeader("month", "September"); + return builder.build(); + } + @SuppressWarnings("unused") private static class MultipleMappingAnnotationTestBean { public void test(@Payload("payload") @Header("foo") String s) { } + } @@ -379,7 +401,8 @@ public class MethodInvokingMessageProcessorAnnotationTests { return lastName + ", " + firstName; } - public String optionalAndRequiredHeader(@Header(required = false) String prop, @Header(value = "num", required = true) Integer num) { + public String optionalAndRequiredHeader(@Header(required = false) String prop, + @Header(value = "num", required = true) Integer num) { return prop + num; } @@ -444,13 +467,7 @@ public class MethodInvokingMessageProcessorAnnotationTests { ids.add(id); return "foo"; } - } - private Message getMessage() { - MessageBuilder builder = MessageBuilder.withPayload(employee); - builder.setHeader("day", "monday"); - builder.setHeader("month", "September"); - return builder.build(); } @@ -472,6 +489,7 @@ public class MethodInvokingMessageProcessorAnnotationTests { public String getLname() { return lname; } + } } diff --git a/spring-integration-core/src/test/java/org/springframework/integration/handler/MethodInvokingMessageProcessorTests.java b/spring-integration-core/src/test/java/org/springframework/integration/handler/MethodInvokingMessageProcessorTests.java index a627587778..fd3d9c06e8 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/handler/MethodInvokingMessageProcessorTests.java +++ b/spring-integration-core/src/test/java/org/springframework/integration/handler/MethodInvokingMessageProcessorTests.java @@ -17,6 +17,7 @@ package org.springframework.integration.handler; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -244,13 +245,13 @@ public class MethodInvokingMessageProcessorTests { public void payloadAndHeaderAnnotationMethodParametersAndObjectAsReturnValue() { MethodInvokingMessageProcessor processor = new MethodInvokingMessageProcessor(new TestBean(), "acceptPayloadAndHeaderAndReturnObject"); - Message request = MessageBuilder.withPayload("testing").setHeader("number", new Integer(123)).build(); + Message request = MessageBuilder.withPayload("testing").setHeader("number", 123).build(); Object result = processor.processMessage(request); assertEquals("testing-123", result); } @Test - public void testVoidMethodsIncludedbyDefault() { + public void testVoidMethodsIncludedByDefault() { MethodInvokingMessageProcessor processor = new MethodInvokingMessageProcessor(new TestBean(), "testVoidReturningMethods"); assertNull(processor.processMessage(MessageBuilder.withPayload("Something").build())); @@ -271,8 +272,8 @@ public class MethodInvokingMessageProcessorTests { AnnotatedTestService service = new AnnotatedTestService(); Method method = service.getClass().getMethod("integerMethod", Integer.class); MethodInvokingMessageProcessor processor = new MethodInvokingMessageProcessor(service, method); - Object result = processor.processMessage(new GenericMessage(new Integer(123))); - assertEquals(new Integer(123), result); + Object result = processor.processMessage(new GenericMessage<>(123)); + assertEquals(123, result); } @Test @@ -281,7 +282,7 @@ public class MethodInvokingMessageProcessorTests { Method method = service.getClass().getMethod("integerMethod", Integer.class); MethodInvokingMessageProcessor processor = new MethodInvokingMessageProcessor(service, method); Object result = processor.processMessage(new GenericMessage("456")); - assertEquals(new Integer(456), result); + assertEquals(456, result); } @Test(expected = MessageHandlingException.class) @@ -733,10 +734,37 @@ public class MethodInvokingMessageProcessorTests { compilerConfigAccessor.setPropertyValue("compilerMode", SpelCompilerMode.OFF); } + + @Test + public void testNoSpElFallbackWhenUserException() { + class A { + + @SuppressWarnings("unused") + public void myMethod(Object payload) { + throw new IllegalStateException(new IllegalArgumentException("argument type mismatch")); + } + } + + MethodInvokingMessageProcessor processor = new MethodInvokingMessageProcessor(new A(), "myMethod"); + + try { + processor.processMessage(new GenericMessage<>("foo")); + } + catch (Exception e) { + assertThat(e.getCause(), instanceOf(IllegalStateException.class)); + assertThat(e.getCause().getCause(), instanceOf(IllegalArgumentException.class)); + assertEquals(A.class.getName(), e.getCause().getStackTrace()[0].getClassName()); + } + + assertEquals(0, + TestUtils.getPropertyValue(processor, "delegate.handlerMethod.failedAttempts")); + + } + private DirectFieldAccessor compileImmediate(MethodInvokingMessageProcessor processor) { // Update the parser configuration compiler mode SpelParserConfiguration config = TestUtils.getPropertyValue(processor, - "delegate.handlerMethod.EXPRESSION_PARSER.configuration", SpelParserConfiguration.class); + "delegate.EXPRESSION_PARSER.configuration", SpelParserConfiguration.class); DirectFieldAccessor accessor = new DirectFieldAccessor(config); accessor.setPropertyValue("compilerMode", SpelCompilerMode.IMMEDIATE); return accessor; diff --git a/spring-integration-ip/src/test/java/org/springframework/integration/ip/tcp/connection/TcpNioConnectionTests.java b/spring-integration-ip/src/test/java/org/springframework/integration/ip/tcp/connection/TcpNioConnectionTests.java index b2df37605e..4240673df0 100644 --- a/spring-integration-ip/src/test/java/org/springframework/integration/ip/tcp/connection/TcpNioConnectionTests.java +++ b/spring-integration-ip/src/test/java/org/springframework/integration/ip/tcp/connection/TcpNioConnectionTests.java @@ -716,8 +716,8 @@ public class TcpNioConnectionTests { @Override public boolean onMessage(Message message) { if (!(message instanceof ErrorMessage)) { - assemblerLatch.countDown(); assembler.set(Thread.currentThread()); + assemblerLatch.countDown(); } return false; }