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 0abf493c6..e03a270cf 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 @@ -46,7 +46,8 @@ import org.springframework.batch.retry.support.DefaultRetryState; * allows for skipping or retry of items that cause exceptions during writing. * */ -public class FaultTolerantChunkProcessor extends SimpleChunkProcessor { +public class FaultTolerantChunkProcessor extends + SimpleChunkProcessor { private SkipPolicy itemProcessSkipPolicy = new LimitCheckingItemSkipPolicy(); @@ -54,7 +55,8 @@ public class FaultTolerantChunkProcessor extends SimpleChunkProcessor rollbackClassifier = new BinaryExceptionClassifier(true); + private Classifier rollbackClassifier = new BinaryExceptionClassifier( + true); private Log logger = LogFactory.getLog(getClass()); @@ -71,21 +73,24 @@ public class FaultTolerantChunkProcessor extends SimpleChunkProcessor extends SimpleChunkProcessor rollbackClassifier) { + public void setRollbackClassifier( + Classifier rollbackClassifier) { this.rollbackClassifier = rollbackClassifier; } @@ -125,14 +131,17 @@ public class FaultTolerantChunkProcessor extends SimpleChunkProcessor itemProcessor, - ItemWriter itemWriter, BatchRetryTemplate batchRetryTemplate) { + public FaultTolerantChunkProcessor( + ItemProcessor itemProcessor, + ItemWriter itemWriter, + BatchRetryTemplate batchRetryTemplate) { super(itemProcessor, itemWriter); this.batchRetryTemplate = batchRetryTemplate; } @@ -193,16 +202,19 @@ public class FaultTolerantChunkProcessor extends SimpleChunkProcessor transform(final StepContribution contribution, Chunk inputs) throws Exception { + protected Chunk transform(final StepContribution contribution, + Chunk inputs) throws Exception { Chunk outputs = new Chunk(); @SuppressWarnings("unchecked") final UserData data = (UserData) inputs.getUserData(); final Chunk cache = data.getOutputs(); - final Iterator cacheIterator = cache.isEmpty() ? null : new ArrayList(cache.getItems()).iterator(); + final Iterator cacheIterator = cache.isEmpty() ? null + : new ArrayList(cache.getItems()).iterator(); final AtomicInteger count = new AtomicInteger(0); - for (final Chunk.ChunkIterator iterator = inputs.iterator(); iterator.hasNext();) { + for (final Chunk.ChunkIterator iterator = inputs.iterator(); iterator + .hasNext();) { final int scanLimit = processorTransactional ? 1 : 0; final I item = iterator.next(); @@ -213,7 +225,8 @@ public class FaultTolerantChunkProcessor extends SimpleChunkProcessor scanLimit) { /* * If there is a cached chunk then we must be @@ -223,30 +236,29 @@ public class FaultTolerantChunkProcessor extends SimpleChunkProcessor extends SimpleChunkProcessor extends SimpleChunkProcessor extends SimpleChunkProcessor inputs, final Chunk outputs) - throws Exception { + protected void write(final StepContribution contribution, + final Chunk inputs, final Chunk outputs) throws Exception { RetryCallback retryCallback = new RetryCallback() { public Object doWithRetry(RetryContext context) throws Exception { @@ -311,8 +326,7 @@ public class FaultTolerantChunkProcessor extends SimpleChunkProcessor 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); + throw new RetryException( + "Invalid retry state during write caused by " + + "exception that does not classify for rollback: ", + e); } Chunk.ChunkIterator inputIterator = inputs.iterator(); - for (Chunk.ChunkIterator outputIterator = outputs.iterator(); outputIterator.hasNext();) { + for (Chunk.ChunkIterator outputIterator = outputs + .iterator(); outputIterator.hasNext();) { inputIterator.next(); outputIterator.next(); - checkSkipPolicy(inputIterator, outputIterator, e, contribution); + checkSkipPolicy(inputIterator, outputIterator, e, + contribution, true); if (!rollbackClassifier.classify(e)) { throw new RetryException( "Invalid retry state during recovery caused by exception that does not classify for rollback: ", @@ -369,10 +387,10 @@ public class FaultTolerantChunkProcessor extends SimpleChunkProcessor recoveryCallback = new RecoveryCallback() { @@ -383,14 +401,15 @@ public class FaultTolerantChunkProcessor extends SimpleChunkProcessor extends SimpleChunkProcessor extends SimpleChunkProcessor extends SimpleChunkProcessor extends SimpleChunkProcessor extends SimpleChunkProcessor.ChunkIterator inputIterator, Chunk.ChunkIterator outputIterator, - Throwable e, StepContribution contribution) { + private void checkSkipPolicy(Chunk.ChunkIterator inputIterator, + Chunk.ChunkIterator outputIterator, Throwable e, + StepContribution contribution, boolean recovery) throws Exception { logger.debug("Checking skip policy after failed write"); if (shouldSkip(itemWriteSkipPolicy, e, contribution.getStepSkipCount())) { contribution.incrementWriteSkipCount(); inputIterator.remove(); outputIterator.remove(e); logger.debug("Skipping after failed write", e); - } - else { - throw new RetryException("Non-skippable exception in recoverer", e); + } else { + if (recovery) { + // Only if already recovering should we check skip policy + throw new RetryException( + "Non-skippable exception in recoverer", e); + } else { + if (e instanceof Exception) { + throw (Exception) e; + } else if (e instanceof Error) { + throw (Error) e; + } else { + throw new RetryException( + "Non-skippable throwable in recoverer", e); + } + } } } - private void scan(final StepContribution contribution, final Chunk inputs, final Chunk outputs, - ChunkMonitor chunkMonitor) throws Exception { + private void scan(final StepContribution contribution, + final Chunk inputs, final Chunk outputs, + ChunkMonitor chunkMonitor, boolean recovery) throws Exception { if (logger.isDebugEnabled()) { - logger.debug("Scanning for failed item on write: " + inputs); + if (recovery) { + logger.debug("Scanning for failed item on recovery from write: " + + inputs); + } else { + logger.debug("Scanning for failed item on write: " + inputs); + } } if (outputs.isEmpty()) { inputs.setBusy(false); @@ -527,15 +569,15 @@ public class FaultTolerantChunkProcessor extends SimpleChunkProcessor list = new ArrayList(); private List after = new ArrayList(); - + private List writeError = new ArrayList(); private FaultTolerantChunkProcessor processor; - private StepContribution contribution = new StepExecution("foo", new JobExecution(0L)).createStepContribution(); + private StepContribution contribution = new StepExecution("foo", + new JobExecution(0L)).createStepContribution(); @Before public void setUp() { batchRetryTemplate = new BatchRetryTemplate(); - processor = new FaultTolerantChunkProcessor(new PassThroughItemProcessor(), + processor = new FaultTolerantChunkProcessor( + new PassThroughItemProcessor(), new ItemWriter() { - public void write(List items) throws Exception { + public void write(List items) + throws Exception { if (items.contains("fail")) { throw new RuntimeException("Planned failure!"); } @@ -90,8 +96,7 @@ public class FaultTolerantChunkProcessorTests { try { processor.process(contribution, inputs); fail("Expected Exception"); - } - catch (Exception e) { + } catch (Exception e) { assertEquals("Skippable", e.getMessage()); } processor.process(contribution, inputs); @@ -102,6 +107,7 @@ public class FaultTolerantChunkProcessorTests { /** * An Error can be retried or skipped but by default it is just propagated + * * @throws Exception */ @Test @@ -114,12 +120,12 @@ public class FaultTolerantChunkProcessorTests { } } }); - Chunk inputs = new Chunk(Arrays.asList("3", "fail", "2")); + Chunk inputs = new Chunk( + Arrays.asList("3", "fail", "2")); try { processor.process(contribution, inputs); fail("Expected Error"); - } - catch (Error e) { + } catch (Error e) { assertEquals("Expected Error!", e.getMessage()); } processor.process(contribution, inputs); @@ -135,20 +141,19 @@ public class FaultTolerantChunkProcessorTests { } } }); - Chunk inputs = new Chunk(Arrays.asList("3", "fail", "2")); + Chunk inputs = new Chunk( + Arrays.asList("3", "fail", "2")); try { processor.process(contribution, inputs); fail("Expected RuntimeException"); - } - catch (RuntimeException e) { + } catch (RuntimeException e) { assertEquals("Expected Exception!", e.getMessage()); } processor.process(contribution, inputs); try { processor.process(contribution, inputs); fail("Expected RuntimeException"); - } - catch (RuntimeException e) { + } catch (RuntimeException e) { assertEquals("Expected Exception!", e.getMessage()); } assertEquals(1, contribution.getSkipCount()); @@ -170,16 +175,15 @@ public class FaultTolerantChunkProcessorTests { try { processor.process(contribution, inputs); fail("Expected RuntimeException"); - } - catch (RuntimeException e) { + } catch (RuntimeException e) { assertEquals("Expected Exception!", e.getMessage()); } - // BATCH-1518: ideally we would not want this to be necessary, but it still is... + // BATCH-1518: ideally we would not want this to be necessary, but it + // still is... try { processor.process(contribution, inputs); fail("Expected RuntimeException"); - } - catch (RuntimeException e) { + } catch (RuntimeException e) { assertEquals("Expected Exception!", e.getMessage()); } processor.process(contribution, inputs); @@ -198,8 +202,11 @@ public class FaultTolerantChunkProcessorTests { } }); processor.setProcessSkipPolicy(new AlwaysSkipItemSkipPolicy()); - processor.setRollbackClassifier(new BinaryExceptionClassifier(Collections - .> singleton(DataIntegrityViolationException.class), false)); + processor + .setRollbackClassifier(new BinaryExceptionClassifier( + Collections + .> singleton(DataIntegrityViolationException.class), + false)); Chunk inputs = new Chunk(Arrays.asList("1", "2")); processor.process(contribution, inputs); assertEquals(1, list.size()); @@ -207,13 +214,15 @@ public class FaultTolerantChunkProcessorTests { @Test public void testAfterWrite() throws Exception { - Chunk chunk = new Chunk(Arrays.asList("foo", "fail", "bar")); - processor.setListeners(Arrays.asList(new ItemListenerSupport() { - @Override - public void afterWrite(List item) { - after.addAll(item); - } - })); + Chunk chunk = new Chunk(Arrays.asList("foo", "fail", + "bar")); + processor.setListeners(Arrays + .asList(new ItemListenerSupport() { + @Override + public void afterWrite(List item) { + after.addAll(item); + } + })); processor.setWriteSkipPolicy(new AlwaysSkipItemSkipPolicy()); processAndExpectPlannedRuntimeException(chunk); processor.process(contribution, chunk); @@ -225,16 +234,19 @@ public class FaultTolerantChunkProcessorTests { // foo is written once because it the failure is detected before it is // committed the first time assertEquals("[foo, bar]", list.toString()); - // the after listener is called once per successful item, which is important + // the after listener is called once per successful item, which is + // important assertEquals("[foo, bar]", after.toString()); } @Test public void testAfterWriteAllPassedInRecovery() throws Exception { Chunk chunk = new Chunk(Arrays.asList("foo", "bar")); - processor = new FaultTolerantChunkProcessor(new PassThroughItemProcessor(), + processor = new FaultTolerantChunkProcessor( + new PassThroughItemProcessor(), new ItemWriter() { - public void write(List items) throws Exception { + public void write(List items) + throws Exception { // Fail if there is more than one item if (items.size() > 1) { throw new RuntimeException("Planned failure!"); @@ -242,12 +254,13 @@ public class FaultTolerantChunkProcessorTests { list.addAll(items); } }, batchRetryTemplate); - processor.setListeners(Arrays.asList(new ItemListenerSupport() { - @Override - public void afterWrite(List item) { - after.addAll(item); - } - })); + processor.setListeners(Arrays + .asList(new ItemListenerSupport() { + @Override + public void afterWrite(List item) { + after.addAll(item); + } + })); processor.setWriteSkipPolicy(new AlwaysSkipItemSkipPolicy()); processAndExpectPlannedRuntimeException(chunk); @@ -257,57 +270,216 @@ public class FaultTolerantChunkProcessorTests { assertEquals("[foo, bar]", list.toString()); assertEquals("[foo, bar]", after.toString()); } - + @Test - public void testOnErrorInWrite() throws Exception{ + public void testOnErrorInWrite() throws Exception { Chunk chunk = new Chunk(Arrays.asList("foo", "fail")); - processor.setListeners(Arrays.asList(new ItemListenerSupport() { - @Override - public void onWriteError(Exception e, List item) { - writeError.addAll(item); - } - })); + processor.setListeners(Arrays + .asList(new ItemListenerSupport() { + @Override + public void onWriteError(Exception e, + List item) { + writeError.addAll(item); + } + })); processor.setWriteSkipPolicy(new AlwaysSkipItemSkipPolicy()); - - processAndExpectPlannedRuntimeException(chunk);//Process foo, fail - processor.process(contribution, chunk);;//Process foo - processAndExpectPlannedRuntimeException(chunk);//Process fail - + + processAndExpectPlannedRuntimeException(chunk);// Process foo, fail + processor.process(contribution, chunk); + ;// Process foo + processAndExpectPlannedRuntimeException(chunk);// Process fail + assertEquals("[foo, fail, fail]", writeError.toString()); } - + @Test - public void testOnErrorInWriteAllItemsFail() throws Exception{ + public void testOnErrorInWriteAllItemsFail() throws Exception { Chunk chunk = new Chunk(Arrays.asList("foo", "bar")); - processor = new FaultTolerantChunkProcessor(new PassThroughItemProcessor(), + processor = new FaultTolerantChunkProcessor( + new PassThroughItemProcessor(), new ItemWriter() { - public void write(List items) throws Exception { - //Always fail in writer + public void write(List items) + throws Exception { + // Always fail in writer throw new RuntimeException("Planned failure!"); } }, batchRetryTemplate); - processor.setListeners(Arrays.asList(new ItemListenerSupport() { - @Override - public void onWriteError(Exception e, List item) { - writeError.addAll(item); - } - })); + processor.setListeners(Arrays + .asList(new ItemListenerSupport() { + @Override + public void onWriteError(Exception e, + List item) { + writeError.addAll(item); + } + })); processor.setWriteSkipPolicy(new AlwaysSkipItemSkipPolicy()); - - processAndExpectPlannedRuntimeException(chunk);//Process foo, bar - processAndExpectPlannedRuntimeException(chunk);//Process foo - processAndExpectPlannedRuntimeException(chunk);//Process bar - + + processAndExpectPlannedRuntimeException(chunk);// Process foo, bar + processAndExpectPlannedRuntimeException(chunk);// Process foo + processAndExpectPlannedRuntimeException(chunk);// Process bar + assertEquals("[foo, bar, foo, bar]", writeError.toString()); } + @Test + public void testWriteRetryOnException() throws Exception { + SimpleRetryPolicy retryPolicy = new SimpleRetryPolicy(); + retryPolicy.setMaxAttempts(2); + batchRetryTemplate.setRetryPolicy(retryPolicy); + processor.setWriteSkipPolicy(new AlwaysSkipItemSkipPolicy()); + processor.setItemWriter(new ItemWriter() { + public void write(List items) throws Exception { + if (items.contains("fail")) { + throw new IllegalArgumentException("Expected Exception!"); + } + } + }); + Chunk inputs = new Chunk( + Arrays.asList("3", "fail", "2")); + try { + processor.process(contribution, inputs); + fail("Expected RuntimeException"); + } catch (RuntimeException e) { + assertEquals("Expected Exception!", e.getMessage()); + } + try { + // first retry + processor.process(contribution, inputs); + fail("Expected RuntimeException"); + } catch (RuntimeException e) { + assertEquals("Expected Exception!", e.getMessage()); + } + // retry exhausted, now scanning + processor.process(contribution, inputs); + try { + // skip on this attempt + processor.process(contribution, inputs); + fail("Expected RuntimeException"); + } catch (RuntimeException e) { + assertEquals("Expected Exception!", e.getMessage()); + } + // finish chunk + processor.process(contribution, inputs); + assertEquals(1, contribution.getSkipCount()); + assertEquals(2, contribution.getWriteCount()); + assertEquals(0, contribution.getFilterCount()); + } + + @Test + public void testWriteRetryOnTwoExceptions() throws Exception { + SimpleRetryPolicy retryPolicy = new SimpleRetryPolicy(); + retryPolicy.setMaxAttempts(2); + batchRetryTemplate.setRetryPolicy(retryPolicy); + processor.setWriteSkipPolicy(new AlwaysSkipItemSkipPolicy()); + processor.setItemWriter(new ItemWriter() { + public void write(List items) throws Exception { + if (items.contains("fail")) { + throw new IllegalArgumentException("Expected Exception!"); + } + } + }); + Chunk inputs = new Chunk(Arrays.asList("3", "fail", + "fail", "4")); + try { + processor.process(contribution, inputs); + fail("Expected RuntimeException"); + } catch (RuntimeException e) { + assertEquals("Expected Exception!", e.getMessage()); + } + try { + // first retry + processor.process(contribution, inputs); + fail("Expected RuntimeException"); + } catch (RuntimeException e) { + assertEquals("Expected Exception!", e.getMessage()); + } + // retry exhausted, now scanning + processor.process(contribution, inputs); + try { + // skip on this attempt + processor.process(contribution, inputs); + fail("Expected RuntimeException"); + } catch (RuntimeException e) { + assertEquals("Expected Exception!", e.getMessage()); + } + try { + // 2nd exception detected + processor.process(contribution, inputs); + fail("Expected RuntimeException"); + } catch (RuntimeException e) { + assertEquals("Expected Exception!", e.getMessage()); + } + // still scanning + processor.process(contribution, inputs); + assertEquals(2, contribution.getSkipCount()); + assertEquals(2, contribution.getWriteCount()); + assertEquals(0, contribution.getFilterCount()); + } + + @Test + // BATCH-1804 + public void testWriteRetryOnNonSkippableException() throws Exception { + SimpleRetryPolicy retryPolicy = new SimpleRetryPolicy(); + retryPolicy.setMaxAttempts(2); + batchRetryTemplate.setRetryPolicy(retryPolicy); + processor.setWriteSkipPolicy(new LimitCheckingItemSkipPolicy(1, + Collections., Boolean> singletonMap( + IllegalArgumentException.class, true))); + processor.setItemWriter(new ItemWriter() { + public void write(List items) throws Exception { + if (items.contains("fail")) { + throw new IllegalArgumentException("Expected Exception!"); + } + if (items.contains("2")) { + throw new RuntimeException( + "Expected Non-Skippable Exception!"); + } + } + }); + Chunk inputs = new Chunk( + Arrays.asList("3", "fail", "2")); + try { + processor.process(contribution, inputs); + fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + assertEquals("Expected Exception!", e.getMessage()); + } + try { + // first retry + processor.process(contribution, inputs); + fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + assertEquals("Expected Exception!", e.getMessage()); + } + // retry exhausted, now scanning + processor.process(contribution, inputs); + try { + // skip on this attempt + processor.process(contribution, inputs); + fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + assertEquals("Expected Exception!", e.getMessage()); + } + try { + // should retry + processor.process(contribution, inputs); + fail("Expected RuntimeException"); + } catch (RetryException e) { + throw e; + } catch (RuntimeException e) { + assertEquals("Expected Non-Skippable Exception!", e.getMessage()); + } + assertEquals(1, contribution.getSkipCount()); + assertEquals(1, contribution.getWriteCount()); + assertEquals(0, contribution.getFilterCount()); + } + protected void processAndExpectPlannedRuntimeException(Chunk chunk) throws Exception { try { processor.process(contribution, chunk); fail(); - } - catch (RuntimeException e) { + } catch (RuntimeException e) { assertEquals("Planned failure!", e.getMessage()); } }