RESOLVED - issue BATCH-1334: No-rollback for non-skippable exceptions.

spring-batch-core/src/test/java/org/springframework/batch/core/AbstractExceptionTests.java

@@ -20,7 +20,7 @@ import static org.junit.Assert.assertEquals;
 
 import org.junit.Test;
 
-public abstract class AbstractExceptionTests {
+public abstract class AbstractExceptionTests extends AbstractExceptionWithCauseTests {
 
 	@Test
 	public void testExceptionString() throws Exception {
@@ -28,14 +28,6 @@ public abstract class AbstractExceptionTests {
 		assertEquals("foo", exception.getMessage());
 	}
 
-	@Test
-	public void testExceptionStringThrowable() throws Exception {
-		Exception exception = getException("foo", new IllegalStateException());
-		assertEquals("foo", exception.getMessage().substring(0, 3));
-	}
-
 	public abstract Exception getException(String msg) throws Exception;
 
-	public abstract Exception getException(String msg, Throwable t) throws Exception;
-
 }

spring-batch-core/src/test/java/org/springframework/batch/core/AbstractExceptionWithCauseTests.java

@@ -0,0 +1,33 @@
+/*
+ * 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.core;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+
+public abstract class AbstractExceptionWithCauseTests {
+
+	@Test
+	public void testExceptionStringThrowable() throws Exception {
+		Exception exception = getException("foo", new IllegalStateException());
+		assertEquals("foo", exception.getMessage().substring(0, 3));
+	}
+
+	public abstract Exception getException(String msg, Throwable t) throws Exception;
+
+}

spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantExceptionClassesTests.java

@@ -118,9 +118,9 @@ public class FaultTolerantExceptionClassesTests implements ApplicationContextAwa
 		writer.setExceptionType(Exception.class);
 		StepExecution stepExecution = launchStep("skippableFatalStep");
 		assertEquals(BatchStatus.FAILED, stepExecution.getStatus());
-		// BATCH-1327: 
+		// BATCH-1327:
 		assertEquals("[1, 2, 3]", writer.getWritten().toString());
-		// BATCH-1327: 
+		// BATCH-1327:
 		assertEquals("[]", writer.getCommitted().toString());
 	}
 
@@ -163,10 +163,13 @@ public class FaultTolerantExceptionClassesTests implements ApplicationContextAwa
 
 	@Test
 	public void testRetryableFatal() throws Exception {
+		// User wants all exceptions to be retried, but only some are skippable
+		// FatalRuntimeException is not skippable, but is a subclass of another
+		// skippable
 		writer.setExceptionType(FatalRuntimeException.class);
 		StepExecution stepExecution = launchStep("retryable");
 		assertEquals(BatchStatus.FAILED, stepExecution.getStatus());
-		// TODO BATCH-1318: assertEquals("[1, 2, 3, 1, 2, 3, 1, 2, 3]",
+		// TODO BATCH-1333: assertEquals("[1, 2, 3, 1, 2, 3, 1, 2, 3]",
 		// writer.getWritten().toString());
 		assertEquals("[]", writer.getCommitted().toString());
 	}
@@ -195,32 +198,51 @@ public class FaultTolerantExceptionClassesTests implements ApplicationContextAwa
 		writer.setExceptionType(FatalException.class);
 		StepExecution stepExecution = launchStep("retryable");
 		assertEquals(BatchStatus.FAILED, stepExecution.getStatus());
-		// TODO BATCH-1318: assertEquals("[1, 2, 3, 1, 2, 3, 1, 2, 3]",
+		// TODO BATCH-1333: assertEquals("[1, 2, 3, 1, 2, 3, 1, 2, 3]",
 		// writer.getWritten().toString());
 		assertEquals("[]", writer.getCommitted().toString());
 	}
 
 	@Test
 	public void testNoRollbackDefaultRollbackException() throws Exception {
-		writer.setExceptionType(RuntimeException.class);
+		// Exception is neither no-rollback nor skippable
+		writer.setExceptionType(Exception.class);
 		StepExecution stepExecution = launchStep("noRollbackDefault");
 		assertEquals(BatchStatus.FAILED, stepExecution.getStatus());
 		// BATCH-1318:
 		assertEquals("[1, 2, 3]", writer.getWritten().toString());
 		// BATCH-1318:
 		assertEquals("[]", writer.getCommitted().toString());
+		assertEquals(0, stepExecution.getWriteSkipCount());
 	}
 
 	@Test
 	public void testNoRollbackDefaultNoRollbackException() throws Exception {
-		writer.setExceptionType(SkippableRuntimeException.class);
+		// Exception is no-rollback and not skippable
+		writer.setExceptionType(IllegalStateException.class);
 		StepExecution stepExecution = launchStep("noRollbackDefault");
 		assertNotNull(stepExecution);
-		// TODO BATCH-1318: assertEquals(BatchStatus.FAILED, stepExecution.getStatus());
-		// TODO BATCH-1318: assertEquals("[1, 2, 3]",
-		// writer.getWritten().toString());
-		// TODO BATCH-1318: assertEquals("[1, 2, 3]",
-		// writer.getCommitted().toString());
+		assertEquals(BatchStatus.COMPLETED, stepExecution.getStatus());
+		// BATCH-1334:
+		assertEquals("[1, 2, 3, 1, 2, 3, 4]", writer.getWritten().toString());
+		// BATCH-1334:
+		assertEquals("[1, 2, 3, 4]", writer.getCommitted().toString());
+		// BATCH-1334:
+		assertEquals(0, stepExecution.getWriteSkipCount());
+	}
+
+	@Test
+	public void testNoRollbackPathology() throws Exception {
+		// Exception is neither no-rollback nor skippable and no-rollback is
+		// RuntimeException (potentially pathological because other obviously
+		// rollback signalling Exceptions also extend RuntimeException)
+		writer.setExceptionType(Exception.class);
+		StepExecution stepExecution = launchStep("noRollbackPathology");
+		assertEquals(BatchStatus.FAILED, stepExecution.getStatus());
+		// BATCH-1335:
+		assertEquals("[1, 2, 3]", writer.getWritten().toString());
+		// BATCH-1335:
+		assertEquals("[]", writer.getCommitted().toString());
 	}
 
 	@Test
@@ -238,8 +260,10 @@ public class FaultTolerantExceptionClassesTests implements ApplicationContextAwa
 		StepExecution stepExecution = launchStep("noRollbackSkippable");
 		assertEquals(BatchStatus.COMPLETED, stepExecution.getStatus());
 		assertEquals("[1, 2, 3, 1, 2, 3, 4]", writer.getWritten().toString());
-		// TODO BATCH-1332: assertEquals("[1, 2, 4]", writer.getCommitted().toString());
-		// Skipped but also committed!
+		// BATCH-1332: 
+		assertEquals("[1, 2, 3, 4]", writer.getCommitted().toString());
+		// TODO BATCH-1334: 
+		// Not skipped but also committed (because it was marked as no-rollback)
 		assertEquals(1, stepExecution.getWriteSkipCount());
 	}
 

spring-batch-core/src/test/java/org/springframework/batch/core/step/item/FaultTolerantStepFactoryBeanRollbackTests.java

@@ -152,7 +152,7 @@ public class FaultTolerantStepFactoryBeanRollbackTests {
 
 		step.execute(stepExecution);
 		assertEquals(BatchStatus.COMPLETED, stepExecution.getStatus());
-		assertEquals(2, stepExecution.getSkipCount());
+		assertEquals(0, stepExecution.getSkipCount());
 		assertEquals(0, stepExecution.getRollbackCount());
 	}
 

spring-batch-core/src/test/java/org/springframework/batch/core/step/item/ForceRollbackForWriteSkipExceptionTests.java

@@ -0,0 +1,35 @@
+/*
+ * 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.core.step.item;
+
+import org.springframework.batch.core.AbstractExceptionWithCauseTests;
+
+
+/**
+ * @author Dave Syer
+ *
+ */
+public class ForceRollbackForWriteSkipExceptionTests extends AbstractExceptionWithCauseTests {
+
+	/* (non-Javadoc)
+	 * @see org.springframework.batch.core.listener.AbstractDoubleExceptionTests#getException(java.lang.String, java.lang.RuntimeException, java.lang.Throwable)
+	 */
+	@Override
+	public Exception getException(String msg, Throwable e) throws Exception {
+		return new ForceRollbackForWriteSkipException(msg, e);
+	}
+
+}

spring-batch-core/src/test/java/org/springframework/batch/core/step/skip/NonSkippableReadExceptionTests.java

@@ -0,0 +1,35 @@
+/*
+ * 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.core.step.skip;
+
+import org.springframework.batch.core.AbstractExceptionWithCauseTests;
+
+
+/**
+ * @author Dave Syer
+ *
+ */
+public class NonSkippableReadExceptionTests extends AbstractExceptionWithCauseTests {
+
+	/* (non-Javadoc)
+	 * @see org.springframework.batch.core.listener.AbstractDoubleExceptionTests#getException(java.lang.String, java.lang.RuntimeException, java.lang.Throwable)
+	 */
+	@Override
+	public Exception getException(String msg, Throwable e) throws Exception {
+		return new NonSkippableReadException(msg, e);
+	}
+
+}

spring-batch-core/src/test/java/org/springframework/batch/core/step/skip/NonSkippableWriteExceptionTests.java

@@ -0,0 +1,35 @@
+/*
+ * 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.core.step.skip;
+
+import org.springframework.batch.core.AbstractExceptionWithCauseTests;
+
+
+/**
+ * @author Dave Syer
+ *
+ */
+public class NonSkippableWriteExceptionTests extends AbstractExceptionWithCauseTests {
+
+	/* (non-Javadoc)
+	 * @see org.springframework.batch.core.listener.AbstractDoubleExceptionTests#getException(java.lang.String, java.lang.RuntimeException, java.lang.Throwable)
+	 */
+	@Override
+	public Exception getException(String msg, Throwable e) throws Exception {
+		return new NonSkippableWriteException(msg, e);
+	}
+
+}