INT-2439 PollerParser adviceChain.add(customBean)

Fix PollerParser#configureAdviceChain for adding 'customBeanDefinition' into 'adviceChain' List.
PollerParserTests: adding <tx:advice> as customElement into poller's <advice-chain>.

Commit is mostly parser tidy-up. The crux of the fix is to add the
missing call to adviceChain.add().

spring-integration-core/src/main/java/org/springframework/integration/config/xml/PollerParser.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2011 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.
@@ -35,6 +35,13 @@ import org.springframework.beans.factory.support.ManagedList;
 import org.springframework.beans.factory.xml.AbstractBeanDefinitionParser;
 import org.springframework.beans.factory.xml.ParserContext;
 import org.springframework.integration.context.IntegrationContextUtils;
+import org.springframework.integration.channel.MessagePublishingErrorHandler;
+import org.springframework.integration.scheduling.PollerMetadata;
+import org.springframework.scheduling.support.CronTrigger;
+import org.springframework.scheduling.support.PeriodicTrigger;
+import org.springframework.transaction.interceptor.DefaultTransactionAttribute;
+import org.springframework.transaction.interceptor.MatchAlwaysTransactionAttributeSource;
+import org.springframework.transaction.interceptor.TransactionInterceptor;
 import org.springframework.util.StringUtils;
 import org.springframework.util.xml.DomUtils;
 
@@ -44,6 +51,7 @@ import org.springframework.util.xml.DomUtils;
  * @author Mark Fisher
  * @author Marius Bogoevici
  * @author Oleg Zhurakousky
+ * @author Artem Bilan
  */
 public class PollerParser extends AbstractBeanDefinitionParser {
 
@@ -51,11 +59,6 @@ public class PollerParser extends AbstractBeanDefinitionParser {
 
 	private static final String NO_TRIGGER_DEFINITIONS = "A <poller> must have one and only one trigger configuration.";
 
-	private static final String PERIODIC_TRIGGER_CLASSNAME = "org.springframework.scheduling.support.PeriodicTrigger";
-
-	private static final String CRON_TRIGGER_CLASSNAME = "org.springframework.scheduling.support.CronTrigger";
-
-
 	@Override
 	protected String resolveId(Element element, AbstractBeanDefinition definition, ParserContext parserContext) throws BeanDefinitionStoreException {
 		String id = super.resolveId(element, definition, parserContext);
@@ -81,8 +84,7 @@ public class PollerParser extends AbstractBeanDefinitionParser {
 
 	@Override
 	protected AbstractBeanDefinition parseInternal(Element element, ParserContext parserContext) {
-		BeanDefinitionBuilder metadataBuilder = BeanDefinitionBuilder.genericBeanDefinition(
+		BeanDefinitionBuilder metadataBuilder = BeanDefinitionBuilder.genericBeanDefinition(PollerMetadata.class);
-				IntegrationNamespaceUtils.BASE_PACKAGE + ".scheduling.PollerMetadata");
 		if (element.hasAttribute("ref")) {
 			parserContext.getReaderContext().error(
 					"the 'ref' attribute must not be present on the top-level 'poller' element", element);
@@ -98,8 +100,7 @@ public class PollerParser extends AbstractBeanDefinitionParser {
 		IntegrationNamespaceUtils.setReferenceIfAttributeDefined(metadataBuilder, element, "task-executor");
 		String errorChannel = element.getAttribute("error-channel");
 		if (StringUtils.hasText(errorChannel)) {
-			BeanDefinitionBuilder errorHandler = BeanDefinitionBuilder.genericBeanDefinition(
+			BeanDefinitionBuilder errorHandler = BeanDefinitionBuilder.genericBeanDefinition(MessagePublishingErrorHandler.class);
-					"org.springframework.integration.channel.MessagePublishingErrorHandler");
 			errorHandler.addPropertyReference("defaultErrorChannel", errorChannel);
 			metadataBuilder.addPropertyValue("errorHandler", errorHandler.getBeanDefinition());
 		}
@@ -121,7 +122,7 @@ public class PollerParser extends AbstractBeanDefinitionParser {
 			triggerBeanNames.add(triggerAttribute);
 		}
 		if (StringUtils.hasText(fixedRateAttribute)) {
-			BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition(PERIODIC_TRIGGER_CLASSNAME);
+			BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition(PeriodicTrigger.class);
 			builder.addConstructorArgValue(fixedRateAttribute);
 			if (StringUtils.hasText(timeUnit)) {
 				builder.addConstructorArgValue(timeUnit);
@@ -132,7 +133,7 @@ public class PollerParser extends AbstractBeanDefinitionParser {
 			triggerBeanNames.add(triggerBeanName);
 		}
 		if (StringUtils.hasText(fixedDelayAttribute)) {
-			BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition(PERIODIC_TRIGGER_CLASSNAME);
+			BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition(PeriodicTrigger.class);
 			builder.addConstructorArgValue(fixedDelayAttribute);
 			if (StringUtils.hasText(timeUnit)) {
 				builder.addConstructorArgValue(timeUnit);
@@ -146,7 +147,7 @@ public class PollerParser extends AbstractBeanDefinitionParser {
 			if (StringUtils.hasText(timeUnit)) {
 				parserContext.getReaderContext().error("The 'time-unit' attribute cannot be used with a 'cron' trigger.", pollerElement);
 			}
-			BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition(CRON_TRIGGER_CLASSNAME);
+			BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition(CronTrigger.class);
 			builder.addConstructorArgValue(cronAttribute);
 			String triggerBeanName = BeanDefinitionReaderUtils.registerWithGeneratedName(
 					builder.getBeanDefinition(), parserContext.getRegistry());
@@ -166,19 +167,15 @@ public class PollerParser extends AbstractBeanDefinitionParser {
 	 * and other "transactionDefinition" properties. This advisor will be applied on the Polling Task proxy
 	 * (see {@link org.springframework.integration.endpoint.AbstractPollingEndpoint}).
 	 */
-	private BeanDefinition configureTransactionAttributes(Element txElement, BeanDefinitionBuilder targetBuilder, ParserContext parserContext) {
+	private BeanDefinition configureTransactionAttributes(Element txElement) {
-		String TX_PKG_PREFIX = "org.springframework.transaction.interceptor";
+		BeanDefinitionBuilder txDefinitionBuilder = BeanDefinitionBuilder.genericBeanDefinition(DefaultTransactionAttribute.class);
-		BeanDefinitionBuilder txDefinitionBuilder = 
-				BeanDefinitionBuilder.genericBeanDefinition(TX_PKG_PREFIX + ".DefaultTransactionAttribute");
 		txDefinitionBuilder.addPropertyValue("propagationBehaviorName", "PROPAGATION_" + txElement.getAttribute("propagation"));
 		txDefinitionBuilder.addPropertyValue("isolationLevelName", "ISOLATION_" + txElement.getAttribute("isolation"));
 		txDefinitionBuilder.addPropertyValue("timeout", txElement.getAttribute("timeout"));
 		txDefinitionBuilder.addPropertyValue("readOnly", txElement.getAttribute("read-only"));
-		BeanDefinitionBuilder attributeSourceBuilder = 
+		BeanDefinitionBuilder attributeSourceBuilder = BeanDefinitionBuilder.genericBeanDefinition(MatchAlwaysTransactionAttributeSource.class);
-				BeanDefinitionBuilder.genericBeanDefinition(TX_PKG_PREFIX + ".MatchAlwaysTransactionAttributeSource");
 		attributeSourceBuilder.addPropertyValue("transactionAttribute", txDefinitionBuilder.getBeanDefinition());
-		BeanDefinitionBuilder txInterceptorBuilder = 
+		BeanDefinitionBuilder txInterceptorBuilder = BeanDefinitionBuilder.genericBeanDefinition(TransactionInterceptor.class);
-				BeanDefinitionBuilder.genericBeanDefinition(TX_PKG_PREFIX + ".TransactionInterceptor");
 		txInterceptorBuilder.addPropertyReference("transactionManager", txElement.getAttribute("transaction-manager"));
 		txInterceptorBuilder.addPropertyValue("transactionAttributeSource", attributeSourceBuilder.getBeanDefinition());
 		return txInterceptorBuilder.getBeanDefinition();
@@ -190,8 +187,9 @@ public class PollerParser extends AbstractBeanDefinitionParser {
 	@SuppressWarnings({ "unchecked", "rawtypes" })
 	private void configureAdviceChain(Element adviceChainElement, Element txElement, BeanDefinitionBuilder targetBuilder, ParserContext parserContext) {
 		ManagedList adviceChain = new ManagedList();
+		// Schema validation ensures txElement and adviceChainElement are mutually exclusive
 		if (txElement != null) {
-			adviceChain.add(this.configureTransactionAttributes(txElement, targetBuilder, parserContext));
+			adviceChain.add(this.configureTransactionAttributes(txElement));
 		}
 		if (adviceChainElement != null) {
 			NodeList childNodes = adviceChainElement.getChildNodes();
@@ -217,6 +215,7 @@ public class PollerParser extends AbstractBeanDefinitionParser {
 							parserContext.getReaderContext().error(
 									"failed to parse custom element '" + localName + "'", childElement);
 						}
+						adviceChain.add(customBeanDefinition);
 					}
 				}
 			}

spring-integration-core/src/test/java/org/springframework/integration/config/xml/PollerParserTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2011 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.
@@ -19,11 +19,15 @@ package org.springframework.integration.config.xml;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertSame;
 
+import java.util.HashMap;
 import java.util.concurrent.TimeUnit;
 
 import org.junit.Test;
 
+import org.aopalliance.aop.Advice;
+
 import org.springframework.beans.factory.parsing.BeanDefinitionParsingException;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.support.ClassPathXmlApplicationContext;
@@ -32,10 +36,14 @@ import org.springframework.integration.context.IntegrationContextUtils;
 import org.springframework.integration.scheduling.PollerMetadata;
 import org.springframework.integration.test.util.TestUtils;
 import org.springframework.scheduling.support.PeriodicTrigger;
+import org.springframework.transaction.interceptor.NameMatchTransactionAttributeSource;
+import org.springframework.transaction.interceptor.TransactionAttributeSource;
+import org.springframework.transaction.interceptor.TransactionInterceptor;
 
 /**
  * @author Mark Fisher
  * @author Oleg Zhurakousky
+ * @author Artem Bilan
  */
 public class PollerParserTests {
 
@@ -78,11 +86,19 @@ public class PollerParserTests {
 		assertNotNull(poller);
 		PollerMetadata metadata = (PollerMetadata) poller;
 		assertNotNull(metadata.getAdviceChain());
-		assertEquals(3, metadata.getAdviceChain().size());
+		assertEquals(4, metadata.getAdviceChain().size());
-		assertEquals(context.getBean("adviceBean1"), metadata.getAdviceChain().get(0));
+		assertSame(context.getBean("adviceBean1"), metadata.getAdviceChain().get(0));
 		assertEquals(TestAdviceBean.class, metadata.getAdviceChain().get(1).getClass());
 		assertEquals(2, ((TestAdviceBean) metadata.getAdviceChain().get(1)).getId());
-		assertEquals(context.getBean("adviceBean3"), metadata.getAdviceChain().get(2));
+		assertSame(context.getBean("adviceBean3"), metadata.getAdviceChain().get(2));
+		Advice txAdvice = metadata.getAdviceChain().get(3);
+		assertEquals(TransactionInterceptor.class, txAdvice.getClass());
+		TransactionAttributeSource transactionAttributeSource = ((TransactionInterceptor) txAdvice).getTransactionAttributeSource();
+		assertEquals(NameMatchTransactionAttributeSource.class, transactionAttributeSource.getClass());
+		HashMap nameMap = TestUtils.getPropertyValue(transactionAttributeSource, "nameMap", HashMap.class);
+		assertEquals(1, nameMap.size());
+		assertEquals("{*=PROPAGATION_REQUIRES_NEW,ISOLATION_DEFAULT,readOnly}", nameMap.toString());
+
 	}
 
 	@Test

spring-integration-core/src/test/java/org/springframework/integration/config/xml/pollerWithAdviceChain.xml

@@ -1,11 +1,15 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<beans:beans xmlns="http://www.springframework.org/schema/integration"
+<beans:beans xmlns="http://www.springframework.org/schema/integration"
-	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+			 xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-	xmlns:beans="http://www.springframework.org/schema/beans"
+			 xmlns:beans="http://www.springframework.org/schema/beans"
-	xsi:schemaLocation="http://www.springframework.org/schema/beans
+			 xmlns:tx="http://www.springframework.org/schema/tx"
+			 xsi:schemaLocation="http://www.springframework.org/schema/beans
 			http://www.springframework.org/schema/beans/spring-beans.xsd
 			http://www.springframework.org/schema/integration
-			http://www.springframework.org/schema/integration/spring-integration.xsd">
+			http://www.springframework.org/schema/integration/spring-integration.xsd
+			http://www.springframework.org/schema/tx http://www.springframework.org/schema/tx/spring-tx.xsd">
+
+	<beans:bean id="transactionManager" class="org.springframework.integration.util.TestTransactionManager"/>
 
 	<poller id="poller" fixed-delay="5000">
 		<advice-chain>
@@ -14,6 +18,11 @@
 				<beans:constructor-arg value="2"/>
 			</beans:bean>
 			<ref bean="adviceBean3"/>
+			<tx:advice>
+				<tx:attributes>
+					<tx:method name="*" read-only="true" propagation="REQUIRES_NEW"/>
+				</tx:attributes>
+			</tx:advice>
 		</advice-chain>
 	</poller>