RESOLVED - issue BATCH-1102: Classes with "listener" annotations should be auto-registered

Added convenience methods to *ListenerFactoryBean and then used in *StepFactoryBean

archetypes/simple-cli/pom.xml

@@ -138,9 +138,9 @@
 				<plugin>
 					<groupId>com.springsource.bundlor</groupId>
 					<artifactId>com.springsource.bundlor.maven</artifactId>
+					<version>1.0.0.BUILD-SNAPSHOT</version>
 					<configuration>
 						<outputFile>${project.basedir}/src/main/resources/META-INF/MANIFEST.MF</outputFile>
-						<manifestTemplatePath>${project.basedir}/template.mf</manifestTemplatePath>
 					</configuration>
 				</plugin>
 				<plugin>

spring-batch-core/src/main/java/org/springframework/batch/core/ExitStatus.java

@@ -17,12 +17,11 @@ package org.springframework.batch.core;
 
 import java.io.Serializable;
 
-import org.springframework.batch.repeat.RepeatOperations;
 import org.springframework.util.StringUtils;
 
 /**
  * Value object used to carry information about the status of a
- * {@link RepeatOperations}.
+ * job or step execution.
  * 
  * ExitStatus is immutable and therefore thread-safe.
  * 

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

@@ -35,80 +35,88 @@ import org.springframework.beans.factory.InitializingBean;
 import org.springframework.util.Assert;
 
 /**
- * {@link FactoryBean} implementation that builds a {@link JobExecutionListener} 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 JobExecutionListener} lifecyle:
+ * {@link FactoryBean} implementation that builds a {@link JobExecutionListener}
+ * 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 JobExecutionListener} lifecyle:
  * 
  * <ul>
- * 	<li>Interface implementation: By implementing JobExecutionListener, methods on said
- * interface will be called.
- *  <li>Annotations: Annotating a method will result in registration. 
- *  <li>String name of the method to be called, which is tied to {@link JobListenerMetaData} in the 
- *  metaDatMap.
- * </ul> 
+ * <li>Interface implementation: By implementing JobExecutionListener, methods
+ * on said interface will be called.
+ * <li>Annotations: Annotating a method will result in registration.
+ * <li>String name of the method to be called, which is tied to
+ * {@link JobListenerMetaData} in the metaDatMap.
+ * </ul>
  * 
- * It should be noted that methods obtained by name or annotation that don't match the listener method 
- * signatures to which they belong, will cause errors.  However, it is acceptable to have no parameters at all.
- * If the same method is marked in more than one way. (i.e. the method name is given and it's annotated) the
- * method will only be called once.  However, if the same class has multiple methods tied to a particular
- * listener, each method will be called.
+ * It should be noted that methods obtained by name or annotation that don't
+ * match the listener method signatures to which they belong, will cause errors.
+ * However, it is acceptable to have no parameters at all. If the same method is
+ * marked in more than one way. (i.e. the method name is given and it's
+ * annotated) the method will only be called once. However, if the same class
+ * has multiple methods tied to a particular listener, each method will be
+ * called.
  * 
  * @author Lucas Ward
  * @since 2.0
  * @see JobListenerMetaData
  */
