diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/xml/AbstractCorrelatingMessageHandlerParser.java b/spring-integration-core/src/main/java/org/springframework/integration/config/xml/AbstractCorrelatingMessageHandlerParser.java index 28f60122e8..88075d00e5 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/config/xml/AbstractCorrelatingMessageHandlerParser.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/config/xml/AbstractCorrelatingMessageHandlerParser.java @@ -18,6 +18,9 @@ import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.xml.ParserContext; import org.springframework.integration.aggregator.AbstractCorrelatingMessageHandler; import org.springframework.util.StringUtils; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.w3c.dom.Element; /** @@ -29,6 +32,8 @@ import org.w3c.dom.Element; */ public abstract class AbstractCorrelatingMessageHandlerParser extends AbstractConsumerEndpointParser { + private final Log logger = LogFactory.getLog(this.getClass()); + private static final String CORRELATION_STRATEGY_REF_ATTRIBUTE = "correlation-strategy"; private static final String CORRELATION_STRATEGY_METHOD_ATTRIBUTE = "correlation-strategy-method"; @@ -58,23 +63,36 @@ public abstract class AbstractCorrelatingMessageHandlerParser extends AbstractCo protected void injectPropertyWithAdapter(String beanRefAttribute, String methodRefAttribute, String expressionAttribute, String beanProperty, String adapterClass, Element element, BeanDefinitionBuilder builder, BeanMetadataElement processor, ParserContext parserContext) { + final String beanRef = element.getAttribute(beanRefAttribute); final String beanMethod = element.getAttribute(methodRefAttribute); final String expression = element.getAttribute(expressionAttribute); + + final boolean hasBeanRef = StringUtils.hasText(beanRef); + final boolean hasExpression = StringUtils.hasText(expression); + + if (hasBeanRef && hasExpression) { + this.logger.warn("Exactly one of the '" + beanRefAttribute + "' or '" + expressionAttribute + + "' attribute is allowed. The '" + expressionAttribute + + "' is ignored when both are provided." + + "NOTE: This is a warning message only, to avoid a breaking change in a point release and should " + + "be treated as an error. In a future release this condition will result in the actual exception"); + } + BeanMetadataElement adapter = null; - if (StringUtils.hasText(beanRef)) { + if (hasBeanRef) { adapter = this.createAdapter(new RuntimeBeanReference(beanRef), beanMethod, adapterClass, parserContext); } - else if (processor != null) { - adapter = this.createAdapter(processor, beanMethod, adapterClass, parserContext); - } - else if (StringUtils.hasText(expression)) { + else if (hasExpression) { BeanDefinitionBuilder adapterBuilder = BeanDefinitionBuilder .genericBeanDefinition(IntegrationNamespaceUtils.BASE_PACKAGE + ".aggregator.ExpressionEvaluating" + adapterClass); adapterBuilder.addConstructorArgValue(expression); adapter = adapterBuilder.getBeanDefinition(); } + else if (processor != null) { + adapter = this.createAdapter(processor, beanMethod, adapterClass, parserContext); + } else { adapter = this.createAdapter(null, beanMethod, adapterClass, parserContext); } diff --git a/spring-integration-core/src/test/java/org/springframework/integration/config/AggregatorParserTests.java b/spring-integration-core/src/test/java/org/springframework/integration/config/AggregatorParserTests.java index c01ee4502f..d75857f4e1 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/config/AggregatorParserTests.java +++ b/spring-integration-core/src/test/java/org/springframework/integration/config/AggregatorParserTests.java @@ -24,6 +24,7 @@ import java.util.concurrent.atomic.AtomicReference; import org.junit.Assert; import org.junit.Before; import org.junit.Test; + import org.springframework.beans.DirectFieldAccessor; import org.springframework.beans.factory.BeanCreationException; import org.springframework.context.ApplicationContext; @@ -35,6 +36,9 @@ import org.springframework.integration.MessageHandlingException; import org.springframework.integration.MessageRejectedException; import org.springframework.integration.aggregator.AggregatingMessageHandler; import org.springframework.integration.aggregator.CorrelationStrategy; +import org.springframework.integration.aggregator.ExpressionEvaluatingCorrelationStrategy; +import org.springframework.integration.aggregator.ExpressionEvaluatingReleaseStrategy; +import org.springframework.integration.aggregator.MethodInvokingMessageGroupProcessor; import org.springframework.integration.aggregator.MethodInvokingReleaseStrategy; import org.springframework.integration.aggregator.ReleaseStrategy; import org.springframework.integration.core.MessageHandler; @@ -47,6 +51,7 @@ import org.springframework.integration.test.util.TestUtils; import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -217,10 +222,29 @@ public class AggregatorParserTests { context = new ClassPathXmlApplicationContext("invalidReleaseStrategyMethod.xml", this.getClass()); } + @Test + public void testAggregationWithExpressionsAndPojoAggregator() { + EventDrivenConsumer aggregatorConsumer = (EventDrivenConsumer) context.getBean("aggregatorWithExpressionsAndPojoAggregator"); + AggregatingMessageHandler aggregatingMessageHandler = (AggregatingMessageHandler) TestUtils.getPropertyValue(aggregatorConsumer, "handler"); + MethodInvokingMessageGroupProcessor messageGroupProcessor = (MethodInvokingMessageGroupProcessor) TestUtils.getPropertyValue(aggregatingMessageHandler, "outputProcessor"); + Object messageGroupProcessorTargetObject = TestUtils.getPropertyValue(messageGroupProcessor, "processor.delegate.targetObject"); + assertSame(context.getBean("aggregatorBean"), messageGroupProcessorTargetObject); + ReleaseStrategy releaseStrategy = (ReleaseStrategy) TestUtils.getPropertyValue(aggregatingMessageHandler, "releaseStrategy"); + CorrelationStrategy correlationStrategy = (CorrelationStrategy) TestUtils.getPropertyValue(aggregatingMessageHandler, "correlationStrategy"); + + assertTrue(ExpressionEvaluatingReleaseStrategy.class.equals(releaseStrategy.getClass())); + assertTrue(ExpressionEvaluatingCorrelationStrategy.class.equals(correlationStrategy.getClass())); + } + + // should be re-enabled for INT-2497 +// @Test(expected=BeanDefinitionParsingException.class) +// public void testAggregatorFailureIfMutuallyExclusivityPresent() { +// this.context = new ClassPathXmlApplicationContext("aggregatorParserFailTests.xml", this.getClass()); +// } + private static Message createMessage(T payload, Object correlationId, int sequenceSize, int sequenceNumber, MessageChannel outputChannel) { return MessageBuilder.withPayload(payload).setCorrelationId(correlationId).setSequenceSize(sequenceSize) .setSequenceNumber(sequenceNumber).setReplyChannel(outputChannel).build(); } - } diff --git a/spring-integration-core/src/test/java/org/springframework/integration/config/aggregatorParserFailTests.xml b/spring-integration-core/src/test/java/org/springframework/integration/config/aggregatorParserFailTests.xml new file mode 100644 index 0000000000..c515cb108c --- /dev/null +++ b/spring-integration-core/src/test/java/org/springframework/integration/config/aggregatorParserFailTests.xml @@ -0,0 +1,24 @@ + + + + + + + + + + + diff --git a/spring-integration-core/src/test/java/org/springframework/integration/config/aggregatorParserTests.xml b/spring-integration-core/src/test/java/org/springframework/integration/config/aggregatorParserTests.xml index 1964ed742b..334591a19b 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/config/aggregatorParserTests.xml +++ b/spring-integration-core/src/test/java/org/springframework/integration/config/aggregatorParserTests.xml @@ -1,82 +1,88 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +