diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/AbstractStepParser.java b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/AbstractStepParser.java index 4fdd09ff8..d4b59dfc8 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/AbstractStepParser.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/AbstractStepParser.java @@ -107,8 +107,8 @@ public abstract class AbstractStepParser { } - private void parseTasklet(Element stepElement, Element taskletElement, AbstractBeanDefinition bd, ParserContext parserContext, - boolean stepUnderspecified) { + private void parseTasklet(Element stepElement, Element taskletElement, AbstractBeanDefinition bd, + ParserContext parserContext, boolean stepUnderspecified) { bd.setBeanClass(StepParserStepFactoryBean.class); bd.setAttribute("isNamespaceStep", true); @@ -205,16 +205,12 @@ public abstract class AbstractStepParser { if (children.size() == 1) { Element child = children.get(0); String exceptions = DomUtils.getTextValue(child); - if (StringUtils.hasLength(exceptions)) { - String[] exceptionArray = StringUtils.tokenizeToStringArray(exceptions, ",\n"); - if (exceptionArray.length > 0) { - ManagedList managedList = new ManagedList(); - managedList.setMergeEnabled(child.hasAttribute(MERGE_ATTR) - && Boolean.valueOf(child.getAttribute(MERGE_ATTR))); - managedList.addAll(Arrays.asList(exceptionArray)); - propertyValues.addPropertyValue(propertyName, managedList); - } - } + String[] exceptionArray = StringUtils.tokenizeToStringArray(exceptions, ",\n"); + ManagedList managedList = new ManagedList(); + managedList.setMergeEnabled(child.hasAttribute(MERGE_ATTR) + && Boolean.valueOf(child.getAttribute(MERGE_ATTR))); + managedList.addAll(Arrays.asList(exceptionArray)); + propertyValues.addPropertyValue(propertyName, managedList); } else if (children.size() > 1) { parserContext.getReaderContext().error( diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/JobParserTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/JobParserTests.java index fae03eb83..e9e420db2 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/JobParserTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/JobParserTests.java @@ -181,4 +181,11 @@ public class JobParserTests { assertTrue(e.getMessage().contains("Missing state for [StateTransition: [state=s2, pattern=*, next=s3]]")); } } + + @Test + public void testListenerClearingJob() throws Exception { + // TODO BATCH-1357: + // assertEquals(0, getListeners("listenerClearingJob", jobParserParentAttributeTestsCtx).size()); + } + } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/StepParserTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/StepParserTests.java index 52091d7ac..af84a9a33 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/StepParserTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/StepParserTests.java @@ -18,6 +18,9 @@ package org.springframework.batch.core.configuration.xml; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; import java.util.List; import java.util.Map; @@ -26,15 +29,25 @@ import org.junit.Test; import org.springframework.aop.framework.Advised; import org.springframework.batch.core.Step; import org.springframework.batch.core.StepExecutionListener; +import org.springframework.batch.core.StepListener; import org.springframework.batch.core.job.AbstractJob; +import org.springframework.batch.core.listener.CompositeStepExecutionListener; import org.springframework.batch.core.listener.StepExecutionListenerSupport; import org.springframework.batch.core.repository.JobRepository; import org.springframework.batch.core.repository.support.SimpleJobRepository; import org.springframework.batch.core.step.AbstractStep; +import org.springframework.batch.core.step.item.FatalException; +import org.springframework.batch.core.step.item.FatalRuntimeException; +import org.springframework.batch.core.step.item.SkippableException; +import org.springframework.batch.core.step.item.SkippableRuntimeException; import org.springframework.batch.core.step.tasklet.Tasklet; import org.springframework.batch.core.step.tasklet.TaskletStep; +import org.springframework.batch.item.ItemStream; +import org.springframework.batch.item.support.CompositeItemStream; import org.springframework.batch.repeat.CompletionPolicy; import org.springframework.batch.repeat.policy.SimpleCompletionPolicy; +import org.springframework.batch.retry.RetryListener; +import org.springframework.batch.retry.listener.RetryListenerSupport; import org.springframework.batch.support.transaction.ResourcelessTransactionManager; import org.springframework.beans.factory.parsing.BeanDefinitionParsingException; import org.springframework.beans.factory.xml.XmlBeanFactory; @@ -42,6 +55,7 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.support.ClassPathXmlApplicationContext; import org.springframework.core.io.ClassPathResource; +import org.springframework.dao.DeadlockLoserDataAccessException; import org.springframework.test.util.ReflectionTestUtils; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.TransactionDefinition; @@ -196,7 +210,7 @@ public class StepParserTests { } @SuppressWarnings("unchecked") - private StepExecutionListener getListener(String stepName, ApplicationContext ctx) throws Exception { + private List getListeners(String stepName, ApplicationContext ctx) throws Exception { assertTrue(ctx.containsBean(stepName)); Step step = (Step) ctx.getBean(stepName); assertTrue(step instanceof TaskletStep); @@ -204,13 +218,20 @@ public class StepParserTests { Object composite = ReflectionTestUtils.getField(compositeListener, "list"); List list = (List) ReflectionTestUtils .getField(composite, "list"); - - assertEquals(1, list.size()); - StepExecutionListener listener = list.get(0); - if (listener instanceof Advised) { - listener = (StepExecutionListener) ((Advised) listener).getTargetSource().getTarget(); + List unwrappedList = new ArrayList(); + for (StepExecutionListener listener : list) { + while (listener instanceof Advised) { + listener = (StepExecutionListener) ((Advised) listener).getTargetSource().getTarget(); + } + unwrappedList.add(listener); } - return listener; + return unwrappedList; + } + + private StepExecutionListener getListener(String stepName, ApplicationContext ctx) throws Exception { + List list = getListeners(stepName, ctx); + assertEquals(1, list.size()); + return list.get(0); } private DefaultTransactionAttribute getTransactionAttribute(ApplicationContext ctx, String stepName) { @@ -393,4 +414,120 @@ public class StepParserTests { Object dummyStep = ctx.getBean("dummyStep"); assertTrue(dummyStep instanceof DummyStep); } + + @SuppressWarnings("unchecked") + @Test + public void testStepWithListsMerge() throws Exception { + ApplicationContext ctx = stepParserParentAttributeTestsCtx; + + List> skippable = Arrays.asList(SkippableRuntimeException.class, + SkippableException.class); + Collection> fatal = Arrays.asList(FatalRuntimeException.class, FatalException.class); + Collection> retryable = Arrays.asList(DeadlockLoserDataAccessException.class, + FatalException.class); + List> streams = Arrays.asList(CompositeItemStream.class, TestReader.class); + List> retryListeners = Arrays.asList(RetryListenerSupport.class, + DummyRetryListener.class); + List> stepListeners = Arrays.asList(StepExecutionListenerSupport.class, + CompositeStepExecutionListener.class); + List> noRollback = Arrays.asList(FatalRuntimeException.class, + SkippableRuntimeException.class); + + StepParserStepFactoryBean fb = (StepParserStepFactoryBean) ctx.getBean("&stepWithListsMerge"); + + Collection> skippableFound = getExceptionList(fb, "skippableExceptionClasses"); + Collection> fatalFound = getExceptionList(fb, "fatalExceptionClasses"); + Collection> retryableFound = getExceptionList(fb, "retryableExceptionClasses"); + ItemStream[] streamsFound = (ItemStream[]) ReflectionTestUtils.getField(fb, "streams"); + RetryListener[] retryListenersFound = (RetryListener[]) ReflectionTestUtils.getField(fb, "retryListeners"); + StepListener[] stepListenersFound = (StepListener[]) ReflectionTestUtils.getField(fb, "listeners"); + Collection> noRollbackFound = getExceptionList(fb, "noRollbackExceptionClasses"); + + assertSameCollections(skippable, skippableFound); + assertSameCollections(fatal, fatalFound); + assertSameCollections(retryable, retryableFound); + assertSameCollections(streams, toClassCollection(streamsFound)); + assertSameCollections(retryListeners, toClassCollection(retryListenersFound)); + assertSameCollections(stepListeners, toClassCollection(stepListenersFound)); + assertSameCollections(noRollback, noRollbackFound); + } + + @SuppressWarnings("unchecked") + @Test + public void testStepWithListsNoMerge() throws Exception { + ApplicationContext ctx = stepParserParentAttributeTestsCtx; + + List> skippable = Arrays.asList(SkippableException.class); + List> fatal = Arrays.asList(FatalException.class); + List> retryable = Arrays.asList(FatalException.class); + List> streams = Arrays.asList(CompositeItemStream.class); + List> retryListeners = Arrays.asList(DummyRetryListener.class); + List> stepListeners = Arrays.asList(CompositeStepExecutionListener.class); + List> noRollback = Arrays.asList(SkippableRuntimeException.class); + + StepParserStepFactoryBean fb = (StepParserStepFactoryBean) ctx.getBean("&stepWithListsNoMerge"); + + Collection> skippableFound = getExceptionList(fb, "skippableExceptionClasses"); + Collection> fatalFound = getExceptionList(fb, "fatalExceptionClasses"); + Collection> retryableFound = getExceptionList(fb, "retryableExceptionClasses"); + ItemStream[] streamsFound = (ItemStream[]) ReflectionTestUtils.getField(fb, "streams"); + RetryListener[] retryListenersFound = (RetryListener[]) ReflectionTestUtils.getField(fb, "retryListeners"); + StepListener[] stepListenersFound = (StepListener[]) ReflectionTestUtils.getField(fb, "listeners"); + Collection> noRollbackFound = getExceptionList(fb, "noRollbackExceptionClasses"); + + assertSameCollections(skippable, skippableFound); + assertSameCollections(fatal, fatalFound); + assertSameCollections(retryable, retryableFound); + assertSameCollections(streams, toClassCollection(streamsFound)); + assertSameCollections(retryListeners, toClassCollection(retryListenersFound)); + assertSameCollections(stepListeners, toClassCollection(stepListenersFound)); + assertSameCollections(noRollback, noRollbackFound); + } + + @Test + public void testStepWithListsOverrideWithEmpty() throws Exception { + ApplicationContext ctx = stepParserParentAttributeTestsCtx; + + StepParserStepFactoryBean fb = (StepParserStepFactoryBean) ctx + .getBean("&stepWithListsOverrideWithEmpty"); + + assertEquals(0, getExceptionList(fb, "skippableExceptionClasses").size()); + assertEquals(0, getExceptionList(fb, "fatalExceptionClasses").size()); + assertEquals(0, getExceptionList(fb, "retryableExceptionClasses").size()); + // TODO BATCH-1357: + // assertEquals(0, ((ItemStream[]) ReflectionTestUtils.getField(fb, "streams")).length); + // TODO BATCH-1357: + // assertEquals(0, ((RetryListener[]) ReflectionTestUtils.getField(fb, "retryListeners")).length); + // TODO BATCH-1357: + // assertEquals(0, ((StepListener[]) ReflectionTestUtils.getField(fb, "listeners")).length); + assertEquals(0, getExceptionList(fb, "noRollbackExceptionClasses").size()); + } + + @SuppressWarnings("unchecked") + private Collection> getExceptionList(StepParserStepFactoryBean fb, + String propertyName) { + return (Collection>) ReflectionTestUtils.getField(fb, propertyName); + } + + private void assertSameCollections(Collection expected, Collection actual) { + assertEquals(expected.size(), actual.size()); + assertTrue(expected.containsAll(actual)); + } + + private Collection> toClassCollection(T[] in) throws Exception { + return toClassCollection(Arrays.asList(in)); + } + + @SuppressWarnings("unchecked") + private Collection> toClassCollection(Collection in) throws Exception { + Collection> out = new ArrayList>(); + for (T item : in) { + while (item instanceof Advised) { + item = (T) ((Advised) item).getTargetSource().getTarget(); + } + Class cls = (Class) item.getClass(); + out.add(cls); + } + return out; + } } diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/JobParserParentAttributeTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/JobParserParentAttributeTests-context.xml index 712d95555..6717a607a 100644 --- a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/JobParserParentAttributeTests-context.xml +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/JobParserParentAttributeTests-context.xml @@ -4,9 +4,6 @@ xsi:schemaLocation="http://www.springframework.org/schema/batch http://www.springframework.org/schema/batch/spring-batch-2.0.xsd http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-2.5.xsd"> - - - @@ -69,4 +66,9 @@ + + + + + diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/StepParserParentAttributeTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/StepParserParentAttributeTests-context.xml index 08ba0b543..84fdbd538 100644 --- a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/StepParserParentAttributeTests-context.xml +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/StepParserParentAttributeTests-context.xml @@ -144,4 +144,106 @@ + + + + + + org.springframework.batch.core.step.item.SkippableRuntimeException + + + org.springframework.batch.core.step.item.FatalRuntimeException + + + org.springframework.dao.DeadlockLoserDataAccessException + + + + + + + + + + + + + org.springframework.batch.core.step.item.FatalRuntimeException + + + + + + + + + org.springframework.batch.core.step.item.SkippableException + + + org.springframework.batch.core.step.item.FatalException + + + org.springframework.batch.core.step.item.FatalException + + + + + + + + + + + + + org.springframework.batch.core.step.item.SkippableRuntimeException + + + + + + + + + org.springframework.batch.core.step.item.SkippableException + + + org.springframework.batch.core.step.item.FatalException + + + org.springframework.batch.core.step.item.FatalException + + + + + + + + + + + + + org.springframework.batch.core.step.item.SkippableRuntimeException + + + + + + + + + + + + + + + + + + + + +