BATCH-1118: Modified StepListenerFactoryBean so that listener methods specified via annotations or the metaDataMap are checked for correct signatures. This allows incorrect methods to fail eagerly. For added flexibility, methods are also allowed if they do not specify any parameters.

spring-batch-core/src/main/java/org/springframework/batch/core/listener/StepListenerFactoryBean.java

@@ -15,9 +15,11 @@
  */
 package org.springframework.batch.core.listener;
 
-import static org.springframework.batch.support.MethodInvokerUtils.getMethodInvokerByAnnotation;
 import static org.springframework.batch.support.MethodInvokerUtils.getMethodInvokerForInterface;
+import static org.springframework.batch.support.MethodInvokerUtils.getParamTypesString;
 
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Method;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
@@ -31,13 +33,15 @@ import org.springframework.batch.support.MethodInvoker;
 import org.springframework.batch.support.MethodInvokerUtils;
 import org.springframework.beans.factory.FactoryBean;
 import org.springframework.beans.factory.InitializingBean;
+import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.util.Assert;
+import org.springframework.util.ReflectionUtils;
 
 /**
  * {@link FactoryBean} implementation that builds a {@link StepListener} based
  * on the various lifecycle methods or annotations that are provided. There are
  * three possible ways of having a method called as part of a
- * {@link StepListener} lifecyle:
+ * {@link StepListener} lifecycle:
  * 
  * <ul>
  * <li>Interface implementation: By implementing any of the subclasses of
@@ -86,12 +90,12 @@ public class StepListenerFactoryBean implements FactoryBean, InitializingBean {
 		// For every entry in the map, try and find a method by interface, name,
 		// or annotation. If the same
 		for (Entry<String, String> entry : metaDataMap.entrySet()) {
-			StepListenerMetaData metaData = StepListenerMetaData.fromPropertyName(entry.getKey());
+			final StepListenerMetaData metaData = StepListenerMetaData.fromPropertyName(entry.getKey());
 			Set<MethodInvoker> invokers = new NullIgnoringSet<MethodInvoker>();
 			invokers.add(getMethodInvokerByName(entry.getValue(), delegate, metaData.getParamTypes()));
 			invokers.add(getMethodInvokerForInterface(metaData.getListenerInterface(), metaData.getMethodName(),
 					delegate, metaData.getParamTypes()));
-			invokers.add(getMethodInvokerByAnnotation(metaData.getAnnotation(), delegate));
+			invokers.add(getMethodInvokerByAnnotation(metaData, metaData.getAnnotation()));
 			if (!invokers.isEmpty()) {
 				invokerMap.put(metaData.getMethodName(), invokers);
 				listenerInterfaces.add(metaData.getListenerInterface());
@@ -110,6 +114,43 @@ public class StepListenerFactoryBean implements FactoryBean, InitializingBean {
 		return proxyFactory.getProxy();
 	}
 
+	/**
+	 * Create a MethodInvoker from the delegate based on the annotationType.
+	 * Ensure that the annotated method has a valid set of parameters.
+	 * 
+	 * @param metaData
+	 * @param annotationType
+	 * @return
+	 */
+	private MethodInvoker getMethodInvokerByAnnotation(final StepListenerMetaData metaData,
+			final Class<? extends Annotation> annotationType) {
+		MethodInvoker mi = MethodInvokerUtils.getMethodInvokerByAnnotation(annotationType, delegate);
+		if (mi != null) {
+			ReflectionUtils.doWithMethods(delegate.getClass(), new ReflectionUtils.MethodCallback() {
+				public void doWith(Method method) throws IllegalArgumentException, IllegalAccessException {
+					Annotation annotation = AnnotationUtils.findAnnotation(method, annotationType);
+					if (annotation != null) {
+						Class<?>[] paramTypes = method.getParameterTypes();
+						if (paramTypes.length > 0) {
+							Class<?>[] expectedParamTypes = metaData.getParamTypes();
+
+							String errorMsg = "The method [" + method.getName() + "] on target class ["
+									+ delegate.getClass().getSimpleName() + "] is incompatable with the signature ["
+									+ getParamTypesString(expectedParamTypes) + "] expected for the annotation ["
+									+ metaData.getAnnotation().getSimpleName() + "].";
+
+							Assert.isTrue(paramTypes.length == expectedParamTypes.length, errorMsg);
+							for (int i = 0; i < paramTypes.length; i++) {
+								Assert.isTrue(paramTypes[i].equals(expectedParamTypes[i]), errorMsg);
+							}
+						}
+					}
+				}
+			});
+		}
+		return mi;
+	}
+
 	private MethodInvoker getMethodInvokerByName(String methodName, Object candidate, Class<?>... params) {
 		if (methodName != null) {
 			return MethodInvokerUtils.createMethodInvokerByName(candidate, methodName, false, params);
@@ -139,6 +180,7 @@ public class StepListenerFactoryBean implements FactoryBean, InitializingBean {
 	/**
 	 * Convenience method to wrap any object and expose the appropriate
 	 * {@link StepListener} interfaces.
+	 * 
 	 * @param delegate a delegate object
 	 * @return a StepListener instance constructed from the delegate
 	 */
@@ -151,16 +193,18 @@ public class StepListenerFactoryBean implements FactoryBean, InitializingBean {
 	/**
 	 * Convenience method to check whether the given object is or can be made
 	 * into a {@link StepListener}.
+	 * 
 	 * @param delegate the object to check
 	 * @return true if the delegate is an instance of any of the
-	 * {@link StepListener} interfaces, or contains the marker annotations
+	 *         {@link StepListener} interfaces, or contains the marker
+	 *         annotations
 	 */
 	public static boolean isListener(Object delegate) {
 		if (delegate instanceof StepListener) {
 			return true;
 		}
 		for (StepListenerMetaData metaData : StepListenerMetaData.values()) {
-			if (getMethodInvokerByAnnotation(metaData.getAnnotation(), delegate) != null) {
+			if (MethodInvokerUtils.getMethodInvokerByAnnotation(metaData.getAnnotation(), delegate) != null) {
 				return true;
 			}
 		}

spring-batch-core/src/test/java/org/springframework/batch/core/listener/StepListenerFactoryBeanTests.java

@@ -20,8 +20,10 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.springframework.batch.core.listener.StepListenerMetaData.AFTER_CHUNK;
 import static org.springframework.batch.core.listener.StepListenerMetaData.AFTER_STEP;
+import static org.springframework.batch.core.listener.StepListenerMetaData.AFTER_WRITE;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -50,6 +52,7 @@ import org.springframework.batch.core.annotation.BeforeWrite;
 import org.springframework.batch.core.annotation.OnProcessError;
 import org.springframework.batch.core.annotation.OnReadError;
 import org.springframework.batch.core.annotation.OnWriteError;
+import org.springframework.batch.core.configuration.xml.AbstractTestComponent;
 import org.springframework.util.Assert;
 
 /**
@@ -189,6 +192,109 @@ public class StepListenerFactoryBeanTests {
 		assertTrue(listener instanceof StepListener);
 	}
 	
+	@Test
+	public void testEmptySignatureAnnotation() {
+		AbstractTestComponent delegate = new AbstractTestComponent() {
+			@SuppressWarnings("unused")
+			@AfterWrite
+			public void aMethod() {
+				executed = true;
+			}
+		};
+		factoryBean.setDelegate(delegate);
+		@SuppressWarnings("unchecked")
+		ItemWriteListener<String> listener = (ItemWriteListener<String>) factoryBean.getObject();
+		listener.afterWrite(Arrays.asList("foo", "bar"));
+		assertTrue(delegate.isExecuted());
+	}
+
+	@Test
+	public void testRightSignatureAnnotation() {
+		AbstractTestComponent delegate = new AbstractTestComponent() {
+			@SuppressWarnings("unused")
+			@AfterWrite
+			public void aMethod(List<String> items) {
+				executed = true;
+				assertEquals("foo", items.get(0));
+				assertEquals("bar", items.get(1));
+			}
+		};
+		factoryBean.setDelegate(delegate);
+		@SuppressWarnings("unchecked")
+		ItemWriteListener<String> listener = (ItemWriteListener<String>) factoryBean.getObject();
+		listener.afterWrite(Arrays.asList("foo", "bar"));
+		assertTrue(delegate.isExecuted());
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void testWrongSignatureAnnotation() {
+		AbstractTestComponent delegate = new AbstractTestComponent() {
+			@SuppressWarnings("unused")
+			@AfterWrite
+			public void aMethod(Integer item) {
+				executed = true;
+			}
+		};
+		factoryBean.setDelegate(delegate);
+		factoryBean.getObject();
+	}
+
+	@Test
+	public void testEmptySignatureNamedMethod() {
+		AbstractTestComponent delegate = new AbstractTestComponent() {
+			@SuppressWarnings("unused")
+			@AfterWrite
+			public void aMethod() {
+				executed = true;
+			}
+		};
+		factoryBean.setDelegate(delegate);
+		Map<String, String> metaDataMap = new HashMap<String, String>();
+		metaDataMap.put(AFTER_WRITE.getPropertyName(), "aMethod");
+		factoryBean.setMetaDataMap(metaDataMap);
+		@SuppressWarnings("unchecked")
+		ItemWriteListener<String> listener = (ItemWriteListener<String>) factoryBean.getObject();
+		listener.afterWrite(Arrays.asList("foo", "bar"));
+		assertTrue(delegate.isExecuted());
+	}
+
+	@Test
+	public void testRightSignatureNamedMethod() {
+		AbstractTestComponent delegate = new AbstractTestComponent() {
+			@SuppressWarnings("unused")
+			@AfterWrite
+			public void aMethod(List<String> items) {
+				executed = true;
+				assertEquals("foo", items.get(0));
+				assertEquals("bar", items.get(1));
+			}
+		};
+		factoryBean.setDelegate(delegate);
+		Map<String, String> metaDataMap = new HashMap<String, String>();
+		metaDataMap.put(AFTER_WRITE.getPropertyName(), "aMethod");
+		factoryBean.setMetaDataMap(metaDataMap);
+		@SuppressWarnings("unchecked")
+		ItemWriteListener<String> listener = (ItemWriteListener<String>) factoryBean.getObject();
+		listener.afterWrite(Arrays.asList("foo", "bar"));
+		assertTrue(delegate.isExecuted());
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void testWrongSignatureNamedMethod() {
+		AbstractTestComponent delegate = new AbstractTestComponent() {
+			@SuppressWarnings("unused")
+			@AfterWrite
+			public void aMethod(Integer item) {
+				executed = true;
+			}
+		};
+		factoryBean.setDelegate(delegate);
+		Map<String, String> metaDataMap = new HashMap<String, String>();
+		metaDataMap.put(AFTER_WRITE.getPropertyName(), "aMethod");
+		factoryBean.setMetaDataMap(metaDataMap);
+		factoryBean.getObject();
+	}
+
 	private class MultipleAfterStep implements StepExecutionListener {
 
 		int callcount = 0;

spring-batch-infrastructure/src/main/java/org/springframework/batch/support/MethodInvokerUtils.java

@@ -50,18 +50,34 @@ public class MethodInvokerUtils {
 			Class<?>... paramTypes) {
 		Assert.notNull(object, "Object to invoke must not be null");
 		Method method = ClassUtils.getMethodIfAvailable(object.getClass(), methodName, paramTypes);
-		// if no method was found for the given parameters, and the parameters
-		// aren't required
-		if (method == null && !paramsRequired) {
-			// try with no params
-			method = ClassUtils.getMethodIfAvailable(object.getClass(), methodName, new Class[] {});
-		}
 		if (method == null) {
-			return null;
+			String errorMsg = "no method found with name [" + methodName + "] on class ["
+					+ object.getClass().getSimpleName() + "] compatable with the signature ["
+					+ getParamTypesString(paramTypes) + "].";
+			Assert.isTrue(!paramsRequired, errorMsg);
+			// if no method was found for the given parameters, and the
+			// parameters aren't required, then try with no params
+			method = ClassUtils.getMethodIfAvailable(object.getClass(), methodName, new Class[] {});
+			Assert.notNull(method, errorMsg);
 		}
-		else {
-			return new SimpleMethodInvoker(object, method);
+		return new SimpleMethodInvoker(object, method);
+	}
+
+	/**
+	 * Create a String representation of the array of parameter types.
+	 * 
+	 * @param paramTypes
+	 * @return String
+	 */
+	public static String getParamTypesString(Class<?>... paramTypes) {
+		StringBuffer paramTypesList = new StringBuffer("(");
+		for (int i = 0; i < paramTypes.length; i++) {
+			paramTypesList.append(paramTypes[i].getSimpleName());
+			if (i + 1 < paramTypes.length) {
+				paramTypesList.append(", ");
+			}
 		}
+		return paramTypesList.append(")").toString();
 	}
 
 	/**