diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/FlowParser.java b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/FlowParser.java index 41055719a..016c9203f 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/FlowParser.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/FlowParser.java @@ -16,8 +16,13 @@ package org.springframework.batch.core.configuration.xml; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.Set; import org.springframework.batch.core.job.flow.FlowExecutionStatus; import org.springframework.batch.core.job.flow.support.SimpleFlow; @@ -39,6 +44,8 @@ import org.w3c.dom.NodeList; */ public class FlowParser extends AbstractSingleBeanDefinitionParser { + private static final String ID_ATTR = "id"; + private static final String STEP_ELE = "step"; private static final String DECISION_ELE = "decision"; @@ -80,7 +87,7 @@ public class FlowParser extends AbstractSingleBeanDefinitionParser { * * @param flowName the name of the flow * @param jobFactoryRef the reference to the {@link JobParserJobFactoryBean} - * from the enclosing tag + * from the enclosing tag */ public FlowParser(String flowName, String jobFactoryRef) { this.flowName = flowName; @@ -112,23 +119,34 @@ public class FlowParser extends AbstractSingleBeanDefinitionParser { parserContext.pushContainingComponent(compositeDef); boolean stepExists = false; + Map> reachableElementMap = new HashMap>(); + String startElement = null; NodeList children = element.getChildNodes(); for (int i = 0; i < children.getLength(); i++) { Node node = children.item(i); if (node instanceof Element) { String nodeName = node.getLocalName(); + Element child = (Element) node; if (nodeName.equals(STEP_ELE)) { - stateTransitions.addAll(stepParser.parse((Element) node, parserContext, jobFactoryRef)); + stateTransitions.addAll(stepParser.parse(child, parserContext, jobFactoryRef)); stepExists = true; } else if (nodeName.equals(DECISION_ELE)) { - stateTransitions.addAll(decisionParser.parse((Element) node, parserContext)); + stateTransitions.addAll(decisionParser.parse(child, parserContext)); } else if (nodeName.equals(SPLIT_ELE)) { - stateTransitions.addAll(splitParser.parse((Element) node, new ParserContext(parserContext - .getReaderContext(), parserContext.getDelegate(), builder.getBeanDefinition()))); + stateTransitions.addAll(splitParser + .parse(child, new ParserContext(parserContext.getReaderContext(), parserContext + .getDelegate(), builder.getBeanDefinition()))); stepExists = true; } + + if (Arrays.asList(STEP_ELE, DECISION_ELE, SPLIT_ELE).contains(nodeName)) { + reachableElementMap.put(child.getAttribute(ID_ATTR), findReachableElements(child)); + if (startElement == null) { + startElement = child.getAttribute(ID_ATTR); + } + } } } @@ -137,6 +155,15 @@ public class FlowParser extends AbstractSingleBeanDefinitionParser { element); } + // Ensure that all elements are reachable + Set allReachableElements = new HashSet(); + findAllReachableElements(startElement, reachableElementMap, allReachableElements); + for (String elementId : reachableElementMap.keySet()) { + if (!allReachableElements.contains(elementId)) { + parserContext.getReaderContext().error("The element [" + elementId + "] is unreachable", element); + } + } + builder.addConstructorArgValue(flowName); ManagedList managedList = new ManagedList(); @SuppressWarnings( { "unchecked", "unused" }) @@ -149,13 +176,65 @@ public class FlowParser extends AbstractSingleBeanDefinitionParser { } + /** + * Find all of the elements that are pointed to by this element. + * + * @param element + * @return a collection of reachable element names + */ + private Set findReachableElements(Element element) { + Set reachableElements = new HashSet(); + + String nextAttribute = element.getAttribute(NEXT_ATTR); + if (StringUtils.hasText(nextAttribute)) { + reachableElements.add(nextAttribute); + } + + @SuppressWarnings("unchecked") + List nextElements = (List) DomUtils.getChildElementsByTagName(element, NEXT_ELE); + for (Element nextElement : nextElements) { + String toAttribute = nextElement.getAttribute(TO_ATTR); + reachableElements.add(toAttribute); + } + + @SuppressWarnings("unchecked") + List stopElements = (List) DomUtils.getChildElementsByTagName(element, STOP_ELE); + for (Element stopElement : stopElements) { + String restartAttribute = stopElement.getAttribute(RESTART_ATTR); + reachableElements.add(restartAttribute); + } + + return reachableElements; + } + + /** + * Find all of the elements reachable from the startElement. + * + * @param startElement + * @param reachableElementMap + * @param accumulator a collection of reachable element names + */ + private void findAllReachableElements(String startElement, Map> reachableElementMap, + Set accumulator) { + Set reachableIds = reachableElementMap.get(startElement); + accumulator.add(startElement); + if (reachableIds != null) { + for (String reachable : reachableIds) { + // don't explore a previously explored element; prevent loop + if (!accumulator.contains(reachable)) { + findAllReachableElements(reachable, reachableElementMap, accumulator); + } + } + } + } + /** * @param parserContext the parser context for the bean factory * @param stateDef The bean definition for the current state * @param element the <step/gt; element to parse * @return a collection of - * {@link org.springframework.batch.core.job.flow.support.StateTransition} - * references + * {@link org.springframework.batch.core.job.flow.support.StateTransition} + * references */ protected static Collection getNextElements(ParserContext parserContext, BeanDefinition stateDef, Element element) { @@ -165,12 +244,12 @@ public class FlowParser extends AbstractSingleBeanDefinitionParser { /** * @param parserContext the parser context for the bean factory * @param stepId the id of the current state if it is a step state, null - * otherwise + * otherwise * @param stateDef The bean definition for the current state * @param element the <step/gt; element to parse * @return a collection of - * {@link org.springframework.batch.core.job.flow.support.StateTransition} - * references + * {@link org.springframework.batch.core.job.flow.support.StateTransition} + * references */ protected static Collection getNextElements(ParserContext parserContext, String stepId, BeanDefinition stateDef, Element element) { @@ -234,8 +313,8 @@ public class FlowParser extends AbstractSingleBeanDefinitionParser { * @param stateDef The bean definition for the current state * @param parserContext the parser context for the bean factory * @param a collection of - * {@link org.springframework.batch.core.job.flow.support.StateTransition} - * references + * {@link org.springframework.batch.core.job.flow.support.StateTransition} + * references */ private static Collection parseTransitionElement(Element transitionElement, String stateId, BeanDefinition stateDef, ParserContext parserContext) { @@ -255,18 +334,18 @@ public class FlowParser extends AbstractSingleBeanDefinitionParser { /** * @param status The batch status that this transition will set. Use - * BatchStatus.UNKNOWN if not applicable. + * BatchStatus.UNKNOWN if not applicable. * @param on The pattern that this transition should match. Use null for - * "no restriction" (same as "*"). + * "no restriction" (same as "*"). * @param next The state to which this transition should go. Use null if not - * applicable. + * applicable. * @param exitCode The exit code that this transition will set. Use null to - * default to batchStatus. + * default to batchStatus. * @param stateDef The bean definition for the current state * @param parserContext the parser context for the bean factory * @param a collection of - * {@link org.springframework.batch.core.job.flow.support.StateTransition} - * references + * {@link org.springframework.batch.core.job.flow.support.StateTransition} + * references */ private static Collection createTransition(FlowExecutionStatus status, String on, String next, String exitCode, BeanDefinition stateDef, ParserContext parserContext, boolean abandon) { @@ -336,7 +415,7 @@ public class FlowParser extends AbstractSingleBeanDefinitionParser { * @param on the pattern value * @param next the next step id * @return a bean definition for a - * {@link org.springframework.batch.core.job.flow.support.StateTransition} + * {@link org.springframework.batch.core.job.flow.support.StateTransition} */ public static BeanDefinition getStateTransitionReference(ParserContext parserContext, BeanDefinition stateDefinition, String on, String next) { 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 4cdff6d02..e3224c78a 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 @@ -17,10 +17,12 @@ package org.springframework.batch.core.configuration.xml; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.util.ArrayList; import java.util.List; +import org.junit.BeforeClass; import org.junit.Test; import org.springframework.aop.framework.Advised; import org.springframework.batch.core.Job; @@ -29,6 +31,7 @@ import org.springframework.batch.core.job.AbstractJob; import org.springframework.batch.core.listener.JobExecutionListenerSupport; import org.springframework.batch.core.repository.JobRepository; import org.springframework.batch.core.repository.support.SimpleJobRepository; +import org.springframework.beans.factory.parsing.BeanDefinitionParsingException; import org.springframework.context.ApplicationContext; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.support.ClassPathXmlApplicationContext; @@ -40,8 +43,13 @@ import org.springframework.test.util.ReflectionTestUtils; */ public class JobParserTests { - ConfigurableApplicationContext jobParserParentAttributeTestsCtx = new ClassPathXmlApplicationContext( - "org/springframework/batch/core/configuration/xml/JobParserParentAttributeTests-context.xml"); + private static ConfigurableApplicationContext jobParserParentAttributeTestsCtx; + + @BeforeClass + public static void loadAppCtx() { + jobParserParentAttributeTestsCtx = new ClassPathXmlApplicationContext( + "org/springframework/batch/core/configuration/xml/JobParserParentAttributeTests-context.xml"); + } @Test public void testInheritListeners() throws Exception { @@ -136,4 +144,28 @@ public class JobParserTests { assertTrue(jobRepository instanceof JobRepository); return (JobRepository) jobRepository; } + + @Test + public void testUnreachableStep() { + try { + new ClassPathXmlApplicationContext( + "org/springframework/batch/core/configuration/xml/JobParserUnreachableStepTests-context.xml"); + fail("Error expected"); + } + catch (BeanDefinitionParsingException e) { + assertTrue(e.getMessage().contains("The element [s2] is unreachable")); + } + } + + @Test + public void testNextOutOfScope() { + try { + new ClassPathXmlApplicationContext( + "org/springframework/batch/core/configuration/xml/JobParserNextOutOfScopeTests-context.xml"); + fail("Error expected"); + } + catch (BeanDefinitionParsingException e) { + assertTrue(e.getMessage().contains("Missing state for [StateTransition: [state=s2, pattern=*, next=s3]]")); + } + } } 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 091973650..23dcd739e 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 @@ -21,6 +21,7 @@ import static org.junit.Assert.assertTrue; import java.util.List; import java.util.Map; +import org.junit.BeforeClass; import org.junit.Test; import org.springframework.aop.framework.Advised; import org.springframework.batch.core.Step; @@ -52,8 +53,13 @@ import org.springframework.transaction.interceptor.DefaultTransactionAttribute; */ public class StepParserTests { - private static final ApplicationContext stepParserParentAttributeTestsCtx = new ClassPathXmlApplicationContext( - "org/springframework/batch/core/configuration/xml/StepParserParentAttributeTests-context.xml"); + private static ApplicationContext stepParserParentAttributeTestsCtx; + + @BeforeClass + public static void loadAppCtx() { + stepParserParentAttributeTestsCtx = new ClassPathXmlApplicationContext( + "org/springframework/batch/core/configuration/xml/StepParserParentAttributeTests-context.xml"); + } @SuppressWarnings("unchecked") @Test diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ChunkElementParentAttributeParserTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ChunkElementParentAttributeParserTests-context.xml index 0e576029e..7ed66a0f4 100644 --- a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ChunkElementParentAttributeParserTests-context.xml +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/ChunkElementParentAttributeParserTests-context.xml @@ -7,7 +7,7 @@ - + diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/JobParserNextOutOfScopeTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/JobParserNextOutOfScopeTests-context.xml new file mode 100644 index 000000000..94a766b4c --- /dev/null +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/JobParserNextOutOfScopeTests-context.xml @@ -0,0 +1,21 @@ + + + + + + + + + + + + + + + + + + diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/JobParserUnreachableStepTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/JobParserUnreachableStepTests-context.xml new file mode 100644 index 000000000..098f27711 --- /dev/null +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/JobParserUnreachableStepTests-context.xml @@ -0,0 +1,15 @@ + + + + + + + + + + + + diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/StepListenerParserTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/StepListenerParserTests-context.xml index a1270b1ad..610a073b2 100644 --- a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/StepListenerParserTests-context.xml +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/StepListenerParserTests-context.xml @@ -7,7 +7,7 @@ - + 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 c3b8268b0..08ba0b543 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 @@ -35,11 +35,11 @@ - - + + - + @@ -104,20 +104,20 @@ - - + + - - + + - - + + @@ -135,9 +135,9 @@ - - - + + +