From 371919841899aed0911f80756aa19d5da67fd2e1 Mon Sep 17 00:00:00 2001 From: dsyer Date: Tue, 10 Mar 2009 14:32:34 +0000 Subject: [PATCH] OPEN - issue BATCH-1130: Ensure Ordered is respected by generated listeners Fixed for Step listeners --- .../MethodInvokerMethodInterceptor.java | 11 + .../listener/StepListenerFactoryBean.java | 41 ++- .../StepListenerFactoryBeanTests.java | 254 ++++++++++++------ 3 files changed, 215 insertions(+), 91 deletions(-) diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/listener/MethodInvokerMethodInterceptor.java b/spring-batch-core/src/main/java/org/springframework/batch/core/listener/MethodInvokerMethodInterceptor.java index 8ed87fd6f..7d8d39b9d 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/listener/MethodInvokerMethodInterceptor.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/listener/MethodInvokerMethodInterceptor.java @@ -40,14 +40,24 @@ import org.springframework.batch.support.MethodInvoker; public class MethodInvokerMethodInterceptor implements MethodInterceptor { private final Map> invokerMap; + private final boolean ordered; public MethodInvokerMethodInterceptor(Map> invokerMap) { + this(invokerMap, false); + } + + public MethodInvokerMethodInterceptor(Map> invokerMap, boolean ordered) { + this.ordered = ordered; this.invokerMap = invokerMap; } public Object invoke(MethodInvocation invocation) throws Throwable { String methodName = invocation.getMethod().getName(); + if (ordered && methodName.equals("getOrder")) { + return invocation.proceed(); + } + Set invokers = invokerMap.get(methodName); if (invokers == null) { @@ -66,6 +76,7 @@ public class MethodInvokerMethodInterceptor implements MethodInterceptor { } } + // The only possible return values are ExitStatus or int (from Ordered) return status; } 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 e7c51f498..0bebf4fea 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 @@ -26,6 +26,9 @@ 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; @@ -33,6 +36,7 @@ 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; @@ -65,6 +69,8 @@ import org.springframework.util.ReflectionUtils; */ public class StepListenerFactoryBean implements FactoryBean, InitializingBean { + private static Log logger = LogFactory.getLog(StepListenerFactoryBean.class); + private Object delegate; private Map metaDataMap; @@ -85,7 +91,7 @@ public class StepListenerFactoryBean implements FactoryBean, InitializingBean { } } - Set> listenerInterfaces = new HashSet>(); + Set> listenerInterfaces = new HashSet>(); // For every entry in the map, try and find a method by interface, name, // or annotation. If the same @@ -106,11 +112,18 @@ public class StepListenerFactoryBean implements FactoryBean, InitializingBean { listenerInterfaces.add(StepListener.class); } + boolean ordered = false; + if (delegate instanceof Ordered) { + ordered = true; + listenerInterfaces.add(Ordered.class); + } + // create a proxy listener for only the interfaces that have methods to // be called ProxyFactory proxyFactory = new ProxyFactory(); + proxyFactory.setTarget(delegate); proxyFactory.setInterfaces(listenerInterfaces.toArray(new Class[0])); - proxyFactory.addAdvisor(new DefaultPointcutAdvisor(new MethodInvokerMethodInterceptor(invokerMap))); + proxyFactory.addAdvisor(new DefaultPointcutAdvisor(new MethodInvokerMethodInterceptor(invokerMap, ordered))); return proxyFactory.getProxy(); } @@ -170,7 +183,17 @@ public class StepListenerFactoryBean implements FactoryBean, InitializingBean { } public void setDelegate(Object delegate) { - this.delegate = 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) { @@ -196,13 +219,21 @@ public class StepListenerFactoryBean implements FactoryBean, InitializingBean { * * @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; } + 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; 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 34f215966..1a3e47152 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 @@ -30,6 +30,7 @@ import java.util.Map; import org.junit.Before; import org.junit.Test; +import org.springframework.aop.framework.ProxyFactory; import org.springframework.batch.core.ChunkListener; import org.springframework.batch.core.ExitStatus; import org.springframework.batch.core.ItemProcessListener; @@ -53,54 +54,60 @@ 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.beans.factory.InitializingBean; +import org.springframework.core.Ordered; import org.springframework.util.Assert; /** * @author Lucas Ward - * + * */ public class StepListenerFactoryBeanTests { private StepListenerFactoryBean factoryBean; + private TestListener testListener; - private JobExecution jobExecution = new JobExecution(11L); + + private JobExecution jobExecution = new JobExecution(11L); + private StepExecution stepExecution = new StepExecution("testStep", jobExecution); - + @Before - public void setUp(){ + public void setUp() { factoryBean = new StepListenerFactoryBean(); testListener = new TestListener(); } - + @Test @SuppressWarnings("unchecked") - public void testStepAndChunk() throws Exception{ - + public void testStepAndChunk() throws Exception { + factoryBean.setDelegate(testListener); - Map metaDataMap = new HashMap();; + Map metaDataMap = new HashMap(); + ; metaDataMap.put(AFTER_STEP.getPropertyName(), "destroy"); metaDataMap.put(AFTER_CHUNK.getPropertyName(), "afterChunk"); factoryBean.setMetaDataMap(metaDataMap); Object item = new Object(); List items = new ArrayList(); items.add(item); - StepListener listener = (StepListener) factoryBean.getObject(); - ((StepExecutionListener)listener).beforeStep(stepExecution); - ((StepExecutionListener)listener).afterStep(stepExecution); - ((ChunkListener)listener).beforeChunk(); - ((ChunkListener)listener).afterChunk(); - ((ItemReadListener)listener).beforeRead(); - ((ItemReadListener)listener).afterRead(item); - ((ItemReadListener)listener).onReadError(new Exception()); - ((ItemProcessListener)listener).beforeProcess(item); - ((ItemProcessListener)listener).afterProcess(item, item); - ((ItemProcessListener)listener).onProcessError(item, new Exception()); - ((ItemWriteListener)listener).beforeWrite(items); - ((ItemWriteListener)listener).afterWrite(items); - ((ItemWriteListener)listener).onWriteError(new Exception(), items); - ((SkipListener)listener).onSkipInRead(new Throwable()); - ((SkipListener)listener).onSkipInProcess(item, new Throwable()); - ((SkipListener)listener).onSkipInWrite(item, new Throwable()); + StepListener listener = (StepListener) factoryBean.getObject(); + ((StepExecutionListener) listener).beforeStep(stepExecution); + ((StepExecutionListener) listener).afterStep(stepExecution); + ((ChunkListener) listener).beforeChunk(); + ((ChunkListener) listener).afterChunk(); + ((ItemReadListener) listener).beforeRead(); + ((ItemReadListener) listener).afterRead(item); + ((ItemReadListener) listener).onReadError(new Exception()); + ((ItemProcessListener) listener).beforeProcess(item); + ((ItemProcessListener) listener).afterProcess(item, item); + ((ItemProcessListener) listener).onProcessError(item, new Exception()); + ((ItemWriteListener) listener).beforeWrite(items); + ((ItemWriteListener) listener).afterWrite(items); + ((ItemWriteListener) listener).onWriteError(new Exception(), items); + ((SkipListener) listener).onSkipInRead(new Throwable()); + ((SkipListener) listener).onSkipInProcess(item, new Throwable()); + ((SkipListener) listener).onSkipInWrite(item, new Throwable()); assertTrue(testListener.beforeStepCalled); assertTrue(testListener.beforeChunkCalled); assertTrue(testListener.afterChunkCalled); @@ -117,58 +124,104 @@ public class StepListenerFactoryBeanTests { assertTrue(testListener.onSkipInProcessCalled); assertTrue(testListener.onSkipInWriteCalled); } - + @Test - public void testAllThreeTypes() throws Exception{ - //Test to make sure if someone has annotated a method, implemented the interface, and given a string - //method name, that all three will be called + public void testAllThreeTypes() throws Exception { + // Test to make sure if someone has annotated a method, implemented the + // interface, and given a string + // method name, that all three will be called ThreeStepExecutionListener delegate = new ThreeStepExecutionListener(); factoryBean.setDelegate(delegate); - Map metaDataMap = new HashMap();; + Map metaDataMap = new HashMap(); + ; metaDataMap.put(AFTER_STEP.getPropertyName(), "destroy"); factoryBean.setMetaDataMap(metaDataMap); StepListener listener = (StepListener) factoryBean.getObject(); - ((StepExecutionListener)listener).afterStep(stepExecution); + ((StepExecutionListener) listener).afterStep(stepExecution); assertEquals(3, delegate.callcount); } - + @Test - public void testAnnotatingInterfaceResultsInOneCall() throws Exception{ + public void testAnnotatingInterfaceResultsInOneCall() throws Exception { MultipleAfterStep delegate = new MultipleAfterStep(); factoryBean.setDelegate(delegate); Map metaDataMap = new HashMap(); metaDataMap.put(AFTER_STEP.getPropertyName(), "afterStep"); factoryBean.setMetaDataMap(metaDataMap); StepListener listener = (StepListener) factoryBean.getObject(); - ((StepExecutionListener)listener).afterStep(stepExecution); + ((StepExecutionListener) listener).afterStep(stepExecution); assertEquals(1, delegate.callcount); } - + @Test - public void testVanillaInterface() throws Exception{ + public void testVanillaInterface() throws Exception { MultipleAfterStep delegate = new MultipleAfterStep(); factoryBean.setDelegate(delegate); Object listener = factoryBean.getObject(); assertTrue(listener instanceof StepExecutionListener); - ((StepExecutionListener)listener).beforeStep(stepExecution); + ((StepExecutionListener) listener).beforeStep(stepExecution); assertEquals(1, delegate.callcount); } - + @Test - public void testFactoryMethod() throws Exception{ + public void testVanillaInterfaceWithProxy() throws Exception { + MultipleAfterStep delegate = new MultipleAfterStep(); + ProxyFactory factory = new ProxyFactory(delegate); + factoryBean.setDelegate(factory.getProxy()); + 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); + ((StepExecutionListener) listener).beforeStep(stepExecution); assertEquals(1, delegate.callcount); } - + + @Test + public void testAnnotationsWithOrdered() throws Exception { + Object delegate = new Ordered() { + @SuppressWarnings("unused") + @BeforeStep + public void foo(StepExecution execution) { + } + + public int getOrder() { + return 3; + } + }; + StepListener listener = StepListenerFactoryBean.getListener(delegate); + assertTrue("Listener is not of correct type", listener instanceof Ordered); + assertEquals(3, ((Ordered) listener).getOrder()); + } + + @Test + public void testProxiedAnnotationsFactoryMethod() throws Exception { + Object delegate = new InitializingBean() { + @SuppressWarnings("unused") + @BeforeStep + public void foo(StepExecution execution) { + } + + public void afterPropertiesSet() throws Exception { + } + }; + ProxyFactory factory = new ProxyFactory(delegate); + assertTrue("Listener is not of correct type", + StepListenerFactoryBean.getListener(factory.getProxy()) instanceof StepExecutionListener); + } + @Test public void testInterfaceIsListener() throws Exception { assertTrue(StepListenerFactoryBean.isListener(new ThreeStepExecutionListener())); } - + @Test public void testAnnotationsIsListener() throws Exception { assertTrue(StepListenerFactoryBean.isListener(new Object() { @@ -178,20 +231,35 @@ public class StepListenerFactoryBeanTests { } })); } - + + @Test + public void testProxiedAnnotationsIsListener() throws Exception { + Object delegate = new InitializingBean() { + @SuppressWarnings("unused") + @BeforeStep + public void foo(StepExecution execution) { + } + + public void afterPropertiesSet() throws Exception { + } + }; + ProxyFactory factory = new ProxyFactory(delegate); + assertTrue(StepListenerFactoryBean.isListener(factory.getProxy())); + } + @Test public void testMixedIsListener() throws Exception { assertTrue(StepListenerFactoryBean.isListener(new MultipleAfterStep())); } - + @Test - public void testNonListener() throws Exception{ + public void testNonListener() throws Exception { Object delegate = new Object(); factoryBean.setDelegate(delegate); StepListener listener = (StepListener) factoryBean.getObject(); assertTrue(listener instanceof StepListener); } - + @Test public void testEmptySignatureAnnotation() { AbstractTestComponent delegate = new AbstractTestComponent() { @@ -298,7 +366,7 @@ public class StepListenerFactoryBeanTests { private class MultipleAfterStep implements StepExecutionListener { int callcount = 0; - + @AfterStep public ExitStatus afterStep(StepExecution stepExecution) { Assert.notNull(stepExecution); @@ -309,118 +377,132 @@ public class StepListenerFactoryBeanTests { public void beforeStep(StepExecution stepExecution) { callcount++; } - - + } - - private class ThreeStepExecutionListener implements StepExecutionListener{ + + private class ThreeStepExecutionListener implements StepExecutionListener { int callcount = 0; - + public ExitStatus afterStep(StepExecution stepExecution) { Assert.notNull(stepExecution); callcount++; return null; } - + public void beforeStep(StepExecution stepExecution) { callcount++; } - - public void destroy(){ + + public void destroy() { callcount++; } - + @AfterStep - public void after(){ + public void after() { callcount++; } } - - private class TestListener implements SkipListener{ + + private class TestListener implements SkipListener { boolean beforeStepCalled = false; + boolean afterStepCalled = false; + boolean beforeChunkCalled = false; + boolean afterChunkCalled = false; + boolean beforeReadCalled = false; + boolean afterReadCalled = false; + boolean onReadErrorCalled = false; + boolean beforeProcessCalled = false; + boolean afterProcessCalled = false; + boolean onProcessErrorCalled = false; + boolean beforeWriteCalled = false; + boolean afterWriteCalled = false; + boolean onWriteErrorCalled = false; + boolean onSkipInReadCalled = false; + boolean onSkipInProcessCalled = false; + boolean onSkipInWriteCalled = false; - + @BeforeStep - public void initStep(){ + public void initStep() { beforeStepCalled = true; } - - public void destroy(){ + + public void destroy() { afterStepCalled = true; } - + @BeforeChunk - public void before(){ + public void before() { beforeChunkCalled = true; } - - public void afterChunk(){ + + public void afterChunk() { afterChunkCalled = true; } - + @BeforeRead - public void beforeReadMethod(){ + public void beforeReadMethod() { beforeReadCalled = true; } - + @AfterRead - public void afterReadMethod(Object item){ + public void afterReadMethod(Object item) { Assert.notNull(item); afterReadCalled = true; } - + @OnReadError - public void onErrorInRead(){ + public void onErrorInRead() { onReadErrorCalled = true; } - + @BeforeProcess - public void beforeProcess(){ + public void beforeProcess() { beforeProcessCalled = true; } - + @AfterProcess - public void afterProcess(){ + public void afterProcess() { afterProcessCalled = true; } - + @OnProcessError - public void processError(){ + public void processError() { onProcessErrorCalled = true; } - + @BeforeWrite - public void beforeWrite(){ + public void beforeWrite() { beforeWriteCalled = true; } - + @AfterWrite - public void afterWrite(){ + public void afterWrite() { afterWriteCalled = true; } @OnWriteError - public void writeError(){ + public void writeError() { onWriteErrorCalled = true; } - + public void onSkipInProcess(Object item, Throwable t) { onSkipInProcessCalled = true; } @@ -432,6 +514,6 @@ public class StepListenerFactoryBeanTests { public void onSkipInWrite(Object item, Throwable t) { onSkipInWriteCalled = true; } - + } }