From 82bc42d10247a1d0c37a75209d635307c60ad5ca Mon Sep 17 00:00:00 2001 From: dsyer Date: Thu, 9 Apr 2009 11:51:31 +0000 Subject: [PATCH] RESOLVED - issue BATCH-1187: Step shouldn't exit with status=EXECUTING AbstractStep now sets the ExitStatus (by ANDing with the existing value), so a Tasklet does not have to set it manually. --- spring-batch-core/pom.xml | 5 ----- .../springframework/batch/core/StepExecution.java | 8 ++++++-- .../batch/core/job/AbstractJob.java | 5 ++++- .../batch/core/job/flow/support/SimpleFlow.java | 9 +++++++++ .../batch/core/step/AbstractStep.java | 6 ++++-- .../core/step/item/ChunkOrientedTasklet.java | 4 ---- .../xml/FailTransitionJobParserTests.java | 2 +- .../configuration/xml/NameStoringTasklet.java | 2 -- .../batch/core/configuration/xml/TestTasklet.java | 2 -- .../batch/core/step/AbstractStepTests.java | 14 +++++++------- .../core/step/item/ChunkOrientedTaskletTests.java | 4 +++- .../core/step/item/TaskletStepExceptionTests.java | 6 ++++-- .../xml/FailTransitionJobParserTests-context.xml | 4 +++- .../batch/integration/job/TestTasklet.java | 2 -- spring-batch-test/pom.xml | 5 +++++ .../src/test/resources/log4j.properties | 15 +++++++++++++++ 16 files changed, 61 insertions(+), 32 deletions(-) create mode 100644 spring-batch-test/src/test/resources/log4j.properties diff --git a/spring-batch-core/pom.xml b/spring-batch-core/pom.xml index 4a5ae4111..1e86f997e 100644 --- a/spring-batch-core/pom.xml +++ b/spring-batch-core/pom.xml @@ -130,11 +130,6 @@ javax.annotation com.springsource.javax.annotation - - org.apache.log4j - com.springsource.org.apache.log4j - false - org.apache.log4j com.springsource.org.apache.log4j diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/StepExecution.java b/spring-batch-core/src/main/java/org/springframework/batch/core/StepExecution.java index 8f3fda2b4..842c64752 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/StepExecution.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/StepExecution.java @@ -500,11 +500,15 @@ public class StepExecution extends Entity { } public String toString() { + return String.format(getSummary() + ", exitDescription=%s", exitStatus.getExitDescription()); + } + + public String getSummary() { return super.toString() + String.format( ", name=%s, status=%s, exitStatus=%s, readCount=%d, filterCount=%d, writeCount=%d readSkipCount=%d, writeSkipCount=%d" - + ", commitCount=%d, rollbackCount=%d", stepName, status, exitStatus, readCount, - filterCount, writeCount, readSkipCount, writeSkipCount, commitCount, rollbackCount); + + ", commitCount=%d, rollbackCount=%d", stepName, status, exitStatus.getExitCode(), + readCount, filterCount, writeCount, readSkipCount, writeSkipCount, commitCount, rollbackCount); } } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/job/AbstractJob.java b/spring-batch-core/src/main/java/org/springframework/batch/core/job/AbstractJob.java index 020bfff87..fab7f2c53 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/job/AbstractJob.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/job/AbstractJob.java @@ -235,6 +235,8 @@ public abstract class AbstractJob implements Job, StepLocator, BeanNameAware, In */ public final void execute(JobExecution execution) { + logger.debug("Job execution starting: "+execution); + try { if (execution.getStatus() != BatchStatus.STOPPING) { @@ -246,6 +248,7 @@ public abstract class AbstractJob implements Job, StepLocator, BeanNameAware, In try { doExecute(execution); + logger.debug("Job execution complete: "+execution); } catch (RepeatException e) { throw e.getCause(); } @@ -256,6 +259,7 @@ public abstract class AbstractJob implements Job, StepLocator, BeanNameAware, In // with it in the same way as any other interruption. execution.setStatus(BatchStatus.STOPPED); execution.setExitStatus(ExitStatus.COMPLETED); + logger.debug("Job execution was stopped: "+execution); } @@ -290,7 +294,6 @@ public abstract class AbstractJob implements Job, StepLocator, BeanNameAware, In jobRepository.update(execution); - } } 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 6eabea8d3..b0677b67e 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 @@ -25,6 +25,8 @@ import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.batch.core.JobExecutionException; import org.springframework.batch.core.Step; import org.springframework.batch.core.job.flow.Flow; @@ -47,6 +49,8 @@ import org.springframework.beans.factory.InitializingBean; */ public class SimpleFlow implements Flow, InitializingBean { + private static final Log logger = LogFactory.getLog(SimpleFlow.class); + private State startState; private Map> transitionMap = new HashMap>(); @@ -128,12 +132,15 @@ public class SimpleFlow implements Flow, InitializingBean { FlowExecutionStatus status = FlowExecutionStatus.UNKNOWN; State state = stateMap.get(stateName); + logger.debug("Resuming state="+stateName+" with status="+status); + // Terminate if there are no more states while (state != null && status!=FlowExecutionStatus.STOPPED) { stateName = state.getName(); try { + logger.debug("Handling state="+stateName); status = state.handle(executor); } catch (Exception e) { @@ -141,6 +148,8 @@ public class SimpleFlow implements Flow, InitializingBean { throw new FlowExecutionException(String.format("Ended flow=%s at state=%s with exception", name, stateName), e); } + + logger.debug("Completed state="+stateName+" with status="+status); state = nextState(stateName, status); diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/AbstractStep.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/AbstractStep.java index 0762ade04..5090c600c 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/AbstractStep.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/AbstractStep.java @@ -199,7 +199,7 @@ public abstract class AbstractStep implements Step, InitializingBean, BeanNameAw catch (RepeatException e) { throw e.getCause(); } - exitStatus = stepExecution.getExitStatus(); + exitStatus = ExitStatus.COMPLETED.and(stepExecution.getExitStatus()); // Check if someone is trying to stop us if (stepExecution.isTerminateOnly()) { @@ -218,6 +218,8 @@ public abstract class AbstractStep implements Step, InitializingBean, BeanNameAw finally { try { + // Update the step execution to the latest known value so the listeners can act on it + stepExecution.setExitStatus(exitStatus); exitStatus = exitStatus.and(getCompositeListener().afterStep(stepExecution)); } catch (Exception e) { @@ -259,7 +261,7 @@ public abstract class AbstractStep implements Step, InitializingBean, BeanNameAw StepSynchronizationManager.release(); - logger.debug("Step execution complete: " + stepExecution); + logger.debug("Step execution complete: " + stepExecution.getSummary()); } } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ChunkOrientedTasklet.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ChunkOrientedTasklet.java index 15c73e373..4dda2caba 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ChunkOrientedTasklet.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ChunkOrientedTasklet.java @@ -16,7 +16,6 @@ package org.springframework.batch.core.step.item; -import org.springframework.batch.core.ExitStatus; import org.springframework.batch.core.StepContribution; import org.springframework.batch.core.scope.context.ChunkContext; import org.springframework.batch.core.step.tasklet.Tasklet; @@ -81,9 +80,6 @@ public class ChunkOrientedTasklet implements Tasklet { chunkContext.removeAttribute(INPUTS_KEY); chunkContext.setComplete(); - if (inputs.isEnd()) { - contribution.setExitStatus(ExitStatus.COMPLETED); - } return RepeatStatus.continueIf(!inputs.isEnd()); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/FailTransitionJobParserTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/FailTransitionJobParserTests.java index e08501ba8..e74b7bdab 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/FailTransitionJobParserTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/FailTransitionJobParserTests.java @@ -48,7 +48,7 @@ public class FailTransitionJobParserTests extends AbstractJobParserTests { assertTrue(stepNamesList.contains("fail")); assertEquals(BatchStatus.FAILED, jobExecution.getStatus()); - assertEquals("FAILED EARLY TERMINATION", jobExecution.getExitStatus() + assertEquals("EARLY TERMINATION", jobExecution.getExitStatus() .getExitCode()); StepExecution stepExecution1 = getStepExecution(jobExecution, "s1"); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/NameStoringTasklet.java b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/NameStoringTasklet.java index cf10ac325..fa4c52c13 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/NameStoringTasklet.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/NameStoringTasklet.java @@ -17,7 +17,6 @@ package org.springframework.batch.core.configuration.xml; import java.util.List; -import org.springframework.batch.core.ExitStatus; import org.springframework.batch.core.StepContribution; import org.springframework.batch.core.StepExecution; import org.springframework.batch.core.listener.StepExecutionListenerSupport; @@ -44,7 +43,6 @@ public class NameStoringTasklet extends StepExecutionListenerSupport implements if (stepNamesList != null) { stepNamesList.add(stepName); } - contribution.setExitStatus(ExitStatus.COMPLETED); return RepeatStatus.FINISHED; } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/TestTasklet.java b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/TestTasklet.java index f16791526..ff9cb420b 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/TestTasklet.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/configuration/xml/TestTasklet.java @@ -1,6 +1,5 @@ package org.springframework.batch.core.configuration.xml; -import org.springframework.batch.core.ExitStatus; import org.springframework.batch.core.StepContribution; import org.springframework.batch.core.scope.context.ChunkContext; import org.springframework.batch.core.step.tasklet.Tasklet; @@ -11,7 +10,6 @@ public class TestTasklet extends AbstractTestComponent implements Tasklet { public RepeatStatus execute(StepContribution contribution, ChunkContext chunkContext) throws Exception { executed = true; - contribution.setExitStatus(ExitStatus.COMPLETED); return RepeatStatus.FINISHED; } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/AbstractStepTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/AbstractStepTests.java index a4863f057..f5c107460 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/AbstractStepTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/AbstractStepTests.java @@ -85,7 +85,7 @@ public class AbstractStepTests { public ExitStatus afterStep(StepExecution stepExecution) { assertSame(execution, stepExecution); - events.add(getEvent("afterStep")); + events.add(getEvent("afterStep("+stepExecution.getExitStatus().getExitCode()+")")); stepExecution.getExecutionContext().putString("afterStep", "afterStep"); return stepExecution.getExitStatus(); } @@ -175,8 +175,8 @@ public class AbstractStepTests { assertEquals("listener2#beforeStep", events.get(i++)); assertEquals("open", events.get(i++)); assertEquals("doExecute", events.get(i++)); - assertEquals("listener2#afterStep", events.get(i++)); - assertEquals("listener1#afterStep", events.get(i++)); + assertEquals("listener2#afterStep(COMPLETED)", events.get(i++)); + assertEquals("listener1#afterStep(COMPLETED)", events.get(i++)); assertEquals("close", events.get(i++)); assertEquals(7, events.size()); @@ -213,8 +213,8 @@ public class AbstractStepTests { assertEquals("listener2#beforeStep", events.get(i++)); assertEquals("open", events.get(i++)); assertEquals("doExecute", events.get(i++)); - assertEquals("listener2#afterStep", events.get(i++)); - assertEquals("listener1#afterStep", events.get(i++)); + assertEquals("listener2#afterStep(FAILED)", events.get(i++)); + assertEquals("listener1#afterStep(FAILED)", events.get(i++)); assertEquals("close", events.get(i++)); assertEquals(7, events.size()); @@ -251,8 +251,8 @@ public class AbstractStepTests { assertEquals("listener2#beforeStep", events.get(i++)); assertEquals("open", events.get(i++)); assertEquals("doExecute", events.get(i++)); - assertEquals("listener2#afterStep", events.get(i++)); - assertEquals("listener1#afterStep", events.get(i++)); + assertEquals("listener2#afterStep(STOPPED)", events.get(i++)); + assertEquals("listener1#afterStep(STOPPED)", events.get(i++)); assertEquals("close", events.get(i++)); assertEquals(7, events.size()); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/ChunkOrientedTaskletTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/ChunkOrientedTaskletTests.java index 2e818f860..135067a2e 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/ChunkOrientedTaskletTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/ChunkOrientedTaskletTests.java @@ -100,8 +100,10 @@ public class ChunkOrientedTaskletTests { }); StepContribution contribution = new StepContribution(new StepExecution("foo", new JobExecution(new JobInstance( 123L, new JobParameters(), "job")))); + ExitStatus expected = contribution.getExitStatus(); handler.execute(contribution, context); - assertEquals(ExitStatus.COMPLETED.getExitCode(), contribution.getExitStatus().getExitCode()); + // The tasklet does not change the exit code + assertEquals(expected, contribution.getExitStatus()); } } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/TaskletStepExceptionTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/TaskletStepExceptionTests.java index a9abeb9cf..89a969be9 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/TaskletStepExceptionTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/TaskletStepExceptionTests.java @@ -74,6 +74,7 @@ public class TaskletStepExceptionTests { taskletStep.execute(stepExecution); assertEquals(FAILED, stepExecution.getStatus()); + assertEquals(FAILED.toString(), stepExecution.getExitStatus().getExitCode()); } @Test @@ -81,6 +82,7 @@ public class TaskletStepExceptionTests { taskletStep.setStepExecutionListeners(new StepExecutionListener[] { new InterruptionListener() }); taskletStep.execute(stepExecution); assertEquals(STOPPED, stepExecution.getStatus()); + assertEquals(STOPPED.toString(), stepExecution.getExitStatus().getExitCode()); } @Test @@ -116,7 +118,7 @@ public class TaskletStepExceptionTests { } @Test - public void testAfterStepFailure() throws Exception { + public void testAfterStepFailureWhenTaskletSucceeds() throws Exception { final RuntimeException exception = new RuntimeException(); taskletStep.setStepExecutionListeners(new StepExecutionListenerSupport[] { new StepExecutionListenerSupport() { @@ -142,7 +144,7 @@ public class TaskletStepExceptionTests { /* * Exception in afterStep is ignored (only logged). */ - public void testAfterStepFAilure() throws Exception { + public void testAfterStepFailureWhenTaskletFails() throws Exception { final RuntimeException exception = new RuntimeException(); taskletStep.setStepExecutionListeners(new StepExecutionListenerSupport[] { new StepExecutionListenerSupport() { diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/FailTransitionJobParserTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/FailTransitionJobParserTests-context.xml index 5fd93ffce..ca0b25ec2 100644 --- a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/FailTransitionJobParserTests-context.xml +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/FailTransitionJobParserTests-context.xml @@ -11,8 +11,10 @@ - + + + \ No newline at end of file diff --git a/spring-batch-integration/src/test/java/org/springframework/batch/integration/job/TestTasklet.java b/spring-batch-integration/src/test/java/org/springframework/batch/integration/job/TestTasklet.java index dd7754eb5..6d99da878 100644 --- a/spring-batch-integration/src/test/java/org/springframework/batch/integration/job/TestTasklet.java +++ b/spring-batch-integration/src/test/java/org/springframework/batch/integration/job/TestTasklet.java @@ -15,7 +15,6 @@ */ package org.springframework.batch.integration.job; -import org.springframework.batch.core.ExitStatus; import org.springframework.batch.core.StepContribution; import org.springframework.batch.core.scope.context.ChunkContext; import org.springframework.batch.core.step.tasklet.Tasklet; @@ -32,7 +31,6 @@ public class TestTasklet implements Tasklet { * */ public RepeatStatus execute(StepContribution contribution, ChunkContext chunkContext) throws Exception { - contribution.setExitStatus(ExitStatus.COMPLETED); return RepeatStatus.FINISHED; } diff --git a/spring-batch-test/pom.xml b/spring-batch-test/pom.xml index 8ee5da344..141cde906 100755 --- a/spring-batch-test/pom.xml +++ b/spring-batch-test/pom.xml @@ -72,6 +72,11 @@ com.springsource.org.apache.commons.collections + + org.apache.log4j + com.springsource.org.apache.log4j + true + diff --git a/spring-batch-test/src/test/resources/log4j.properties b/spring-batch-test/src/test/resources/log4j.properties new file mode 100644 index 000000000..0fa986064 --- /dev/null +++ b/spring-batch-test/src/test/resources/log4j.properties @@ -0,0 +1,15 @@ +log4j.rootCategory=INFO, stdout + +log4j.appender.stdout=org.apache.log4j.ConsoleAppender +log4j.appender.stdout.layout=org.apache.log4j.PatternLayout +log4j.appender.stdout.layout.ConversionPattern=%d{ABSOLUTE} %5p %t %c{2} - %m%n + +log4j.category.org.apache.activemq=ERROR +log4j.category.org.springframework.batch=DEBUG +log4j.category.org.springframework.batch.support=INFO +# log4j.category.org.springframework.transaction=INFO +log4j.category.org.springframework.jdbc=DEBUG + +# log4j.category.org.hibernate.SQL=DEBUG +# for debugging datasource initialization +# log4j.category.test.jdbc=DEBUG