From 28d04d424acb5f6459092c7c0a72f390d302fca2 Mon Sep 17 00:00:00 2001 From: dhgarrette Date: Tue, 10 Mar 2009 19:57:18 +0000 Subject: [PATCH] BATCH-1118: *Updated JobListenerFactoryBean so that listener methods specified via annotations or the metaDataMap are checked eagerly for correct signatures *Extracted common processing of Job/StepListenerFactoryBean classes into new superclass AbstractListenerFactoryBean *Created AbstractListenerMetaData interface for Job/StepListenerMetaData enums *Fixed JobExecutionListenerParser which had a bug in parsing listener method attributes *Corrected job listener attributes in xsd to use before/after-job-method *Added unit tests for job listener annotations and metaDataMap *Fixed typo in StepListenerFactoryBeanTests --- .../xml/JobExecutionListenerParser.java | 17 +- .../listener/AbstractListenerFactoryBean.java | 208 ++++++++++++++++++ .../listener/AbstractListenerMetaData.java | 38 ++++ .../core/listener/JobListenerFactoryBean.java | 118 ++-------- .../core/listener/JobListenerMetaData.java | 8 +- .../listener/StepListenerFactoryBean.java | 171 +------------- .../core/listener/StepListenerMetaData.java | 2 +- .../configuration/xml/spring-batch-2.0.xsd | 7 +- .../listener/JobListenerFactoryBeanTests.java | 98 +++++++++ .../StepListenerFactoryBeanTests.java | 3 - ...obExecutionListenerParserTests-context.xml | 4 +- 11 files changed, 395 insertions(+), 279 deletions(-) create mode 100644 spring-batch-core/src/main/java/org/springframework/batch/core/listener/AbstractListenerFactoryBean.java create mode 100644 spring-batch-core/src/main/java/org/springframework/batch/core/listener/AbstractListenerMetaData.java diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/JobExecutionListenerParser.java b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/JobExecutionListenerParser.java index 0f01b08a3..43b28765a 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/JobExecutionListenerParser.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/configuration/xml/JobExecutionListenerParser.java @@ -83,14 +83,15 @@ public class JobExecutionListenerParser { } ManagedMap metaDataMap = new ManagedMap(); - String beforeMethod = listenerElement.getAttribute("before-method"); - if (StringUtils.hasText(beforeMethod)) { - metaDataMap.put("beforeMethod", beforeMethod); - } - - String afterMethod = listenerElement.getAttribute("after-method"); - if (StringUtils.hasText(beforeMethod)) { - metaDataMap.put("afterMethod", afterMethod); + String[] methodNameAttributes = new String[] { + "before-job-method", + "after-job-method" + }; + for (String metaDataPropertyName : methodNameAttributes) { + String listenerMethod = listenerElement.getAttribute(metaDataPropertyName); + if(StringUtils.hasText(listenerMethod)){ + metaDataMap.put(metaDataPropertyName, listenerMethod); + } } listenerBuilder.addPropertyValue("metaDataMap", metaDataMap); AbstractBeanDefinition beanDef = listenerBuilder.getBeanDefinition(); diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/listener/AbstractListenerFactoryBean.java b/spring-batch-core/src/main/java/org/springframework/batch/core/listener/AbstractListenerFactoryBean.java new file mode 100644 index 000000000..ae797c384 --- /dev/null +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/listener/AbstractListenerFactoryBean.java @@ -0,0 +1,208 @@ +/* + * Copyright 2002-2008 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.batch.core.listener; + +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; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.springframework.aop.framework.Advised; +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 listener 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 listener lifecycle: + * + * + * + * 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 is + * 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. Also note that the same annotations cannot be applied to two separate + * methods in a single class. + * + * @author Lucas Ward + * @author Dan Garrette + * @since 2.0 + * @see StepListenerMetaData + */ +public abstract class AbstractListenerFactoryBean implements FactoryBean, InitializingBean { + + private static Log logger = LogFactory.getLog(StepListenerFactoryBean.class); + + private Object delegate; + + private Map metaDataMap; + + public Object getObject() { + + if (metaDataMap == null) { + metaDataMap = new HashMap(); + } + // Because all annotations and interfaces should be checked for, make + // sure that each meta data + // entry is represented. + for (AbstractListenerMetaData metaData : this.getMetaDataValues()) { + if (!metaDataMap.containsKey(metaData.getPropertyName())) { + // put null so that the annotation and interface is checked + metaDataMap.put(metaData.getPropertyName(), null); + } + } + + return this.doGetObject(delegate, metaDataMap); + } + + protected abstract Object doGetObject(Object delegate, Map metaDataMap); + + protected abstract AbstractListenerMetaData[] getMetaDataValues(); + + /** + * Create a MethodInvoker from the delegate based on the annotationType. + * Ensure that the annotated method has a valid set of parameters. + * + * @param metaData + * @return a MethodInvoker + */ + protected MethodInvoker getMethodInvokerByAnnotation(final AbstractListenerMetaData metaData) { + final Class annotationType = metaData.getAnnotation(); + 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; + } + + protected MethodInvoker getMethodInvokerByName(String methodName, Object candidate, Class... params) { + if (methodName != null) { + return MethodInvokerUtils.createMethodInvokerByName(candidate, methodName, false, params); + } + else { + return null; + } + } + + public boolean isSingleton() { + return false; + } + + public void setDelegate(Object delegate) { + if (delegate instanceof Advised) { + try { + setDelegate(((Advised) delegate).getTargetSource().getTarget()); + } catch (Exception e) { + throw new IllegalStateException("Cannot generate listener for proxy with no target", e); + } + } + else { + this.delegate = delegate; + } + } + + public void setMetaDataMap(Map metaDataMap) { + this.metaDataMap = metaDataMap; + } + + /* + * Extension of HashSet that ignores nulls, rather than putting them into + * the set. + */ + protected static class NullIgnoringSet extends HashSet { + + @Override + public boolean add(E e) { + if (e == null) { + return false; + } + else { + return super.add(e); + } + }; + } + + public void afterPropertiesSet() throws Exception { + Assert.notNull(delegate, "Delegate must not be null"); + } + + /** + * Convenience method to check whether the given object is or can be made + * into a listener. + * + * @param delegate the object to check + * @return true if the delegate is an instance of any of the listener + * interface, or contains the marker annotations + */ + public static boolean isListener(Object delegate, Class listenerType, AbstractListenerMetaData[] metaDataValues) { + if (listenerType.isInstance(delegate)) { + return true; + } + if (delegate instanceof Advised) { + try { + return isListener(((Advised) delegate).getTargetSource().getTarget(), listenerType, metaDataValues); + } catch (Exception e) { + logger.debug("Error obtaining target for Proxy. Assume not a listener.", e); + return false; + } + } + for (AbstractListenerMetaData metaData : metaDataValues) { + if (MethodInvokerUtils.getMethodInvokerByAnnotation(metaData.getAnnotation(), delegate) != null) { + return true; + } + } + return false; + } +} diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/listener/AbstractListenerMetaData.java b/spring-batch-core/src/main/java/org/springframework/batch/core/listener/AbstractListenerMetaData.java new file mode 100644 index 000000000..359a89359 --- /dev/null +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/listener/AbstractListenerMetaData.java @@ -0,0 +1,38 @@ +/* + * Copyright 2002-2008 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.batch.core.listener; + +import java.lang.annotation.Annotation; + +/** + * A common interface for ListenerMetaData enumerations. + * + * @author Dan Garrette + * @since 2.0 + * @see JobListenerMetaData + * @see StepListenerMetaData + */ +public interface AbstractListenerMetaData { + + public String getMethodName(); + + public Class getAnnotation(); + + public String getPropertyName(); + + public Class[] getParamTypes(); + +} diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/listener/JobListenerFactoryBean.java b/spring-batch-core/src/main/java/org/springframework/batch/core/listener/JobListenerFactoryBean.java index 9fb962002..d7f4a83e1 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/listener/JobListenerFactoryBean.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/listener/JobListenerFactoryBean.java @@ -15,11 +15,9 @@ */ package org.springframework.batch.core.listener; -import static org.springframework.batch.support.MethodInvokerUtils.getMethodInvokerByAnnotation; import static org.springframework.batch.support.MethodInvokerUtils.getMethodInvokerForInterface; import java.util.HashMap; -import java.util.HashSet; import java.util.Map; import java.util.Set; import java.util.Map.Entry; @@ -29,76 +27,33 @@ import org.springframework.aop.support.DefaultPointcutAdvisor; import org.springframework.batch.core.JobExecution; import org.springframework.batch.core.JobExecutionListener; 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.Ordered; -import org.springframework.util.Assert; +import org.springframework.batch.core.listener.MethodInvokerMethodInterceptor; /** - * {@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: - * - *
    - *
  • Interface implementation: By implementing JobExecutionListener, methods - * on said interface will be called. - *
  • Annotations: Annotating a method will result in registration. - *
  • String name of the method to be called, which is tied to - * {@link JobListenerMetaData} in the metaDatMap. - *
- * - * 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. + * This {@link AbstractListenerFactoryBean} implementation is used to create a + * {@link JobExecutionListener}. * * @author Lucas Ward + * @author Dan Garrette * @since 2.0 + * @see AbstractListenerFactoryBean * @see JobListenerMetaData */ -public class JobListenerFactoryBean implements FactoryBean, InitializingBean { +public class JobListenerFactoryBean extends AbstractListenerFactoryBean { - private Object delegate; - - private Map metaDataMap; - - public void setDelegate(Object delegate) { - this.delegate = delegate; - } - - public void setMetaDataMap(Map metaDataMap) { - this.metaDataMap = metaDataMap; - } - - public Object getObject() { - Map> invokerMap = new HashMap>(); - if (metaDataMap == null) { - metaDataMap = new HashMap(); - } - // 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); - } - } + public Object doGetObject(Object delegate, Map metaDataMap) { // For every entry in the map, try and find a method by interface, name, // or annotation. If the same + Map> invokerMap = new HashMap>(); for (Entry entry : metaDataMap.entrySet()) { JobListenerMetaData metaData = JobListenerMetaData.fromPropertyName(entry.getKey()); Set invokers = new NullIgnoringSet(); - invokers.add(getMethodInvokerByName(entry.getValue(), delegate, JobExecution.class)); + invokers.add(getMethodInvokerByName(entry.getValue(), delegate, metaData.getParamTypes())); invokers.add(getMethodInvokerForInterface(JobExecutionListener.class, metaData.getMethodName(), delegate, JobExecution.class)); - invokers.add(getMethodInvokerByAnnotation(metaData.getAnnotation(), delegate)); + invokers.add(getMethodInvokerByAnnotation(metaData)); if (!invokers.isEmpty()) { invokerMap.put(metaData.getMethodName(), invokers); } @@ -108,7 +63,7 @@ public class JobListenerFactoryBean implements FactoryBean, InitializingBean { // be called ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.setTarget(delegate); - + boolean ordered = false; if (delegate instanceof Ordered) { ordered = true; @@ -120,13 +75,8 @@ public class JobListenerFactoryBean implements FactoryBean, InitializingBean { return proxyFactory.getProxy(); } - private MethodInvoker getMethodInvokerByName(String methodName, Object candidate, Class... params) { - if (methodName != null) { - return MethodInvokerUtils.createMethodInvokerByName(candidate, methodName, false, params); - } - else { - return null; - } + protected AbstractListenerMetaData[] getMetaDataValues() { + return JobListenerMetaData.values(); } @SuppressWarnings("unchecked") @@ -134,17 +84,10 @@ public class JobListenerFactoryBean implements FactoryBean, InitializingBean { return JobExecutionListener.class; } - public boolean isSingleton() { - return false; - } - - 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 */ @@ -157,38 +100,13 @@ public class JobListenerFactoryBean implements FactoryBean, InitializingBean { /** * 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 + * {@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 invokers = new NullIgnoringSet(); - 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 extends HashSet { - - @Override - public boolean add(E e) { - if (e == null) { - return false; - } - else { - return super.add(e); - } - }; + return AbstractListenerFactoryBean.isListener(delegate, JobExecutionListener.class, JobListenerMetaData + .values()); } } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/listener/JobListenerMetaData.java b/spring-batch-core/src/main/java/org/springframework/batch/core/listener/JobListenerMetaData.java index ba1c4c36f..dfb1645ec 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/listener/JobListenerMetaData.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/listener/JobListenerMetaData.java @@ -19,6 +19,7 @@ import java.lang.annotation.Annotation; import java.util.HashMap; import java.util.Map; +import org.springframework.batch.core.JobExecution; import org.springframework.batch.core.JobExecutionListener; import org.springframework.batch.core.annotation.AfterJob; import org.springframework.batch.core.annotation.BeforeJob; @@ -31,7 +32,7 @@ import org.springframework.batch.core.annotation.BeforeJob; * @since 2.0 * @see JobListenerFactoryBean */ -public enum JobListenerMetaData { +public enum JobListenerMetaData implements AbstractListenerMetaData { BEFORE_JOB("beforeJob", "before-job-method", BeforeJob.class), AFTER_JOB("afterJob", "after-job-method", AfterJob.class); @@ -63,11 +64,14 @@ public enum JobListenerMetaData { return annotation; } - public String getPropertyName() { return propertyName; } + public Class[] getParamTypes() { + return new Class[]{ JobExecution.class }; + } + /** * Return the relevant meta data for the provided property name. * diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/listener/StepListenerFactoryBean.java b/spring-batch-core/src/main/java/org/springframework/batch/core/listener/StepListenerFactoryBean.java index 0bebf4fea..f4724ff83 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/listener/StepListenerFactoryBean.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/listener/StepListenerFactoryBean.java @@ -16,92 +16,45 @@ package org.springframework.batch.core.listener; 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; import java.util.Set; import java.util.Map.Entry; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; -import org.springframework.aop.framework.Advised; import org.springframework.aop.framework.ProxyFactory; import org.springframework.aop.support.DefaultPointcutAdvisor; import org.springframework.batch.core.StepListener; 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.Ordered; -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} lifecycle: - * - *
    - *
  • Interface implementation: By implementing any of the subclasses of - * StepListener, methods on said interface will be called - *
  • Annotations: Annotating a method will result in registration. - *
  • String name of the method to be called, which is tied to - * {@link StepListenerMetaData} in the metaDatMap. - *
- * - * 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. + * This {@link AbstractListenerFactoryBean} implementation is used to create a + * {@link StepListener}. * * @author Lucas Ward + * @author Dan Garrette * @since 2.0 + * @see AbstractListenerFactoryBean * @see StepListenerMetaData */ -public class StepListenerFactoryBean implements FactoryBean, InitializingBean { +public class StepListenerFactoryBean extends AbstractListenerFactoryBean { - private static Log logger = LogFactory.getLog(StepListenerFactoryBean.class); - - private Object delegate; - - private Map metaDataMap; - - public Object getObject() { - - Map> invokerMap = new HashMap>(); - if (metaDataMap == null) { - metaDataMap = new HashMap(); - } - // 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); - } - } + public Object doGetObject(Object delegate, Map metaDataMap) { Set> listenerInterfaces = new HashSet>(); // For every entry in the map, try and find a method by interface, name, // or annotation. If the same + Map> invokerMap = new HashMap>(); for (Entry entry : metaDataMap.entrySet()) { final StepListenerMetaData metaData = StepListenerMetaData.fromPropertyName(entry.getKey()); Set invokers = new NullIgnoringSet(); invokers.add(getMethodInvokerByName(entry.getValue(), delegate, metaData.getParamTypes())); invokers.add(getMethodInvokerForInterface(metaData.getListenerInterface(), metaData.getMethodName(), delegate, metaData.getParamTypes())); - invokers.add(getMethodInvokerByAnnotation(metaData, metaData.getAnnotation())); + invokers.add(getMethodInvokerByAnnotation(metaData)); if (!invokers.isEmpty()) { invokerMap.put(metaData.getMethodName(), invokers); listenerInterfaces.add(metaData.getListenerInterface()); @@ -127,50 +80,8 @@ 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 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); - } - else { - return null; - } + protected AbstractListenerMetaData[] getMetaDataValues() { + return StepListenerMetaData.values(); } @SuppressWarnings("unchecked") @@ -178,28 +89,6 @@ public class StepListenerFactoryBean implements FactoryBean, InitializingBean { return StepListener.class; } - public boolean isSingleton() { - return false; - } - - public void setDelegate(Object delegate) { - if (delegate instanceof Advised) { - try { - setDelegate(((Advised) delegate).getTargetSource().getTarget()); - } - catch (Exception e) { - throw new IllegalStateException("Cannot generate listener for proxy with no target", e); - } - } - else { - this.delegate = delegate; - } - } - - public void setMetaDataMap(Map metaDataMap) { - this.metaDataMap = metaDataMap; - } - /** * Convenience method to wrap any object and expose the appropriate * {@link StepListener} interfaces. @@ -222,44 +111,6 @@ public class StepListenerFactoryBean implements FactoryBean, InitializingBean { * {@link StepListener} interfaces, or contains the marker annotations */ public static boolean isListener(Object delegate) { - if (delegate instanceof StepListener) { - return true; - } - if (delegate instanceof Advised) { - try { - return isListener(((Advised) delegate).getTargetSource().getTarget()); - } - catch (Exception e) { - logger.debug("Error obtaining target for Proxy. Assume not a listener.", e); - return false; - } - } - for (StepListenerMetaData metaData : StepListenerMetaData.values()) { - if (MethodInvokerUtils.getMethodInvokerByAnnotation(metaData.getAnnotation(), delegate) != null) { - return true; - } - } - return false; - } - - /* - * Extension of HashSet that ignores nulls, rather than putting them into - * the set. - */ - private static class NullIgnoringSet extends HashSet { - - @Override - public boolean add(E e) { - if (e == null) { - return false; - } - else { - return super.add(e); - } - }; - } - - public void afterPropertiesSet() throws Exception { - Assert.notNull(delegate, "Delegate must not be null"); + return AbstractListenerFactoryBean.isListener(delegate, StepListener.class, StepListenerMetaData.values()); } } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/listener/StepListenerMetaData.java b/spring-batch-core/src/main/java/org/springframework/batch/core/listener/StepListenerMetaData.java index 4806b01b8..1acc5683f 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/listener/StepListenerMetaData.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/listener/StepListenerMetaData.java @@ -53,7 +53,7 @@ import org.springframework.batch.core.annotation.OnWriteError; * @since 2.0 * @see StepListenerFactoryBean */ -public enum StepListenerMetaData { +public enum StepListenerMetaData implements AbstractListenerMetaData { BEFORE_STEP("beforeStep", "before-step-method", BeforeStep.class, StepExecutionListener.class, StepExecution.class), AFTER_STEP("afterStep", "after-step-method", AfterStep.class, StepExecutionListener.class, StepExecution.class), diff --git a/spring-batch-core/src/main/resources/org/springframework/batch/core/configuration/xml/spring-batch-2.0.xsd b/spring-batch-core/src/main/resources/org/springframework/batch/core/configuration/xml/spring-batch-2.0.xsd index 47dcf5111..4ffecceea 100644 --- a/spring-batch-core/src/main/resources/org/springframework/batch/core/configuration/xml/spring-batch-2.0.xsd +++ b/spring-batch-core/src/main/resources/org/springframework/batch/core/configuration/xml/spring-batch-2.0.xsd @@ -229,7 +229,8 @@ - A reference to a JobExecutionListener or a POJO (with before-method / after-method). + A reference to a JobExecutionListener or a POJO + (with before-job-method / after-job-method). @@ -248,8 +249,8 @@ - - + + diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/listener/JobListenerFactoryBeanTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/listener/JobListenerFactoryBeanTests.java index 756e5e9ff..4cf2b0916 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/listener/JobListenerFactoryBeanTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/listener/JobListenerFactoryBeanTests.java @@ -17,8 +17,11 @@ package org.springframework.batch.core.listener; import static junit.framework.Assert.assertTrue; import static org.junit.Assert.assertEquals; +import static org.springframework.batch.core.listener.JobListenerMetaData.AFTER_JOB; +import java.util.HashMap; import java.util.HashSet; +import java.util.Map; import java.util.Set; import org.junit.Before; @@ -28,6 +31,7 @@ import org.springframework.batch.core.JobExecution; import org.springframework.batch.core.JobExecutionListener; import org.springframework.batch.core.annotation.AfterJob; import org.springframework.batch.core.annotation.BeforeJob; +import org.springframework.batch.core.configuration.xml.AbstractTestComponent; import org.springframework.core.Ordered; /** @@ -138,6 +142,100 @@ public class JobListenerFactoryBeanTests { assertEquals(listener1, listener2); } + @Test + public void testEmptySignatureAnnotation() { + AbstractTestComponent delegate = new AbstractTestComponent() { + @SuppressWarnings("unused") + @AfterJob + public void aMethod() { + executed = true; + } + }; + factoryBean.setDelegate(delegate); + JobExecutionListener listener = (JobExecutionListener) factoryBean.getObject(); + listener.afterJob(new JobExecution(1L)); + assertTrue(delegate.isExecuted()); + } + + @Test + public void testRightSignatureAnnotation() { + AbstractTestComponent delegate = new AbstractTestComponent() { + @SuppressWarnings("unused") + @AfterJob + public void aMethod(JobExecution jobExecution) { + executed = true; + assertEquals(new Long(25), jobExecution.getId()); + } + }; + factoryBean.setDelegate(delegate); + JobExecutionListener listener = (JobExecutionListener) factoryBean.getObject(); + listener.afterJob(new JobExecution(25L)); + assertTrue(delegate.isExecuted()); + } + + @Test(expected = IllegalArgumentException.class) + public void testWrongSignatureAnnotation() { + AbstractTestComponent delegate = new AbstractTestComponent() { + @SuppressWarnings("unused") + @AfterJob + public void aMethod(Integer item) { + executed = true; + } + }; + factoryBean.setDelegate(delegate); + factoryBean.getObject(); + } + + @Test + public void testEmptySignatureNamedMethod() { + AbstractTestComponent delegate = new AbstractTestComponent() { + @SuppressWarnings("unused") + public void aMethod() { + executed = true; + } + }; + factoryBean.setDelegate(delegate); + Map metaDataMap = new HashMap(); + metaDataMap.put(AFTER_JOB.getPropertyName(), "aMethod"); + factoryBean.setMetaDataMap(metaDataMap); + JobExecutionListener listener = (JobExecutionListener) factoryBean.getObject(); + listener.afterJob(new JobExecution(1L)); + assertTrue(delegate.isExecuted()); + } + + @Test + public void testRightSignatureNamedMethod() { + AbstractTestComponent delegate = new AbstractTestComponent() { + @SuppressWarnings("unused") + public void aMethod(JobExecution jobExecution) { + executed = true; + assertEquals(new Long(25), jobExecution.getId()); + } + }; + factoryBean.setDelegate(delegate); + Map metaDataMap = new HashMap(); + metaDataMap.put(AFTER_JOB.getPropertyName(), "aMethod"); + factoryBean.setMetaDataMap(metaDataMap); + JobExecutionListener listener = (JobExecutionListener) factoryBean.getObject(); + listener.afterJob(new JobExecution(25L)); + assertTrue(delegate.isExecuted()); + } + + @Test(expected = IllegalArgumentException.class) + public void testWrongSignatureNamedMethod() { + AbstractTestComponent delegate = new AbstractTestComponent() { + @SuppressWarnings("unused") + public void aMethod(Integer item) { + executed = true; + } + }; + factoryBean.setDelegate(delegate); + Map metaDataMap = new HashMap(); + metaDataMap.put(AFTER_JOB.getPropertyName(), "aMethod"); + factoryBean.setMetaDataMap(metaDataMap); + factoryBean.getObject(); + } + private class JobListenerWithInterface implements JobExecutionListener { boolean beforeJobCalled = false; diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/listener/StepListenerFactoryBeanTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/listener/StepListenerFactoryBeanTests.java index 1a3e47152..83e7eb9b9 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/listener/StepListenerFactoryBeanTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/listener/StepListenerFactoryBeanTests.java @@ -311,7 +311,6 @@ public class StepListenerFactoryBeanTests { public void testEmptySignatureNamedMethod() { AbstractTestComponent delegate = new AbstractTestComponent() { @SuppressWarnings("unused") - @AfterWrite public void aMethod() { executed = true; } @@ -330,7 +329,6 @@ public class StepListenerFactoryBeanTests { public void testRightSignatureNamedMethod() { AbstractTestComponent delegate = new AbstractTestComponent() { @SuppressWarnings("unused") - @AfterWrite public void aMethod(List items) { executed = true; assertEquals("foo", items.get(0)); @@ -351,7 +349,6 @@ public class StepListenerFactoryBeanTests { public void testWrongSignatureNamedMethod() { AbstractTestComponent delegate = new AbstractTestComponent() { @SuppressWarnings("unused") - @AfterWrite public void aMethod(Integer item) { executed = true; } diff --git a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/JobExecutionListenerParserTests-context.xml b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/JobExecutionListenerParserTests-context.xml index 0d759ae09..5d449a1a2 100644 --- a/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/JobExecutionListenerParserTests-context.xml +++ b/spring-batch-core/src/test/resources/org/springframework/batch/core/configuration/xml/JobExecutionListenerParserTests-context.xml @@ -9,8 +9,8 @@ - - + +