From 95f29507a7f6df2786a27577f96c6ad753f2d986 Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Tue, 7 Feb 2012 17:03:04 +0200 Subject: [PATCH] INT-2405 fix MessageProcessor for 1. MethodInvokingHeaderValueMessageProcessor => MessageProcessingHeaderValueMessageProcessor because now he uses MessageProcessor 2. polishing HeaderEnricherParserSupport 3. GroovyHeaderEnricherTests: add check to HeaderValueMessageProcessor for Groovy scripts. 4. build.gradle: add dependency to 'spring-integration-test' for 'spring-integration-groovy'. 5. Fix HeaderEnricherParserSupport like in INT-2188 --- build.gradle | 1 + .../xml/HeaderEnricherParserSupport.java | 88 ++++++++----- .../transformer/HeaderEnricher.java | 119 +++++++++++++----- .../GroovyHeaderEnricherTests-context.xml | 2 +- .../config/GroovyHeaderEnricherTests.java | 26 +++- 5 files changed, 171 insertions(+), 65 deletions(-) diff --git a/build.gradle b/build.gradle index c76bd059c4..f3a27fab1f 100644 --- a/build.gradle +++ b/build.gradle @@ -224,6 +224,7 @@ project('spring-integration-groovy') { compile project(":spring-integration-core") compile "org.codehaus.groovy:groovy-all:1.7.5" compile "org.springframework:spring-context-support:$springVersion" + testCompile project(":spring-integration-test") } } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/xml/HeaderEnricherParserSupport.java b/spring-integration-core/src/main/java/org/springframework/integration/config/xml/HeaderEnricherParserSupport.java index 11a6d097ff..99cb364bc1 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/config/xml/HeaderEnricherParserSupport.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/config/xml/HeaderEnricherParserSupport.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2010 the original author or authors. + * Copyright 2002-2012 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. @@ -38,6 +38,7 @@ import org.springframework.util.xml.DomUtils; * * @author Mark Fisher * @author Oleg Zhurakousky + * @author Artem Bilan * @since 2.0 */ public abstract class HeaderEnricherParserSupport extends AbstractTransformerParser { @@ -116,33 +117,53 @@ public abstract class HeaderEnricherParserSupport extends AbstractTransformerPar String ref = headerElement.getAttribute("ref"); String method = headerElement.getAttribute("method"); String expression = headerElement.getAttribute("expression"); - - List subElements = DomUtils.getChildElements(headerElement); - - BeanDefinition innerComponentDefinition = null; + + Element beanElement = null; + Element scriptElement = null; Element expressionElement = null; - if (subElements != null && subElements.size() == 1) { - Element beanElement = subElements.get(0); - if ("expression".equals(beanElement.getNodeName())){ - expressionElement = beanElement; - if (StringUtils.hasText(expression) && expressionElement != null) { - parserContext.getReaderContext().error("The 'expression' attribute and sub-element are mutually exclusive", element); - return; - } - } - else if ("bean".equals(beanElement.getNodeName())){ - innerComponentDefinition = parserContext.getDelegate().parseBeanDefinitionElement(beanElement).getBeanDefinition(); + + List subElements = DomUtils.getChildElements(headerElement); + if (!subElements.isEmpty()) { + Element subElement = subElements.get(0); + String subElementLocalName = subElement.getLocalName(); + if ("bean".equals(subElementLocalName)) { + beanElement = subElement; } - else { - innerComponentDefinition = parserContext.getDelegate().parseCustomElement(beanElement); + else if ("script".equals(subElementLocalName)) { + scriptElement = subElement; + } + else if ("expression".equals(subElementLocalName)) { + expressionElement = subElement; + } + if (beanElement == null && scriptElement == null && expressionElement == null) { + parserContext.getReaderContext().error("Only 'bean', 'script' or 'expression' can be defined as a sub-element", element); } } + if (StringUtils.hasText(expression) && expressionElement != null) { + parserContext.getReaderContext().error("The 'expression' attribute and sub-element are mutually exclusive", element); + } + boolean isValue = StringUtils.hasText(value); boolean isRef = StringUtils.hasText(ref); boolean hasMethod = StringUtils.hasText(method); boolean isExpression = StringUtils.hasText(expression) || expressionElement != null; + boolean isScript = scriptElement != null; + + BeanDefinition innerComponentDefinition = null; + + if (beanElement != null) { + innerComponentDefinition = parserContext.getDelegate().parseBeanDefinitionElement(beanElement).getBeanDefinition(); + } + else if (isScript) { + innerComponentDefinition = parserContext.getDelegate().parseCustomElement(scriptElement); + } + boolean isCustomBean = innerComponentDefinition != null; - + + if (hasMethod && isScript) { + parserContext.getReaderContext().error("The 'method' attribute cannot be used when a 'script' sub-element is defined", element); + } + if (!(isValue ^ (isRef ^ (isExpression ^ isCustomBean)))) { parserContext.getReaderContext().error( "Exactly one of the 'ref', 'value', 'expression' or inner bean is required.", element); @@ -178,17 +199,24 @@ public abstract class HeaderEnricherParserSupport extends AbstractTransformerPar } valueProcessorBuilder.addConstructorArgValue(headerType); } - else if (isCustomBean){ - valueProcessorBuilder = BeanDefinitionBuilder.genericBeanDefinition( - IntegrationNamespaceUtils.BASE_PACKAGE + ".transformer.HeaderEnricher$MethodInvokingHeaderValueMessageProcessor"); - valueProcessorBuilder.addConstructorArgValue(innerComponentDefinition); - if (hasMethod){ - valueProcessorBuilder.addConstructorArgValue(method); - } + else if (isCustomBean) { + if (StringUtils.hasText(headerElement.getAttribute("type"))) { + parserContext.getReaderContext().error( + "The 'type' attribute cannot be used with an inner bean.", element); + } + if (hasMethod || isScript) { + valueProcessorBuilder = BeanDefinitionBuilder.genericBeanDefinition( + IntegrationNamespaceUtils.BASE_PACKAGE + ".transformer.HeaderEnricher$MessageProcessingHeaderValueMessageProcessor"); + valueProcessorBuilder.addConstructorArgValue(innerComponentDefinition); + if (hasMethod) { + valueProcessorBuilder.addConstructorArgValue(method); + } + } else { - valueProcessorBuilder.addConstructorArgValue(null); - } - headers.put(headerName, valueProcessorBuilder.getBeanDefinition()); + valueProcessorBuilder = BeanDefinitionBuilder.genericBeanDefinition( + IntegrationNamespaceUtils.BASE_PACKAGE + ".transformer.HeaderEnricher$StaticHeaderValueMessageProcessor"); + valueProcessorBuilder.addConstructorArgValue(innerComponentDefinition); + } } else { if (StringUtils.hasText(headerElement.getAttribute("type"))) { @@ -197,7 +225,7 @@ public abstract class HeaderEnricherParserSupport extends AbstractTransformerPar } if (hasMethod) { valueProcessorBuilder = BeanDefinitionBuilder.genericBeanDefinition( - IntegrationNamespaceUtils.BASE_PACKAGE + ".transformer.HeaderEnricher$MethodInvokingHeaderValueMessageProcessor"); + IntegrationNamespaceUtils.BASE_PACKAGE + ".transformer.HeaderEnricher$MessageProcessingHeaderValueMessageProcessor"); valueProcessorBuilder.addConstructorArgReference(ref); valueProcessorBuilder.addConstructorArgValue(method); } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/transformer/HeaderEnricher.java b/spring-integration-core/src/main/java/org/springframework/integration/transformer/HeaderEnricher.java index 5945b400af..918f659e1a 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/transformer/HeaderEnricher.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/transformer/HeaderEnricher.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2010 the original author or authors. + * Copyright 2002-2012 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. @@ -23,6 +23,8 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; +import org.springframework.beans.factory.BeanNameAware; +import org.springframework.beans.factory.InitializingBean; import org.springframework.expression.Expression; import org.springframework.expression.ExpressionParser; import org.springframework.expression.spel.SpelParserConfiguration; @@ -36,17 +38,18 @@ import org.springframework.integration.support.MessageBuilder; /** * A Transformer that adds statically configured header values to a Message. - * Accepts the boolean 'overwrite' property that specifies whether values - * should be overwritten. By default, any existing header values for - * a given key, will not be replaced. + * Accepts the boolean 'overwrite' property that specifies whether values should + * be overwritten. By default, any existing header values for a given key, will + * not be replaced. * * @author Mark Fisher + * @author David Turanski + * @author Artem Bilan */ -public class HeaderEnricher implements Transformer { +public class HeaderEnricher implements Transformer, BeanNameAware, InitializingBean { private static final Log logger = LogFactory.getLog(HeaderEnricher.class); - private final Map> headersToAdd; private volatile MessageProcessor messageProcessor; @@ -55,6 +58,7 @@ public class HeaderEnricher implements Transformer { private volatile boolean shouldSkipNulls = true; + private Object beanName; public HeaderEnricher() { this(null); @@ -64,10 +68,10 @@ public class HeaderEnricher implements Transformer { * Create a HeaderEnricher with the given map of headers. */ public HeaderEnricher(Map> headersToAdd) { - this.headersToAdd = (headersToAdd != null) ? headersToAdd : new HashMap>(); + this.headersToAdd = (headersToAdd != null) ? headersToAdd + : new HashMap>(); } - public void setMessageProcessor(MessageProcessor messageProcessor) { this.messageProcessor = messageProcessor; } @@ -77,9 +81,11 @@ public class HeaderEnricher implements Transformer { } /** - * Specify whether null values, such as might be returned from an expression evaluation, - * should be skipped. The default value is true. Set this to false if a - * null value should trigger removal of the corresponding header instead. + * Specify whether null values, such as might be returned from + * an expression evaluation, should be skipped. The default value is + * true. Set this to false if a + * null value should trigger removal of the + * corresponding header instead. */ public void setShouldSkipNulls(boolean shouldSkipNulls) { this.shouldSkipNulls = shouldSkipNulls; @@ -92,20 +98,29 @@ public class HeaderEnricher implements Transformer { for (Map.Entry> entry : this.headersToAdd.entrySet()) { String key = entry.getKey(); HeaderValueMessageProcessor valueProcessor = entry.getValue(); + Boolean shouldOverwrite = valueProcessor.isOverwrite(); if (shouldOverwrite == null) { shouldOverwrite = this.defaultOverwrite; } - Object value = valueProcessor.processMessage(message); - if ((value != null && shouldOverwrite) || headerMap.get(key) == null || (value == null && !this.shouldSkipNulls)) { - headerMap.put(key, value); + + boolean headerDoesNotExist = headerMap.get(key) == null; + + /** + * Only evaluate value expression if necessary + */ + if (headerDoesNotExist || shouldOverwrite) { + Object value = valueProcessor.processMessage(message); + if (value != null || !this.shouldSkipNulls) { + headerMap.put(key, value); + } } } - return MessageBuilder.withPayload(message.getPayload()).copyHeaders(headerMap).build(); - } + return MessageBuilder.withPayload(message.getPayload()).copyHeaders(headerMap).build(); + } catch (Exception e) { - throw new MessagingException(message, "failed to transform message headers", e); - } + throw new MessagingException(message, "failed to transform message headers", e); + } } @SuppressWarnings("rawtypes") @@ -131,17 +146,16 @@ public class HeaderEnricher implements Transformer { } } - public static interface HeaderValueMessageProcessor extends MessageProcessor { Boolean isOverwrite(); } - static abstract class AbstractHeaderValueMessageProcessor implements HeaderValueMessageProcessor { - // null indicates no explicit setting; use header-enricher's 'default-overwrite' value + // null indicates no explicit setting; use header-enricher's + // 'default-overwrite' value private volatile Boolean overwrite = null; public void setOverwrite(Boolean overwrite) { @@ -154,7 +168,6 @@ public class HeaderEnricher implements Transformer { } - static class StaticHeaderValueMessageProcessor extends AbstractHeaderValueMessageProcessor { private final T value; @@ -168,24 +181,27 @@ public class HeaderEnricher implements Transformer { } } + static class ExpressionEvaluatingHeaderValueMessageProcessor extends AbstractHeaderValueMessageProcessor + implements BeanFactoryAware { - static class ExpressionEvaluatingHeaderValueMessageProcessor extends AbstractHeaderValueMessageProcessor implements BeanFactoryAware { - - private static final ExpressionParser expressionParser = new SpelExpressionParser(new SpelParserConfiguration(true, true)); + private static final ExpressionParser expressionParser = new SpelExpressionParser(new SpelParserConfiguration( + true, true)); private final ExpressionEvaluatingMessageProcessor targetProcessor; /** - * Create a header value processor for the given Expression and the expected type - * of the expression evaluation result. The expectedType may be null if unknown. + * Create a header value processor for the given Expression and the + * expected type of the expression evaluation result. The expectedType + * may be null if unknown. */ public ExpressionEvaluatingHeaderValueMessageProcessor(Expression expression, Class expectedType) { this.targetProcessor = new ExpressionEvaluatingMessageProcessor(expression, expectedType); } /** - * Create a header value processor for the given expression string and the expected type - * of the expression evaluation result. The expectedType may be null if unknown. + * Create a header value processor for the given expression string and + * the expected type of the expression evaluation result. The + * expectedType may be null if unknown. */ public ExpressionEvaluatingHeaderValueMessageProcessor(String expressionString, Class expectedType) { Expression expression = expressionParser.parseExpression(expressionString); @@ -202,12 +218,51 @@ public class HeaderEnricher implements Transformer { } + /* + * (non-Javadoc) + * + * @see + * org.springframework.beans.factory.BeanNameAware#setBeanName(java.lang + * .String) + */ + public void setBeanName(String beanName) { + this.beanName = beanName; - static class MethodInvokingHeaderValueMessageProcessor extends AbstractHeaderValueMessageProcessor { + } - private final MethodInvokingMessageProcessor targetProcessor; + /* + * (non-Javadoc) + * + * @see + * org.springframework.beans.factory.InitializingBean#afterPropertiesSet() + */ + public void afterPropertiesSet() throws Exception { + boolean shouldOverwrite = this.defaultOverwrite; + for (HeaderValueMessageProcessor processor : this.headersToAdd.values()) { + Boolean processerOverwrite = processor.isOverwrite(); + if (processerOverwrite != null) { + shouldOverwrite |= processerOverwrite.booleanValue(); + } + } + if (!shouldOverwrite && !this.shouldSkipNulls) { + logger.warn(this.beanName + + " is configured to not overwrite existing headers. 'shouldSkipNulls = false' will have no effect"); + } + } - public MethodInvokingHeaderValueMessageProcessor(Object targetObject, String method) { + static class MessageProcessingHeaderValueMessageProcessor extends AbstractHeaderValueMessageProcessor { + + private final MessageProcessor targetProcessor; + + public MessageProcessingHeaderValueMessageProcessor(MessageProcessor targetProcessor) { + this.targetProcessor = targetProcessor; + } + + public MessageProcessingHeaderValueMessageProcessor(Object targetObject) { + this(targetObject, null); + } + + public MessageProcessingHeaderValueMessageProcessor(Object targetObject, String method) { this.targetProcessor = new MethodInvokingMessageProcessor(targetObject, method); } diff --git a/spring-integration-groovy/src/test/java/org/springframework/integration/groovy/config/GroovyHeaderEnricherTests-context.xml b/spring-integration-groovy/src/test/java/org/springframework/integration/groovy/config/GroovyHeaderEnricherTests-context.xml index 586dbbfe96..405e67ef75 100644 --- a/spring-integration-groovy/src/test/java/org/springframework/integration/groovy/config/GroovyHeaderEnricherTests-context.xml +++ b/spring-integration-groovy/src/test/java/org/springframework/integration/groovy/config/GroovyHeaderEnricherTests-context.xml @@ -22,7 +22,7 @@ - + ("Hello")); assertEquals("groovy", outputA.receive(1000).getHeaders().get("TEST_HEADER")); } - + + @SuppressWarnings("unchecked") @Test public void inlineScript() throws Exception{ + Map headers = + TestUtils.getPropertyValue(headerEnricherWithInlineGroovyScript, "handler.transformer.headersToAdd", Map.class); + assertEquals(1, headers.size()); + HeaderEnricher.HeaderValueMessageProcessor headerValueMessageProcessor = headers.get("TEST_HEADER"); + assertThat(headerValueMessageProcessor.getClass().getName(), Matchers.containsString("HeaderEnricher$MessageProcessingHeaderValueMessageProcessor")); + Object targetProcessor = TestUtils.getPropertyValue(headerValueMessageProcessor, "targetProcessor"); + assertEquals(GroovyScriptExecutingMessageProcessor.class, targetProcessor.getClass()); + inputB.send(new GenericMessage("Hello")); assertEquals("groovy", outputB.receive(1000).getHeaders().get("TEST_HEADER")); }