Skip to content

Commit 8cade4d

Browse files
committed
Align skip policy with retry cause traversal and fix write-path behavior
We added support for traverses exception causes for skip, which was not available with retry, and also fixed some related inaccuracies, such as inversion in ChunkOrientedStep. Closes: gh-5127 Signed-off-by: Andrey Litvitski <andrey1010102008@gmail.com>
1 parent 1d50d82 commit 8cade4d

File tree

3 files changed

+53
-13
lines changed

3 files changed

+53
-13
lines changed

spring-batch-core/src/main/java/org/springframework/batch/core/step/item/ChunkOrientedStep.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.apache.commons.logging.LogFactory;
2525
import org.jspecify.annotations.Nullable;
2626

27+
import org.springframework.batch.core.ExitStatus;
2728
import org.springframework.batch.core.job.JobInterruptedException;
2829
import org.springframework.batch.core.listener.ChunkListener;
2930
import org.springframework.batch.core.listener.CompositeChunkListener;
@@ -87,6 +88,7 @@
8788
* @param <I> type of input items
8889
* @param <O> type of output items
8990
* @author Mahmoud Ben Hassine
91+
* @author Andrey Litvitski
9092
* @since 6.0
9193
*/
9294
public class ChunkOrientedStep<I, O> extends AbstractStep {
@@ -690,7 +692,7 @@ private void writeChunk(Chunk<O> chunk, StepContribution contribution) throws Ex
690692
observation.lowCardinalityKeyValue(fullyQualifiedMetricName + ".status", BatchMetrics.STATUS_FAILURE);
691693
observation.error(exception);
692694
if (this.faultTolerant && exception instanceof RetryException retryException
693-
&& !this.skipPolicy.shouldSkip(retryException.getCause(), -1)) {
695+
&& this.skipPolicy.shouldSkip(retryException.getCause(), -1)) {
694696
logger.info("Retry exhausted while attempting to write items, scanning the chunk", retryException);
695697
ChunkScanEvent chunkScanEvent = new ChunkScanEvent(contribution.getStepExecution().getStepName(),
696698
contribution.getStepExecution().getId());

spring-batch-core/src/main/java/org/springframework/batch/core/step/skip/LimitCheckingExceptionHierarchySkipPolicy.java

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
* limit.
2727
*
2828
* @author Mahmoud Ben Hassine
29+
* @author Andrey Litvitski
2930
* @since 6.0
3031
*/
3132
public class LimitCheckingExceptionHierarchySkipPolicy implements SkipPolicy {
@@ -49,29 +50,30 @@ public LimitCheckingExceptionHierarchySkipPolicy(Set<Class<? extends Throwable>>
4950

5051
@Override
5152
public boolean shouldSkip(Throwable t, long skipCount) throws SkipLimitExceededException {
53+
boolean skippable = isSkippable(t);
5254
if (skipCount < 0) {
53-
return !isSkippable(t);
55+
return skippable;
5456
}
55-
if (!isSkippable(t)) {
57+
if (!skippable) {
5658
return false;
5759
}
58-
if (skipCount < this.skipLimit) {
59-
return true;
60-
}
61-
else {
60+
if (skipCount >= this.skipLimit) {
6261
throw new SkipLimitExceededException(this.skipLimit, t);
6362
}
63+
return true;
6464
}
6565

6666
private boolean isSkippable(Throwable t) {
67-
boolean isSkippable = false;
68-
for (Class<? extends Throwable> skippableException : this.skippableExceptions) {
69-
if (skippableException.isAssignableFrom(t.getClass())) {
70-
isSkippable = true;
71-
break;
67+
Throwable current = t;
68+
while (current != null) {
69+
for (Class<? extends Throwable> skippableException : this.skippableExceptions) {
70+
if (skippableException.isInstance(current)) {
71+
return true;
72+
}
7273
}
74+
current = current.getCause();
7375
}
74-
return isSkippable;
76+
return false;
7577
}
7678

7779
}

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151

5252
/**
5353
* @author Mahmoud Ben Hassine
54+
* @author Andrey Litvitski
5455
*/
5556
public class ChunkOrientedStepTests {
5657

@@ -283,4 +284,39 @@ void testDoSkipInProcessShouldThrowNonSkippableProcessExceptionWhenSkipPolicyRet
283284
assertEquals(0, stepExecution.getProcessSkipCount(), "Process skip count should be 0");
284285
}
285286

287+
@Test
288+
void testSkippableExceptionsTraversal() throws Exception {
289+
// given
290+
class SkippableException extends RuntimeException {
291+
292+
}
293+
ItemReader<String> reader = new ListItemReader<>(List.of("item1"));
294+
ItemWriter<String> writer = chunk -> {
295+
throw new RuntimeException(new SkippableException());
296+
};
297+
298+
JobRepository jobRepository = new ResourcelessJobRepository();
299+
ChunkOrientedStep<String, String> step = new StepBuilder("step", jobRepository).<String, String>chunk(1)
300+
.reader(reader)
301+
.writer(writer)
302+
.faultTolerant()
303+
.retry(SkippableException.class)
304+
.retryLimit(1)
305+
.skip(SkippableException.class)
306+
.skipLimit(1)
307+
.build();
308+
309+
JobInstance jobInstance = new JobInstance(1L, "job");
310+
JobExecution jobExecution = new JobExecution(1L, jobInstance, new JobParameters());
311+
StepExecution stepExecution = new StepExecution(1L, "step", jobExecution);
312+
313+
// when - execute step
314+
step.execute(stepExecution);
315+
316+
// then - should skip the exception thrown by the writer
317+
ExitStatus stepExecutionExitStatus = stepExecution.getExitStatus();
318+
assertEquals(ExitStatus.COMPLETED.getExitCode(), stepExecutionExitStatus.getExitCode());
319+
assertEquals(1, stepExecution.getSkipCount());
320+
}
321+
286322
}

0 commit comments

Comments
 (0)