From c5c874f5635cb68383e3b84fac97e8f2a69e1423 Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Fri, 27 Jan 2017 10:16:31 -0500 Subject: [PATCH] INT-4217: SpEL Compilable for Required Header Args JIRA: https://jira.spring.io/browse/INT-4217 Previously, SpEL method calls with required header parameters had the following form: #target.messageAndHeader(message, headers['number'] != null ? headers['number'] : T(org.springframework.util.Assert).isTrue(false, 'required header not available: number')) The SpEL compiler cannot compile this because the else clause of the ternary has no `exitDescriptor` to indicate the type. Change the expression to use a function for required headers. Also use an Elvis operator when possible. Some examples of new expressions: #target.optionalAndRequiredHeader(headers['prop'] ?: null, #requiredHeader(headers, 'num')) #target.optionalAndRequiredDottedHeader(headers['dot1'] != null ? headers['dot1'].foo : null, #requiredHeader(headers, 'dot2').baz In the second case, we can't use an Elvis because we're accessing a `foo` property of the header. __cherry-pick to 4.3.x__ - make `ParametersWrapper` `static` - minor conflicts in imports - remove the perf test * Polishing: remove redundant annotation args for their default values usage --- .../util/MessagingMethodInvokerHelper.java | 36 +++- .../MethodInvokingMessageProcessorTests.java | 159 +++++++++++++++++- 2 files changed, 181 insertions(+), 14 deletions(-) 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 41a8faf6db..d311defa42 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 @@ -31,6 +31,7 @@ import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Properties; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; @@ -314,7 +315,7 @@ public class MessagingMethodInvokerHelper extends AbstractExpressionEvaluator this.displayString = sb.toString() + "]"; } - private void prepareEvaluationContext() { + private void prepareEvaluationContext() throws Exception { StandardEvaluationContext context = getEvaluationContext(false); Class targetType = AopUtils.getTargetClass(this.targetObject); if (this.method != null) { @@ -333,6 +334,8 @@ public class MessagingMethodInvokerHelper extends AbstractExpressionEvaluator context.registerMethodFilter(targetType, filter); } context.setVariable("target", this.targetObject); + context.registerFunction("requiredHeader", ParametersWrapper.class.getDeclaredMethod("getHeader", + Map.class, String.class)); } private boolean canReturnExpectedType(AnnotatedMethodFilter filter, Class targetType, @@ -949,12 +952,16 @@ public class MessagingMethodInvokerHelper extends AbstractExpressionEvaluator + "disabled or header name is not explicitly provided via @Header annotation."); String headerRetrievalExpression = "headers['" + headerName + "']"; String fullHeaderExpression = headerRetrievalExpression + relativeExpression; - String fallbackExpression = (annotationAttributes.getBoolean("required") - && !methodParameter.getParameterType().getName().equals("java.util.Optional")) - ? "T(org.springframework.util.Assert).isTrue(false, 'required header not available: " - + headerName + "')" - : "null"; - return headerRetrievalExpression + " != null ? " + fullHeaderExpression + " : " + fallbackExpression; + if (annotationAttributes.getBoolean("required") + && !methodParameter.getParameterType().equals(Optional.class)) { + return "#requiredHeader(headers, '" + headerName + "')" + relativeExpression; + } + else if (!StringUtils.hasLength(relativeExpression)) { + return headerRetrievalExpression + " ?: null"; + } + else { + return headerRetrievalExpression + " != null ? " + fullHeaderExpression + " : null"; + } } private void setExclusiveTargetParameterType(TypeDescriptor targetParameterType, @@ -999,6 +1006,21 @@ public class MessagingMethodInvokerHelper extends AbstractExpressionEvaluator this.message = null; } + /** + * SpEL Function to retrieve a required header. + * @param headers the headers. + * @param header the header name + * @return the header + * @throws IllegalArgumentException if the header does not exist + */ + public static Object getHeader(Map headers, String header) { + Object object = headers.get(header); + if (object == null) { + throw new IllegalArgumentException("required header not available: " + header); + } + return object; + } + public Object getPayload() { Assert.state(this.payload != null, "Invalid method parameter for payload: was expecting collection."); 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 b619cc1cd4..90fc626e6b 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 @@ -16,6 +16,7 @@ package org.springframework.integration.handler; +import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -67,7 +68,7 @@ import org.springframework.util.StopWatch; * @author Gunnar Hillert * @author Artem Bilan */ -@SuppressWarnings({"rawtypes", "unchecked"}) +@SuppressWarnings({ "rawtypes", "unchecked" }) public class MethodInvokingMessageProcessorTests { private static final Log logger = LogFactory.getLog(MethodInvokingMessageProcessorTests.class); @@ -78,6 +79,7 @@ public class MethodInvokingMessageProcessorTests { @Test public void testHandlerInheritanceMethodImplInSuper() { class A { + @SuppressWarnings("unused") public Message myMethod(final Message msg) { return MessageBuilder.fromMessage(msg).setHeader("A", "A").build(); @@ -85,10 +87,12 @@ public class MethodInvokingMessageProcessorTests { } class B extends A { + } @SuppressWarnings("unused") class C extends B { + } MethodInvokingMessageProcessor processor = new MethodInvokingMessageProcessor(new B(), "myMethod"); @@ -99,6 +103,7 @@ public class MethodInvokingMessageProcessorTests { @Test public void testHandlerInheritanceMethodImplInLatestSuper() { class A { + @SuppressWarnings("unused") public Message myMethod(Message msg) { return MessageBuilder.fromMessage(msg).setHeader("A", "A").build(); @@ -106,6 +111,7 @@ public class MethodInvokingMessageProcessorTests { } class B extends A { + @Override public Message myMethod(Message msg) { return MessageBuilder.fromMessage(msg).setHeader("B", "B").build(); @@ -114,6 +120,7 @@ public class MethodInvokingMessageProcessorTests { @SuppressWarnings("unused") class C extends B { + } MethodInvokingMessageProcessor processor = new MethodInvokingMessageProcessor(new B(), "myMethod"); @@ -123,6 +130,7 @@ public class MethodInvokingMessageProcessorTests { public void testHandlerInheritanceMethodImplInSubClass() { class A { + @SuppressWarnings("unused") public Message myMethod(Message msg) { return MessageBuilder.fromMessage(msg).setHeader("A", "A").build(); @@ -130,6 +138,7 @@ public class MethodInvokingMessageProcessorTests { } class B extends A { + @Override public Message myMethod(Message msg) { return MessageBuilder.fromMessage(msg).setHeader("B", "B").build(); @@ -137,6 +146,7 @@ public class MethodInvokingMessageProcessorTests { } class C extends B { + @Override public Message myMethod(Message msg) { return MessageBuilder.fromMessage(msg).setHeader("C", "C").build(); @@ -150,6 +160,7 @@ public class MethodInvokingMessageProcessorTests { public void testHandlerInheritanceMethodImplInSubClassAndSuper() { class A { + @SuppressWarnings("unused") public Message myMethod(Message msg) { return MessageBuilder.fromMessage(msg).setHeader("A", "A").build(); @@ -157,9 +168,11 @@ public class MethodInvokingMessageProcessorTests { } class B extends A { + } class C extends B { + @Override public Message myMethod(Message msg) { return MessageBuilder.fromMessage(msg).setHeader("C", "C").build(); @@ -347,6 +360,110 @@ public class MethodInvokingMessageProcessorTests { assertEquals("bar-42", result); } + @Test + public void optionalAndRequiredWithAnnotatedMethod() throws Exception { + AnnotatedTestService service = new AnnotatedTestService(); + Method method = service.getClass().getMethod("optionalAndRequiredHeader", String.class, Integer.class); + MethodInvokingMessageProcessor processor = new MethodInvokingMessageProcessor(service, method); + processor.setUseSpelInvoker(true); + optionalAndRequiredWithAnnotatedMethodGuts(processor, false); + } + + @Test + public void compiledOptionalAndRequiredWithAnnotatedMethod() throws Exception { + AnnotatedTestService service = new AnnotatedTestService(); + Method method = service.getClass().getMethod("optionalAndRequiredHeader", String.class, Integer.class); + MethodInvokingMessageProcessor processor = new MethodInvokingMessageProcessor(service, method); + processor.setUseSpelInvoker(true); + DirectFieldAccessor compilerConfigAccessor = compileImmediate(processor); + optionalAndRequiredWithAnnotatedMethodGuts(processor, true); + assertNotNull(TestUtils.getPropertyValue(processor, "delegate.handlerMethod.expression.compiledAst")); + optionalAndRequiredWithAnnotatedMethodGuts(processor, true); + compilerConfigAccessor.setPropertyValue("compilerMode", SpelCompilerMode.OFF); + } + + private void optionalAndRequiredWithAnnotatedMethodGuts(MethodInvokingMessageProcessor processor, + boolean compiled) { + Message message = MessageBuilder.withPayload("foo") + .setHeader("num", 42) + .build(); + Object result = processor.processMessage(message); + assertEquals("null42", result); + message = MessageBuilder.withPayload("foo") + .setHeader("prop", "bar") + .setHeader("num", 42) + .build(); + result = processor.processMessage(message); + assertEquals("bar42", result); + message = MessageBuilder.withPayload("foo") + .setHeader("prop", "bar") + .build(); + try { + result = processor.processMessage(message); + fail("Expected MessageHandlingException"); + } + catch (MessageHandlingException e) { + if (compiled) { + assertThat(e.getCause().getMessage(), equalTo("required header not available: num")); + } + else { + assertThat(e.getCause().getCause().getMessage(), equalTo("required header not available: num")); + } + } + } + + @Test + public void optionalAndRequiredDottedWithAnnotatedMethod() throws Exception { + AnnotatedTestService service = new AnnotatedTestService(); + Method method = service.getClass().getMethod("optionalAndRequiredDottedHeader", String.class, Integer.class); + MethodInvokingMessageProcessor processor = new MethodInvokingMessageProcessor(service, method); + processor.setUseSpelInvoker(true); + optionalAndRequiredDottedWithAnnotatedMethodGuts(processor, false); + } + + @Test + public void compiledOptionalAndRequiredDottedWithAnnotatedMethod() throws Exception { + AnnotatedTestService service = new AnnotatedTestService(); + Method method = service.getClass().getMethod("optionalAndRequiredDottedHeader", String.class, Integer.class); + MethodInvokingMessageProcessor processor = new MethodInvokingMessageProcessor(service, method); + processor.setUseSpelInvoker(true); + DirectFieldAccessor compilerConfigAccessor = compileImmediate(processor); + optionalAndRequiredDottedWithAnnotatedMethodGuts(processor, true); + assertNotNull(TestUtils.getPropertyValue(processor, "delegate.handlerMethod.expression.compiledAst")); + optionalAndRequiredDottedWithAnnotatedMethodGuts(processor, true); + compilerConfigAccessor.setPropertyValue("compilerMode", SpelCompilerMode.OFF); + } + + private void optionalAndRequiredDottedWithAnnotatedMethodGuts(MethodInvokingMessageProcessor processor, + boolean compiled) { + Message message = MessageBuilder.withPayload("hello") + .setHeader("dot2", new DotBean()) + .build(); + Object result = processor.processMessage(message); + assertEquals("null42", result); + message = MessageBuilder.withPayload("hello") + .setHeader("dot1", new DotBean()) + .setHeader("dot2", new DotBean()) + .build(); + result = processor.processMessage(message); + assertEquals("bar42", result); + message = MessageBuilder.withPayload("hello") + .setHeader("dot1", new DotBean()) + .build(); + try { + result = processor.processMessage(message); + fail("Expected MessageHandlingException"); + } + catch (MessageHandlingException e) { + if (compiled) { + assertThat(e.getCause().getMessage(), equalTo("required header not available: dot2")); + } + else { // interpreted + assertThat(e.getCause().getCause().getMessage(), equalTo("required header not available: dot2")); + } + } + } + @Test public void testOverloadedNonVoidReturningMethodsWithExactMatchForType() { AmbiguousMethodBean bean = new AmbiguousMethodBean(); @@ -601,10 +718,7 @@ public class MethodInvokingMessageProcessorTests { } stopWatch.stop(); - // Update the parser configuration compiler mode - SpelParserConfiguration config = TestUtils.getPropertyValue(processor, - "delegate.handlerMethod.EXPRESSION_PARSER.configuration", SpelParserConfiguration.class); - new DirectFieldAccessor(config).setPropertyValue("compilerMode", SpelCompilerMode.IMMEDIATE); + DirectFieldAccessor compilerConfigAccessor = compileImmediate(processor); processor = new MethodInvokingMessageProcessor(service, method); processor.setUseSpelInvoker(true); @@ -616,6 +730,16 @@ public class MethodInvokingMessageProcessorTests { stopWatch.stop(); logger.warn(stopWatch.prettyPrint()); + compilerConfigAccessor.setPropertyValue("compilerMode", SpelCompilerMode.OFF); + } + + private DirectFieldAccessor compileImmediate(MethodInvokingMessageProcessor processor) { + // Update the parser configuration compiler mode + SpelParserConfiguration config = TestUtils.getPropertyValue(processor, + "delegate.handlerMethod.EXPRESSION_PARSER.configuration", SpelParserConfiguration.class); + DirectFieldAccessor accessor = new DirectFieldAccessor(config); + accessor.setPropertyValue("compilerMode", SpelCompilerMode.IMMEDIATE); + return accessor; } private static class ExceptionCauseMatcher extends TypeSafeMatcher { @@ -748,12 +872,17 @@ public class MethodInvokingMessageProcessorTests { return num; } - public Integer requiredHeader(@Header(value = "num", required = true) Integer num) { + public Integer requiredHeader(@Header(value = "num") Integer num) { return num; } public String optionalAndRequiredHeader(@Header(required = false) String prop, - @Header(value = "num", required = true) Integer num) { + @Header(value = "num") Integer num) { + return prop + num; + } + + public String optionalAndRequiredDottedHeader(@Header(name = "dot1.foo", required = false) String prop, + @Header(name = "dot2.baz") Integer num) { return prop + num; } @@ -845,4 +974,20 @@ public class MethodInvokingMessageProcessorTests { } + public static class DotBean { + + private final String foo = "bar"; + + private final Integer baz = 42; + + public String getFoo() { + return this.foo; + } + + public Integer getBaz() { + return this.baz; + } + + } + }