From d76209df10065b3e04ebe1048b9b1e3caec219bc Mon Sep 17 00:00:00 2001 From: Chris Schaefer Date: Tue, 17 Dec 2013 17:20:51 -0500 Subject: [PATCH] Prevent NPE when checking for a re-runnable continued flow, remove usage of state transition comparator in FlowParser, ensure custom exit code is set at job level. Fixes TCK test testDeciderExitStatusIsSetOnJobContext, contributes but does not fix decider transition tests that include restart. --- .../core/job/flow/support/SimpleFlow.java | 6 ++++- .../jsr/configuration/xml/FlowParser.java | 4 +-- .../core/jsr/job/flow/JsrFlowExecutor.java | 3 +++ .../batch/core/jsr/step/DecisionStep.java | 5 +++- .../core/jsr/step/DecisionStepTests.java | 19 ++++++++++++++ ...Tests-decisionCustomExitStatus-context.xml | 26 +++++++++++++++++++ 6 files changed, 58 insertions(+), 5 deletions(-) create mode 100644 spring-batch-core/src/test/resources/org/springframework/batch/core/jsr/step/DecisionStepTests-decisionCustomExitStatus-context.xml diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/job/flow/support/SimpleFlow.java b/spring-batch-core/src/main/java/org/springframework/batch/core/job/flow/support/SimpleFlow.java index 7b3feed10..fcf228e66 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/job/flow/support/SimpleFlow.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/job/flow/support/SimpleFlow.java @@ -237,7 +237,7 @@ public class SimpleFlow implements Flow, InitializingBean { if(stepExecution != null) { Boolean reRun = (Boolean) stepExecution.getExecutionContext().get("batch.restart"); - if(reRun != null && reRun && status == FlowExecutionStatus.STOPPED && !state.getName().endsWith(stepExecution.getStepName())) { + if(reRun != null && reRun && status == FlowExecutionStatus.STOPPED && stateNameEndsWithStepName(state, stepExecution)) { continued = true; } } @@ -245,6 +245,10 @@ public class SimpleFlow implements Flow, InitializingBean { return continued; } + private boolean stateNameEndsWithStepName(State state, StepExecution stepExecution) { + return !(stepExecution == null || state == null) && !state.getName().endsWith(stepExecution.getStepName()); + } + /** * Analyse the transitions provided and generate all the information needed * to execute the flow. diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/configuration/xml/FlowParser.java b/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/configuration/xml/FlowParser.java index 6670ecacf..5ffdab85d 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/configuration/xml/FlowParser.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/configuration/xml/FlowParser.java @@ -26,10 +26,8 @@ import java.util.Set; import org.springframework.batch.core.configuration.xml.AbstractFlowParser; import org.springframework.batch.core.configuration.xml.SimpleFlowFactoryBean; import org.springframework.batch.core.job.flow.FlowExecutionStatus; -import org.springframework.batch.core.job.flow.support.DefaultStateTransitionComparator; import org.springframework.batch.core.jsr.job.flow.support.DefaultFlow; import org.springframework.beans.factory.config.BeanDefinition; -import org.springframework.beans.factory.config.RuntimeBeanReference; import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.ManagedList; import org.springframework.beans.factory.xml.ParserContext; @@ -143,7 +141,7 @@ public class FlowParser extends AbstractFlowParser { builder.getRawBeanDefinition().setAttribute("flowName", idAttribute); builder.addPropertyValue("name", idAttribute); - builder.addPropertyValue("stateTransitionComparator", new RuntimeBeanReference(DefaultStateTransitionComparator.STATE_TRANSITION_COMPARATOR)); + doParse(element, parserContext, builder); builder.setRole(BeanDefinition.ROLE_INFRASTRUCTURE); diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/job/flow/JsrFlowExecutor.java b/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/job/flow/JsrFlowExecutor.java index c2c2e2435..c4dd26117 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/job/flow/JsrFlowExecutor.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/job/flow/JsrFlowExecutor.java @@ -60,6 +60,9 @@ public class JsrFlowExecutor extends JobFlowExecutor { if(isNonDefaultExitStatus(curStatus.getExitCode())) { exitStatus = exitStatus.and(new ExitStatus(status.getName())); execution.setExitStatus(exitStatus); + } else { + exitStatus = exitStatus.and(curStatus); + execution.setExitStatus(exitStatus); } } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/step/DecisionStep.java b/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/step/DecisionStep.java index 8b57919f8..dbc84b3be 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/step/DecisionStep.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/jsr/step/DecisionStep.java @@ -71,7 +71,10 @@ public class DecisionStep extends AbstractStep { executions[0] = new org.springframework.batch.core.jsr.StepExecution(lastExecution); try { - stepExecution.setExitStatus(new ExitStatus(decider.decide(executions))); + ExitStatus exitStatus = new ExitStatus(decider.decide(executions)); + + stepExecution.getJobExecution().setExitStatus(exitStatus); + stepExecution.setExitStatus(exitStatus); } catch (Exception e) { stepExecution.setTerminateOnly(); stepExecution.addFailureException(e); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/jsr/step/DecisionStepTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/jsr/step/DecisionStepTests.java index 58f4c6df9..ef9f2f631 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/jsr/step/DecisionStepTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/jsr/step/DecisionStepTests.java @@ -86,6 +86,19 @@ public class DecisionStepTests { } } + @Test + public void testDecisionCustomExitStatus() throws Exception { + ApplicationContext context = new GenericXmlApplicationContext("classpath:/org/springframework/batch/core/jsr/step/DecisionStepTests-decisionCustomExitStatus-context.xml"); + + JobLauncher launcher = context.getBean(JobLauncher.class); + Job job = context.getBean(Job.class); + + JobExecution execution = launcher.run(job, new JobParameters()); + assertEquals(BatchStatus.FAILED, execution.getStatus()); + assertEquals(2, execution.getStepExecutions().size()); + assertEquals("CustomFail", execution.getExitStatus().getExitCode()); + } + @Test @Ignore("Flows as first steps are not supported yet") public void testDecisionAfterFlow() throws Exception { @@ -108,6 +121,12 @@ public class DecisionStepTests { @Override public String decide(StepExecution[] executions) throws Exception { + for(StepExecution stepExecution : executions) { + if ("customFailTest".equals(stepExecution.getStepName())) { + return "CustomFail"; + } + } + return "next"; } } diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/jsr/step/DecisionStepTests-decisionCustomExitStatus-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/jsr/step/DecisionStepTests-decisionCustomExitStatus-context.xml new file mode 100644 index 000000000..3b4a895da --- /dev/null +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/jsr/step/DecisionStepTests-decisionCustomExitStatus-context.xml @@ -0,0 +1,26 @@ + + + + + + + + + + + + + + + + + + + + + + +