From 7746cfc304a36fb557fd84e7143fc614b6be8ab0 Mon Sep 17 00:00:00 2001 From: robokaso Date: Tue, 1 Apr 2008 11:07:53 +0000 Subject: [PATCH] RESOLVED - issue BATCH-529: StepExecutionListener.afterStep() should only be called on success RESOLVED - issue BATCH-532: StepExecutionListener can influence exit status but not status of step execution listener.afterStep() call moved from catch block to the end of the try block --- .../core/step/item/ItemOrientedStep.java | 9 +---- .../core/step/item/ItemOrientedStepTests.java | 38 ++++++++++++++++--- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ItemOrientedStep.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ItemOrientedStep.java index 4cc28b2e2..2689b6c4c 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ItemOrientedStep.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ItemOrientedStep.java @@ -350,6 +350,8 @@ public class ItemOrientedStep extends AbstractStep { } }); + + status = status.and(listener.afterStep(stepExecution)); fatalException.setException(updateStatus(stepExecution, BatchStatus.COMPLETED)); } @@ -371,13 +373,6 @@ public class ItemOrientedStep extends AbstractStep { } finally { - try { - status = status.and(listener.afterStep(stepExecution)); - } - catch (RuntimeException e) { - logger.error("Unexpected error in listener after step.", e); - } - stepExecution.setExitStatus(status); stepExecution.setEndTime(new Date(System.currentTimeMillis())); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/ItemOrientedStepTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/ItemOrientedStepTests.java index d773f13c7..5bdba71b8 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/ItemOrientedStepTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/ItemOrientedStepTests.java @@ -278,7 +278,7 @@ public class ItemOrientedStepTests extends TestCase { } }); itemOrientedStep.execute(stepExecution); - + // context saved before processing starts and updated at the end assertEquals(2, list.size()); } @@ -288,9 +288,11 @@ public class ItemOrientedStepTests extends TestCase { final StepExecution stepExecution = new StepExecution(itemOrientedStep, jobExecution); itemOrientedStep.setJobRepository(new JobRepositorySupport() { private int counter = 0; + // initial save before item processing succeeds, later calls fail public void saveOrUpdateExecutionContext(StepExecution stepExecution) { - if (counter > 0) throw new RuntimeException("foo"); + if (counter > 0) + throw new RuntimeException("foo"); counter++; } }); @@ -744,7 +746,7 @@ public class ItemOrientedStepTests extends TestCase { /** * Execution context must not be left empty even if job failed before * commiting first chunk - otherwise ItemStreams won't recognize it is - * restart scenario on next run. + * restart scenario on next run. */ public void testRestartAfterFailureInFirstChunk() throws Exception { MockRestartableItemReader reader = new MockRestartableItemReader() { @@ -755,15 +757,14 @@ public class ItemOrientedStepTests extends TestCase { }; itemOrientedStep.setItemHandler(new SimpleItemHandler(reader, itemWriter)); itemOrientedStep.registerStream(reader); - + StepExecution stepExecution = new StepExecution(itemOrientedStep, new JobExecution(jobInstance)); - + try { itemOrientedStep.execute(stepExecution); fail("Expected InfrastructureException"); } catch (RuntimeException expected) { - // The job actually completed, but the streams couldn't be closed. assertEquals(BatchStatus.FAILED, stepExecution.getStatus()); assertEquals("CRASH!", expected.getMessage()); assertFalse(stepExecution.getExecutionContext().isEmpty()); @@ -771,6 +772,31 @@ public class ItemOrientedStepTests extends TestCase { } } + /** + * Exception in {@link StepExecutionListener#afterStep(StepExecution)} + * causes step to fail. + * @throws JobInterruptedException + */ + public void testStepFailureInAfterStepCallback() throws JobInterruptedException { + StepExecutionListener listener = new StepExecutionListenerSupport() { + public ExitStatus afterStep(StepExecution stepExecution) { + throw new RuntimeException("exception thrown in afterStep to signal failure"); + } + }; + itemOrientedStep.setStepExecutionListeners(new StepExecutionListener[] { listener }); + StepExecution stepExecution = new StepExecution(itemOrientedStep, new JobExecution(jobInstance)); + try { + itemOrientedStep.execute(stepExecution); + fail(); + } + catch (RuntimeException expected) { + assertEquals("exception thrown in afterStep to signal failure", expected.getMessage()); + } + + assertEquals(BatchStatus.FAILED, stepExecution.getStatus()); + + } + private boolean contains(String str, String searchStr) { return str.indexOf(searchStr) != -1; }