From ecddd61b478128dffee33ca8ffc70c145929be2b Mon Sep 17 00:00:00 2001 From: dsyer Date: Thu, 20 Aug 2009 16:41:38 +0000 Subject: [PATCH] RESOLVED - issue BATCH-1354: Infinite loop caused by throwing an Error from the ItemWriter of a skippable step http://jira.springframework.org/browse/BATCH-1354 Previous change reverted: Error should be fatal, so it has to be added to the exception classifiers in FaultTolerantStepFactoryBean. --- .../item/FaultTolerantChunkProcessor.java | 25 +++++++------------ .../item/FaultTolerantStepFactoryBean.java | 4 +-- ...tractExceptionThrowingItemHandlerStub.java | 19 +++++++++++--- .../FaultTolerantChunkProcessorTests.java | 19 ++++++-------- ...tTolerantStepFactoryBeanRollbackTests.java | 16 ++++++++++++ .../core/step/tasklet/TaskletStepTests.java | 1 - 6 files changed, 49 insertions(+), 35 deletions(-) diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/FaultTolerantChunkProcessor.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/FaultTolerantChunkProcessor.java index a8d6aab10..6330fb532 100755 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/FaultTolerantChunkProcessor.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/FaultTolerantChunkProcessor.java @@ -283,11 +283,8 @@ public class FaultTolerantChunkProcessor extends SimpleChunkProcessor extends SimpleChunkProcessor 1 && !rollbackClassifier.classify(e)) { throw new RetryException("Invalid retry state during write caused by " + "exception that does not classify for rollback: ", e); @@ -387,7 +384,7 @@ public class FaultTolerantChunkProcessor extends SimpleChunkProcessor extends SimpleChunkProcessor wrapper : outputs.getSkips()) { - Throwable e = wrapper.getException(); + Exception e = wrapper.getException(); try { getListener().onSkipInWrite(wrapper.getItem(), e); } @@ -467,20 +464,16 @@ public class FaultTolerantChunkProcessor extends SimpleChunkProcessor extends SimpleStepFactoryBean { private Collection failures = Collections.emptyList(); - private Constructor exception; + private Constructor exception; public AbstractExceptionThrowingItemHandlerStub() throws Exception { exception = SkippableRuntimeException.class.getConstructor(String.class); @@ -44,8 +44,12 @@ public abstract class AbstractExceptionThrowingItemHandlerStub { this.failures = new ArrayList(Arrays.asList(failures)); } - public void setExceptionType(Class exceptionType) throws Exception { - exception = exceptionType.getConstructor(String.class); + public void setExceptionType(Class exceptionType) throws Exception { + try { + exception = exceptionType.getConstructor(String.class); + } catch (NoSuchMethodException e) { + exception = exceptionType.getConstructor(Object.class); + } } public void clearFailures() { @@ -54,7 +58,14 @@ public abstract class AbstractExceptionThrowingItemHandlerStub { protected void checkFailure(T item) throws Exception { if (isFailure(item)) { - throw exception.newInstance("Intended Failure: " + item); + Throwable t = exception.newInstance("Intended Failure: " + item); + if (t instanceof Exception) { + throw (Exception) t; + } + if (t instanceof Error) { + throw (Error) t; + } + throw new IllegalStateException("Unexpected non-Error Throwable"); } } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantChunkProcessorTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantChunkProcessorTests.java index a7962adc0..e52028a96 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantChunkProcessorTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantChunkProcessorTests.java @@ -98,7 +98,11 @@ public class FaultTolerantChunkProcessorTests { assertEquals(1, contribution.getFilterCount()); } - @Test + /** + * An Error pops right back up (no skips, no retry) + * @throws Exception + */ + @Test(expected=AssertionError.class) public void testWriteSkipOnError() throws Exception { processor.setWriteSkipPolicy(new AlwaysSkipItemSkipPolicy()); processor.setItemWriter(new ItemWriter() { @@ -113,19 +117,10 @@ public class FaultTolerantChunkProcessorTests { processor.process(contribution, inputs); fail("Expected Error"); } - catch (IllegalStateException e) { - assertEquals("Expected Error!", e.getCause().getMessage()); + catch (Error e) { + assertEquals("Expected Error!", e.getMessage()); } processor.process(contribution, inputs); - try { - processor.process(contribution, inputs); - fail("Expected Error"); - } - catch (IllegalStateException e) { - assertEquals("Expected Error!", e.getCause().getMessage()); - } - assertEquals(1, contribution.getSkipCount()); - assertEquals(1, contribution.getWriteCount()); } @Test diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBeanRollbackTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBeanRollbackTests.java index b03ec641e..259061f0c 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBeanRollbackTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBeanRollbackTests.java @@ -236,6 +236,22 @@ public class FaultTolerantStepFactoryBeanRollbackTests { assertEquals(4, stepExecution.getRollbackCount()); } + /** + * Scenario: Exception in writer that should not cause rollback and scan + */ + @Test + public void testWriterDefaultRollbackOnError() throws Exception { + writer.setFailures("2", "3"); + writer.setExceptionType(AssertionError.class); + + Step step = (Step) factory.getObject(); + + step.execute(stepExecution); + assertEquals(BatchStatus.FAILED, stepExecution.getStatus()); + assertEquals(0, stepExecution.getSkipCount()); + assertEquals(1, stepExecution.getRollbackCount()); + } + /** * Scenario: Exception in writer that should not cause rollback and scan */ diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/tasklet/TaskletStepTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/tasklet/TaskletStepTests.java index abc73cef7..3f1144415 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/tasklet/TaskletStepTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/tasklet/TaskletStepTests.java @@ -26,7 +26,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import org.apache.commons.lang.mutable.MutableInt; import org.junit.Before; import org.junit.Test; import org.springframework.batch.core.BatchStatus;