From d4123d06379fadc0b05043f5ab5a1a2cb264314b Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Sun, 11 Dec 2011 13:00:30 +0000 Subject: [PATCH] Support use of @Scheduled against JDK proxies Prior to this change, ScheduledAnnotationBeanPostProcessor found any @Scheduled methods against the ultimate targetClass for a given bean and then attempted to invoke that method against the bean instance. In cases where the bean instance was in fact a JDK proxy, this attempt would fail because the proxy is not an instance of the target class. Now SABPP still attempts to find @Scheduled methods against the target class, but subsequently checks to see if the bean is a JDK proxy, and if so attempts to find the corresponding method on the proxy itself. If it cannot be found (e.g. the @Scheduled method was declared only at the concrete class level), an appropriate exception is thrown, explaining to the users their options: (a) use proxyTargetClass=true and go with subclass proxies which won't have this problem, or (b) pull the @Scheduled method up into an interface. Issue: SPR-8651 --- .../ScheduledAnnotationBeanPostProcessor.java | 18 +- ...ansactionalAnnotationIntegrationTests.java | 182 ++++++++++++++++++ 2 files changed, 199 insertions(+), 1 deletion(-) create mode 100644 org.springframework.integration-tests/src/test/java/org/springframework/scheduling/annotation/ScheduledAndTransactionalAnnotationIntegrationTests.java diff --git a/org.springframework.context/src/main/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessor.java b/org.springframework.context/src/main/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessor.java index ec06fc0fe4..692270044c 100644 --- a/org.springframework.context/src/main/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessor.java +++ b/org.springframework.context/src/main/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessor.java @@ -110,7 +110,7 @@ public class ScheduledAnnotationBeanPostProcessor } public Object postProcessAfterInitialization(final Object bean, String beanName) { - Class targetClass = AopUtils.getTargetClass(bean); + final Class targetClass = AopUtils.getTargetClass(bean); ReflectionUtils.doWithMethods(targetClass, new MethodCallback() { public void doWith(Method method) throws IllegalArgumentException, IllegalAccessException { Scheduled annotation = AnnotationUtils.getAnnotation(method, Scheduled.class); @@ -119,6 +119,22 @@ public class ScheduledAnnotationBeanPostProcessor "Only void-returning methods may be annotated with @Scheduled."); Assert.isTrue(method.getParameterTypes().length == 0, "Only no-arg methods may be annotated with @Scheduled."); + if (AopUtils.isJdkDynamicProxy(bean)) { + try { + // found a @Scheduled method on the target class for this JDK proxy -> is it + // also present on the proxy itself? + method = bean.getClass().getMethod(method.getName(), method.getParameterTypes()); + } catch (SecurityException ex) { + ReflectionUtils.handleReflectionException(ex); + } catch (NoSuchMethodException ex) { + throw new IllegalStateException(String.format( + "@Scheduled method '%s' found on bean target class '%s', " + + "but not found in any interface(s) for bean JDK proxy. Either " + + "pull the method up to an interface or switch to subclass (CGLIB) " + + "proxies by setting proxy-target-class/proxyTargetClass " + + "attribute to 'true'", method.getName(), targetClass.getSimpleName())); + } + } Runnable runnable = new ScheduledMethodRunnable(bean, method); boolean processedSchedule = false; String errorMessage = "Exactly one of 'cron', 'fixedDelay', or 'fixedRate' is required."; diff --git a/org.springframework.integration-tests/src/test/java/org/springframework/scheduling/annotation/ScheduledAndTransactionalAnnotationIntegrationTests.java b/org.springframework.integration-tests/src/test/java/org/springframework/scheduling/annotation/ScheduledAndTransactionalAnnotationIntegrationTests.java new file mode 100644 index 0000000000..0750058583 --- /dev/null +++ b/org.springframework.integration-tests/src/test/java/org/springframework/scheduling/annotation/ScheduledAndTransactionalAnnotationIntegrationTests.java @@ -0,0 +1,182 @@ +/* + * Copyright 2002-2011 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.scheduling.annotation; + +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.replay; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.Matchers.greaterThan; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.util.concurrent.atomic.AtomicInteger; + +import org.junit.Test; +import org.springframework.aop.support.AopUtils; +import org.springframework.beans.factory.BeanCreationException; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.dao.annotation.PersistenceExceptionTranslationPostProcessor; +import org.springframework.dao.support.PersistenceExceptionTranslator; +import org.springframework.scheduling.annotation.EnableScheduling; +import org.springframework.stereotype.Repository; +import org.springframework.transaction.CallCountingTransactionManager; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.annotation.EnableTransactionManagement; +import org.springframework.transaction.annotation.Transactional; + +/** + * Integration tests cornering bug SPR-8651, which revealed that @Scheduled methods may + * not work well with beans that have already been proxied for other reasons such + * as @Transactional or @Async processing. + * + * @author Chris Beams + * @since 3.1 + */ +public class ScheduledAndTransactionalAnnotationIntegrationTests { + + @Test + public void failsWhenJdkProxyAndScheduledMethodNotPresentOnInterface() { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.register(Config.class, JdkProxyTxConfig.class, RepoConfigA.class); + try { + ctx.refresh(); + fail("expected exception"); + } catch (BeanCreationException ex) { + assertTrue(ex.getRootCause().getMessage().startsWith("@Scheduled method 'scheduled' found")); + } + } + + @Test + public void succeedsWhenSubclassProxyAndScheduledMethodNotPresentOnInterface() throws InterruptedException { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.register(Config.class, SubclassProxyTxConfig.class, RepoConfigA.class); + ctx.refresh(); + + Thread.sleep(10); // allow @Scheduled method to be called several times + + MyRepository repository = ctx.getBean(MyRepository.class); + CallCountingTransactionManager txManager = ctx.getBean(CallCountingTransactionManager.class); + assertThat("repository is not a proxy", AopUtils.isAopProxy(repository), is(true)); + assertThat("@Scheduled method never called", repository.getInvocationCount(), greaterThan(0)); + assertThat("no transactions were committed", txManager.commits, greaterThan(0)); + } + + @Test + public void succeedsWhenJdkProxyAndScheduledMethodIsPresentOnInterface() throws InterruptedException { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.register(Config.class, JdkProxyTxConfig.class, RepoConfigB.class); + ctx.refresh(); + + Thread.sleep(10); // allow @Scheduled method to be called several times + + MyRepositoryWithScheduledMethod repository = ctx.getBean(MyRepositoryWithScheduledMethod.class); + CallCountingTransactionManager txManager = ctx.getBean(CallCountingTransactionManager.class); + assertThat("repository is not a proxy", AopUtils.isAopProxy(repository), is(true)); + assertThat("@Scheduled method never called", repository.getInvocationCount(), greaterThan(0)); + assertThat("no transactions were committed", txManager.commits, greaterThan(0)); + } + + @Configuration + @EnableTransactionManagement + static class JdkProxyTxConfig { } + + @Configuration + @EnableTransactionManagement(proxyTargetClass=true) + static class SubclassProxyTxConfig { } + + @Configuration + static class RepoConfigA { + @Bean + public MyRepository repository() { + return new MyRepositoryImpl(); + } + } + + @Configuration + static class RepoConfigB { + @Bean + public MyRepositoryWithScheduledMethod repository() { + return new MyRepositoryWithScheduledMethodImpl(); + } + } + + @Configuration + @EnableScheduling + static class Config { + + @Bean + public PersistenceExceptionTranslationPostProcessor peTranslationPostProcessor() { + return new PersistenceExceptionTranslationPostProcessor(); + } + + @Bean + public PlatformTransactionManager txManager() { + return new CallCountingTransactionManager(); + } + + @Bean + public PersistenceExceptionTranslator peTranslator() { + PersistenceExceptionTranslator txlator = createMock(PersistenceExceptionTranslator.class); + replay(txlator); + return txlator; + } + } + + public interface MyRepository { + int getInvocationCount(); + } + + @Repository + static class MyRepositoryImpl implements MyRepository { + + private final AtomicInteger count = new AtomicInteger(0); + + @Transactional + @Scheduled(fixedDelay = 5) + public void scheduled() { + this.count.incrementAndGet(); + } + + public int getInvocationCount() { + return this.count.get(); + } + } + + public interface MyRepositoryWithScheduledMethod { + int getInvocationCount(); + public void scheduled(); + } + + @Repository + static class MyRepositoryWithScheduledMethodImpl implements MyRepositoryWithScheduledMethod { + + private final AtomicInteger count = new AtomicInteger(0); + + @Transactional + @Scheduled(fixedDelay = 5) + public void scheduled() { + this.count.incrementAndGet(); + } + + public int getInvocationCount() { + return this.count.get(); + } + } +} \ No newline at end of file