Skip to content

Commit db6ef7b

Browse files
committed
Persist step execution status before calling listeners
Before this commit, the step execution status was not persisted in the job repository before calling step execution listeners. Therefore, listeners see the step's status as STARTED and not COMPLETED or FAILED. This was not coherent with the contract of `afterStep` which states that the method is called after the execution of the step's processing logic (whether successful or failed), where the status (both in memory and in the DB) should not be STARTED at that point, but rather a non-running status. This commit adds a job repository update to persist the step's status before calling step execution listeners. Resolves #4362
1 parent 5c07115 commit db6ef7b

File tree

2 files changed

+26
-17
lines changed

2 files changed

+26
-17
lines changed

spring-batch-core/src/main/java/org/springframework/batch/core/step/AbstractStep.java

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -286,37 +286,46 @@ public final void execute(StepExecution stepExecution)
286286
logger.info("Step: [" + stepExecution.getStepName() + "] executed in "
287287
+ BatchMetrics.formatDuration(stepExecutionDuration));
288288
}
289-
try {
290-
// Update the step execution to the latest known value so the
291-
// listeners can act on it
292-
exitStatus = exitStatus.and(stepExecution.getExitStatus());
293-
stepExecution.setExitStatus(exitStatus);
294-
exitStatus = exitStatus.and(getCompositeListener().afterStep(stepExecution));
295-
}
296-
catch (Exception e) {
297-
logger.error(String.format("Exception in afterStep callback in step %s in job %s", name,
298-
stepExecution.getJobExecution().getJobInstance().getJobName()), e);
299-
}
300289

290+
// Update the step execution to the latest known value so the
291+
// listeners can act on it
292+
exitStatus = exitStatus.and(stepExecution.getExitStatus());
293+
stepExecution.setExitStatus(exitStatus);
294+
295+
// save status in job repository before calling listeners
296+
// https://github.com/spring-projects/spring-batch/issues/4362
301297
try {
298+
getJobRepository().update(stepExecution);
302299
getJobRepository().updateExecutionContext(stepExecution);
303300
}
304301
catch (Exception e) {
305302
stepExecution.setStatus(BatchStatus.UNKNOWN);
306-
exitStatus = exitStatus.and(ExitStatus.UNKNOWN);
303+
stepExecution.setExitStatus(exitStatus.and(ExitStatus.UNKNOWN));
307304
stepExecution.addFailureException(e);
308305
logger.error(String.format(
309306
"Encountered an error saving batch meta data for step %s in job %s. "
310307
+ "This job is now in an unknown state and should not be restarted.",
311308
name, stepExecution.getJobExecution().getJobInstance().getJobName()), e);
312309
}
310+
311+
try {
312+
exitStatus = exitStatus.and(getCompositeListener().afterStep(stepExecution));
313+
}
314+
catch (Exception e) {
315+
logger.error(String.format("Exception in afterStep callback in step %s in job %s", name,
316+
stepExecution.getJobExecution().getJobInstance().getJobName()), e);
317+
}
318+
313319
stepExecutionEvent.exitStatus = stepExecution.getExitStatus().getExitCode();
314320
stepExecutionEvent.commit();
315321
stopObservation(stepExecution, observation);
316322
stepExecution.setExitStatus(exitStatus);
317323

324+
// save status in job repository after calling listeners (since afterStep
325+
// might have changed it)
318326
try {
319327
getJobRepository().update(stepExecution);
328+
getJobRepository().updateExecutionContext(stepExecution);
320329
}
321330
catch (Exception e) {
322331
stepExecution.setStatus(BatchStatus.UNKNOWN);

spring-batch-core/src/test/java/org/springframework/batch/core/step/item/TaskletStepExceptionTests.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ public void open(ExecutionContext executionContext) throws ItemStreamException {
128128
taskletStep.execute(stepExecution);
129129
assertEquals(FAILED, stepExecution.getStatus());
130130
assertTrue(stepExecution.getFailureExceptions().contains(exception));
131-
assertEquals(2, jobRepository.getUpdateCount());
131+
assertEquals(3, jobRepository.getUpdateCount());
132132
}
133133

134134
@Test
@@ -144,7 +144,7 @@ public void beforeStep(StepExecution stepExecution) {
144144
taskletStep.execute(stepExecution);
145145
assertEquals(FAILED, stepExecution.getStatus());
146146
assertTrue(stepExecution.getFailureExceptions().contains(exception));
147-
assertEquals(2, jobRepository.getUpdateCount());
147+
assertEquals(3, jobRepository.getUpdateCount());
148148
}
149149

150150
@Test
@@ -170,7 +170,7 @@ public RepeatStatus execute(StepContribution contribution, ChunkContext attribut
170170
taskletStep.execute(stepExecution);
171171
assertEquals(COMPLETED, stepExecution.getStatus());
172172
assertFalse(stepExecution.getFailureExceptions().contains(exception));
173-
assertEquals(3, jobRepository.getUpdateCount());
173+
assertEquals(4, jobRepository.getUpdateCount());
174174
}
175175

176176
/*
@@ -191,7 +191,7 @@ public ExitStatus afterStep(StepExecution stepExecution) {
191191
assertEquals(FAILED, stepExecution.getStatus());
192192
assertTrue(stepExecution.getFailureExceptions().contains(taskletException));
193193
assertFalse(stepExecution.getFailureExceptions().contains(exception));
194-
assertEquals(2, jobRepository.getUpdateCount());
194+
assertEquals(3, jobRepository.getUpdateCount());
195195
}
196196

197197
@Test
@@ -210,7 +210,7 @@ public void close() throws ItemStreamException {
210210
assertEquals(FAILED, stepExecution.getStatus());
211211
assertEquals(stepExecution.getFailureExceptions().get(0), taskletException);
212212
assertEquals(stepExecution.getFailureExceptions().get(1).getSuppressed()[0], exception);
213-
assertEquals(2, jobRepository.getUpdateCount());
213+
assertEquals(3, jobRepository.getUpdateCount());
214214
}
215215

216216
@Test

0 commit comments

Comments
 (0)