RESOLVED - BATCH-1465: Unwrap exceptions in AbstractMethodInvokingDelegator

spring-batch-infrastructure/src/main/java/org/springframework/batch/item/adapter/AbstractMethodInvokingDelegator.java

@@ -31,6 +31,11 @@ import org.springframework.util.MethodInvoker;
  * injected object. Provides convenient API for dynamic method invocation
  * shielding subclasses from low-level details and exception handling.
  * 
+ * {@link Exception}s thrown by a successfully invoked delegate method are
+ * re-thrown without wrapping. In case the delegate method throws a
+ * {@link Throwable} that doesn't subclass {@link Exception} it will be wrapped
+ * by {@link InvocationTargetThrowableWrapper}.
+ * 
  * @author Robert Kasanicky
  */
 public abstract class AbstractMethodInvokingDelegator<T> implements InitializingBean {
@@ -48,7 +53,7 @@ public abstract class AbstractMethodInvokingDelegator<T> implements Initializing
 	 * @throws DynamicMethodInvocationException if the {@link MethodInvoker}
 	 * used throws exception
 	 */
-	protected T invokeDelegateMethod() {
+	protected T invokeDelegateMethod() throws Exception {
 		MethodInvoker invoker = createMethodInvoker(targetObject, targetMethod);
 		invoker.setArguments(arguments);
 		return doInvoke(invoker);
@@ -61,7 +66,7 @@ public abstract class AbstractMethodInvokingDelegator<T> implements Initializing
 	 * @throws DynamicMethodInvocationException if the {@link MethodInvoker}
 	 * used throws exception
 	 */
-	protected T invokeDelegateMethodWithArgument(Object object) {
+	protected T invokeDelegateMethodWithArgument(Object object) throws Exception {
 		MethodInvoker invoker = createMethodInvoker(targetObject, targetMethod);
 		invoker.setArguments(new Object[] { object });
 		return doInvoke(invoker);
@@ -74,7 +79,7 @@ public abstract class AbstractMethodInvokingDelegator<T> implements Initializing
 	 * @throws DynamicMethodInvocationException if the {@link MethodInvoker}
 	 * used throws exception
 	 */
-	protected T invokeDelegateMethodWithArguments(Object[] args) {
+	protected T invokeDelegateMethodWithArguments(Object[] args) throws Exception {
 		MethodInvoker invoker = createMethodInvoker(targetObject, targetMethod);
 		invoker.setArguments(args);
 		return doInvoke(invoker);
@@ -97,7 +102,7 @@ public abstract class AbstractMethodInvokingDelegator<T> implements Initializing
 	 * @return return value of the invoked method
 	 */
 	@SuppressWarnings("unchecked")
-	private T doInvoke(MethodInvoker invoker) {
+	private T doInvoke(MethodInvoker invoker) throws Exception {
 		try {
 			invoker.prepare();
 		}
@@ -112,7 +117,12 @@ public abstract class AbstractMethodInvokingDelegator<T> implements Initializing
 			return (T) invoker.invoke();
 		}
 		catch (InvocationTargetException e) {
-			throw new DynamicMethodInvocationException(e);
+			if (e.getCause() instanceof Exception) {
+				throw (Exception) e.getCause();
+			}
+			else {
+				throw new InvocationTargetThrowableWrapper(e.getCause());
+			}
 		}
 		catch (IllegalAccessException e) {
 			throw new DynamicMethodInvocationException(e);
@@ -132,14 +142,14 @@ public abstract class AbstractMethodInvokingDelegator<T> implements Initializing
 	 */
 	private boolean targetClassDeclaresTargetMethod() {
 		MethodInvoker invoker = createMethodInvoker(targetObject, targetMethod);
-		
+
 		Method[] memberMethods = invoker.getTargetClass().getMethods();
 		Method[] declaredMethods = invoker.getTargetClass().getDeclaredMethods();
-		
-		List<Method> allMethods = new ArrayList<Method>(); 
+
+		List<Method> allMethods = new ArrayList<Method>();
 		allMethods.addAll(Arrays.asList(memberMethods));
 		allMethods.addAll(Arrays.asList(declaredMethods));
-		
+
 		String targetMethodName = invoker.getTargetMethod();
 
 		for (Method method : allMethods) {
@@ -199,4 +209,18 @@ public abstract class AbstractMethodInvokingDelegator<T> implements Initializing
 	public void setArguments(Object[] arguments) {
 		this.arguments = arguments;
 	}
+
+	/**
+	 * Used to wrap a {@link Throwable} (not an {@link Exception}) thrown by a
+	 * reflectively-invoked delegate.
+	 * 
+	 * @author Robert Kasanicky
+	 */
+	public static class InvocationTargetThrowableWrapper extends RuntimeException {
+
+		public InvocationTargetThrowableWrapper(Throwable cause) {
+			super(cause);
+		}
+
+	}
 }

spring-batch-infrastructure/src/main/java/org/springframework/batch/item/adapter/DynamicMethodInvocationException.java

@@ -19,20 +19,23 @@ package org.springframework.batch.item.adapter;
 import org.springframework.util.MethodInvoker;
 
 /**
- * Indicates an error has been encountered
- * while trying to dynamically call a method e.g. using {@link MethodInvoker}.
+ * Indicates an error has been encountered while trying to dynamically invoke a
+ * method e.g. using {@link MethodInvoker}.
+ * 
+ * The exception should be caused by a failed invocation of a method, it
+ * shouldn't be used to wrap an exception thrown by successfully invoked method.
  * 
  * @author Robert Kasanicky
  */
 public class DynamicMethodInvocationException extends RuntimeException {
-	
-	//generated value
+
+	// generated value
 	private static final long serialVersionUID = -6056786139731564040L;
 
-	public DynamicMethodInvocationException(Throwable cause){
+	public DynamicMethodInvocationException(Throwable cause) {
 		super(cause);
 	}
-	
+
 	public DynamicMethodInvocationException(String message, Throwable cause) {
 		super(message, cause);
 	}

spring-batch-infrastructure/src/test/java/org/springframework/batch/item/adapter/AbstractDelegatorTests.java

@@ -4,6 +4,7 @@ import junit.framework.TestCase;
 
 import org.springframework.batch.item.adapter.AbstractMethodInvokingDelegator;
 import org.springframework.batch.item.adapter.DynamicMethodInvocationException;
+import org.springframework.batch.item.adapter.AbstractMethodInvokingDelegator.InvocationTargetThrowableWrapper;
 import org.springframework.batch.item.sample.Foo;
 import org.springframework.batch.item.sample.FooService;
 import org.springframework.util.Assert;
@@ -181,4 +182,37 @@ public class AbstractDelegatorTests extends TestCase {
 		}
 	}
 
+	/**
+	 * Exception scenario - target method is successfully invoked but throws
+	 * exception. Such 'business' exception should be re-thrown as is (without
+	 * wrapping).
+	 */
+	public void testDelegateException() throws Exception {
+		delegator.setTargetMethod("fail");
+		delegator.afterPropertiesSet();
+		try {
+			delegator.invokeDelegateMethod();
+			fail();
+		}
+		catch (Exception expected) {
+			assertEquals(Foo.FAILURE_MESSAGE, expected.getMessage());
+		}
+
+	}
+	
+	/**
+	 * Exception scenario - target method is successfully invoked but throws a {@link Throwable} (not an {@link Exception}).
+	 */
+	public void testDelegateThrowable() throws Exception {
+		delegator.setTargetMethod("failUgly");
+		delegator.afterPropertiesSet();
+		try {
+			delegator.invokeDelegateMethod();
+			fail();
+		}
+		catch (InvocationTargetThrowableWrapper expected) {
+			assertEquals(Foo.UGLY_FAILURE_MESSAGE, expected.getCause().getMessage());
+		}
+	}
+
 }

spring-batch-infrastructure/src/test/java/org/springframework/batch/item/sample/Foo.java

@@ -14,6 +14,10 @@ import javax.persistence.Id;
 @Table(name = "T_FOOS")
 public class Foo {
 	
+	public static final String FAILURE_MESSAGE = "Foo Failure!";
+	
+	public static final String UGLY_FAILURE_MESSAGE = "Ugly Foo Failure!";
+	
 	@Id
 	private int id;
 	private String name;
@@ -57,5 +61,13 @@ public class Foo {
 	public int hashCode() {
 		return HashCodeBuilder.reflectionHashCode(this);
 	}
+	
+	public void fail() throws Exception {
+		throw new Exception(FAILURE_MESSAGE);
+	}
+	
+	public void failUgly() throws Throwable {
+		throw new Throwable(UGLY_FAILURE_MESSAGE);
+	}
 
 }