diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/StepExecutionListener.java b/spring-batch-core/src/main/java/org/springframework/batch/core/StepExecutionListener.java index 443f3c64e..e1e069b89 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/StepExecutionListener.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/StepExecutionListener.java @@ -33,23 +33,14 @@ public interface StepExecutionListener extends StepListener { */ void beforeStep(StepExecution stepExecution); - /** - * The value returned will be combined with the normal exit status using - * {@link ExitStatus#and(ExitStatus)}. - * - * @param e an exception thrown by the step execution - * - * @return an exit status to be combined with the normal one, or null - */ - ExitStatus onErrorInStep(StepExecution stepExecution, Throwable e); - /** * Give a listener a chance to modify the exit status from a step. The value * returned will be combined with the normal exit status using * {@link ExitStatus#and(ExitStatus)}. * - * Called after successful execution of step's processing logic. Throwing - * exception in this method will cause step to fail. + * Called after execution of step's processing logic (both successful or + * failed). Throwing exception in this method has no effect, it will only be + * logged. * * @return an {@link ExitStatus} to combine with the normal value. Return * null to leave the old value unchanged. diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/listener/CompositeStepExecutionListener.java b/spring-batch-core/src/main/java/org/springframework/batch/core/listener/CompositeStepExecutionListener.java index 015453370..6989df3d6 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/listener/CompositeStepExecutionListener.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/listener/CompositeStepExecutionListener.java @@ -77,19 +77,4 @@ public class CompositeStepExecutionListener implements StepExecutionListener { } } - /** - * Call the registered listeners in reverse order, respecting and - * prioritising those that implement {@link Ordered}. - * @see org.springframework.batch.core.StepExecutionListener#onErrorInStep(StepExecution, - * java.lang.Throwable) - */ - public ExitStatus onErrorInStep(StepExecution stepExecution, Throwable e) { - ExitStatus status = null; - for (Iterator iterator = list.reverse(); iterator.hasNext();) { - StepExecutionListener listener = (StepExecutionListener) iterator.next(); - ExitStatus close = listener.onErrorInStep(stepExecution, e); - status = status != null ? status.and(close) : close; - } - return status; - } } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/listener/MulticasterBatchListener.java b/spring-batch-core/src/main/java/org/springframework/batch/core/listener/MulticasterBatchListener.java index 0f57854aa..4d266b679 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/listener/MulticasterBatchListener.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/listener/MulticasterBatchListener.java @@ -168,20 +168,6 @@ public class MulticasterBatchListener implements StepExecutionListener, Ch } } - /** - * @param t - * @see org.springframework.batch.core.listener.CompositeStepExecutionListener#onErrorInStep(StepExecution, - * Throwable) - */ - public ExitStatus onErrorInStep(StepExecution stepExecution, Throwable t) { - try { - return stepListener.onErrorInStep(stepExecution, t); - } - catch (RuntimeException e) { - throw new StepListenerFailedException("Error in onErrorInStep.", t, e); - } - } - /** * * @see org.springframework.batch.core.listener.CompositeChunkListener#afterChunk() 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 fa37fdd70..2f43ab000 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,6 @@ public abstract class AbstractStep implements Step, InitializingBean, BeanNameAw } stepExecution.setStatus(BatchStatus.COMPLETED); - exitStatus = exitStatus.and(getCompositeListener().afterStep(stepExecution)); try { getJobRepository().update(stepExecution); @@ -219,7 +218,6 @@ public abstract class AbstractStep implements Step, InitializingBean, BeanNameAw stepExecution.addFailureException(e); try { - exitStatus = exitStatus.and(getCompositeListener().onErrorInStep(stepExecution, e)); getJobRepository().updateExecutionContext(stepExecution); } catch (Exception ex) { @@ -228,7 +226,14 @@ public abstract class AbstractStep implements Step, InitializingBean, BeanNameAw } } finally { - + + try { + exitStatus = exitStatus.and(getCompositeListener().afterStep(stepExecution)); + } + catch (Exception e){ + logger.error("Excption in afterStep callback", e); + } + stepExecution.setExitStatus(exitStatus); stepExecution.setEndTime(new Date()); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/listener/CompositeStepExecutionListenerTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/listener/CompositeStepExecutionListenerTests.java index 77f364163..2dd18caae 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/listener/CompositeStepExecutionListenerTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/listener/CompositeStepExecutionListenerTests.java @@ -83,19 +83,4 @@ public class CompositeStepExecutionListenerTests extends TestCase { assertEquals(1, list.size()); } - /** - * Test method for - * {@link org.springframework.batch.core.listener.CompositeStepExecutionListener#beforeStep(StepExecution)}. - */ - public void testOnError() { - listener.register(new StepExecutionListenerSupport() { - public ExitStatus onErrorInStep(StepExecution stepExecution, Throwable e) { - list.add("foo"); - return null; - } - }); - listener.onErrorInStep(null, new RuntimeException()); - assertEquals(1, list.size()); - } - } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/listener/MulticasterBatchListenerTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/listener/MulticasterBatchListenerTests.java index 9c1390ffa..19e174f4c 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/listener/MulticasterBatchListenerTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/listener/MulticasterBatchListenerTests.java @@ -126,37 +126,6 @@ public class MulticasterBatchListenerTests { assertEquals(1, count); } - /** - * Test method for - * {@link org.springframework.batch.core.listener.MulticasterBatchListener#onErrorInStep(org.springframework.batch.core.StepExecution, java.lang.Throwable)} - * . - */ - @Test - public void testOnErrorInStep() { - multicast.onErrorInStep(null, new RuntimeException("foo")); - assertEquals(1, count); - } - - /** - * Test method for - * {@link org.springframework.batch.core.listener.MulticasterBatchListener#onErrorInStep(org.springframework.batch.core.StepExecution, java.lang.Throwable)} - * . - */ - @Test - public void testOnErrorInStepFails() { - error = true; - try { - multicast.onErrorInStep(null, new RuntimeException("foo")); - fail("Expected StepListenerFailedException"); - } - catch (StepListenerFailedException e) { - // expected - String message = e.getCause().getMessage(); - assertEquals("Wrong message: " + message, "foo", message); - } - assertEquals(1, count); - } - /** * Test method for * {@link org.springframework.batch.core.listener.MulticasterBatchListener#afterChunk()} 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 bcf6c43ad..62fed8182 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 @@ -175,15 +175,15 @@ public class AbstractStepTests extends TestCase { assertEquals("listener2#beforeStep", events.get(i++)); assertEquals("open", events.get(i++)); assertEquals("doExecute", events.get(i++)); - assertEquals("listener2#onErrorInStep", events.get(i++)); - assertEquals("listener1#onErrorInStep", events.get(i++)); + assertEquals("listener2#afterStep", events.get(i++)); + assertEquals("listener1#afterStep", events.get(i++)); assertEquals("close", events.get(i++)); assertEquals(7, events.size()); assertEquals(ExitStatus.FAILED.getExitCode(), execution.getExitStatus().getExitCode()); assertTrue("Execution context modifications made by listener should be persisted", repository.saved - .containsKey("onErrorInStep")); + .containsKey("afterStep")); } /** @@ -209,15 +209,15 @@ public class AbstractStepTests extends TestCase { assertEquals("listener2#beforeStep", events.get(i++)); assertEquals("open", events.get(i++)); assertEquals("doExecute", events.get(i++)); - assertEquals("listener2#onErrorInStep", events.get(i++)); - assertEquals("listener1#onErrorInStep", events.get(i++)); + assertEquals("listener2#afterStep", events.get(i++)); + assertEquals("listener1#afterStep", events.get(i++)); assertEquals("close", events.get(i++)); assertEquals(7, events.size()); assertEquals("JOB_INTERRUPTED", execution.getExitStatus().getExitCode()); assertTrue("Execution context modifications made by listener should be persisted", repository.saved - .containsKey("onErrorInStep")); + .containsKey("afterStep")); } /**