diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/SimpleRetryExceptionHandler.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/SimpleRetryExceptionHandler.java index 465047680..40149014d 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/SimpleRetryExceptionHandler.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/SimpleRetryExceptionHandler.java @@ -60,7 +60,7 @@ public class SimpleRetryExceptionHandler extends RetryListenerSupport implements this.retryPolicy = retryPolicy; this.exceptionHandler = exceptionHandler; this.fatalExceptionClassifier = new BinaryExceptionClassifier(); - fatalExceptionClassifier.setExceptionClasses(fatalExceptionClasses); + fatalExceptionClassifier.setTypes(fatalExceptionClasses); } /** @@ -74,7 +74,7 @@ public class SimpleRetryExceptionHandler extends RetryListenerSupport implements public void handleException(RepeatContext context, Throwable throwable) throws Throwable { // Only bother to check the delegate exception handler if we know that // retry is exhausted - if (!fatalExceptionClassifier.isDefault(throwable) || context.hasAttribute(EXHAUSTED)) { + if (fatalExceptionClassifier.classify(throwable) || context.hasAttribute(EXHAUSTED)) { exceptionHandler.handleException(context, throwable); } } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/SkipLimitStepFactoryBean.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/SkipLimitStepFactoryBean.java index e9f0d0f02..f8135cdf1 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/SkipLimitStepFactoryBean.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/item/SkipLimitStepFactoryBean.java @@ -30,11 +30,9 @@ import org.springframework.batch.retry.RetryState; import org.springframework.batch.retry.backoff.BackOffPolicy; import org.springframework.batch.retry.policy.ExceptionClassifierRetryPolicy; import org.springframework.batch.retry.policy.MapRetryContextCache; -import org.springframework.batch.retry.policy.NeverRetryPolicy; import org.springframework.batch.retry.policy.RetryContextCache; import org.springframework.batch.retry.policy.SimpleRetryPolicy; import org.springframework.batch.retry.support.RetryTemplate; -import org.springframework.batch.support.SubclassExceptionClassifier; /** * Factory bean for step that provides options for configuring skip behaviour. @@ -214,17 +212,11 @@ public class SkipLimitStepFactoryBean extends SimpleStepFactoryBean simpleRetryPolicy.setFatalExceptionClasses(fatalExceptionClasses); ExceptionClassifierRetryPolicy classifierRetryPolicy = new ExceptionClassifierRetryPolicy(); - SubclassExceptionClassifier exceptionClassifier = new SubclassExceptionClassifier(); - HashMap, String> exceptionTypeMap = new HashMap, String>(); + HashMap, RetryPolicy> exceptionTypeMap = new HashMap, RetryPolicy>(); for (Class cls : retryableExceptionClasses) { - exceptionTypeMap.put(cls, "retry"); + exceptionTypeMap.put(cls, simpleRetryPolicy); } - exceptionClassifier.setTypeMap(exceptionTypeMap); - HashMap retryPolicyMap = new HashMap(); - retryPolicyMap.put("retry", simpleRetryPolicy); - retryPolicyMap.put("default", new NeverRetryPolicy()); - classifierRetryPolicy.setPolicyMap(retryPolicyMap); - classifierRetryPolicy.setExceptionClassifier(exceptionClassifier); + classifierRetryPolicy.setPolicyMap(exceptionTypeMap); retryPolicy = classifierRetryPolicy; } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/skip/LimitCheckingItemSkipPolicy.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/skip/LimitCheckingItemSkipPolicy.java index d6773a43e..fe6f22571 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/skip/LimitCheckingItemSkipPolicy.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/skip/LimitCheckingItemSkipPolicy.java @@ -18,15 +18,11 @@ package org.springframework.batch.core.step.skip; import java.io.FileNotFoundException; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Map; - import org.springframework.batch.core.Step; import org.springframework.batch.core.StepExecution; import org.springframework.batch.item.file.FlatFileParseException; +import org.springframework.batch.support.BinaryExceptionClassifier; import org.springframework.batch.support.Classifier; -import org.springframework.batch.support.SubclassExceptionClassifier; /** *

@@ -58,30 +54,20 @@ import org.springframework.batch.support.SubclassExceptionClassifier; */ public class LimitCheckingItemSkipPolicy implements ItemSkipPolicy { - /** - * Label for classifying skippable exceptions. - */ - private static final String SKIP = "skip"; - - /** - * Label for classifying fatal exceptions - these are never skipped. - */ - private static final String NEVER_SKIP = "neverSkip"; - private final int skipLimit; - private Classifier exceptionClassifier; + private final Classifier fatalExceptionClassifier; + + private final Classifier skippableExceptionClassifier; /** * Convenience constructor that assumes all exception types are skippable * and none are fatal. * @param skipLimit the number of exceptions allowed to skip */ - @SuppressWarnings("unchecked") public LimitCheckingItemSkipPolicy(int skipLimit) { - this(skipLimit, - new HashSet>(){{add(Exception.class);}}, - Collections.EMPTY_LIST); + this(skipLimit, Collections.> singleton(Exception.class), Collections + .> emptyList()); } /** @@ -92,18 +78,11 @@ public class LimitCheckingItemSkipPolicy implements ItemSkipPolicy { * (non-critical) * @param fatalExceptions exception classes that should never be skipped */ - public LimitCheckingItemSkipPolicy(int skipLimit, Collection> skippableExceptions, Collection>fatalExceptions) { + public LimitCheckingItemSkipPolicy(int skipLimit, Collection> skippableExceptions, + Collection> fatalExceptions) { this.skipLimit = skipLimit; - SubclassExceptionClassifier exceptionClassifier = new SubclassExceptionClassifier(); - Map, String> typeMap = new HashMap, String>(); - for (Class throwable : skippableExceptions) { - typeMap.put(throwable, SKIP); - } - for (Class throwable : fatalExceptions) { - typeMap.put(throwable, NEVER_SKIP); - } - exceptionClassifier.setTypeMap(typeMap); - this.exceptionClassifier = exceptionClassifier; + skippableExceptionClassifier = new BinaryExceptionClassifier(skippableExceptions); + fatalExceptionClassifier = new BinaryExceptionClassifier(fatalExceptions); } /** @@ -116,10 +95,10 @@ public class LimitCheckingItemSkipPolicy implements ItemSkipPolicy { * {@link SkipLimitExceededException} will be thrown. */ public boolean shouldSkip(Throwable t, int skipCount) { - if (exceptionClassifier.classify(t).equals(NEVER_SKIP)) { + if (fatalExceptionClassifier.classify(t)) { return false; } - if (exceptionClassifier.classify(t).equals(SKIP)) { + if (skippableExceptionClassifier.classify(t)) { if (skipCount < skipLimit) { return true; } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/listener/CompositeItemProcessListenerTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/listener/CompositeItemProcessListenerTests.java index ba695c29e..e51ce96f3 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/listener/CompositeItemProcessListenerTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/listener/CompositeItemProcessListenerTests.java @@ -19,7 +19,7 @@ import static org.easymock.EasyMock.createMock; import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.verify; -import java.util.ArrayList; +import java.util.Collections; import org.junit.Before; import org.junit.Test; @@ -74,11 +74,8 @@ public class CompositeItemProcessListenerTests { @Test public void testSetListeners() throws Exception { - compositeListener.setListeners(new ArrayList>() { - { - add(listener); - } - }); + compositeListener.setListeners(Collections + .> singletonList(listener)); listener.beforeProcess(null); replay(listener); compositeListener.beforeProcess(null); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/AbstractExecutionContextDaoTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/AbstractExecutionContextDaoTests.java index 9e1f60bfb..a287766d2 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/AbstractExecutionContextDaoTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/AbstractExecutionContextDaoTests.java @@ -1,8 +1,9 @@ package org.springframework.batch.core.repository.dao; -import java.util.HashMap; +import static org.junit.Assert.assertEquals; -import static org.junit.Assert.*; +import java.util.Collections; +import java.util.HashMap; import org.junit.Before; import org.junit.Test; @@ -17,32 +18,30 @@ import org.springframework.transaction.annotation.Transactional; /** * Tests for {@link ExecutionContextDao} implementations. */ -public abstract class AbstractExecutionContextDaoTests extends AbstractTransactionalJUnit4SpringContextTests{ - +public abstract class AbstractExecutionContextDaoTests extends AbstractTransactionalJUnit4SpringContextTests { + private ExecutionContextDao dao; - + private JobExecution jobExecution = new JobExecution(new JobInstance(1L, new JobParameters(), "jobName"), 1L); - + private StepExecution stepExecution = new StepExecution("stepName", jobExecution, 1L); - + @Before public void setUp() { dao = getExecutionContextDao(); } - + /** - * @return Configured {@link ExecutionContextDao} implementation ready for use. + * @return Configured {@link ExecutionContextDao} implementation ready for + * use. */ protected abstract ExecutionContextDao getExecutionContextDao(); - - @Transactional @Test + + @Transactional + @Test public void testSaveAndFindContext() { - - ExecutionContext ctx = new ExecutionContext(new HashMap() { - { - put("key", "value"); - } - }); + + ExecutionContext ctx = new ExecutionContext(Collections. singletonMap("key", "value")); jobExecution.setExecutionContext(ctx); dao.persistExecutionContext(jobExecution); @@ -50,9 +49,10 @@ public abstract class AbstractExecutionContextDaoTests extends AbstractTransacti assertEquals(ctx, retrieved); } - @Transactional @Test + @Transactional + @Test public void testSaveAndFindEmptyContext() { - + ExecutionContext ctx = new ExecutionContext(); jobExecution.setExecutionContext(ctx); dao.persistExecutionContext(jobExecution); @@ -61,14 +61,12 @@ public abstract class AbstractExecutionContextDaoTests extends AbstractTransacti assertEquals(ctx, retrieved); } - @Transactional @Test + @Transactional + @Test public void testUpdateContext() { - - ExecutionContext ctx = new ExecutionContext(new HashMap() { - { - put("key", "value"); - } - }); + + ExecutionContext ctx = new ExecutionContext(Collections + . singletonMap("key", "value")); jobExecution.setExecutionContext(ctx); dao.persistExecutionContext(jobExecution); @@ -79,25 +77,23 @@ public abstract class AbstractExecutionContextDaoTests extends AbstractTransacti assertEquals(ctx, retrieved); assertEquals(7, retrieved.getLong("longKey")); } - - @Transactional @Test + + @Transactional + @Test public void testSaveAndFindStepContext() { - - ExecutionContext ctx = new ExecutionContext(new HashMap() { - { - put("key", "value"); - } - }); + + ExecutionContext ctx = new ExecutionContext(Collections. singletonMap("key", "value")); stepExecution.setExecutionContext(ctx); dao.persistExecutionContext(stepExecution); ExecutionContext retrieved = dao.getExecutionContext(stepExecution); assertEquals(ctx, retrieved); } - - @Transactional @Test + + @Transactional + @Test public void testSaveAndFindEmptyStepContext() { - + ExecutionContext ctx = new ExecutionContext(); stepExecution.setExecutionContext(ctx); dao.persistExecutionContext(stepExecution); @@ -106,14 +102,11 @@ public abstract class AbstractExecutionContextDaoTests extends AbstractTransacti assertEquals(ctx, retrieved); } - @Transactional @Test + @Transactional + @Test public void testUpdateStepContext() { - - ExecutionContext ctx = new ExecutionContext(new HashMap() { - { - put("key", "value"); - } - }); + + ExecutionContext ctx = new ExecutionContext(Collections. singletonMap("key", "value")); stepExecution.setExecutionContext(ctx); dao.persistExecutionContext(stepExecution); @@ -124,10 +117,11 @@ public abstract class AbstractExecutionContextDaoTests extends AbstractTransacti assertEquals(ctx, retrieved); assertEquals(7, retrieved.getLong("longKey")); } - - @Transactional @Test - public void testStoreInteger(){ - + + @Transactional + @Test + public void testStoreInteger() { + ExecutionContext ec = new ExecutionContext(); ec.put("intValue", new Integer(343232)); stepExecution.setExecutionContext(ec); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/SimpleRetryExceptionHandlerTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/SimpleRetryExceptionHandlerTests.java index 98b232424..1d2ca4564 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/SimpleRetryExceptionHandlerTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/SimpleRetryExceptionHandlerTests.java @@ -16,7 +16,7 @@ package org.springframework.batch.core.step.item; import java.util.Collection; -import java.util.HashSet; +import java.util.Collections; import junit.framework.TestCase; @@ -57,20 +57,15 @@ public class SimpleRetryExceptionHandlerTests extends TestCase { /** * Test method for - * {@link org.springframework.batch.core.step.item.SimpleRetryExceptionHandler#handleException(org.springframework.batch.repeat.RepeatContext, java.lang.Throwable)} - * . + * {@link org.springframework.batch.core.step.item.SimpleRetryExceptionHandler#handleException(org.springframework.batch.repeat.RepeatContext, java.lang.Throwable)} . */ public void testRethrowWhenRetryExhausted() throws Throwable { RetryPolicy retryPolicy = new NeverRetryPolicy(); RuntimeException ex = new RuntimeException("foo"); - SimpleRetryExceptionHandler handler = getHandlerAfterRetry(retryPolicy, ex, - new HashSet>() { - { - add(Error.class); - } - }); + SimpleRetryExceptionHandler handler = getHandlerAfterRetry(retryPolicy, ex, Collections + .> singleton(Error.class)); // Then pretend to handle the exception in the parent context... try { @@ -89,20 +84,15 @@ public class SimpleRetryExceptionHandlerTests extends TestCase { /** * Test method for - * {@link org.springframework.batch.core.step.item.SimpleRetryExceptionHandler#handleException(org.springframework.batch.repeat.RepeatContext, java.lang.Throwable)} - * . + * {@link org.springframework.batch.core.step.item.SimpleRetryExceptionHandler#handleException(org.springframework.batch.repeat.RepeatContext, java.lang.Throwable)} . */ public void testNoRethrowWhenRetryNotExhausted() throws Throwable { RetryPolicy retryPolicy = new AlwaysRetryPolicy(); RuntimeException ex = new RuntimeException("foo"); - SimpleRetryExceptionHandler handler = getHandlerAfterRetry(retryPolicy, ex, - new HashSet>() { - { - add(Error.class); - } - }); + SimpleRetryExceptionHandler handler = getHandlerAfterRetry(retryPolicy, ex, Collections + .> singleton(Error.class)); // Then pretend to handle the exception in the parent context... handler.handleException(context.getParent(), ex); @@ -113,20 +103,15 @@ public class SimpleRetryExceptionHandlerTests extends TestCase { /** * Test method for - * {@link org.springframework.batch.core.step.item.SimpleRetryExceptionHandler#handleException(org.springframework.batch.repeat.RepeatContext, java.lang.Throwable)} - * . + * {@link org.springframework.batch.core.step.item.SimpleRetryExceptionHandler#handleException(org.springframework.batch.repeat.RepeatContext, java.lang.Throwable)} . */ public void testRethrowWhenFatal() throws Throwable { RetryPolicy retryPolicy = new AlwaysRetryPolicy(); RuntimeException ex = new RuntimeException("foo"); - SimpleRetryExceptionHandler handler = getHandlerAfterRetry(retryPolicy, ex, - new HashSet>() { - { - add(RuntimeException.class); - } - }); + SimpleRetryExceptionHandler handler = getHandlerAfterRetry(retryPolicy, ex, Collections + .> singleton(RuntimeException.class)); // Then pretend to handle the exception in the parent context... try { diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/SimpleStepFactoryBeanTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/SimpleStepFactoryBeanTests.java index 343bfc9fb..231b218bb 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/SimpleStepFactoryBeanTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/SimpleStepFactoryBeanTests.java @@ -108,7 +108,7 @@ public class SimpleStepFactoryBeanTests { public void testSimpleConcurrentJob() throws Exception { job.setSteps(new ArrayList()); - SimpleStepFactoryBean factory = getStepFactory("foo", "bar"); + SimpleStepFactoryBean factory = getStepFactory("foo", "bar"); factory.setTaskExecutor(new SimpleAsyncTaskExecutor()); factory.setThrottleLimit(1); @@ -127,14 +127,14 @@ public class SimpleStepFactoryBeanTests { @Test public void testSimpleJobWithItemListeners() throws Exception { - SimpleStepFactoryBean factory = getStepFactory(new String[] { "foo", "bar", "spam" }); + SimpleStepFactoryBean factory = getStepFactory(new String[] { "foo", "bar", "spam" }); factory.setItemWriter(new ItemWriter() { public void write(List data) throws Exception { throw new RuntimeException("Error!"); } }); - factory.setListeners(new StepListener[] { new ItemListenerSupport() { + factory.setListeners(new StepListener[] { new ItemListenerSupport() { @Override public void onReadError(Exception ex) { listened.add(ex); @@ -154,7 +154,8 @@ public class SimpleStepFactoryBeanTests { try { job.execute(jobExecution); fail("Expected RuntimeException"); - } catch (RuntimeException e) { + } + catch (RuntimeException e) { // expected assertEquals("Error!", e.getMessage()); } @@ -168,7 +169,7 @@ public class SimpleStepFactoryBeanTests { @Test public void testExceptionTerminates() throws Exception { - SimpleStepFactoryBean factory = getStepFactory(new String[] { "foo", "bar", "spam" }); + SimpleStepFactoryBean factory = getStepFactory(new String[] { "foo", "bar", "spam" }); factory.setBeanName("exceptionStep"); factory.setItemWriter(new ItemWriter() { public void write(List data) throws Exception { @@ -192,9 +193,11 @@ public class SimpleStepFactoryBeanTests { @Test public void testExceptionHandler() throws Exception { - SimpleStepFactoryBean factory = getStepFactory(new String[] { "foo", "bar", "spam" }); + SimpleStepFactoryBean factory = getStepFactory(new String[] { "foo", "bar", "spam" }); factory.setBeanName("exceptionStep"); - factory.setExceptionHandler(new SimpleLimitExceptionHandler(1)); + SimpleLimitExceptionHandler exceptionHandler = new SimpleLimitExceptionHandler(1); + exceptionHandler.afterPropertiesSet(); + factory.setExceptionHandler(exceptionHandler); factory.setItemWriter(new ItemWriter() { int count = 0; @@ -208,9 +211,9 @@ public class SimpleStepFactoryBeanTests { job.setSteps(Collections.singletonList((Step) step)); JobExecution jobExecution = repository.createJobExecution(job, new JobParameters()); - + job.execute(jobExecution); - + assertEquals(BatchStatus.COMPLETED, jobExecution.getStatus()); } @@ -219,7 +222,7 @@ public class SimpleStepFactoryBeanTests { String[] items = new String[] { "1", "2", "3", "4", "5", "6", "7" }; int commitInterval = 3; - SimpleStepFactoryBean factory = getStepFactory(items); + SimpleStepFactoryBean factory = getStepFactory(items); class CountingChunkListener implements ChunkListener { int beforeCount = 0; @@ -259,7 +262,7 @@ public class SimpleStepFactoryBeanTests { */ @Test public void testCommitIntervalMustBeGreaterThanZero() throws Exception { - SimpleStepFactoryBean factory = getStepFactory("foo"); + SimpleStepFactoryBean factory = getStepFactory("foo"); // nothing wrong here factory.getObject(); @@ -281,7 +284,7 @@ public class SimpleStepFactoryBeanTests { */ @Test public void testCommitIntervalAndCompletionPolicyBothSet() throws Exception { - SimpleStepFactoryBean factory = getStepFactory("foo"); + SimpleStepFactoryBean factory = getStepFactory("foo"); // but exception expected after setting commit interval and completion // policy @@ -296,9 +299,9 @@ public class SimpleStepFactoryBeanTests { } } - - private SimpleStepFactoryBean getStepFactory(String... args) throws Exception { - SimpleStepFactoryBean factory = new SimpleStepFactoryBean(); + + private SimpleStepFactoryBean getStepFactory(String... args) throws Exception { + SimpleStepFactoryBean factory = new SimpleStepFactoryBean(); List items = new ArrayList(); items.addAll(Arrays.asList(args)); @@ -311,5 +314,5 @@ public class SimpleStepFactoryBeanTests { factory.setBeanName("stepName"); return factory; } - + } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/SkipLimitStepFactoryBeanTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/SkipLimitStepFactoryBeanTests.java index 628454f54..b1e443065 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/SkipLimitStepFactoryBeanTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/SkipLimitStepFactoryBeanTests.java @@ -46,12 +46,9 @@ public class SkipLimitStepFactoryBeanTests { private SkipLimitStepFactoryBean factory = new SkipLimitStepFactoryBean(); - private Collection> skippableExceptions = new HashSet>() { - { - add(SkippableException.class); - add(SkippableRuntimeException.class); - } - }; + @SuppressWarnings("unchecked") + private Collection> skippableExceptions = new HashSet>(Arrays + .> asList(SkippableException.class, SkippableRuntimeException.class)); private SkipReaderStub reader = new SkipReaderStub(); @@ -139,11 +136,8 @@ public class SkipLimitStepFactoryBeanTests { */ @Test public void testFatalException() throws Exception { - factory.setFatalExceptionClasses(new HashSet>() { - { - add(FatalRuntimeException.class); - } - }); + factory.setFatalExceptionClasses(Collections + .> singleton(FatalRuntimeException.class)); factory.setItemWriter(new SkipWriterStub() { public void write(List items) { throw new FatalRuntimeException("Ouch!"); @@ -204,11 +198,7 @@ public class SkipLimitStepFactoryBeanTests { factory.setSkipLimit(3); factory.setItemReader(reader); - factory.setSkippableExceptionClasses(new HashSet>() { - { - add(Exception.class); - } - }); + factory.setSkippableExceptionClasses(Collections.> singleton(Exception.class)); Step step = (Step) factory.getObject(); @@ -246,17 +236,13 @@ public class SkipLimitStepFactoryBeanTests { factory.setSkipLimit(3); factory.setItemReader(reader); - factory.setListeners(new StepListener[] { new SkipListenerSupport() { + factory.setListeners(new StepListener[] { new SkipListenerSupport() { @Override public void onSkipInRead(Throwable t) { throw new RuntimeException("oops"); } } }); - factory.setSkippableExceptionClasses(new HashSet>() { - { - add(Exception.class); - } - }); + factory.setSkippableExceptionClasses(Collections.> singleton(Exception.class)); Step step = (Step) factory.getObject(); @@ -287,17 +273,13 @@ public class SkipLimitStepFactoryBeanTests { factory.setSkipLimit(3); factory.setItemReader(reader); - factory.setListeners(new StepListener[] { new SkipListenerSupport() { + factory.setListeners(new StepListener[] { new SkipListenerSupport() { @Override public void onSkipInWrite(String item, Throwable t) { throw new RuntimeException("oops"); } } }); - factory.setSkippableExceptionClasses(new HashSet>() { - { - add(Exception.class); - } - }); + factory.setSkippableExceptionClasses(Collections.> singleton(Exception.class)); Step step = (Step) factory.getObject(); @@ -377,11 +359,7 @@ public class SkipLimitStepFactoryBeanTests { @Test public void testDefaultSkipPolicy() throws Exception { - factory.setSkippableExceptionClasses(new HashSet>() { - { - add(Exception.class); - } - }); + factory.setSkippableExceptionClasses(Collections.> singleton(Exception.class)); factory.setSkipLimit(1); List items = Arrays.asList(new String[] { "a", "b", "c" }); ItemReader provider = new ListItemReader(items) { @@ -411,17 +389,13 @@ public class SkipLimitStepFactoryBeanTests { @Test public void testSkipOverLimitOnReadWithAllSkipsAtEnd() throws Exception { - reader = new SkipReaderStub(StringUtils.commaDelimitedListToStringArray("1,2,3,4,5,6,7,8,9,10,11,12,13,14,15"), Arrays - .asList(StringUtils.commaDelimitedListToStringArray("6,12,13,14,15"))); + reader = new SkipReaderStub(StringUtils.commaDelimitedListToStringArray("1,2,3,4,5,6,7,8,9,10,11,12,13,14,15"), + Arrays.asList(StringUtils.commaDelimitedListToStringArray("6,12,13,14,15"))); factory.setCommitInterval(5); factory.setSkipLimit(3); factory.setItemReader(reader); - factory.setSkippableExceptionClasses(new HashSet>() { - { - add(Exception.class); - } - }); + factory.setSkippableExceptionClasses(Collections.> singleton(Exception.class)); Step step = (Step) factory.getObject(); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/tasklet/ConfigurableSystemProcessExitCodeMapperTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/tasklet/ConfigurableSystemProcessExitCodeMapperTests.java index 7f7310105..c0fc9fbc5 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/tasklet/ConfigurableSystemProcessExitCodeMapperTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/tasklet/ConfigurableSystemProcessExitCodeMapperTests.java @@ -3,6 +3,7 @@ package org.springframework.batch.core.step.tasklet; import static org.junit.Assert.assertSame; import static org.junit.Assert.fail; +import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -16,36 +17,38 @@ import org.springframework.batch.repeat.ExitStatus; public class ConfigurableSystemProcessExitCodeMapperTests { private ConfigurableSystemProcessExitCodeMapper mapper = new ConfigurableSystemProcessExitCodeMapper(); - + /** * Regular usage scenario - mapping adheres to injected values */ @Test public void testMapping() { - Map mappings = new HashMap() {{ - put(0, ExitStatus.FINISHED); - put(1, ExitStatus.FAILED); - put(2, ExitStatus.CONTINUABLE); - put(3, ExitStatus.NOOP); - put(4, ExitStatus.UNKNOWN); - put(ConfigurableSystemProcessExitCodeMapper.ELSE_KEY, ExitStatus.UNKNOWN); - }}; - + Map mappings = new HashMap() { + { + put(0, ExitStatus.FINISHED); + put(1, ExitStatus.FAILED); + put(2, ExitStatus.CONTINUABLE); + put(3, ExitStatus.NOOP); + put(4, ExitStatus.UNKNOWN); + put(ConfigurableSystemProcessExitCodeMapper.ELSE_KEY, ExitStatus.UNKNOWN); + } + }; + mapper.setMappings(mappings); - - //check explicitly defined values + + // check explicitly defined values for (Map.Entry entry : mappings.entrySet()) { - if (entry.getKey().equals(ConfigurableSystemProcessExitCodeMapper.ELSE_KEY)) continue; - + if (entry.getKey().equals(ConfigurableSystemProcessExitCodeMapper.ELSE_KEY)) + continue; + int exitCode = (Integer) entry.getKey(); assertSame(entry.getValue(), mapper.getExitStatus(exitCode)); } - - //check the else clause - assertSame(mappings.get(ConfigurableSystemProcessExitCodeMapper.ELSE_KEY), - mapper.getExitStatus(5)); + + // check the else clause + assertSame(mappings.get(ConfigurableSystemProcessExitCodeMapper.ELSE_KEY), mapper.getExitStatus(5)); } - + /** * Else clause is required in the injected map - setter checks its presence. */ @@ -59,12 +62,10 @@ public class ConfigurableSystemProcessExitCodeMapperTests { catch (IllegalArgumentException e) { // expected } - - Map containsElse = new HashMap() {{ - put(ConfigurableSystemProcessExitCodeMapper.ELSE_KEY, ExitStatus.FAILED); - }}; + + Map containsElse = Collections. singletonMap( + ConfigurableSystemProcessExitCodeMapper.ELSE_KEY, ExitStatus.FAILED); // no error expected now mapper.setMappings(containsElse); } - } diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/ExecutionContext.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/ExecutionContext.java index ad2e1684c..ead6eed23 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/ExecutionContext.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/item/ExecutionContext.java @@ -52,7 +52,7 @@ public class ExecutionContext implements Serializable { * @param map Initial contents of context. */ public ExecutionContext(Map map) { - this.map = map; + this.map = new HashMap(map); } /** diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/repeat/context/RepeatContextCounter.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/repeat/context/RepeatContextCounter.java index cf2f550e9..c062b7bc3 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/repeat/context/RepeatContextCounter.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/repeat/context/RepeatContextCounter.java @@ -33,15 +33,15 @@ import org.springframework.util.Assert; */ public class RepeatContextCounter { - private String countKey; + final private String countKey; /** * Flag to indicate whether the count is stored at the level of the parent * context, or just local to the current context. Default value is false. */ - private boolean useParent = false; + final private boolean useParent; - private RepeatContext context; + final private RepeatContext context; /** * Increment the counter. @@ -82,7 +82,7 @@ public class RepeatContextCounter { super(); - Assert.notNull(context, "The context must be provided"); + Assert.notNull(context, "The context must be provided to initialize a counter"); this.countKey = countKey; this.useParent = useParent; diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/repeat/exception/LogOrRethrowExceptionHandler.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/repeat/exception/LogOrRethrowExceptionHandler.java index 0d87b923c..d65aaabff 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/repeat/exception/LogOrRethrowExceptionHandler.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/repeat/exception/LogOrRethrowExceptionHandler.java @@ -21,12 +21,12 @@ import org.apache.commons.logging.LogFactory; import org.springframework.batch.repeat.RepeatContext; import org.springframework.batch.repeat.RepeatException; import org.springframework.batch.support.Classifier; -import org.springframework.batch.support.ExceptionClassifierSupport; +import org.springframework.batch.support.ClassifierSupport; /** - * Implementation of {@link ExceptionHandler} based on an {@link Classifier}. The classifier determines - * whether to log the exception or rethrow it. The keys in the classifier must be the same as the static contants in - * this class. + * Implementation of {@link ExceptionHandler} based on an {@link Classifier}. + * The classifier determines whether to log the exception or rethrow it. The + * keys in the classifier must be the same as the static enum in this class. * * @author Dave Syer * @@ -34,46 +34,57 @@ import org.springframework.batch.support.ExceptionClassifierSupport; public class LogOrRethrowExceptionHandler implements ExceptionHandler { /** - * Key for {@link Classifier} signalling that the throwable should be rethrown. If the throwable is not a - * RuntimeException it is wrapped in a {@link RepeatException}. + * Logging levels for the handler. + * + * @author Dave Syer + * */ - public static final String RETHROW = "rethrow"; + public static enum Level { - /** - * Key for {@link Classifier} signalling that the throwable should be logged at debug level. - */ - public static final String DEBUG = "debug"; + /** + * Key for {@link Classifier} signalling that the throwable should be + * rethrown. If the throwable is not a RuntimeException it is wrapped in + * a {@link RepeatException}. + */ + RETHROW, - /** - * Key for {@link Classifier} signalling that the throwable should be logged at warn level. - */ - public static final String WARN = "warn"; + /** + * Key for {@link Classifier} signalling that the throwable should be + * logged at debug level. + */ + DEBUG, - /** - * Key for {@link Classifier} signalling that the throwable should be logged at error level. - */ - public static final String ERROR = "error"; + /** + * Key for {@link Classifier} signalling that the throwable should be + * logged at warn level. + */ + WARN, + + /** + * Key for {@link Classifier} signalling that the throwable should be + * logged at error level. + */ + ERROR + + } protected final Log logger = LogFactory.getLog(LogOrRethrowExceptionHandler.class); - private Classifier exceptionClassifier = new ExceptionClassifierSupport() { - public String classify(Throwable throwable) { - return RETHROW; - } - }; + private Classifier exceptionClassifier = new ClassifierSupport(Level.RETHROW); /** - * Setter for the {@link Classifier} used by this handler. The default is to map all throwable instances to - * {@link #RETHROW}. + * Setter for the {@link Classifier} used by this handler. The default is to + * map all throwable instances to {@link Level#RETHROW}. * * @param exceptionClassifier the ExceptionClassifier to use */ - public void setExceptionClassifier(Classifier exceptionClassifier) { + public void setExceptionClassifier(Classifier exceptionClassifier) { this.exceptionClassifier = exceptionClassifier; } /** - * Classify the throwables and decide whether to rethrow based on the result. The context is not used. + * Classify the throwables and decide whether to rethrow based on the + * result. The context is not used. * * @throws Throwable * @@ -81,18 +92,18 @@ public class LogOrRethrowExceptionHandler implements ExceptionHandler { */ public void handleException(RepeatContext context, Throwable throwable) throws Throwable { - String key = exceptionClassifier.classify(throwable); - if (ERROR.equals(key)) { + Level key = exceptionClassifier.classify(throwable); + if (Level.ERROR.equals(key)) { logger.error("Exception encountered in batch repeat.", throwable); - } else if (WARN.equals(key)) { + } + else if (Level.WARN.equals(key)) { logger.warn("Exception encountered in batch repeat.", throwable); - } else if (DEBUG.equals(key) && logger.isDebugEnabled()) { + } + else if (Level.DEBUG.equals(key) && logger.isDebugEnabled()) { logger.debug("Exception encountered in batch repeat.", throwable); - } else if (RETHROW.equals(key)) { + } + else if (Level.RETHROW.equals(key)) { throw throwable; - } else { - throw new IllegalStateException( - "Unclassified exception encountered. Did you mean to classifiy this as 'rethrow'?"); } } diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/repeat/exception/RethrowOnThresholdExceptionHandler.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/repeat/exception/RethrowOnThresholdExceptionHandler.java index 1baf09ee8..a8dc91f70 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/repeat/exception/RethrowOnThresholdExceptionHandler.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/repeat/exception/RethrowOnThresholdExceptionHandler.java @@ -24,24 +24,27 @@ import org.apache.commons.logging.LogFactory; import org.springframework.batch.repeat.RepeatContext; import org.springframework.batch.repeat.context.RepeatContextCounter; import org.springframework.batch.support.Classifier; -import org.springframework.batch.support.ExceptionClassifierSupport; +import org.springframework.batch.support.SubclassClassifier; +import org.springframework.util.ObjectUtils; /** * Implementation of {@link ExceptionHandler} that rethrows when exceptions of a - * given type reach a threshold. Requires an {@link Classifier} that - * maps exception types to unique keys, and also a map from those keys to - * threshold values (Integer type). + * given type reach a threshold. Requires an {@link Classifier} that maps + * exception types to unique keys, and also a map from those keys to threshold + * values (Integer type). * * @author Dave Syer * */ public class RethrowOnThresholdExceptionHandler implements ExceptionHandler { + protected static final IntegerHolder ZERO = new IntegerHolder(0); + protected final Log logger = LogFactory.getLog(RethrowOnThresholdExceptionHandler.class); - private Classifier exceptionClassifier = new ExceptionClassifierSupport(); - - private Map thresholds = new HashMap(); + private Classifier exceptionClassifier = new Classifier() { + public RethrowOnThresholdExceptionHandler.IntegerHolder classify(Throwable classifiable) { return ZERO;} + }; private boolean useParent = false; @@ -63,30 +66,19 @@ public class RethrowOnThresholdExceptionHandler implements ExceptionHandler { */ public RethrowOnThresholdExceptionHandler() { super(); - thresholds.put(ExceptionClassifierSupport.DEFAULT, 0); } /** - * A map from classifier keys to a threshold value of type Integer. The keys - * are usually String literals, depending on the {@link Classifier} - * implementation used. + * A map from exception classes to a threshold value of type Integer. * * @param thresholds the threshold value map. */ - public void setThresholds(Map thresholds) { - this.thresholds = thresholds; - } - - /** - * Setter for the {@link Classifier} used by this handler. The - * default is to map all throwable instances to - * {@link ExceptionClassifierSupport#DEFAULT}, which are then mapped to a - * threshold of 0 by the {@link #setThresholds(Map)} map. - * - * @param exceptionClassifier ExceptionClassifier to use - */ - public void setExceptionClassifier(Classifier exceptionClassifier) { - this.exceptionClassifier = exceptionClassifier; + public void setThresholds(Map, Integer> thresholds) { + Map, IntegerHolder> typeMap = new HashMap, IntegerHolder>(); + for (Class type : thresholds.keySet()) { + typeMap.put(type, new IntegerHolder(thresholds.get(type))); + } + exceptionClassifier = new SubclassClassifier(typeMap, ZERO); } /** @@ -99,21 +91,55 @@ public class RethrowOnThresholdExceptionHandler implements ExceptionHandler { */ public void handleException(RepeatContext context, Throwable throwable) throws Throwable { - Object key = exceptionClassifier.classify(throwable); + IntegerHolder key = exceptionClassifier.classify(throwable); + RepeatContextCounter counter = getCounter(context, key); counter.increment(); int count = counter.getCount(); - Integer threshold = thresholds.get(key); - if (threshold == null || count > threshold) { + int threshold = key.getValue(); + if (count > threshold) { throw throwable; } } - private RepeatContextCounter getCounter(RepeatContext context, Object key) { - String attribute = RethrowOnThresholdExceptionHandler.class.getName() + "." + key.toString(); + private RepeatContextCounter getCounter(RepeatContext context, IntegerHolder key) { + String attribute = RethrowOnThresholdExceptionHandler.class.getName() + "." + key; // Creates a new counter and stores it in the correct context: return new RepeatContextCounter(context, attribute, useParent); } + /** + * @author Dave Syer + * + */ + private static class IntegerHolder { + + private final int value; + + /** + * @param value + */ + public IntegerHolder(int value) { + this.value = value; + } + + /** + * Public getter for the value. + * @return the value + */ + public int getValue() { + return value; + } + + /* (non-Javadoc) + * @see java.lang.Object#toString() + */ + @Override + public String toString() { + return ObjectUtils.getIdentityHexString(this)+"."+value; + } + + } + } diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/repeat/exception/SimpleLimitExceptionHandler.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/repeat/exception/SimpleLimitExceptionHandler.java index 768d1a223..6390f8133 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/repeat/exception/SimpleLimitExceptionHandler.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/repeat/exception/SimpleLimitExceptionHandler.java @@ -16,10 +16,13 @@ package org.springframework.batch.repeat.exception; +import java.util.Collection; +import java.util.Collections; import java.util.HashMap; +import java.util.Map; import org.springframework.batch.repeat.RepeatContext; -import org.springframework.batch.support.ExceptionClassifierSupport; +import org.springframework.beans.factory.InitializingBean; /** * Simple implementation of exception handler which looks for given exception @@ -32,24 +35,37 @@ import org.springframework.batch.support.ExceptionClassifierSupport; * @author Dave Syer * @author Robert Kasanicky */ -public class SimpleLimitExceptionHandler implements ExceptionHandler { - - /** - * Name of exception classifier key for the nominated exception types. - */ - private static final String TX_INVALID = "TX_INVALID"; - - /** - * Name of exception classifier key for the fatal exception types (not - * counted, immediately rethrown). - */ - private static final String FATAL = "FATAL"; +public class SimpleLimitExceptionHandler implements ExceptionHandler, InitializingBean { private RethrowOnThresholdExceptionHandler delegate = new RethrowOnThresholdExceptionHandler(); - private Class[] exceptionClasses = new Class[] { Exception.class }; + private Collection> exceptionClasses = Collections + .> singleton(Exception.class); - private Class[] fatalExceptionClasses = new Class[] { Error.class }; + private Collection> fatalExceptionClasses = Collections + .> singleton(Error.class); + + private int limit = 0; + + /** + * Apply the provided properties to create a delegate handler. + * + * @see org.springframework.beans.factory.InitializingBean#afterPropertiesSet() + */ + public void afterPropertiesSet() throws Exception { + if (limit <= 0) { + return; + } + Map, Integer> thresholds = new HashMap, Integer>(); + for (Class type : exceptionClasses) { + thresholds.put(type, limit); + } + // do the fatalExceptionClasses last so they override the others + for (Class type : fatalExceptionClasses) { + thresholds.put(type, 0); + } + delegate.setThresholds(thresholds); + } /** * Flag to indicate the the exception counters should be shared between @@ -67,12 +83,12 @@ public class SimpleLimitExceptionHandler implements ExceptionHandler { /** * Convenience constructor for the {@link SimpleLimitExceptionHandler} to * set the limit. - * + * * @param limit the limit */ public SimpleLimitExceptionHandler(int limit) { this(); - setLimit(limit); + this.limit = limit; } /** @@ -80,28 +96,13 @@ public class SimpleLimitExceptionHandler implements ExceptionHandler { */ public SimpleLimitExceptionHandler() { super(); - delegate.setExceptionClassifier(new ExceptionClassifierSupport() { - public String classify(Throwable throwable) { - for (Class fatalExceptionClass : fatalExceptionClasses) { - if (fatalExceptionClass.isAssignableFrom(throwable.getClass())) { - return FATAL; - } - } - for (Class exceptionClass : exceptionClasses) { - if (exceptionClass.isAssignableFrom(throwable.getClass())) { - return TX_INVALID; - } - } - return super.classify(throwable); - } - }); } /** * Rethrows only if the limit is breached for this context on the exception * type specified. * - * @see #setExceptionClasses(Class[]) + * @see #setExceptionClasses(Collection) * @see #setLimit(int) * * @see org.springframework.batch.repeat.exception.ExceptionHandler#handleException(org.springframework.batch.repeat.RepeatContext, @@ -118,34 +119,28 @@ public class SimpleLimitExceptionHandler implements ExceptionHandler { * @param limit the limit */ public void setLimit(final int limit) { - delegate.setThresholds(new HashMap() { - { - put(ExceptionClassifierSupport.DEFAULT, 0); - put(TX_INVALID, limit); - put(FATAL, 0); - } - }); + this.limit = limit; } /** - * Setter for the Throwable exceptionClasses that this handler counts. - * Defaults to {@link Exception}. If more exceptionClasses are specified - * handler uses single counter that is incremented when one of the - * recognized exception exceptionClasses is handled. + * Setter for the exception classes that this handler counts. Defaults to + * {@link Exception}. If more exceptionClasses are specified handler uses + * single counter that is incremented when one of the recognized exception + * exceptionClasses is handled. * @param classes exceptionClasses */ - public void setExceptionClasses(Class[] classes) { + public void setExceptionClasses(Collection> classes) { this.exceptionClasses = classes; } /** - * Setter for the Throwable exceptionClasses that shouldn't be counted, but - * rethrown immediately. This list has higher priority than - * {@link #setExceptionClasses(Class[])}. + * Setter for the exception classes that shouldn't be counted, but rethrown + * immediately. This list has higher priority than + * {@link #setExceptionClasses(Collection)}. * * @param fatalExceptionClasses defaults to {@link Error} */ - public void setFatalExceptionClasses(Class[] fatalExceptionClasses) { + public void setFatalExceptionClasses(Collection> fatalExceptionClasses) { this.fatalExceptionClasses = fatalExceptionClasses; } diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/policy/ExceptionClassifierRetryPolicy.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/policy/ExceptionClassifierRetryPolicy.java index 6be7ad0d2..9fc9c3daa 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/policy/ExceptionClassifierRetryPolicy.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/policy/ExceptionClassifierRetryPolicy.java @@ -23,7 +23,8 @@ import org.springframework.batch.retry.RetryContext; import org.springframework.batch.retry.RetryPolicy; import org.springframework.batch.retry.context.RetryContextSupport; import org.springframework.batch.support.Classifier; -import org.springframework.batch.support.ExceptionClassifierSupport; +import org.springframework.batch.support.ClassifierSupport; +import org.springframework.batch.support.SubclassClassifier; import org.springframework.util.Assert; /** @@ -35,34 +36,32 @@ import org.springframework.util.Assert; */ public class ExceptionClassifierRetryPolicy implements RetryPolicy { - private Classifier exceptionClassifier = new ExceptionClassifierSupport(); - - private Map policyMap = new HashMap(); - - public ExceptionClassifierRetryPolicy() { - policyMap.put(ExceptionClassifierSupport.DEFAULT, new NeverRetryPolicy()); - } + private Classifier exceptionClassifier = new ClassifierSupport( + new NeverRetryPolicy()); /** * Setter for policy map. This property should not be changed dynamically - * set it once, e.g. in configuration, and then don't change it during a - * running application. + * running application. Either this property or the exception classifier + * directly should be set, but not both. * - * @param policyMap a map of String to {@link RetryPolicy} that will be - * applied to the result of the {@link Classifier} to locate a - * policy. + * @param policyMap a map of String to {@link RetryPolicy} that will be used + * to create a {@link Classifier} to locate a policy. */ - public void setPolicyMap(Map policyMap) { - this.policyMap = policyMap; + public void setPolicyMap(Map, RetryPolicy> policyMap) { + SubclassClassifier subclassClassifier = new SubclassClassifier( + policyMap, (RetryPolicy) new NeverRetryPolicy()); + this.exceptionClassifier = subclassClassifier; } /** * Setter for an exception classifier. The classifier is responsible for - * translating exceptions to keys in the policy map. + * translating exceptions to concrete retry policies. Either this property + * or the policy map should be used, but not both. * * @param exceptionClassifier ExceptionClassifier to use */ - public void setExceptionClassifier(Classifier exceptionClassifier) { + public void setExceptionClassifier(Classifier exceptionClassifier) { this.exceptionClassifier = exceptionClassifier; } @@ -110,22 +109,20 @@ public class ExceptionClassifierRetryPolicy implements RetryPolicy { private class ExceptionClassifierRetryContext extends RetryContextSupport implements RetryPolicy { - private Classifier exceptionClassifier; + final private Classifier exceptionClassifier; // Dynamic: depends on the latest exception: - RetryPolicy policy; + private RetryPolicy policy; // Dynamic: depends on the policy: - RetryContext context; + private RetryContext context; - Map contexts = new HashMap(); + final private Map contexts = new HashMap(); - public ExceptionClassifierRetryContext(RetryContext parent, Classifier exceptionClassifier) { + public ExceptionClassifierRetryContext(RetryContext parent, + Classifier exceptionClassifier) { super(parent); this.exceptionClassifier = exceptionClassifier; - Object key = exceptionClassifier.getDefault(); - policy = getPolicy(key); - Assert.notNull(policy, "Could not locate default policy: key=[" + key + "]."); } public boolean canRetry(RetryContext context) { @@ -139,7 +136,7 @@ public class ExceptionClassifierRetryPolicy implements RetryPolicy { public void close(RetryContext context) { // Only close those policies that have been used (opened): for (RetryPolicy policy : contexts.keySet()) { - policy.close(getContext(policy)); + policy.close(getContext(policy, context.getParent())); } } @@ -148,26 +145,21 @@ public class ExceptionClassifierRetryPolicy implements RetryPolicy { } public void registerThrowable(RetryContext context, Exception throwable) { - policy = getPolicy(exceptionClassifier.classify(throwable)); - this.context = getContext(policy); + policy = exceptionClassifier.classify(throwable); + Assert.notNull(policy, "Could not locate policy for exception=[" + throwable + "]."); + this.context = getContext(policy, context.getParent()); policy.registerThrowable(this.context, throwable); } - private RetryContext getContext(RetryPolicy policy) { + private RetryContext getContext(RetryPolicy policy, RetryContext parent) { RetryContext context = contexts.get(policy); if (context == null) { - context = policy.open(null); + context = policy.open(parent); contexts.put(policy, context); } return context; } - private RetryPolicy getPolicy(Object key) { - RetryPolicy result = policyMap.get(key); - Assert.notNull(result, "Could not locate policy for key=[" + key + "]."); - return result; - } - } } diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/policy/SimpleRetryPolicy.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/policy/SimpleRetryPolicy.java index 329b74a9a..802f34cc8 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/policy/SimpleRetryPolicy.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/policy/SimpleRetryPolicy.java @@ -107,7 +107,7 @@ public class SimpleRetryPolicy implements RetryPolicy { * @param retryableExceptionClasses defaults to {@link Exception}. */ public final void setRetryableExceptionClasses(Collection> retryableExceptionClasses) { - retryableClassifier.setExceptionClasses(retryableExceptionClasses); + retryableClassifier.setTypes(retryableExceptionClasses); } /** @@ -118,7 +118,7 @@ public class SimpleRetryPolicy implements RetryPolicy { * @param fatalExceptionClasses defaults to {@link Exception}. */ public final void setFatalExceptionClasses(Collection> fatalExceptionClasses) { - fatalClassifier.setExceptionClasses(fatalExceptionClasses); + fatalClassifier.setTypes(fatalExceptionClasses); } /** @@ -162,6 +162,6 @@ public class SimpleRetryPolicy implements RetryPolicy { * retryable. */ private boolean retryForException(Throwable ex) { - return fatalClassifier.isDefault(ex) && !retryableClassifier.isDefault(ex); + return !fatalClassifier.classify(ex) && retryableClassifier.classify(ex); } } diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/support/RetryTemplate.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/support/RetryTemplate.java index f68ac91bd..33600b8c9 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/support/RetryTemplate.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/retry/support/RetryTemplate.java @@ -39,6 +39,7 @@ import org.springframework.batch.retry.backoff.NoBackOffPolicy; import org.springframework.batch.retry.policy.MapRetryContextCache; import org.springframework.batch.retry.policy.RetryContextCache; import org.springframework.batch.retry.policy.SimpleRetryPolicy; +import org.springframework.batch.support.Classifier; /** * Template class that simplifies the execution of operations with retry @@ -78,6 +79,45 @@ public class RetryTemplate implements RetryOperations { private RetryContextCache retryContextCache = new MapRetryContextCache(); + private Classifier rollbackClassifier = null; + + /** + * Public setter for the rollback classifier. This classifier answers its + * default if the exception provided should not cause a rollback. I.e. + * anything other than the default will lead to a rollback.

+ * + * The decision whether to rollback or not is unrelated to that of the + * {@link RetryPolicy}, but the policy can be accidentally inconsistent + * with the rollback decision. E.g. in a stateless retry the policy might be + * configured to allow retry on a rollback, but that wouldn't make sense - a + * stateful retry should have been used. The best we can do in such + * circumstances is throw a {@link RetryException} from + * {@link #execute(RetryCallback)} or + * {@link #execute(RetryCallback, RecoveryCallback)}. The recovery path + * will not be taken in such situations.

+ * + * For stateless retry it is often adequate to use the default behaviour, as + * long as one is careful with the retry policy (exceptions which should + * cause rollback are still not really retryable in a transactional + * setting).

+ * + * For stateful retry adding a classifier will allow an optimisation: + * exceptions which are not marked for rollback can still be retried, but + * without paying the cost of a rollback. Effectively one is overriding the + * stateful quality of the retry dynamically, according to the exception + * type.

+ * + * Example usage would be for a stateful retry to specify a validation exception as not for rollback + * + * If not set then the default is to rollback for all exceptions when the + * retry is stateful, and for none when it is stateless. + * + * @param rollbackClassifier the rollback classifier to set + */ + public void setRollbackClassifier(Classifier rollbackClassifier) { + this.rollbackClassifier = rollbackClassifier; + } + /** * Public setter for the {@link RetryContextCache}. * @param retryContextCache the {@link RetryContextCache} to set. @@ -406,7 +446,17 @@ public class RetryTemplate implements RetryOperations { * otherwise */ protected boolean shouldRethrow(RetryPolicy retryPolicy, RetryContext context, RetryState state) { - // TODO: allow stateless behaviour to take over for certain exception types + // Allow stateless behaviour to take over for certain exception types + if (rollbackClassifier != null) { + boolean rollback = rollbackClassifier.classify(context.getLastThrowable()); + if (rollback && state == null && retryPolicy.canRetry(context)) { + throw new RetryException("Inconsistent configuration. The retry policy says we can retry but " + + "the exception has been marked for rollback.", context.getLastThrowable()); + } + return rollback; + } + // If no classifier is provided, just assume the all exceptions are for + // rollback if the execution is stateful, and none otherwise. return state != null; } diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/BinaryExceptionClassifier.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/BinaryExceptionClassifier.java index 9bd90aa4a..efe4d8894 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/BinaryExceptionClassifier.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/BinaryExceptionClassifier.java @@ -20,59 +20,65 @@ import java.util.HashMap; import java.util.Map; /** - * A {@link Classifier} that has only two classes of exception. - * Provides convenient methods for setting up and querying the classification - * with boolean return type. + * A {@link Classifier} for exceptions that has only two classes (true and + * false). Classifies objects according to their inheritance relation with the + * supplied types. If the object to be classified is one of the provided types, + * or is a subclass of one of the types, then the non-default value is returned + * (usually true). + * + * @see SubclassClassifier * * @author Dave Syer * */ -public class BinaryExceptionClassifier extends ExceptionClassifierSupport { +public class BinaryExceptionClassifier extends SubclassClassifier { /** - * The classifier result for a non-default exception. + * Create a binary exception classifier with the provided default value. + * @param defaultValue */ - public static final String NON_DEFAULT = "NON_DEFAULT"; - - private SubclassExceptionClassifier delegate = new SubclassExceptionClassifier(); + public BinaryExceptionClassifier(boolean defaultValue) { + super(defaultValue); + } /** - * Set the special exceptions. Any exception on the list, or subclasses - * thereof, will be classified as non-default. + * Create a binary exception classifier with the default value false. + */ + public BinaryExceptionClassifier() { + this(false); + } + + /** + * Create a binary exception classifier with the provided classes and their + * subclasses. The mapped value for these exceptions will be the one + * provided (which will be the opposite of the default). + * @param value + */ + public BinaryExceptionClassifier(Collection> exceptionClasses, boolean value) { + this(!value); + setTypes(exceptionClasses); + } + + /** + * Create a binary exception classifier with the default value false and + * value mapping true for the provided classes and their subclasses. + */ + public BinaryExceptionClassifier(Collection> exceptionClasses) { + this(exceptionClasses, true); + } + + /** + * Set of Throwable class types to keys for the classifier. Any subclass of + * the type provided will be classified as of non-default type. * - * @param exceptionClasses defaults to {@link Exception}. + * @param types the types to classify as non-default */ - public final void setExceptionClasses(Collection> exceptionClasses) { - Map, String> temp = new HashMap, String>(); - for (Class exceptionClass : exceptionClasses) { - temp.put(exceptionClass, NON_DEFAULT); + public final void setTypes(Collection> types) { + Map, Boolean> map = new HashMap, Boolean>(); + for (Class type : types) { + map.put(type, !getDefault()); } - this.delegate.setTypeMap(temp); - } - - /** - * Convenience method to return boolean if the throwable is classified as - * default. - * - * @param throwable the Throwable to classify - * @return true if it is default classified (i.e. not on the list provided - * in {@link #setExceptionClasses(Collection)}. - */ - public boolean isDefault(Throwable throwable) { - return classify(throwable).equals(DEFAULT); - } - - /** - * Returns either {@link ExceptionClassifierSupport#DEFAULT} or - * {@link #NON_DEFAULT} depending on the type of the throwable. If the type - * of the throwable or one of its ancestors is on the exception class list - * the classification is as {@link #NON_DEFAULT}. - * - * @see #setExceptionClasses(Collection) - * @see ExceptionClassifierSupport#classify(Throwable) - */ - public String classify(Throwable throwable) { - return delegate.classify(throwable); + setTypeMap(map); } } diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/Classifier.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/Classifier.java index 95c6e07df..4b3ddd1c6 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/Classifier.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/Classifier.java @@ -26,20 +26,12 @@ package org.springframework.batch.support; public interface Classifier { /** - * Get a default value, normally the same as would be returned by - * {@link #classify(Object)} with null argument. - * - * @return the default value. - */ - T getDefault(); - - /** - * Classify the given object and return a non-null object. The return type - * depends on the implementation but typically would be a key in a map which - * the client maintains. + * Classify the given object and return an object. The return type depends + * on the implementation. * * @param classifiable the input object. Can be null. - * @return an object. + * @return an object. Can be null, but implementations should declare if + * this is the case. */ T classify(C classifiable); diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/ExceptionClassifierSupport.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/ClassifierSupport.java similarity index 61% rename from spring-batch-infrastructure/src/main/java/org/springframework/batch/support/ExceptionClassifierSupport.java rename to spring-batch-infrastructure/src/main/java/org/springframework/batch/support/ClassifierSupport.java index b1c20e945..cfce64af8 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/ExceptionClassifierSupport.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/ClassifierSupport.java @@ -17,35 +17,32 @@ package org.springframework.batch.support; /** - * Base class for {@link Classifier} implementations. Provides default - * behaviour and some convenience members, like constants. + * Base class for {@link Classifier} implementations. Provides default behaviour + * and some convenience members, like constants. * * @author Dave Syer * */ -public class ExceptionClassifierSupport implements Classifier { +public class ClassifierSupport implements Classifier { + + final private T defaultValue; /** - * Default classification key. + * @param defaultValue */ - public static final String DEFAULT = "default"; + public ClassifierSupport(T defaultValue) { + super(); + this.defaultValue = defaultValue; + } /** - * Always returns the value of {@link #DEFAULT}. + * Always returns the default value. This is the main extension point for + * subclasses, so it must be able to classify null. * * @see org.springframework.batch.support.Classifier#classify(Object) */ - public String classify(Throwable throwable) { - return DEFAULT; - } - - /** - * Wrapper for a call to {@link #classify(Throwable)} with argument null. - * - * @see org.springframework.batch.support.Classifier#getDefault() - */ - public String getDefault() { - return classify(null); + public T classify(C throwable) { + return defaultValue; } } diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/SubclassClassifier.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/SubclassClassifier.java new file mode 100644 index 000000000..66d757bca --- /dev/null +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/SubclassClassifier.java @@ -0,0 +1,147 @@ +/* + * Copyright 2006-2007 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.batch.support; + +import java.util.Comparator; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import java.util.TreeSet; + +/** + * A {@link Classifier} for a parameterised object type based on a map. + * Classifies objects according to their inheritance relation with the supplied + * type map. If the object to be classified is one of the keys of the provided + * map, or is a subclass of one of the keys, then the map entry vale for that + * key is returned. Otherwise returns the default value which is null by + * default. + * + * @author Dave Syer + * + */ +public class SubclassClassifier implements Classifier { + + private Map, C> classified = new HashMap, C>(); + + private C defaultValue = null; + + /** + * Create a {@link SubclassClassifier} with null default value. + * + */ + public SubclassClassifier() { + this(null); + } + + /** + * Create a {@link SubclassClassifier} with supplied default value. + * + * @param defaultValue + */ + public SubclassClassifier(C defaultValue) { + this(new HashMap, C>(), defaultValue); + } + + /** + * Create a {@link SubclassClassifier} with supplied default value. + * + * @param defaultValue + */ + public SubclassClassifier(Map, C> typeMap, C defaultValue) { + super(); + setTypeMap(typeMap); + this.defaultValue = defaultValue; + } + + /** + * Public setter for the default value for mapping keys that are not found + * in the map (or their subclasses). Defaults to false. + * + * @param defaultValue the default value to set + */ + public void setDefaultValue(C defaultValue) { + this.defaultValue = defaultValue; + } + + /** + * Set the classifications up as a map. The keys are types and these will be + * mapped along with all their subclasses to the corresponding value. The + * most specific types will match first. + * + * @param map a map from type to class + */ + public void setTypeMap(Map, C> map) { + this.classified = new HashMap, C>(map); + } + + /** + * Return the value from the type map whose key is the class of the given + * Throwable, or its nearest ancestor if a subclass. + * + */ + public C classify(T classifiable) { + + if (classifiable == null) { + return defaultValue; + } + + @SuppressWarnings("unchecked") + Class exceptionClass = (Class) classifiable.getClass(); + if (classified.containsKey(exceptionClass)) { + return classified.get(exceptionClass); + } + + // check for subclasses + Set> classes = new TreeSet>(new ClassComparator()); + classes.addAll(classified.keySet()); + for (Class cls : classes) { + if (cls.isAssignableFrom(exceptionClass)) { + C value = classified.get(cls); + this.classified.put(exceptionClass, value); + return value; + } + } + + return defaultValue; + } + + /** + * Return the default value supplied in the constructor (default false). + */ + final public C getDefault() { + return defaultValue; + } + + /** + * Comparator for classes to order by inheritance. + * + * @author Dave Syer + * + */ + private static class ClassComparator implements Comparator> { + /** + * @return 1 if arg0 is assignable from arg1, -1 otherwise + * @see java.util.Comparator#compare(java.lang.Object, java.lang.Object) + */ + public int compare(Class arg0, Class arg1) { + if (arg0.isAssignableFrom(arg1)) { + return 1; + } + return -1; + } + } + +} diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/SubclassExceptionClassifier.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/SubclassExceptionClassifier.java deleted file mode 100644 index 249d7b875..000000000 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/SubclassExceptionClassifier.java +++ /dev/null @@ -1,105 +0,0 @@ -/* - * Copyright 2006-2007 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.batch.support; - -import java.util.Comparator; -import java.util.HashMap; -import java.util.Map; -import java.util.Set; -import java.util.TreeSet; - -import org.springframework.util.Assert; - -/** - * - * @author Dave Syer - * - */ -public class SubclassExceptionClassifier extends ExceptionClassifierSupport { - - private Map, String> classified = new HashMap, String>(); - - /** - * Map of Throwable class types to keys for the classifier. Any subclass of - * the type provided will be classified as of the type given by the - * corresponding map entry value. - * - * @param typeMap the typeMap to set - */ - public final void setTypeMap(Map, String> typeMap) { - Map, String> map = new HashMap, String>(); - for (Map.Entry, String> entry : typeMap.entrySet()) { - addRetryableExceptionClass(entry.getKey(), entry.getValue(), map); - } - this.classified = map; - } - - /** - * Return the value from the type map whose key is the class of the given - * Throwable, or its nearest ancestor if a subclass. - * - * @see org.springframework.batch.support.ExceptionClassifierSupport#classify(java.lang.Throwable) - */ - public String classify(Throwable throwable) { - - if (throwable == null) { - return super.classify(throwable); - } - - Class exceptionClass = throwable.getClass(); - if (classified.containsKey(exceptionClass)) { - return classified.get(exceptionClass); - } - - // check for subclasses - Set> classes = new TreeSet>(new ClassComparator()); - classes.addAll(classified.keySet()); - for (Class cls : classes) { - if (cls.isAssignableFrom(exceptionClass)) { - String value = classified.get(cls); - addRetryableExceptionClass(exceptionClass, value, this.classified); - return value; - } - } - - return super.classify(throwable); - } - - private void addRetryableExceptionClass(Class exceptionClass, String classifiedAs, Map, String> map) { - Assert.isAssignable(Throwable.class, exceptionClass); - map.put(exceptionClass, classifiedAs); - } - - /** - * Comparator for classes to order by inheritance. - * - * @author Dave Syer - * - */ - private class ClassComparator implements Comparator> { - /** - * @return 1 if arg0 is assignable from arg1, -1 otherwise - * @see java.util.Comparator#compare(java.lang.Object, java.lang.Object) - */ - public int compare(Class arg0, Class arg1) { - if (arg0.isAssignableFrom(arg1)) { - return 1; - } - return -1; - } - } - -} diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java index 611aa293a..95608a88c 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/item/xml/StaxEventItemWriterTests.java @@ -11,7 +11,6 @@ import static org.junit.Assert.fail; import java.io.File; import java.io.IOException; import java.util.Collections; -import java.util.HashMap; import java.util.List; import javax.xml.stream.XMLEventFactory; @@ -48,7 +47,7 @@ public class StaxEventItemWriterTests { // test item for writing to output private Object item = new Object() { public String toString() { - return ClassUtils.getShortName(StaxEventItemWriter.class)+"-testString"; + return ClassUtils.getShortName(StaxEventItemWriter.class) + "-testString"; } }; @@ -73,7 +72,7 @@ public class StaxEventItemWriterTests { writer.write(items); writer.close(executionContext); String content = outputFileContent(); - assertTrue("Wrong content: "+content, content.contains(TEST_STRING)); + assertTrue("Wrong content: " + content, content.contains(TEST_STRING)); } /** @@ -96,7 +95,7 @@ public class StaxEventItemWriterTests { // check the output is concatenation of 'before restart' and 'after // restart' writes. String outputFile = outputFileContent(); - + assertEquals(2, StringUtils.countOccurrencesOf(outputFile, TEST_STRING)); } @@ -107,13 +106,13 @@ public class StaxEventItemWriterTests { public void testWriteWithHeader() throws Exception { Object header1 = new Object(); Object header2 = new Object(); - writer.setHeaderItems(new Object[] {header1, header2}); + writer.setHeaderItems(new Object[] { header1, header2 }); writer.open(executionContext); writer.write(items); String content = outputFileContent(); - assertTrue("Wrong content: "+content, content.contains((""))); - assertTrue("Wrong content: "+content, content.contains((""))); - assertTrue("Wrong content: "+content, content.contains(TEST_STRING)); + assertTrue("Wrong content: " + content, content.contains((""))); + assertTrue("Wrong content: " + content, content.contains((""))); + assertTrue("Wrong content: " + content, content.contains(TEST_STRING)); } /** @@ -123,8 +122,7 @@ public class StaxEventItemWriterTests { public void testStreamContext() throws Exception { writer.open(executionContext); final int NUMBER_OF_RECORDS = 10; - assertFalse(executionContext.containsKey(ClassUtils.getShortName(StaxEventItemWriter.class) - + ".record.count")); + assertFalse(executionContext.containsKey(ClassUtils.getShortName(StaxEventItemWriter.class) + ".record.count")); for (int i = 1; i <= NUMBER_OF_RECORDS; i++) { writer.write(items); writer.update(executionContext); @@ -141,25 +139,21 @@ public class StaxEventItemWriterTests { @Test public void testOpenAndClose() throws Exception { writer.setRootTagName("testroot"); - writer.setRootElementAttributes(new HashMap() { - { - put("attribute", "value"); - } - }); + writer.setRootElementAttributes(Collections. singletonMap("attribute", "value")); writer.open(executionContext); writer.close(null); String content = outputFileContent(); assertTrue(content.contains("")); assertTrue(content.endsWith("")); } - + @Test public void testNonExistantResource() throws Exception { Resource doesntExist = createMock(Resource.class); expect(doesntExist.getFile()).andReturn(File.createTempFile("arbitrary", null)); expect(doesntExist.exists()).andReturn(false); replay(doesntExist); - + writer.setResource(doesntExist); try { diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/repeat/exception/LogOrRethrowExceptionHandlerTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/repeat/exception/LogOrRethrowExceptionHandlerTests.java index 1f323e39e..77fc53154 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/repeat/exception/LogOrRethrowExceptionHandlerTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/repeat/exception/LogOrRethrowExceptionHandlerTests.java @@ -20,34 +20,37 @@ import java.io.StringWriter; import junit.framework.TestCase; -import org.apache.log4j.Level; import org.apache.log4j.Logger; import org.apache.log4j.SimpleLayout; import org.apache.log4j.WriterAppender; import org.springframework.batch.repeat.RepeatContext; -import org.springframework.batch.support.ExceptionClassifierSupport; +import org.springframework.batch.repeat.exception.LogOrRethrowExceptionHandler.Level; +import org.springframework.batch.support.ClassifierSupport; public class LogOrRethrowExceptionHandlerTests extends TestCase { private LogOrRethrowExceptionHandler handler = new LogOrRethrowExceptionHandler(); + private StringWriter writer; + private RepeatContext context = null; - + protected void setUp() throws Exception { super.setUp(); Logger logger = Logger.getLogger(LogOrRethrowExceptionHandler.class); - logger.setLevel(Level.DEBUG); + logger.setLevel(org.apache.log4j.Level.DEBUG); writer = new StringWriter(); logger.removeAllAppenders(); logger.getParent().removeAllAppenders(); logger.addAppender(new WriterAppender(new SimpleLayout(), writer)); } - + public void testRuntimeException() throws Throwable { try { handler.handleException(context, new RuntimeException("Foo")); fail("Expected RuntimeException"); - } catch (RuntimeException e) { + } + catch (RuntimeException e) { assertEquals("Foo", e.getMessage()); } } @@ -56,15 +59,16 @@ public class LogOrRethrowExceptionHandlerTests extends TestCase { try { handler.handleException(context, new Error("Foo")); fail("Expected Error"); - } catch (Error e) { + } + catch (Error e) { assertEquals("Foo", e.getMessage()); } } - + public void testNotRethrownErrorLevel() throws Throwable { - handler.setExceptionClassifier(new ExceptionClassifierSupport() { - public String classify(Throwable throwable) { - return LogOrRethrowExceptionHandler.ERROR; + handler.setExceptionClassifier(new ClassifierSupport(Level.RETHROW) { + public Level classify(Throwable throwable) { + return Level.ERROR; } }); // No exception... @@ -73,20 +77,9 @@ public class LogOrRethrowExceptionHandlerTests extends TestCase { } public void testNotRethrownWarnLevel() throws Throwable { - handler.setExceptionClassifier(new ExceptionClassifierSupport() { - public String classify(Throwable throwable) { - return LogOrRethrowExceptionHandler.WARN; - } - }); - // No exception... - handler.handleException(context, new Error("Foo")); - assertNotNull(writer.toString()); - } - - public void testNotRethrownDebugLevel() throws Throwable { - handler.setExceptionClassifier(new ExceptionClassifierSupport() { - public String classify(Throwable throwable) { - return LogOrRethrowExceptionHandler.DEBUG; + handler.setExceptionClassifier(new ClassifierSupport(Level.RETHROW) { + public Level classify(Throwable throwable) { + return Level.WARN; } }); // No exception... @@ -94,18 +87,15 @@ public class LogOrRethrowExceptionHandlerTests extends TestCase { assertNotNull(writer.toString()); } - public void testUnclassifiedException() throws Throwable { - handler.setExceptionClassifier(new ExceptionClassifierSupport() { - public String classify(Throwable throwable) { - return "DEFAULT"; + public void testNotRethrownDebugLevel() throws Throwable { + handler.setExceptionClassifier(new ClassifierSupport(Level.RETHROW) { + public Level classify(Throwable throwable) { + return Level.DEBUG; } }); - try { - handler.handleException(context, new Error("Foo")); - fail("Expected IllegalStateException"); - } catch (IllegalStateException e) { - assertTrue(e.getMessage().toLowerCase().indexOf("unclassified")>=0); - } + // No exception... + handler.handleException(context, new Error("Foo")); + assertNotNull(writer.toString()); } } diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/repeat/exception/RethrowOnThresholdExceptionHandlerTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/repeat/exception/RethrowOnThresholdExceptionHandlerTests.java index 093b62a8f..9866dd7ae 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/repeat/exception/RethrowOnThresholdExceptionHandlerTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/repeat/exception/RethrowOnThresholdExceptionHandlerTests.java @@ -16,60 +16,60 @@ package org.springframework.batch.repeat.exception; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.fail; + import java.util.Collections; +import java.util.concurrent.atomic.AtomicInteger; -import junit.framework.TestCase; - +import org.junit.Test; import org.springframework.batch.repeat.RepeatContext; -import org.springframework.batch.repeat.context.RepeatContextCounter; import org.springframework.batch.repeat.context.RepeatContextSupport; -import org.springframework.batch.support.ExceptionClassifierSupport; -public class RethrowOnThresholdExceptionHandlerTests extends TestCase { +public class RethrowOnThresholdExceptionHandlerTests { private RethrowOnThresholdExceptionHandler handler = new RethrowOnThresholdExceptionHandler(); + private RepeatContext parent = new RepeatContextSupport(null); + private RepeatContext context = new RepeatContextSupport(parent); - + + @Test public void testRuntimeException() throws Throwable { try { handler.handleException(context, new RuntimeException("Foo")); fail("Expected RuntimeException"); - } catch (RuntimeException e) { + } + catch (RuntimeException e) { assertEquals("Foo", e.getMessage()); } } + @Test public void testError() throws Throwable { try { handler.handleException(context, new Error("Foo")); fail("Expected Error"); - } catch (Error e) { + } + catch (Error e) { assertEquals("Foo", e.getMessage()); } } - + + @Test public void testNotRethrownWithThreshold() throws Throwable { - handler.setExceptionClassifier(new ExceptionClassifierSupport() { - public String classify(Throwable throwable) { - return "RuntimeException"; - } - }); - handler.setThresholds(Collections.singletonMap("RuntimeException", new Integer(1))); + handler.setThresholds(Collections., Integer> singletonMap(Exception.class, 1)); // No exception... handler.handleException(context, new RuntimeException("Foo")); - RepeatContextCounter counter = new RepeatContextCounter(context, RethrowOnThresholdExceptionHandler.class.getName() + ".RuntimeException"); + AtomicInteger counter = (AtomicInteger) context.getAttribute(context.attributeNames()[0]); assertNotNull(counter); - assertEquals(1, counter.getCount()); + assertEquals(1, counter.get()); } - + + @Test public void testRethrowOnThreshold() throws Throwable { - handler.setExceptionClassifier(new ExceptionClassifierSupport() { - public String classify(Throwable throwable) { - return "RuntimeException"; - } - }); - handler.setThresholds(Collections.singletonMap("RuntimeException", new Integer(2))); + handler.setThresholds(Collections., Integer> singletonMap(Exception.class, 2)); // No exception... handler.handleException(context, new RuntimeException("Foo")); handler.handleException(context, new RuntimeException("Foo")); @@ -81,14 +81,10 @@ public class RethrowOnThresholdExceptionHandlerTests extends TestCase { assertEquals("Foo", e.getMessage()); } } - + + @Test public void testNotUseParent() throws Throwable { - handler.setExceptionClassifier(new ExceptionClassifierSupport() { - public String classify(Throwable throwable) { - return "RuntimeException"; - } - }); - handler.setThresholds(Collections.singletonMap("RuntimeException", new Integer(1))); + handler.setThresholds(Collections., Integer> singletonMap(Exception.class, 1)); // No exception... handler.handleException(context, new RuntimeException("Foo")); context = new RepeatContextSupport(parent); @@ -101,13 +97,9 @@ public class RethrowOnThresholdExceptionHandlerTests extends TestCase { } } + @Test public void testUseParent() throws Throwable { - handler.setExceptionClassifier(new ExceptionClassifierSupport() { - public String classify(Throwable throwable) { - return "RuntimeException"; - } - }); - handler.setThresholds(Collections.singletonMap("RuntimeException", new Integer(1))); + handler.setThresholds(Collections., Integer> singletonMap(Exception.class, 1)); handler.setUseParent(true); // No exception... handler.handleException(context, new RuntimeException("Foo")); diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/repeat/exception/SimpleLimitExceptionHandlerTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/repeat/exception/SimpleLimitExceptionHandlerTests.java index 1c806f579..fe5219280 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/repeat/exception/SimpleLimitExceptionHandlerTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/repeat/exception/SimpleLimitExceptionHandlerTests.java @@ -16,11 +16,17 @@ package org.springframework.batch.repeat.exception; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + import java.util.ArrayList; +import java.util.Collections; import java.util.List; -import junit.framework.TestCase; - +import org.junit.Before; +import org.junit.Test; import org.springframework.batch.repeat.context.RepeatContextSupport; /** @@ -29,77 +35,110 @@ import org.springframework.batch.repeat.context.RepeatContextSupport; * @author Robert Kasanicky * @author Dave Syer */ -public class SimpleLimitExceptionHandlerTests extends TestCase { +public class SimpleLimitExceptionHandlerTests { // object under test private SimpleLimitExceptionHandler handler = new SimpleLimitExceptionHandler(); + @Before + public void initializeHandler() throws Exception { + handler.afterPropertiesSet(); + } + + @Test public void testInitializeWithNullContext() throws Throwable { try { handler.handleException(null, new RuntimeException("foo")); fail("Expected IllegalArgumentException"); - } catch (IllegalArgumentException e) { + } + catch (IllegalArgumentException e) { // expected } } + @Test public void testInitializeWithNullContextAndNullException() throws Throwable { try { handler.handleException(null, null); - } catch (NullPointerException e) { + } + catch (IllegalArgumentException e) { // expected; } } - /** - * Other than nominated exception type should be rethrown, ignoring the exception limit. - * - * @throws Exception - */ - public void testNormalExceptionThrown() throws Throwable { + @Test + public void testDefaultBehaviour() throws Throwable { Throwable throwable = new RuntimeException("foo"); - - final int MORE_THAN_ZERO = 1; - handler.setLimit(MORE_THAN_ZERO); - handler.setExceptionClasses(new Class[] { IllegalArgumentException.class }); - try { handler.handleException(new RepeatContextSupport(null), throwable); - fail("Exception swallowed."); - } catch (RuntimeException expected) { + fail("Exception was swallowed."); + } + catch (RuntimeException expected) { assertTrue("Exception is rethrown, ignoring the exception limit", true); assertSame(expected, throwable); } } /** - * TransactionInvalidException should only be rethrown below the exception limit. + * Other than nominated exception type should be rethrown, ignoring the + * exception limit. * * @throws Exception */ + @Test + public void testNormalExceptionThrown() throws Throwable { + Throwable throwable = new RuntimeException("foo"); + + final int MORE_THAN_ZERO = 1; + handler.setLimit(MORE_THAN_ZERO); + handler.setExceptionClasses(Collections.> singleton(IllegalArgumentException.class)); + handler.afterPropertiesSet(); + + try { + handler.handleException(new RepeatContextSupport(null), throwable); + fail("Exception was swallowed."); + } + catch (RuntimeException expected) { + assertTrue("Exception is rethrown, ignoring the exception limit", true); + assertSame(expected, throwable); + } + } + + /** + * TransactionInvalidException should only be rethrown below the exception + * limit. + * + * @throws Exception + */ + @Test public void testLimitedExceptionTypeNotThrown() throws Throwable { final int MORE_THAN_ZERO = 1; handler.setLimit(MORE_THAN_ZERO); - handler.setExceptionClasses(new Class[] {RuntimeException.class} ); + handler.setExceptionClasses(Collections.> singleton(RuntimeException.class)); + handler.afterPropertiesSet(); try { handler.handleException(new RepeatContextSupport(null), new RuntimeException("foo")); - } catch (RuntimeException expected) { + } + catch (RuntimeException expected) { fail("Unexpected exception."); } } /** - * TransactionInvalidException should only be rethrown below the exception limit. + * TransactionInvalidException should only be rethrown below the exception + * limit. * * @throws Exception */ + @Test public void testLimitedExceptionNotThrownFromSiblings() throws Throwable { Throwable throwable = new RuntimeException("foo"); final int MORE_THAN_ZERO = 1; handler.setLimit(MORE_THAN_ZERO); - handler.setExceptionClasses(new Class[] {RuntimeException.class}); + handler.setExceptionClasses(Collections.> singleton(RuntimeException.class)); + handler.afterPropertiesSet(); RepeatContextSupport parent = new RepeatContextSupport(null); @@ -108,23 +147,27 @@ public class SimpleLimitExceptionHandlerTests extends TestCase { handler.handleException(context, throwable); context = new RepeatContextSupport(parent); handler.handleException(context, throwable); - } catch (RuntimeException expected) { + } + catch (RuntimeException expected) { fail("Unexpected exception."); } } /** - * TransactionInvalidException should only be rethrown below the exception limit. + * TransactionInvalidException should only be rethrown below the exception + * limit. * * @throws Exception */ + @Test public void testLimitedExceptionThrownFromSiblingsWhenUsingParent() throws Throwable { Throwable throwable = new RuntimeException("foo"); final int MORE_THAN_ZERO = 1; handler.setLimit(MORE_THAN_ZERO); - handler.setExceptionClasses(new Class[] { RuntimeException.class } ); + handler.setExceptionClasses(Collections.> singleton(RuntimeException.class)); handler.setUseParent(true); + handler.afterPropertiesSet(); RepeatContextSupport parent = new RepeatContextSupport(null); @@ -134,19 +177,22 @@ public class SimpleLimitExceptionHandlerTests extends TestCase { context = new RepeatContextSupport(parent); handler.handleException(context, throwable); fail("Expected exception."); - } catch (RuntimeException expected) { + } + catch (RuntimeException expected) { assertSame(throwable, expected); } } /** - * TransactionInvalidExceptions are swallowed until the exception limit is exceeded. After the limit is exceeded - * exceptions are rethrown as BatchCriticalExceptions + * Exceptions are swallowed until the exception limit is exceeded. After the + * limit is exceeded exceptions are rethrown */ + @Test public void testExceptionNotThrownBelowLimit() throws Throwable { final int EXCEPTION_LIMIT = 3; handler.setLimit(EXCEPTION_LIMIT); + handler.afterPropertiesSet(); List throwables = new ArrayList() { { @@ -165,20 +211,24 @@ public class SimpleLimitExceptionHandlerTests extends TestCase { assertTrue("exceptions up to limit are swallowed", true); } - } catch (RuntimeException unexpected) { + } + catch (RuntimeException unexpected) { fail("exception rethrown although exception limit was not exceeded"); } } /** - * TransactionInvalidExceptions are swallowed until the exception limit is exceeded. After the limit is exceeded - * exceptions are rethrown as BatchCriticalExceptions + * TransactionInvalidExceptions are swallowed until the exception limit is + * exceeded. After the limit is exceeded exceptions are rethrown as + * BatchCriticalExceptions */ + @Test public void testExceptionThrownAboveLimit() throws Throwable { final int EXCEPTION_LIMIT = 3; handler.setLimit(EXCEPTION_LIMIT); + handler.afterPropertiesSet(); List throwables = new ArrayList() { { @@ -199,7 +249,8 @@ public class SimpleLimitExceptionHandlerTests extends TestCase { assertTrue("exceptions up to limit are swallowed", true); } - } catch (RuntimeException expected) { + } + catch (RuntimeException expected) { assertEquals("above exception limit", expected.getMessage()); } @@ -208,7 +259,8 @@ public class SimpleLimitExceptionHandlerTests extends TestCase { handler.handleException(context, new RuntimeException("foo")); assertTrue("exceptions up to limit are swallowed", true); - } catch (RuntimeException expected) { + } + catch (RuntimeException expected) { assertEquals("foo", expected.getMessage()); } } diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/policy/ExceptionClassifierRetryPolicyTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/policy/ExceptionClassifierRetryPolicyTests.java index deaf608fb..795118021 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/policy/ExceptionClassifierRetryPolicyTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/policy/ExceptionClassifierRetryPolicyTests.java @@ -18,17 +18,15 @@ package org.springframework.batch.retry.policy; import java.util.Collections; import java.util.HashMap; -import java.util.Map; - import junit.framework.TestCase; import org.springframework.batch.retry.RetryContext; import org.springframework.batch.retry.RetryPolicy; -import org.springframework.batch.support.ExceptionClassifierSupport; +import org.springframework.batch.support.Classifier; public class ExceptionClassifierRetryPolicyTests extends TestCase { - ExceptionClassifierRetryPolicy policy = new ExceptionClassifierRetryPolicy(); + private ExceptionClassifierRetryPolicy policy = new ExceptionClassifierRetryPolicy(); public void testDefaultPolicies() throws Exception { RetryContext context = policy.open(null); @@ -36,28 +34,22 @@ public class ExceptionClassifierRetryPolicyTests extends TestCase { } public void testTrivialPolicies() throws Exception { - policy.setPolicyMap(Collections.singletonMap(ExceptionClassifierSupport.DEFAULT, - (RetryPolicy) new MockRetryPolicySupport())); + policy.setPolicyMap(Collections., RetryPolicy> singletonMap(Exception.class, + new MockRetryPolicySupport())); RetryContext context = policy.open(null); assertNotNull(context); assertTrue(policy.canRetry(context)); } public void testNullPolicies() throws Exception { - policy.setPolicyMap(new HashMap()); - try { - policy.open(null); - fail("Expected IllegalArgumentException"); - } - catch (IllegalArgumentException e) { - // expected - } + policy.setPolicyMap(new HashMap, RetryPolicy>()); + RetryContext context = policy.open(null); + assertNotNull(context); } public void testNullContext() throws Exception { - Map map = new HashMap(); - map.put(ExceptionClassifierSupport.DEFAULT, new NeverRetryPolicy()); - policy.setPolicyMap(map); + policy.setPolicyMap(Collections., RetryPolicy> singletonMap(Exception.class, + new NeverRetryPolicy())); RetryContext context = policy.open(null); assertNotNull(context); @@ -67,53 +59,52 @@ public class ExceptionClassifierRetryPolicyTests extends TestCase { public void testClassifierOperates() throws Exception { - Map map = new HashMap(); - map.put(ExceptionClassifierSupport.DEFAULT, new AlwaysRetryPolicy()); - map.put("foo", new NeverRetryPolicy()); - policy.setPolicyMap(map); - RetryContext context = policy.open(null); assertNotNull(context); assertTrue(policy.canRetry(context)); policy.registerThrowable(context, new IllegalArgumentException()); - assertTrue(policy.canRetry(context)); + assertFalse(policy.canRetry(context)); // NeverRetryPolicy is the + // default - policy.setExceptionClassifier(new ExceptionClassifierSupport() { - public String classify(Throwable throwable) { + policy.setExceptionClassifier(new Classifier() { + public RetryPolicy classify(Throwable throwable) { if (throwable != null) { - return "foo"; + return new AlwaysRetryPolicy(); } - return super.classify(throwable); + return new NeverRetryPolicy(); } }); // The context saves the classifier, so changing it now has no effect - assertTrue(policy.canRetry(context)); + assertFalse(policy.canRetry(context)); policy.registerThrowable(context, new IllegalArgumentException()); - assertTrue(policy.canRetry(context)); + assertFalse(policy.canRetry(context)); // But now the classifier will be active in the new context... context = policy.open(null); assertTrue(policy.canRetry(context)); policy.registerThrowable(context, new IllegalArgumentException()); - assertFalse(policy.canRetry(context)); + assertTrue(policy.canRetry(context)); } int count = 0; public void testClose() throws Exception { - policy.setPolicyMap(Collections.singletonMap(ExceptionClassifierSupport.DEFAULT, - (RetryPolicy) new MockRetryPolicySupport() { + policy.setExceptionClassifier(new Classifier() { + public RetryPolicy classify(Throwable throwable) { + return new MockRetryPolicySupport() { public void close(RetryContext context) { count++; } - })); + }; + } + }); RetryContext context = policy.open(null); // The mapped (child) policy hasn't been used yet, so if we close now - // we don't incur the possible expense of ceating the child context. + // we don't incur the possible expense of creating the child context. policy.close(context); assertEquals(0, count); // not classified yet // This forces a child context to be created and the child policy is diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/policy/FatalExceptionRetryPolicyTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/policy/FatalExceptionRetryPolicyTests.java index ba0149e55..e815f721c 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/policy/FatalExceptionRetryPolicyTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/policy/FatalExceptionRetryPolicyTests.java @@ -16,7 +16,8 @@ package org.springframework.batch.retry.policy; -import java.util.HashSet; +import java.util.Arrays; +import java.util.List; import junit.framework.TestCase; @@ -38,12 +39,10 @@ public class FatalExceptionRetryPolicyTests extends TestCase { retryTemplate.setRetryPolicy(policy); // ...but make sure certain exceptions are fatal - policy.setFatalExceptionClasses(new HashSet>() { - { - add(IllegalArgumentException.class); - add(IllegalStateException.class); - } - }); + @SuppressWarnings("unchecked") + List> list = Arrays.> asList(IllegalArgumentException.class, + IllegalStateException.class); + policy.setFatalExceptionClasses(list); RecoveryCallback recoveryCallback = new RecoveryCallback() { public String recover(RetryContext context) throws Exception { return "bar"; @@ -71,13 +70,11 @@ public class FatalExceptionRetryPolicyTests extends TestCase { SimpleRetryPolicy policy = new SimpleRetryPolicy(3); retryTemplate.setRetryPolicy(policy); - policy.setFatalExceptionClasses(new HashSet>() { - { - add(IllegalArgumentException.class); - add(IllegalStateException.class); - } - }); - RecoveryCallbackrecoveryCallback = new RecoveryCallback() { + @SuppressWarnings("unchecked") + List> list = Arrays.> asList( + IllegalArgumentException.class, IllegalStateException.class); + policy.setFatalExceptionClasses(list); + RecoveryCallback recoveryCallback = new RecoveryCallback() { public String recover(RetryContext context) throws Exception { return "bar"; } diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/support/RetryTemplateTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/support/RetryTemplateTests.java index d36d0bf9a..16d4464cf 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/support/RetryTemplateTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/support/RetryTemplateTests.java @@ -22,19 +22,21 @@ import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertSame; import static org.junit.Assert.fail; -import java.util.HashSet; +import java.util.Collections; import org.junit.Test; import org.springframework.batch.retry.ExhaustedRetryException; import org.springframework.batch.retry.RecoveryCallback; import org.springframework.batch.retry.RetryCallback; import org.springframework.batch.retry.RetryContext; +import org.springframework.batch.retry.RetryState; import org.springframework.batch.retry.backoff.BackOffContext; import org.springframework.batch.retry.backoff.BackOffInterruptedException; import org.springframework.batch.retry.backoff.BackOffPolicy; import org.springframework.batch.retry.backoff.StatelessBackOffPolicy; import org.springframework.batch.retry.policy.NeverRetryPolicy; import org.springframework.batch.retry.policy.SimpleRetryPolicy; +import org.springframework.batch.support.BinaryExceptionClassifier; /** * @author Rob Harrop @@ -71,7 +73,7 @@ public class RetryTemplateTests { } }); assertEquals(2, callback.attempts); - assertEquals(value, result); + assertEquals(value, result); } @Test @@ -117,16 +119,28 @@ public class RetryTemplateTests { assertEquals(attempts, callback.attempts); } + @Test + public void testRollbackClassifierOverridesRetryPolicy() throws Exception { + MockRetryCallback callback = new MockRetryCallback(); + int attempts = 3; + callback.setAttemptsBeforeSuccess(attempts); + callback.setExceptionToThrow(new IllegalArgumentException()); + + RetryTemplate retryTemplate = new RetryTemplate(); + retryTemplate.setRetryPolicy(new SimpleRetryPolicy(attempts)); + BinaryExceptionClassifier classifier = new BinaryExceptionClassifier(Collections + .> singleton(IllegalArgumentException.class), false); + retryTemplate.setRollbackClassifier(classifier); + retryTemplate.execute(callback, new RetryState("foo")); + assertEquals(attempts, callback.attempts); + } + @Test public void testSetExceptions() throws Exception { RetryTemplate template = new RetryTemplate(); SimpleRetryPolicy policy = new SimpleRetryPolicy(); template.setRetryPolicy(policy); - policy.setRetryableExceptionClasses(new HashSet>() { - { - add(RuntimeException.class); - } - }); + policy.setRetryableExceptionClasses(Collections.> singleton(RuntimeException.class)); int attempts = 3; @@ -243,7 +257,7 @@ public class RetryTemplateTests { assertEquals("foo", e.getMessage()); } } - + private static class MockRetryCallback implements RetryCallback { private int attempts; diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/support/StatefulRecoveryRetryTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/support/StatefulRecoveryRetryTests.java index e80c43138..6c407167b 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/support/StatefulRecoveryRetryTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/retry/support/StatefulRecoveryRetryTests.java @@ -23,6 +23,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import org.junit.Test; @@ -39,6 +40,8 @@ import org.springframework.batch.retry.RetryState; import org.springframework.batch.retry.policy.MapRetryContextCache; import org.springframework.batch.retry.policy.NeverRetryPolicy; import org.springframework.batch.retry.policy.SimpleRetryPolicy; +import org.springframework.batch.support.BinaryExceptionClassifier; +import org.springframework.dao.DataAccessException; public class StatefulRecoveryRetryTests { @@ -121,6 +124,37 @@ public class StatefulRecoveryRetryTests { assertEquals(input, list.get(0)); } + @Test + public void testSwitchToStatelessForNoRollback() throws Exception { + retryTemplate.setRetryPolicy(new SimpleRetryPolicy(1)); + // Roll back for these: + BinaryExceptionClassifier classifier = new BinaryExceptionClassifier(Collections + .> singleton(DataAccessException.class)); + // ...but not these: + assertFalse(classifier.classify(new RuntimeException())); + retryTemplate.setRollbackClassifier(classifier); + final String input = "foo"; + RetryState state = new RetryState(input); + RetryCallback callback = new RetryCallback() { + public String doWithRetry(RetryContext context) throws Exception { + throw new RuntimeException("Barf!"); + } + }; + RecoveryCallback recoveryCallback = new RecoveryCallback() { + public String recover(RetryContext context) { + count++; + list.add(input); + return input; + } + }; + Object result = null; + // On the second retry, the recovery path is taken... + result = retryTemplate.execute(callback, recoveryCallback, state); + assertEquals(input, result); // default result is the item + assertEquals(1, count); + assertEquals(input, list.get(0)); + } + @Test public void testExhaustedClearsHistoryAfterLastAttempt() throws Exception { RetryPolicy retryPolicy = new SimpleRetryPolicy(1); diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/support/BinaryExceptionClassifierTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/support/BinaryExceptionClassifierTests.java index de5611288..d48022ec3 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/support/BinaryExceptionClassifierTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/support/BinaryExceptionClassifierTests.java @@ -16,8 +16,7 @@ package org.springframework.batch.support; -import java.util.HashSet; - +import java.util.Collections; import junit.framework.TestCase; public class BinaryExceptionClassifierTests extends TestCase { @@ -25,19 +24,36 @@ public class BinaryExceptionClassifierTests extends TestCase { BinaryExceptionClassifier classifier = new BinaryExceptionClassifier(); public void testClassifyNullIsDefault() { - assertTrue(classifier.isDefault(null)); + assertFalse(classifier.classify(null)); + } + + public void testFalseIsDefault() { + assertFalse(classifier.getDefault()); + } + + public void testDefaultProvided() { + classifier = new BinaryExceptionClassifier(true); + assertTrue(classifier.getDefault()); } public void testClassifyRandomException() { - assertTrue(classifier.isDefault(new IllegalStateException("foo"))); + assertFalse(classifier.classify(new IllegalStateException("foo"))); } public void testClassifyExactMatch() { - classifier.setExceptionClasses(new HashSet>() { - { - add(IllegalStateException.class); - } - }); - assertEquals(false, classifier.isDefault(new IllegalStateException("Foo"))); + classifier.setTypes(Collections.> singleton(IllegalStateException.class)); + assertTrue(classifier.classify(new IllegalStateException("Foo"))); + } + + public void testTypesProvidedInConstructor() { + classifier = new BinaryExceptionClassifier(Collections + .> singleton(IllegalStateException.class)); + assertTrue(classifier.classify(new IllegalStateException("Foo"))); + } + + public void testTypesProvidedInConstructorWithNonDefault() { + classifier = new BinaryExceptionClassifier(Collections + .> singleton(IllegalStateException.class), false); + assertFalse(classifier.classify(new IllegalStateException("Foo"))); } } diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/support/ExceptionClassifierSupportTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/support/ClassifierSupportTests.java similarity index 69% rename from spring-batch-infrastructure/src/test/java/org/springframework/batch/support/ExceptionClassifierSupportTests.java rename to spring-batch-infrastructure/src/test/java/org/springframework/batch/support/ClassifierSupportTests.java index 9692ab585..8a03e95c2 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/support/ExceptionClassifierSupportTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/support/ClassifierSupportTests.java @@ -16,20 +16,18 @@ package org.springframework.batch.support; -import org.springframework.batch.support.ExceptionClassifierSupport; - import junit.framework.TestCase; -public class ExceptionClassifierSupportTests extends TestCase { +public class ClassifierSupportTests extends TestCase { public void testClassifyNullIsDefault() { - ExceptionClassifierSupport classifier = new ExceptionClassifierSupport(); - assertEquals(classifier.classify(null), classifier.getDefault()); + ClassifierSupport classifier = new ClassifierSupport("foo"); + assertEquals(classifier.classify(null), "foo"); } public void testClassifyRandomException() { - ExceptionClassifierSupport classifier = new ExceptionClassifierSupport(); - assertEquals(classifier.classify(new IllegalStateException("Foo")), classifier.getDefault()); + ClassifierSupport classifier = new ClassifierSupport("foo"); + assertEquals(classifier.classify(new IllegalStateException("Foo")), classifier.classify(null)); } } diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/support/SubclassExceptionClassifierTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/support/SubclassExceptionClassifierTests.java index 9d4087cf2..495d2db39 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/support/SubclassExceptionClassifierTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/support/SubclassExceptionClassifierTests.java @@ -16,49 +16,69 @@ package org.springframework.batch.support; -import java.util.LinkedHashMap; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; -import junit.framework.TestCase; +import java.util.Collections; +import java.util.HashMap; -public class SubclassExceptionClassifierTests extends TestCase { +import org.junit.Test; - SubclassExceptionClassifier classifier = new SubclassExceptionClassifier(); +public class SubclassExceptionClassifierTests { + SubclassClassifier classifier = new SubclassClassifier(); + + @Test public void testClassifyNullIsDefault() { assertEquals(classifier.classify(null), classifier.getDefault()); } + @Test + public void testClassifyNull() { + assertNull(classifier.classify(null)); + } + + @Test + public void testClassifyNullNonDefault() { + classifier = new SubclassClassifier("foo"); + assertEquals("foo", classifier.classify(null)); + } + + @Test public void testClassifyRandomException() { - assertEquals(classifier.classify(new IllegalStateException("Foo")), classifier.getDefault()); + assertNull(classifier.classify(new IllegalStateException("Foo"))); } + @Test public void testClassifyExactMatch() { - classifier.setTypeMap(new LinkedHashMap, String>() {{ - put(IllegalStateException.class, "foo"); - }}); + classifier.setTypeMap(Collections., String> singletonMap( + IllegalStateException.class, "foo")); assertEquals("foo", classifier.classify(new IllegalStateException("Foo"))); } + @Test public void testClassifySubclassMatch() { - classifier.setTypeMap(new LinkedHashMap, String>() {{ - put(RuntimeException.class, "foo"); - }}); + classifier.setTypeMap(Collections., String> singletonMap(RuntimeException.class, + "foo")); assertEquals("foo", classifier.classify(new IllegalStateException("Foo"))); } + @Test public void testClassifySuperclassDoesNotMatch() { - classifier.setTypeMap(new LinkedHashMap, String>() {{ - put(IllegalStateException.class, "foo"); - }}); + classifier.setTypeMap(Collections., String> singletonMap( + IllegalStateException.class, "foo")); assertEquals(classifier.getDefault(), classifier.classify(new RuntimeException("Foo"))); } + @Test public void testClassifyAncestorMatch() { - classifier.setTypeMap(new LinkedHashMap, String>() {{ - put(Exception.class, "bar"); - put(IllegalArgumentException.class, "foo"); - put(RuntimeException.class, "bucket"); - }}); - assertEquals("bucket", classifier.classify(new IllegalStateException("Foo"))); + classifier.setTypeMap(new HashMap, String>() { + { + put(Exception.class, "foo"); + put(IllegalArgumentException.class, "bar"); + put(RuntimeException.class, "spam"); + } + }); + assertEquals("spam", classifier.classify(new IllegalStateException("Foo"))); } } diff --git a/spring-batch-samples/src/main/resources/jobs/skipSampleJob.xml b/spring-batch-samples/src/main/resources/jobs/skipSampleJob.xml index 1e1189d9a..9082d2901 100644 --- a/spring-batch-samples/src/main/resources/jobs/skipSampleJob.xml +++ b/spring-batch-samples/src/main/resources/jobs/skipSampleJob.xml @@ -54,7 +54,7 @@ class="org.springframework.batch.item.database.JdbcCursorItemReader"> + value="SELECT isin, quantity, price, customer from TRADE" /> diff --git a/spring-batch-samples/src/main/resources/log4j.properties b/spring-batch-samples/src/main/resources/log4j.properties index fcc475b7a..33ed59a2b 100644 --- a/spring-batch-samples/src/main/resources/log4j.properties +++ b/spring-batch-samples/src/main/resources/log4j.properties @@ -28,7 +28,7 @@ log4j.logger.org.springframework=info #log4j.logger.org.springframework.orm=debug ### debug your specific package or classes with the following example -log4j.logger.org.springframework.batch=info +log4j.logger.org.springframework.batch=debug log4j.logger.org.springframework.batch.sample=debug log4j.logger.org.springframework.batch.sample.module.OrderDataProvider=debug log4j.logger.org.springframework.batch.container.common.module.process.support.DefaultXmlDataProvider=debug diff --git a/spring-batch-samples/src/test/java/org/springframework/batch/sample/AbstractBatchLauncherTests.java b/spring-batch-samples/src/test/java/org/springframework/batch/sample/AbstractBatchLauncherTests.java index 08fea87d8..fb460df37 100644 --- a/spring-batch-samples/src/test/java/org/springframework/batch/sample/AbstractBatchLauncherTests.java +++ b/spring-batch-samples/src/test/java/org/springframework/batch/sample/AbstractBatchLauncherTests.java @@ -42,7 +42,7 @@ public abstract class AbstractBatchLauncherTests implements ApplicationContextAw protected ApplicationContext applicationContext; - protected JobLauncher launcher; + private JobLauncher launcher; private Job job; @@ -77,6 +77,14 @@ public abstract class AbstractBatchLauncherTests implements ApplicationContextAw @Test public void testLaunchJob() throws Exception { - launcher.run(job, jobParameters); + getLauncher().run(job, jobParameters); + } + + /** + * Public getter for the launcher. + * @return the launcher + */ + protected JobLauncher getLauncher() { + return launcher; } } diff --git a/spring-batch-samples/src/test/java/org/springframework/batch/sample/DatabaseShutdownFunctionalTests.java b/spring-batch-samples/src/test/java/org/springframework/batch/sample/DatabaseShutdownFunctionalTests.java index 11c814409..240185d7f 100644 --- a/spring-batch-samples/src/test/java/org/springframework/batch/sample/DatabaseShutdownFunctionalTests.java +++ b/spring-batch-samples/src/test/java/org/springframework/batch/sample/DatabaseShutdownFunctionalTests.java @@ -46,7 +46,7 @@ public class DatabaseShutdownFunctionalTests extends AbstractBatchLauncherTests final JobParameters jobParameters = new JobParameters(); - JobExecution jobExecution = launcher.run(getJob(), jobParameters); + JobExecution jobExecution = getLauncher().run(getJob(), jobParameters); Thread.sleep(1000); diff --git a/spring-batch-samples/src/test/java/org/springframework/batch/sample/GracefulShutdownFunctionalTests.java b/spring-batch-samples/src/test/java/org/springframework/batch/sample/GracefulShutdownFunctionalTests.java index 06648f0e4..2c17d4625 100644 --- a/spring-batch-samples/src/test/java/org/springframework/batch/sample/GracefulShutdownFunctionalTests.java +++ b/spring-batch-samples/src/test/java/org/springframework/batch/sample/GracefulShutdownFunctionalTests.java @@ -45,7 +45,7 @@ public class GracefulShutdownFunctionalTests extends AbstractBatchLauncherTests final JobParameters jobParameters = new JobParameters(); - JobExecution jobExecution = launcher.run(getJob(), jobParameters); + JobExecution jobExecution = getLauncher().run(getJob(), jobParameters); Thread.sleep(1000); diff --git a/spring-batch-samples/src/test/java/org/springframework/batch/sample/RestartFunctionalTests.java b/spring-batch-samples/src/test/java/org/springframework/batch/sample/RestartFunctionalTests.java index 92f92b6c4..e29505d16 100644 --- a/spring-batch-samples/src/test/java/org/springframework/batch/sample/RestartFunctionalTests.java +++ b/spring-batch-samples/src/test/java/org/springframework/batch/sample/RestartFunctionalTests.java @@ -94,7 +94,7 @@ public class RestartFunctionalTests extends AbstractBatchLauncherTests { private void runJobForRestartTest() throws Exception { // The second time we run the job it needs to be a new instance so we // need to make the parameters unique... - launcher.run(getJob(), new DefaultJobParametersConverter().getJobParameters(PropertiesConverter + getLauncher().run(getJob(), new DefaultJobParametersConverter().getJobParameters(PropertiesConverter .stringToProperties("force.new.job.parameters=true"))); } diff --git a/spring-batch-samples/src/test/java/org/springframework/batch/sample/TaskletJobFunctionalTests.java b/spring-batch-samples/src/test/java/org/springframework/batch/sample/TaskletJobFunctionalTests.java index 174ca7d1a..2ca897d10 100644 --- a/spring-batch-samples/src/test/java/org/springframework/batch/sample/TaskletJobFunctionalTests.java +++ b/spring-batch-samples/src/test/java/org/springframework/batch/sample/TaskletJobFunctionalTests.java @@ -1,14 +1,16 @@ package org.springframework.batch.sample; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + import java.io.File; -import org.junit.Before; +import org.junit.BeforeClass; import org.junit.runner.RunWith; import org.springframework.core.io.FileSystemResource; import org.springframework.core.io.Resource; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; -import org.springframework.util.Assert; /** * Deletes files in the given directory. @@ -19,13 +21,13 @@ import org.springframework.util.Assert; @ContextConfiguration() public class TaskletJobFunctionalTests extends AbstractValidatingBatchLauncherTests { - private Resource directory = new FileSystemResource("target/test-outputs/test-dir"); + private static Resource directory = new FileSystemResource("target/test-outputs/test-dir"); /* * Create the directory and some files in it. */ - @Before - public void onSetUp() throws Exception { + @BeforeClass + public static void onSetUp() throws Exception { File dir = directory.getFile(); dir.mkdirs(); new File(dir, "file1").createNewFile(); @@ -37,8 +39,8 @@ public class TaskletJobFunctionalTests extends AbstractValidatingBatchLauncherTe */ @Override protected void validatePreConditions() throws Exception { - Assert.state(directory.getFile().isDirectory()); - Assert.state(directory.getFile().listFiles().length > 0); + assertTrue(directory.getFile().isDirectory()); + assertTrue(directory.getFile().listFiles().length > 0); } /** @@ -46,8 +48,8 @@ public class TaskletJobFunctionalTests extends AbstractValidatingBatchLauncherTe */ @Override protected void validatePostConditions() throws Exception { - Assert.state(directory.getFile().isDirectory()); - Assert.state(directory.getFile().listFiles().length == 0); + assertTrue(directory.getFile().isDirectory()); + assertEquals(0, directory.getFile().listFiles().length); } } diff --git a/spring-batch-samples/src/test/java/org/springframework/batch/sample/TradeJobFunctionalTests.java b/spring-batch-samples/src/test/java/org/springframework/batch/sample/TradeJobFunctionalTests.java index a125730fd..494feb3cc 100644 --- a/spring-batch-samples/src/test/java/org/springframework/batch/sample/TradeJobFunctionalTests.java +++ b/spring-batch-samples/src/test/java/org/springframework/batch/sample/TradeJobFunctionalTests.java @@ -22,7 +22,7 @@ import static org.junit.Assert.assertTrue; import java.math.BigDecimal; import java.sql.ResultSet; import java.sql.SQLException; -import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -80,16 +80,16 @@ public class TradeJobFunctionalTests extends AbstractValidatingBatchLauncherTest // assertTrue(((Resource)applicationContext.getBean("customerFileLocator")).exists()); - customers = new ArrayList() {{add(new Customer("customer1", (credits.get("customer1") - 98.34))); - add(new Customer("customer2", (credits.get("customer2") - 18.12 - 12.78))); - add(new Customer("customer3", (credits.get("customer3") - 109.25))); - add(new Customer("customer4", credits.get("customer4") - 123.39));}}; + customers = Arrays.asList(new Customer("customer1", (credits.get("customer1") - 98.34)), + new Customer("customer2", (credits.get("customer2") - 18.12 - 12.78)), + new Customer("customer3", (credits.get("customer3") - 109.25)), + new Customer("customer4", credits.get("customer4") - 123.39)); - trades = new ArrayList() {{add(new Trade("UK21341EAH45", 978, new BigDecimal("98.34"), "customer1")); - add(new Trade("UK21341EAH46", 112, new BigDecimal("18.12"), "customer2")); - add(new Trade("UK21341EAH47", 245, new BigDecimal("12.78"), "customer2")); - add(new Trade("UK21341EAH48", 108, new BigDecimal("109.25"), "customer3")); - add(new Trade("UK21341EAH49", 854, new BigDecimal("123.39"), "customer4"));}}; + trades = Arrays.asList(new Trade("UK21341EAH45", 978, new BigDecimal("98.34"), "customer1"), + new Trade("UK21341EAH46", 112, new BigDecimal("18.12"), "customer2"), + new Trade("UK21341EAH47", 245, new BigDecimal("12.78"), "customer2"), + new Trade("UK21341EAH48", 108, new BigDecimal("109.25"), "customer3"), + new Trade("UK21341EAH49", 854, new BigDecimal("123.39"), "customer4")); // check content of the trade table simpleJdbcTemplate.getJdbcOperations().query(GET_TRADES, new RowCallbackHandler() {