-public class JobListenerFactoryBean implements FactoryBean, InitializingBean{
+public class JobListenerFactoryBean implements FactoryBean, InitializingBean {
 
 	private Object delegate;
+
 	private Map<String, String> metaDataMap;
-	
+
 	public void setDelegate(Object delegate) {
 		this.delegate = delegate;
 	}
-	
+
 	public void setMetaDataMap(Map<String, String> metaDataMap) {
 		this.metaDataMap = metaDataMap;
 	}
-	
-	public Object getObject() throws Exception {
+
+	public Object getObject() {
 		Map<String, Set<MethodInvoker>> invokerMap = new HashMap<String, Set<MethodInvoker>>();
-		if(metaDataMap == null){
+		if (metaDataMap == null) {
 			metaDataMap = new HashMap<String, String>();
 		}
-		//Because all annotations and interfaces should be checked for, make sure that each meta data 
-		//entry is represented.
-		for(JobListenerMetaData metaData : JobListenerMetaData.values()){
-			if(!metaDataMap.containsKey(metaData.getPropertyName())){
-				//put null so that the annotation and interface is checked
+		// Because all annotations and interfaces should be checked for, make
+		// sure that each meta data
+		// entry is represented.
+		for (JobListenerMetaData metaData : JobListenerMetaData.values()) {
+			if (!metaDataMap.containsKey(metaData.getPropertyName())) {
+				// put null so that the annotation and interface is checked
 				metaDataMap.put(metaData.getPropertyName(), null);
 			}
 		}
-		
-		//For every entry in th emap, try and find a method by interface, name, or annotation.  If the same
-		for(Entry<String, String> entry : metaDataMap.entrySet()){
+
+		// 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()) {
 			JobListenerMetaData metaData = JobListenerMetaData.fromPropertyName(entry.getKey());
 			Set<MethodInvoker> invokers = new NullIgnoringSet<MethodInvoker>();
 			invokers.add(getMethodInvokerByName(entry.getValue(), delegate, JobExecution.class));
-			invokers.add(getMethodInvokerForInterface(JobExecutionListener.class, metaData.getMethodName(), 
-					delegate, JobExecution.class));
+			invokers.add(getMethodInvokerForInterface(JobExecutionListener.class, metaData.getMethodName(), delegate,
+					JobExecution.class));
 			invokers.add(getMethodInvokerByAnnotation(metaData.getAnnotation(), delegate));
-			if(!invokers.isEmpty()){
+			if (!invokers.isEmpty()) {
 				invokerMap.put(metaData.getMethodName(), invokers);
 			}
 		}
-		
-		//create a proxy listener for only the interfaces that have methods to be called
+
+		// create a proxy listener for only the interfaces that have methods to
+		// be called
 		ProxyFactory proxyFactory = new ProxyFactory();
-		proxyFactory.setInterfaces(new Class[]{JobExecutionListener.class});
+		proxyFactory.setTarget(delegate);
+		proxyFactory.setInterfaces(new Class[] { JobExecutionListener.class });
 		proxyFactory.addAdvisor(new DefaultPointcutAdvisor(new MethodInvokerMethodInterceptor(invokerMap)));
 		return proxyFactory.getProxy();
 	}
-	
-	private MethodInvoker getMethodInvokerByName(String methodName, Object candidate, Class<?>... params){
-		if(methodName != null){
+
+	private MethodInvoker getMethodInvokerByName(String methodName, Object candidate, Class<?>... params) {
+		if (methodName != null) {
 			return MethodInvokerUtils.createMethodInvokerByName(candidate, methodName, false, params);
 		}
-		else{
+		else {
 			return null;
 		}
 	}
@@ -125,19 +133,52 @@ public class JobListenerFactoryBean implements FactoryBean, InitializingBean{
 	public void afterPropertiesSet() throws Exception {
 		Assert.notNull(delegate, "Delegate listener must not be null");
 	}
-	
+
+	/**
+	 * Convenience method to wrap any object and expose the appropriate
+	 * {@link JobExecutionListener} interfaces.
+	 * @param delegate a delegate object
+	 * @return a JobListener instance constructed from the delegate
+	 */
+	public static JobExecutionListener getListener(Object delegate) {
+		JobListenerFactoryBean factory = new JobListenerFactoryBean();
+		factory.setDelegate(delegate);
+		return (JobExecutionListener) factory.getObject();
+	}
+
+	/**
+	 * Convenience method to check whether the given object is or can be made
+	 * into a {@link JobExecutionListener}.
+	 * @param delegate the object to check
+	 * @return true if the delegate is an instance of
+	 * {@link JobExecutionListener}, or contains the marker annotations
+	 */
+	public static boolean isListener(Object delegate) {
+		if (delegate instanceof JobExecutionListener) {
+			return true;
+		}
+		for (JobListenerMetaData metaData : JobListenerMetaData.values()) {
+			Set<MethodInvoker> invokers = new NullIgnoringSet<MethodInvoker>();
+			invokers.add(getMethodInvokerByAnnotation(metaData.getAnnotation(), delegate));
+			if (!invokers.isEmpty()) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 	/*
 	 * Extension of HashSet that ignores nulls, rather than putting them into
 	 * the set.
 	 */
-	private static class NullIgnoringSet<E> extends HashSet<E>{
-		
+	private static class NullIgnoringSet<E> extends HashSet<E> {
+
 		@Override
 		public boolean add(E e) {
-			if(e == null){
+			if (e == null) {
 				return false;
 			}
-			else{
+			else {
 				return super.add(e);
 			}
 		};

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

@@ -25,48 +25,68 @@ import org.springframework.batch.core.StepExecutionListener;
 import org.springframework.batch.core.configuration.util.MethodInvoker;
 
 /**
- * {@link MethodInterceptor} that, given a map of method names and {@link MethodInvoker}s,
- * will execute all methods tied to a particular method name, with the provided 
- * arguments.  The only possible return value that is handled is of type ExitStatus, since
- * the only StepListener implementation that isn't void is 
- * {@link StepExecutionListener#afterStep(org.springframework.batch.core.StepExecution)}, which
- * returns ExitStatus.
+ * {@link MethodInterceptor} that, given a map of method names and
+ * {@link MethodInvoker}s, will execute all methods tied to a particular method
+ * name, with the provided arguments. The only possible return value that is
+ * handled is of type ExitStatus, since the only StepListener implementation
+ * that isn't void is
+ * {@link StepExecutionListener#afterStep(org.springframework.batch.core.StepExecution)}
+ * , which returns ExitStatus.
  * 
  * @author Lucas Ward
  * @since 2.0
  * @see MethodInvoker
  */
-public class MethodInvokerMethodInterceptor implements MethodInterceptor{
+public class MethodInvokerMethodInterceptor implements MethodInterceptor {
 
 	private final Map<String, Set<MethodInvoker>> invokerMap;
 
 	public MethodInvokerMethodInterceptor(Map<String, Set<MethodInvoker>> invokerMap) {
 		this.invokerMap = invokerMap;
 	}
-	
+
 	public Object invoke(MethodInvocation invocation) throws Throwable {
-		
+
 		String methodName = invocation.getMethod().getName();
 		Set<MethodInvoker> invokers = invokerMap.get(methodName);
-		
-		if(invokers == null){
+
+		if (invokers == null) {
 			return null;
 		}
 		ExitStatus status = null;
-		for(MethodInvoker invoker : invokers){
+		for (MethodInvoker invoker : invokers) {
 			Object retVal = invoker.invokeMethod(invocation.getArguments());
-			if(retVal instanceof ExitStatus){
-				if(status != null){
+			if (retVal instanceof ExitStatus) {
+				if (status != null) {
 					status = status.and((ExitStatus) retVal);
 				}
-				else{
+				else {
 					status = (ExitStatus) retVal;
 				}
 			}
 		}
-		
+
 		return status;
 	}
+
+	/**
+	 * {@inheritDoc}
+	 */
+	@Override
+	public boolean equals(Object obj) {
+		if (!(obj instanceof MethodInvokerMethodInterceptor)) {
+			return false;
+		}
+		MethodInvokerMethodInterceptor other = (MethodInvokerMethodInterceptor) obj;
+		return invokerMap.equals(other.invokerMap);
+	}
 	
-	
+	/**
+	 * {@inheritDoc}
+	 */
+	@Override
+	public int hashCode() {
+		return invokerMap.hashCode();
+	}
+
 }

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

@@ -34,76 +34,87 @@ import org.springframework.beans.factory.InitializingBean;
 import org.springframework.util.Assert;
 
 /**
- * {@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 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:
  * 
  * <ul>
- * 	<li>Interface implementation: By implementing any of the subclasses of StepListener, methods on said
- * interface will be called
- *  <li>Annotations: Annotating a method will result in registration. 
- *  <li>String name of the method to be called, which is tied to {@link StepListenerMetaData} in the 
- *  metaDatMap.
- * </ul> 
+ * <li>Interface implementation: By implementing any of the subclasses of
+ * StepListener, methods on said interface will be called
+ * <li>Annotations: Annotating a method will result in registration.
+ * <li>String name of the method to be called, which is tied to
+ * {@link StepListenerMetaData} in the metaDatMap.
+ * </ul>
  * 
- * It should be noted that methods obtained by name or annotation that don't match the StepListener method 
- * signatures to which they belong, will cause errors.  However, it is acceptable to have no parameters at all.
- * If the same method is marked in more than one way. (i.e. the method name is given and it's annotated) the
- * method will only be called once.  However, if the same class has multiple methods tied to a particular
- * listener, each method will be called.
+ * It should be noted that methods obtained by name or annotation that don't
+ * match the StepListener method signatures to which they belong, will cause
+ * errors. However, it is acceptable to have no parameters at all. If the same
+ * method is marked in more than one way. (i.e. the method name is given and
+ * it's annotated) the method will only be called once. However, if the same
+ * class has multiple methods tied to a particular listener, each method will be
+ * called.
  * 
  * @author Lucas Ward
  * @since 2.0
  * @see StepListenerMetaData
  */
-public class StepListenerFactoryBean implements FactoryBean, InitializingBean{
+public class StepListenerFactoryBean implements FactoryBean, InitializingBean {
 
 	private Object delegate;
+
 	private Map<String, String> metaDataMap;
-	
-	public Object getObject() throws Exception {
-		
+
+	public Object getObject() {
+
 		Map<String, Set<MethodInvoker>> invokerMap = new HashMap<String, Set<MethodInvoker>>();
-		if(metaDataMap == null){
+		if (metaDataMap == null) {
 			metaDataMap = new HashMap<String, String>();
 		}
-		//Because all annotations and interfaces should be checked for, make sure that each meta data 
-		//entry is represented.
-		for(StepListenerMetaData metaData : StepListenerMetaData.values()){
-			if(!metaDataMap.containsKey(metaData.getPropertyName())){
-				//put null so that the annotation and interface is checked
+		// Because all annotations and interfaces should be checked for, make
+		// sure that each meta data
+		// entry is represented.
+		for (StepListenerMetaData metaData : StepListenerMetaData.values()) {
+			if (!metaDataMap.containsKey(metaData.getPropertyName())) {
+				// put null so that the annotation and interface is checked
 				metaDataMap.put(metaData.getPropertyName(), null);
 			}
 		}
-		
+
 		Set<Class<? extends StepListener>> listenerInterfaces = new HashSet<Class<? extends StepListener>>();
-		
-		//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()){
+
+		// 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());
 			Set<MethodInvoker> invokers = new NullIgnoringSet<MethodInvoker>();
 			invokers.add(getMethodInvokerByName(entry.getValue(), delegate, metaData.getParamTypes()));
-			invokers.add(getMethodInvokerForInterface(metaData.getListenerInterface(), metaData.getMethodName(), 
+			invokers.add(getMethodInvokerForInterface(metaData.getListenerInterface(), metaData.getMethodName(),
 					delegate, metaData.getParamTypes()));
 			invokers.add(getMethodInvokerByAnnotation(metaData.getAnnotation(), delegate));
-			if(!invokers.isEmpty()){
+			if (!invokers.isEmpty()) {
 				invokerMap.put(metaData.getMethodName(), invokers);
 				listenerInterfaces.add(metaData.getListenerInterface());
 			}
 		}
-		
-		//create a proxy listener for only the interfaces that have methods to be called
+
+		if (listenerInterfaces.isEmpty()) {
+			listenerInterfaces.add(StepListener.class);
+		}
+
+		// create a proxy listener for only the interfaces that have methods to
+		// be called
 		ProxyFactory proxyFactory = new ProxyFactory();
 		proxyFactory.setInterfaces(listenerInterfaces.toArray(new Class[0]));
 		proxyFactory.addAdvisor(new DefaultPointcutAdvisor(new MethodInvokerMethodInterceptor(invokerMap)));
 		return proxyFactory.getProxy();
 	}
-	
-	private MethodInvoker getMethodInvokerByName(String methodName, Object candidate, Class<?>... params){
-		if(methodName != null){
+
+	private MethodInvoker getMethodInvokerByName(String methodName, Object candidate, Class<?>... params) {
+		if (methodName != null) {
 			return MethodInvokerUtils.createMethodInvokerByName(candidate, methodName, false, params);
 		}
-		else{
+		else {
 			return null;
 		}
 	}
@@ -116,27 +127,60 @@ public class StepListenerFactoryBean implements FactoryBean, InitializingBean{
 	public boolean isSingleton() {
 		return false;
 	}
-	
+
 	public void setDelegate(Object delegate) {
 		this.delegate = delegate;
 	}
-	
+
 	public void setMetaDataMap(Map<String, String> metaDataMap) {
 		this.metaDataMap = metaDataMap;
 	}
-	
+
+	/**
+	 * 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
+	 */
+	public static StepListener getListener(Object delegate) {
+		StepListenerFactoryBean factory = new StepListenerFactoryBean();
+		factory.setDelegate(delegate);
+		return (StepListener) factory.getObject();
+	}
+
+	/**
+	 * 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
+	 */
+	public static boolean isListener(Object delegate) {
+		if (delegate instanceof StepListener) {
+			return true;
+		}
+		for (StepListenerMetaData metaData : StepListenerMetaData.values()) {
+			Set<MethodInvoker> invokers = new NullIgnoringSet<MethodInvoker>();
+			invokers.add(getMethodInvokerByAnnotation(metaData.getAnnotation(), delegate));
+			if (!invokers.isEmpty()) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 	/*
 	 * Extension of HashSet that ignores nulls, rather than putting them into
 	 * the set.
 	 */
-	private static class NullIgnoringSet<E> extends HashSet<E>{
-		
+	private static class NullIgnoringSet<E> extends HashSet<E> {
+
 		@Override
 		public boolean add(E e) {
-			if(e == null){
+			if (e == null) {
 				return false;
 			}
-			else{
+			else {
 				return super.add(e);
 			}
 		};

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

@@ -29,6 +29,7 @@ import org.springframework.batch.core.SkipListener;
 import org.springframework.batch.core.Step;
 import org.springframework.batch.core.StepExecutionListener;
 import org.springframework.batch.core.StepListener;
+import org.springframework.batch.core.listener.StepListenerFactoryBean;
 import org.springframework.batch.core.repository.JobRepository;
 import org.springframework.batch.core.step.tasklet.TaskletStep;
 import org.springframework.batch.item.ItemProcessor;
@@ -121,10 +122,21 @@ public class SimpleStepFactoryBean<T, S> implements FactoryBean, BeanNameAware {
 		super();
 	}
 
+	/**
+	 * Flag to signal that the reader is transactional (usually a JMS consumer)
+	 * so that items are re-presented after a rollback. The default is false and
+	 * readers are assumed to be forward-only.
+	 * 
+	 * @param isReaderTransactionalQueue the value of the flag
+	 */
 	public void setIsReaderTransactionalQueue(boolean isReaderTransactionalQueue) {
 		this.isReaderTransactionalQueue = isReaderTransactionalQueue;
 	}
 
+	/**
+	 * Convenience method for subclasses.
+	 * @return true if the flag is set (default false)
+	 */
 	protected boolean isReaderTransactionalQueue() {
 		return isReaderTransactionalQueue;
 	}
@@ -142,7 +154,7 @@ public class SimpleStepFactoryBean<T, S> implements FactoryBean, BeanNameAware {
 	}
 
 	/**
-	 * Public getter for the String.
+	 * Public getter for the name of the step.
 	 * @return the name
 	 */
 	public String getName() {
@@ -150,7 +162,7 @@ public class SimpleStepFactoryBean<T, S> implements FactoryBean, BeanNameAware {
 	}
 
 	/**
-	 * Public setter for the startLimit.
+	 * Public setter for the start limit for the step.
 	 * 
 	 * @param startLimit the startLimit to set
 	 */
@@ -159,7 +171,8 @@ public class SimpleStepFactoryBean<T, S> implements FactoryBean, BeanNameAware {
 	}
 
 	/**
-	 * Public setter for the shouldAllowStartIfComplete.
+	 * Public setter for the flag to indicate that the step should be replayed
+	 * on a restart, even if successful the first time.
 	 * 
 	 * @param allowStartIfComplete the shouldAllowStartIfComplete to set
 	 */
@@ -168,21 +181,21 @@ public class SimpleStepFactoryBean<T, S> implements FactoryBean, BeanNameAware {
 	}
 
 	/**
-	 * @param itemReader the itemReader to set
+	 * @param itemReader the {@link ItemReader} to set
 	 */
 	public void setItemReader(ItemReader<? extends T> itemReader) {
 		this.itemReader = itemReader;
 	}
 
 	/**
-	 * @param itemWriter the itemWriter to set
+	 * @param itemWriter the {@link ItemWriter} to set
 	 */
 	public void setItemWriter(ItemWriter<? super S> itemWriter) {
 		this.itemWriter = itemWriter;
 	}
 
 	/**
-	 * @param itemProcessor the itemProcessor to set
+	 * @param itemProcessor the {@link ItemProcessor} to set
 	 */
 	public void setItemProcessor(ItemProcessor<? super T, ? extends S> itemProcessor) {
 		this.itemProcessor = itemProcessor;
@@ -516,11 +529,14 @@ public class SimpleStepFactoryBean<T, S> implements FactoryBean, BeanNameAware {
 			if (itemHandler instanceof ItemStream) {
 				step.registerStream((ItemStream) itemHandler);
 			}
-			if (itemHandler instanceof StepExecutionListener) {
-				step.registerStepExecutionListener((StepExecutionListener) itemHandler);
-			}
-			if (itemHandler instanceof ChunkListener) {
-				chunkListeners.add((StepListener) itemHandler);
+			if (StepListenerFactoryBean.isListener(itemHandler)) {
+				StepListener listener = StepListenerFactoryBean.getListener(itemHandler);
+				if (listener instanceof StepExecutionListener) {
+					step.registerStepExecutionListener((StepExecutionListener) listener);
+				}
+				if (listener instanceof ChunkListener) {
+					chunkListeners.add((StepListener) listener);
+				}
 			}
 		}
 
@@ -551,17 +567,20 @@ public class SimpleStepFactoryBean<T, S> implements FactoryBean, BeanNameAware {
 		// auto-register reader, processor and writer
 		for (Object itemHandler : new Object[] { getItemReader(), getItemWriter(), getItemProcessor() }) {
 
-			if (itemHandler instanceof SkipListener) {
-				chunkProvider.registerListener((StepListener) itemHandler);
-				chunkProcessor.registerListener((StepListener) itemHandler);
-				// already registered with both so avoid double-registering
-				continue;
-			}
-			if (itemHandler instanceof ItemReadListener) {
-				chunkProvider.registerListener((StepListener) itemHandler);
-			}
-			if (itemHandler instanceof ItemProcessListener || itemHandler instanceof ItemWriteListener) {
-				chunkProcessor.registerListener((StepListener) itemHandler);
+			if (StepListenerFactoryBean.isListener(itemHandler)) {
+				StepListener listener = StepListenerFactoryBean.getListener(itemHandler);
+				if (listener instanceof SkipListener) {
+					chunkProvider.registerListener(listener);
+					chunkProcessor.registerListener(listener);
+					// already registered with both so avoid double-registering
+					continue;
+				}
+				if (listener instanceof ItemReadListener) {
+					chunkProvider.registerListener(listener);
+				}
+				if (listener instanceof ItemProcessListener || listener instanceof ItemWriteListener) {
+					chunkProcessor.registerListener(listener);
+				}
 			}
 		}
 	}

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

@@ -15,7 +15,11 @@
  */
 package org.springframework.batch.core.listener;
 
-import static org.junit.Assert.*;
+import static junit.framework.Assert.assertTrue;
+import static org.junit.Assert.assertEquals;
+
+import java.util.HashSet;
+import java.util.Set;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -26,19 +30,19 @@ import org.springframework.batch.core.annotation.BeforeJob;
 
 /**
  * @author Lucas Ward
- *
+ * 
  */
 public class JobListenerFactoryBeanTests {
 
 	JobListenerFactoryBean factoryBean;
-	
+
 	@Before
-	public void setUp(){
+	public void setUp() {
 		factoryBean = new JobListenerFactoryBean();
 	}
-	
+
 	@Test
-	public void testWithInterface() throws Exception{
+	public void testWithInterface() throws Exception {
 		JobListenerWithInterface delegate = new JobListenerWithInterface();
 		factoryBean.setDelegate(delegate);
 		JobExecutionListener listener = (JobExecutionListener) factoryBean.getObject();
@@ -48,9 +52,9 @@ public class JobListenerFactoryBeanTests {
 		assertTrue(delegate.beforeJobCalled);
 		assertTrue(delegate.afterJobCalled);
 	}
-	
+
 	@Test
-	public void testWithAnnotations() throws Exception{
+	public void testWithAnnotations() throws Exception {
 		AnnotatedTestClass delegate = new AnnotatedTestClass();
 		factoryBean.setDelegate(delegate);
 		JobExecutionListener listener = (JobExecutionListener) factoryBean.getObject();
@@ -60,34 +64,81 @@ public class JobListenerFactoryBeanTests {
 		assertTrue(delegate.beforeJobCalled);
 		assertTrue(delegate.afterJobCalled);
 	}
-	
-	private class JobListenerWithInterface implements JobExecutionListener{
+
+	@Test
+	public void testFactoryMethod() throws Exception {
+		JobListenerWithInterface delegate = new JobListenerWithInterface();
+		Object listener = JobListenerFactoryBean.getListener(delegate);
+		assertTrue(listener instanceof JobExecutionListener);
+		((JobExecutionListener) listener).afterJob(new JobExecution(11L));
+		assertTrue(delegate.afterJobCalled);
+	}
+
+	@Test
+	public void testUseInHashSet() throws Exception {
+		JobListenerWithInterface delegate = new JobListenerWithInterface();
+		Object listener = JobListenerFactoryBean.getListener(delegate);
+		Object other = JobListenerFactoryBean.getListener(delegate);
+		assertTrue(listener instanceof JobExecutionListener);
+		Set<JobExecutionListener> listeners = new HashSet<JobExecutionListener>();
+		listeners.add((JobExecutionListener) listener);
+		listeners.add((JobExecutionListener) other);
+		assertTrue(listeners.contains(listener));
+		assertEquals(1, listeners.size());
+	}
+
+	@Test
+	public void testAnnotationsIsListener() throws Exception {
+		assertTrue(JobListenerFactoryBean.isListener(new Object() {
+			@SuppressWarnings("unused")
+			@BeforeJob
+			public void foo(JobExecution execution) {
+			}
+		}));
+	}
+
+	@Test
+	public void testInterfaceIsListener() throws Exception {
+		assertTrue(JobListenerFactoryBean.isListener(new JobListenerWithInterface()));
+	}
+
+	@Test
+	public void testEqualityOfProxies() throws Exception {
+		JobListenerWithInterface delegate = new JobListenerWithInterface();
+		Object listener1 = JobListenerFactoryBean.getListener(delegate);
+		Object listener2 = JobListenerFactoryBean.getListener(delegate);
+		assertEquals(listener1, listener2);
+	}
+
+	private class JobListenerWithInterface implements JobExecutionListener {
 
 		boolean beforeJobCalled = false;
+
 		boolean afterJobCalled = false;
-		
+
 		public void afterJob(JobExecution jobExecution) {
-			beforeJobCalled = true;
+			afterJobCalled = true;
 		}
 
 		public void beforeJob(JobExecution jobExecution) {
-			afterJobCalled = true;
-		}
-		
-	}
-	
-	private class AnnotatedTestClass {
-		
-		boolean beforeJobCalled = false;
-		boolean afterJobCalled = false;
-		
-		@BeforeJob
-		public void before(){
 			beforeJobCalled = true;
 		}
-		
+
+	}
+
+	private class AnnotatedTestClass {
+
+		boolean beforeJobCalled = false;
+
+		boolean afterJobCalled = false;
+
+		@BeforeJob
+		public void before() {
+			beforeJobCalled = true;
+		}
+
 		@AfterJob
-		public void after(){
+		public void after() {
 			afterJobCalled = true;
 		}
 	}

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

@@ -17,6 +17,7 @@ package org.springframework.batch.core.listener;
 
 import static junit.framework.Assert.assertTrue;
 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;
 
@@ -57,22 +58,22 @@ import org.springframework.util.Assert;
  */
 public class StepListenerFactoryBeanTests {
 
-	StepListenerFactoryBean factoryBean;
-	TestClass testClass;
-	JobExecution jobExecution = new JobExecution(11L); 
-	StepExecution stepExecution = new StepExecution("testStep", jobExecution);
+	private StepListenerFactoryBean factoryBean;
+	private TestListener testListener;
+	private JobExecution jobExecution = new JobExecution(11L); 
+	private StepExecution stepExecution = new StepExecution("testStep", jobExecution);
 	
 	@Before
 	public void setUp(){
 		factoryBean = new StepListenerFactoryBean();
-		testClass = new TestClass();
+		testListener = new TestListener();
 	}
 	
 	@Test
 	@SuppressWarnings("unchecked")
 	public void testStepAndChunk() throws Exception{
 		
-		factoryBean.setDelegate(testClass);
+		factoryBean.setDelegate(testListener);
 		Map<String, String> metaDataMap = new HashMap<String, String>();;
 		metaDataMap.put(AFTER_STEP.getPropertyName(), "destroy");
 		metaDataMap.put(AFTER_CHUNK.getPropertyName(), "afterChunk");
@@ -97,21 +98,21 @@ public class StepListenerFactoryBeanTests {
 		((SkipListener<Object, Object>)listener).onSkipInRead(new Throwable());
 		((SkipListener<Object, Object>)listener).onSkipInProcess(item, new Throwable());
 		((SkipListener<Object, Object>)listener).onSkipInWrite(item, new Throwable());
-		assertTrue(testClass.beforeStepCalled);
-		assertTrue(testClass.beforeChunkCalled);
-		assertTrue(testClass.afterChunkCalled);
-		assertTrue(testClass.beforeReadCalled);
-		assertTrue(testClass.afterReadCalled);
-		assertTrue(testClass.onReadErrorCalled);
-		assertTrue(testClass.beforeProcessCalled);
-		assertTrue(testClass.afterProcessCalled);
-		assertTrue(testClass.onProcessErrorCalled);
-		assertTrue(testClass.beforeWriteCalled);
-		assertTrue(testClass.afterWriteCalled);
-		assertTrue(testClass.onWriteErrorCalled);
-		assertTrue(testClass.onSkipInReadCalled);
-		assertTrue(testClass.onSkipInProcessCalled);
-		assertTrue(testClass.onSkipInWriteCalled);
+		assertTrue(testListener.beforeStepCalled);
+		assertTrue(testListener.beforeChunkCalled);
+		assertTrue(testListener.afterChunkCalled);
+		assertTrue(testListener.beforeReadCalled);
+		assertTrue(testListener.afterReadCalled);
+		assertTrue(testListener.onReadErrorCalled);
+		assertTrue(testListener.beforeProcessCalled);
+		assertTrue(testListener.afterProcessCalled);
+		assertTrue(testListener.onProcessErrorCalled);
+		assertTrue(testListener.beforeWriteCalled);
+		assertTrue(testListener.afterWriteCalled);
+		assertTrue(testListener.onWriteErrorCalled);
+		assertTrue(testListener.onSkipInReadCalled);
+		assertTrue(testListener.onSkipInProcessCalled);
+		assertTrue(testListener.onSkipInWriteCalled);
 	}
 	
 	@Test
@@ -132,7 +133,7 @@ public class StepListenerFactoryBeanTests {
 	public void testAnnotatingInterfaceResultsInOneCall() throws Exception{
 		MultipleAfterStep delegate = new MultipleAfterStep();
 		factoryBean.setDelegate(delegate);
-		Map<String, String> metaDataMap = new HashMap<String, String>();;
+		Map<String, String> metaDataMap = new HashMap<String, String>();
 		metaDataMap.put(AFTER_STEP.getPropertyName(), "afterStep");
 		factoryBean.setMetaDataMap(metaDataMap);
 		StepListener listener = (StepListener) factoryBean.getObject();
@@ -140,7 +141,55 @@ public class StepListenerFactoryBeanTests {
 		assertEquals(1, delegate.callcount);
 	}
 	
-	private class MultipleAfterStep implements StepExecutionListener{
+	@Test
+	public void testVanillaInterface() throws Exception{
+		MultipleAfterStep delegate = new MultipleAfterStep();
+		factoryBean.setDelegate(delegate);
+		Object listener = factoryBean.getObject();
+		assertTrue(listener instanceof StepExecutionListener);
+		((StepExecutionListener)listener).beforeStep(stepExecution);
+		assertEquals(1, delegate.callcount);
+	}
+	
+	@Test
+	public void testFactoryMethod() throws Exception{
+		MultipleAfterStep delegate = new MultipleAfterStep();
+		Object listener = StepListenerFactoryBean.getListener(delegate);
+		assertTrue(listener instanceof StepExecutionListener);
+		assertFalse(listener instanceof ChunkListener);
+		((StepExecutionListener)listener).beforeStep(stepExecution);
+		assertEquals(1, delegate.callcount);
+	}
+	
+	@Test
+	public void testInterfaceIsListener() throws Exception {
+		assertTrue(StepListenerFactoryBean.isListener(new ThreeStepExecutionListener()));
+	}
+	
+	@Test
+	public void testAnnotationsIsListener() throws Exception {
+		assertTrue(StepListenerFactoryBean.isListener(new Object() {
+			@SuppressWarnings("unused")
+			@BeforeStep
+			public void foo(StepExecution execution) {
+			}
+		}));
+	}
+	
+	@Test
+	public void testMixedIsListener() throws Exception {
+		assertTrue(StepListenerFactoryBean.isListener(new MultipleAfterStep()));
+	}
+	
+	@Test
+	public void testNonListener() throws Exception{
+		Object delegate = new Object();
+		factoryBean.setDelegate(delegate);
+		StepListener listener = (StepListener) factoryBean.getObject();
+		assertTrue(listener instanceof StepListener);
+	}
+	
+	private class MultipleAfterStep implements StepExecutionListener {
 
 		int callcount = 0;
 		
@@ -183,7 +232,7 @@ public class StepListenerFactoryBeanTests {
 
 	}
 	
-	private class TestClass implements SkipListener<Object, Object>{
+	private class TestListener implements SkipListener<Object, Object>{
 
 		boolean beforeStepCalled = false;
 		boolean afterStepCalled = false;

spring-batch-samples/src/main/resources/jobs/skipSampleJob.xml

@@ -39,7 +39,6 @@
 		<listeners>
 			<listener ref="skipCheckingListener"/>
 			<listener ref="promotionListener"/>
-			<listener ref="tradeWriter"/>
 		</listeners>
 	</step>
 	

spring-batch-samples/src/test/java/org/springframework/batch/sample/common/CustomItemReaderTests.java

@@ -15,11 +15,14 @@
  */
 package org.springframework.batch.sample.common;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
 import java.util.ArrayList;
 import java.util.List;
 
-import junit.framework.TestCase;
-
+import org.junit.Before;
+import org.junit.Test;
 import org.springframework.batch.item.ExecutionContext;
 import org.springframework.batch.item.ItemReader;
 import org.springframework.batch.item.ItemStream;
@@ -34,15 +37,15 @@ import org.springframework.batch.item.UnexpectedInputException;
  * @author Lucas Ward
  *
  */
-public class CustomItemReaderTests extends TestCase {
+public class CustomItemReaderTests {
 
 	ItemReader<String> itemReader;
 	
 	/* (non-Javadoc)
 	 * @see junit.framework.TestCase#setUp()
 	 */
-	protected void setUp() throws Exception {
-		super.setUp();
+	@Before
+	public void setUp() throws Exception {
 		
 		List<String> items = new ArrayList<String>();
 		items.add("1");
@@ -52,6 +55,7 @@ public class CustomItemReaderTests extends TestCase {
 		itemReader = new CustomItemReader<String>(items);
 	}
 	
+	@Test
 	public void testRead() throws Exception{
 	
 		assertEquals("1", itemReader.read());
@@ -60,6 +64,7 @@ public class CustomItemReaderTests extends TestCase {
 		assertNull(itemReader.read());
 	}
 	
+	@Test
 	public void testRestart() throws Exception{
 		
 		ExecutionContext executionContext = new ExecutionContext();

spring-batch-samples/src/test/java/org/springframework/batch/sample/common/StagingItemReaderTests.java

@@ -41,7 +41,7 @@ public class StagingItemReaderTests {
 	@Autowired
 	private StagingItemReader<String> reader;
 
-	private Long jobId = 11L;
+	private Long jobId = 113L;
 
 	@Autowired
 	public void setDataSource(DataSource dataSource) {

spring-batch-samples/src/test/java/org/springframework/batch/sample/support/AbstractRowMapperTests.java

@@ -1,12 +1,13 @@
 package org.springframework.batch.sample.support;
 
-import static org.easymock.EasyMock.*;
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.replay;
+import static org.junit.Assert.assertEquals;
 
 import java.sql.ResultSet;
 import java.sql.SQLException;
 
-import junit.framework.TestCase;
-
+import org.junit.Test;
 import org.springframework.jdbc.core.RowMapper;
 
 /**
@@ -14,38 +15,38 @@ import org.springframework.jdbc.core.RowMapper;
  * 
  * @author Robert Kasanicky
  */
-public abstract class AbstractRowMapperTests extends TestCase {
+public abstract class AbstractRowMapperTests {
 
-	//row number should be irrelevant
+	// row number should be irrelevant
 	private static final int IGNORED_ROW_NUMBER = 0;
-	
-	//mock result set
+
+	// mock result set
 	private ResultSet rs = createMock(ResultSet.class);
 
 	/**
-	 * @return Expected result of mapping the mock <code>ResultSet</code> by
-	 * the mapper being tested.
+	 * @return Expected result of mapping the mock <code>ResultSet</code> by the
+	 * mapper being tested.
 	 */
 	abstract protected Object expectedDomainObject();
-	
+
 	/**
 	 * @return <code>RowMapper</code> implementation that is being tested.
 	 */
 	abstract protected RowMapper rowMapper();
-	
+
 	/*
 	 * Define the behaviour of mock <code>ResultSet</code>.
 	 */
 	abstract protected void setUpResultSetMock(ResultSet rs) throws SQLException;
-	
-	
+
 	/*
 	 * Regular usage scenario.
 	 */
+	@Test
 	public void testRegularUse() throws SQLException {
 		setUpResultSetMock(rs);
 		replay(rs);
-		
+
 		assertEquals(expectedDomainObject(), rowMapper().mapRow(rs, IGNORED_ROW_NUMBER));
 	}
 }