From 61bec2805dda06b25f1dd5c3030066754cdf7b94 Mon Sep 17 00:00:00 2001 From: Mahmoud Ben Hassine Date: Fri, 23 Feb 2018 15:37:50 +0100 Subject: [PATCH] BATCH-2213: unwrap exceptions thrown from annotated step listeners Currently, when a method of an annotated listener throws an exception, the exception is wrapped in a InvocationTargetException (by the reflection API) which in turn is wrapped in a IllegalArgumentException (by Spring Batch). This requires the user to unwrap the original exception from the StepListenerFailedException. This behavior is not consistent with interface based listeners where the original exception is the root cause of StepListenerFailedException. This commit unwraps the original exception and make it the root cause of StepListenerFailedException. Resolves BATCH-2213 --- .../listener/MulticasterBatchListener.java | 33 ++- .../MulticasterBatchListenerTests.java | 195 +++++++++++++++++- 2 files changed, 218 insertions(+), 10 deletions(-) diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/listener/MulticasterBatchListener.java b/spring-batch-core/src/main/java/org/springframework/batch/core/listener/MulticasterBatchListener.java index 102eae5f7..a0761ac8d 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/listener/MulticasterBatchListener.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/listener/MulticasterBatchListener.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2013 the original author or authors. + * Copyright 2006-2018 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. @@ -15,6 +15,7 @@ */ package org.springframework.batch.core.listener; +import java.lang.reflect.InvocationTargetException; import java.util.List; import javax.batch.api.chunk.listener.RetryProcessListener; @@ -37,6 +38,7 @@ import org.springframework.batch.item.ItemStream; * @author Dave Syer * @author Michael Minella * @author Chris Schaefer + * @author Mahmoud Ben Hassine */ public class MulticasterBatchListener implements StepExecutionListener, ChunkListener, ItemReadListener, ItemProcessListener, ItemWriteListener, SkipListener, RetryReadListener, RetryProcessListener, RetryWriteListener { @@ -133,7 +135,7 @@ ItemProcessListener, ItemWriteListener, SkipListener, RetryReadLi itemProcessListener.afterProcess(item, result); } catch (RuntimeException e) { - throw new StepListenerFailedException("Error in afterProcess.", e); + throw new StepListenerFailedException("Error in afterProcess.", getTargetException(e)); } } @@ -146,7 +148,7 @@ ItemProcessListener, ItemWriteListener, SkipListener, RetryReadLi itemProcessListener.beforeProcess(item); } catch (RuntimeException e) { - throw new StepListenerFailedException("Error in beforeProcess.", e); + throw new StepListenerFailedException("Error in beforeProcess.", getTargetException(e)); } } @@ -199,7 +201,7 @@ ItemProcessListener, ItemWriteListener, SkipListener, RetryReadLi chunkListener.afterChunk(context); } catch (RuntimeException e) { - throw new StepListenerFailedException("Error in afterChunk.", e); + throw new StepListenerFailedException("Error in afterChunk.", getTargetException(e)); } } @@ -212,7 +214,7 @@ ItemProcessListener, ItemWriteListener, SkipListener, RetryReadLi chunkListener.beforeChunk(context); } catch (RuntimeException e) { - throw new StepListenerFailedException("Error in beforeChunk.", e); + throw new StepListenerFailedException("Error in beforeChunk.", getTargetException(e)); } } @@ -225,7 +227,7 @@ ItemProcessListener, ItemWriteListener, SkipListener, RetryReadLi itemReadListener.afterRead(item); } catch (RuntimeException e) { - throw new StepListenerFailedException("Error in afterRead.", e); + throw new StepListenerFailedException("Error in afterRead.", getTargetException(e)); } } @@ -238,7 +240,7 @@ ItemProcessListener, ItemWriteListener, SkipListener, RetryReadLi itemReadListener.beforeRead(); } catch (RuntimeException e) { - throw new StepListenerFailedException("Error in beforeRead.", e); + throw new StepListenerFailedException("Error in beforeRead.", getTargetException(e)); } } @@ -264,7 +266,7 @@ ItemProcessListener, ItemWriteListener, SkipListener, RetryReadLi itemWriteListener.afterWrite(items); } catch (RuntimeException e) { - throw new StepListenerFailedException("Error in afterWrite.", e); + throw new StepListenerFailedException("Error in afterWrite.", getTargetException(e)); } } @@ -277,7 +279,7 @@ ItemProcessListener, ItemWriteListener, SkipListener, RetryReadLi itemWriteListener.beforeWrite(items); } catch (RuntimeException e) { - throw new StepListenerFailedException("Error in beforeWrite.", e); + throw new StepListenerFailedException("Error in beforeWrite.", getTargetException(e)); } } @@ -356,4 +358,17 @@ ItemProcessListener, ItemWriteListener, SkipListener, RetryReadLi throw new BatchRuntimeException(e); } } + + /** + * Unwrap the target exception from a wrapped {@link InvocationTargetException}. + * @param e the exception to introspect + * @return the target exception if any + */ + private Throwable getTargetException(RuntimeException e) { + Throwable cause = e.getCause(); + if (cause != null && cause instanceof InvocationTargetException) { + return ((InvocationTargetException) cause).getTargetException(); + } + return e; + } } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/listener/MulticasterBatchListenerTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/listener/MulticasterBatchListenerTests.java index 69629b58b..7323b0e30 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/listener/MulticasterBatchListenerTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/listener/MulticasterBatchListenerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2007 the original author or authors. + * Copyright 2006-2018 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. @@ -16,6 +16,7 @@ package org.springframework.batch.core.listener; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.util.Arrays; @@ -26,10 +27,20 @@ import org.junit.Test; import org.springframework.batch.core.ExitStatus; import org.springframework.batch.core.JobExecution; import org.springframework.batch.core.StepExecution; +import org.springframework.batch.core.StepListener; +import org.springframework.batch.core.annotation.AfterChunk; +import org.springframework.batch.core.annotation.AfterProcess; +import org.springframework.batch.core.annotation.AfterRead; +import org.springframework.batch.core.annotation.AfterWrite; +import org.springframework.batch.core.annotation.BeforeChunk; +import org.springframework.batch.core.annotation.BeforeProcess; +import org.springframework.batch.core.annotation.BeforeRead; +import org.springframework.batch.core.annotation.BeforeWrite; import org.springframework.batch.core.scope.context.ChunkContext; /** * @author Dave Syer + * @author Mahmoud Ben Hassine * */ public class MulticasterBatchListenerTests { @@ -512,6 +523,188 @@ public class MulticasterBatchListenerTests { assertEquals(1, count); } + @Test + public void testBeforeReadFails_withAnnotatedListener() { + StepListener listener = StepListenerFactoryBean.getListener(new AnnotationBasedStepListener()); + multicast.register(listener); + + try { + multicast.beforeRead(); + fail("Expected StepListenerFailedException"); + } catch (StepListenerFailedException e) { + // expected + Throwable cause = e.getCause(); + String message = cause.getMessage(); + assertTrue(cause instanceof IllegalStateException); + assertEquals("Wrong message: " + message, "listener error", message); + } + } + + @Test + public void testAfterReadFails_withAnnotatedListener() { + StepListener listener = StepListenerFactoryBean.getListener(new AnnotationBasedStepListener()); + multicast.register(listener); + + try { + multicast.afterRead(null); + fail("Expected StepListenerFailedException"); + } catch (StepListenerFailedException e) { + // expected + Throwable cause = e.getCause(); + String message = cause.getMessage(); + assertTrue(cause instanceof IllegalStateException); + assertEquals("Wrong message: " + message, "listener error", message); + } + } + + @Test + public void testBeforeProcessFails_withAnnotatedListener() { + StepListener listener = StepListenerFactoryBean.getListener(new AnnotationBasedStepListener()); + multicast.register(listener); + + try { + multicast.beforeProcess(null); + fail("Expected StepListenerFailedException"); + } catch (StepListenerFailedException e) { + // expected + Throwable cause = e.getCause(); + String message = cause.getMessage(); + assertTrue(cause instanceof IllegalStateException); + assertEquals("Wrong message: " + message, "listener error", message); + } + } + + @Test + public void testAfterProcessFails_withAnnotatedListener() { + StepListener listener = StepListenerFactoryBean.getListener(new AnnotationBasedStepListener()); + multicast.register(listener); + + try { + multicast.afterProcess(null, null); + fail("Expected StepListenerFailedException"); + } catch (StepListenerFailedException e) { + // expected + Throwable cause = e.getCause(); + String message = cause.getMessage(); + assertTrue(cause instanceof IllegalStateException); + assertEquals("Wrong message: " + message, "listener error", message); + } + } + + @Test + public void testBeforeWriteFails_withAnnotatedListener() { + StepListener listener = StepListenerFactoryBean.getListener(new AnnotationBasedStepListener()); + multicast.register(listener); + + try { + multicast.beforeWrite(null); + fail("Expected StepListenerFailedException"); + } catch (StepListenerFailedException e) { + // expected + Throwable cause = e.getCause(); + String message = cause.getMessage(); + assertTrue(cause instanceof IllegalStateException); + assertEquals("Wrong message: " + message, "listener error", message); + } + } + + @Test + public void testAfterWriteFails_withAnnotatedListener() { + StepListener listener = StepListenerFactoryBean.getListener(new AnnotationBasedStepListener()); + multicast.register(listener); + + try { + multicast.afterWrite(null); + fail("Expected StepListenerFailedException"); + } catch (StepListenerFailedException e) { + // expected + Throwable cause = e.getCause(); + String message = cause.getMessage(); + assertTrue(cause instanceof IllegalStateException); + assertEquals("Wrong message: " + message, "listener error", message); + } + } + + @Test + public void testBeforeChunkFails_withAnnotatedListener() { + StepListener listener = StepListenerFactoryBean.getListener(new AnnotationBasedStepListener()); + multicast.register(listener); + + try { + multicast.beforeChunk(null); + fail("Expected StepListenerFailedException"); + } catch (StepListenerFailedException e) { + // expected + Throwable cause = e.getCause(); + String message = cause.getMessage(); + assertTrue(cause instanceof IllegalStateException); + assertEquals("Wrong message: " + message, "listener error", message); + } + } + + @Test + public void testAfterChunkFails_withAnnotatedListener() { + StepListener listener = StepListenerFactoryBean.getListener(new AnnotationBasedStepListener()); + multicast.register(listener); + + try { + multicast.afterChunk(null); + fail("Expected StepListenerFailedException"); + } catch (StepListenerFailedException e) { + // expected + Throwable cause = e.getCause(); + String message = cause.getMessage(); + assertTrue(cause instanceof IllegalStateException); + assertEquals("Wrong message: " + message, "listener error", message); + } + } + + private final class AnnotationBasedStepListener { + + private IllegalStateException exception = new IllegalStateException("listener error"); + + @BeforeRead + public void beforeRead() { + throw exception; + } + + @AfterRead + public void afterRead() { + throw exception; + } + + @BeforeProcess + public void beforeProcess() { + throw exception; + } + + @AfterProcess + public void afterProcess() { + throw exception; + } + + @BeforeWrite + public void beforeWrite() { + throw exception; + } + + @AfterWrite + public void afterWrite() { + throw exception; + } + + @BeforeChunk + public void beforeChunk() { + throw exception; + } + + @AfterChunk + public void afterChunk() { + throw exception; + } + + } + /** * @author Dave Syer *