From fd21e0e69aceb0c1f2f2296f282e49d39d8f6ee7 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 29 Dec 2016 22:35:10 +0100 Subject: [PATCH] @Scheduled reliably applies after other post-processors and shuts down before TaskScheduler Issue: SPR-14692 Issue: SPR-15067 (cherry picked from commit edc62be) --- .../support/DefaultListableBeanFactory.java | 2 +- .../ScheduledAnnotationBeanPostProcessor.java | 54 +++++++++++--- .../annotation/EnableSchedulingTests.java | 10 +-- ...msListenerAnnotationBeanPostProcessor.java | 12 +++- ...ansactionalAnnotationIntegrationTests.java | 71 ++++++++++++++++--- 5 files changed, 123 insertions(+), 26 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java index b4e29ec6f8f..ed6de655681 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java @@ -993,7 +993,7 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto if (parent instanceof AutowireCapableBeanFactory) { return ((AutowireCapableBeanFactory) parent).resolveNamedBean(requiredType); } - return null; + throw new NoSuchBeanDefinitionException(requiredType); } @SuppressWarnings("unchecked") diff --git a/spring-context/src/main/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessor.java b/spring-context/src/main/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessor.java index 7b5cb34f6f0..661754ed3e2 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessor.java +++ b/spring-context/src/main/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessor.java @@ -33,12 +33,18 @@ import org.apache.commons.logging.LogFactory; import org.springframework.aop.support.AopUtils; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; +import org.springframework.beans.factory.BeanNameAware; import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.ListableBeanFactory; import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.NoUniqueBeanDefinitionException; import org.springframework.beans.factory.SmartInitializingSingleton; +import org.springframework.beans.factory.config.AutowireCapableBeanFactory; +import org.springframework.beans.factory.config.ConfigurableBeanFactory; import org.springframework.beans.factory.config.DestructionAwareBeanPostProcessor; +import org.springframework.beans.factory.config.NamedBeanHolder; +import org.springframework.beans.factory.support.MergedBeanDefinitionPostProcessor; +import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.context.ApplicationListener; @@ -85,8 +91,9 @@ import org.springframework.util.StringValueResolver; * @see org.springframework.scheduling.config.ScheduledTaskRegistrar * @see AsyncAnnotationBeanPostProcessor */ -public class ScheduledAnnotationBeanPostProcessor implements DestructionAwareBeanPostProcessor, - Ordered, EmbeddedValueResolverAware, BeanFactoryAware, ApplicationContextAware, +public class ScheduledAnnotationBeanPostProcessor + implements MergedBeanDefinitionPostProcessor, DestructionAwareBeanPostProcessor, + Ordered, EmbeddedValueResolverAware, BeanNameAware, BeanFactoryAware, ApplicationContextAware, SmartInitializingSingleton, ApplicationListener, DisposableBean { /** @@ -104,6 +111,8 @@ public class ScheduledAnnotationBeanPostProcessor implements DestructionAwareBea private StringValueResolver embeddedValueResolver; + private String beanName; + private BeanFactory beanFactory; private ApplicationContext applicationContext; @@ -142,6 +151,11 @@ public class ScheduledAnnotationBeanPostProcessor implements DestructionAwareBea this.embeddedValueResolver = resolver; } + @Override + public void setBeanName(String beanName) { + this.beanName = beanName; + } + /** * Making a {@link BeanFactory} available is optional; if not set, * {@link SchedulingConfigurer} beans won't get autodetected and @@ -201,12 +215,11 @@ public class ScheduledAnnotationBeanPostProcessor implements DestructionAwareBea Assert.state(this.beanFactory != null, "BeanFactory must be set to find scheduler by type"); try { // Search for TaskScheduler bean... - this.registrar.setTaskScheduler(this.beanFactory.getBean(TaskScheduler.class)); + this.registrar.setTaskScheduler(resolveSchedulerBean(TaskScheduler.class, false)); } catch (NoUniqueBeanDefinitionException ex) { try { - this.registrar.setTaskScheduler( - this.beanFactory.getBean(DEFAULT_TASK_SCHEDULER_BEAN_NAME, TaskScheduler.class)); + this.registrar.setTaskScheduler(resolveSchedulerBean(TaskScheduler.class, true)); } catch (NoSuchBeanDefinitionException ex2) { if (logger.isInfoEnabled()) { @@ -222,12 +235,11 @@ public class ScheduledAnnotationBeanPostProcessor implements DestructionAwareBea logger.debug("Could not find default TaskScheduler bean", ex); // Search for ScheduledExecutorService bean next... try { - this.registrar.setScheduler(this.beanFactory.getBean(ScheduledExecutorService.class)); + this.registrar.setScheduler(resolveSchedulerBean(ScheduledExecutorService.class, false)); } catch (NoUniqueBeanDefinitionException ex2) { try { - this.registrar.setScheduler( - this.beanFactory.getBean(DEFAULT_TASK_SCHEDULER_BEAN_NAME, ScheduledExecutorService.class)); + this.registrar.setScheduler(resolveSchedulerBean(ScheduledExecutorService.class, true)); } catch (NoSuchBeanDefinitionException ex3) { if (logger.isInfoEnabled()) { @@ -250,6 +262,32 @@ public class ScheduledAnnotationBeanPostProcessor implements DestructionAwareBea this.registrar.afterPropertiesSet(); } + private T resolveSchedulerBean(Class schedulerType, boolean byName) { + if (byName) { + T scheduler = this.beanFactory.getBean(DEFAULT_TASK_SCHEDULER_BEAN_NAME, schedulerType); + if (this.beanFactory instanceof ConfigurableBeanFactory) { + ((ConfigurableBeanFactory) this.beanFactory).registerDependentBean( + DEFAULT_TASK_SCHEDULER_BEAN_NAME, this.beanName); + } + return scheduler; + } + else if (this.beanFactory instanceof AutowireCapableBeanFactory) { + NamedBeanHolder holder = ((AutowireCapableBeanFactory) this.beanFactory).resolveNamedBean(schedulerType); + if (this.beanFactory instanceof ConfigurableBeanFactory) { + ((ConfigurableBeanFactory) this.beanFactory).registerDependentBean( + holder.getBeanName(), this.beanName); + } + return holder.getBeanInstance(); + } + else { + return this.beanFactory.getBean(schedulerType); + } + } + + + @Override + public void postProcessMergedBeanDefinition(RootBeanDefinition beanDefinition, Class beanType, String beanName) { + } @Override public Object postProcessBeforeInitialization(Object bean, String beanName) { diff --git a/spring-context/src/test/java/org/springframework/scheduling/annotation/EnableSchedulingTests.java b/spring-context/src/test/java/org/springframework/scheduling/annotation/EnableSchedulingTests.java index c3a46c6e73a..4cbdbb7305f 100644 --- a/spring-context/src/test/java/org/springframework/scheduling/annotation/EnableSchedulingTests.java +++ b/spring-context/src/test/java/org/springframework/scheduling/annotation/EnableSchedulingTests.java @@ -16,6 +16,7 @@ package org.springframework.scheduling.annotation; +import java.util.Arrays; import java.util.Date; import java.util.concurrent.atomic.AtomicInteger; @@ -31,6 +32,7 @@ import org.springframework.scheduling.TriggerContext; import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler; import org.springframework.scheduling.config.IntervalTask; import org.springframework.scheduling.config.ScheduledTaskRegistrar; +import org.springframework.scheduling.config.TaskManagementConfigUtils; import org.springframework.tests.Assume; import org.springframework.tests.TestGroup; @@ -86,6 +88,8 @@ public class EnableSchedulingTests { Thread.sleep(100); assertThat(ctx.getBean(AtomicInteger.class).get(), greaterThanOrEqualTo(10)); assertThat(ctx.getBean(ExplicitSchedulerConfig.class).threadName, startsWith("explicitScheduler-")); + assertTrue(Arrays.asList(ctx.getDefaultListableBeanFactory().getDependentBeans("myTaskScheduler")).contains( + TaskManagementConfigUtils.SCHEDULED_ANNOTATION_PROCESSOR_BEAN_NAME)); } @Test @@ -201,7 +205,7 @@ public class EnableSchedulingTests { String threadName; @Bean - public TaskScheduler taskScheduler() { + public TaskScheduler myTaskScheduler() { ThreadPoolTaskScheduler scheduler = new ThreadPoolTaskScheduler(); scheduler.setThreadNamePrefix("explicitScheduler-"); return scheduler; @@ -275,10 +279,6 @@ public class EnableSchedulingTests { counter().incrementAndGet(); } - public Object getScheduler() { - return null; - } - @Override public void configureTasks(ScheduledTaskRegistrar taskRegistrar) { taskRegistrar.setScheduler(taskScheduler1()); diff --git a/spring-jms/src/main/java/org/springframework/jms/annotation/JmsListenerAnnotationBeanPostProcessor.java b/spring-jms/src/main/java/org/springframework/jms/annotation/JmsListenerAnnotationBeanPostProcessor.java index dad78130dfd..8bd569dc836 100644 --- a/spring-jms/src/main/java/org/springframework/jms/annotation/JmsListenerAnnotationBeanPostProcessor.java +++ b/spring-jms/src/main/java/org/springframework/jms/annotation/JmsListenerAnnotationBeanPostProcessor.java @@ -34,9 +34,10 @@ import org.springframework.beans.factory.BeanInitializationException; import org.springframework.beans.factory.ListableBeanFactory; import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.SmartInitializingSingleton; -import org.springframework.beans.factory.config.BeanPostProcessor; import org.springframework.beans.factory.config.ConfigurableBeanFactory; import org.springframework.beans.factory.config.EmbeddedValueResolver; +import org.springframework.beans.factory.support.MergedBeanDefinitionPostProcessor; +import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.core.MethodIntrospector; import org.springframework.core.Ordered; import org.springframework.core.annotation.AnnotatedElementUtils; @@ -81,7 +82,7 @@ import org.springframework.util.StringValueResolver; * @see MethodJmsListenerEndpoint */ public class JmsListenerAnnotationBeanPostProcessor - implements BeanPostProcessor, Ordered, BeanFactoryAware, SmartInitializingSingleton { + implements MergedBeanDefinitionPostProcessor, Ordered, BeanFactoryAware, SmartInitializingSingleton { /** * The bean name of the default {@link JmsListenerContainerFactory}. @@ -99,7 +100,8 @@ public class JmsListenerAnnotationBeanPostProcessor private StringValueResolver embeddedValueResolver; - private final MessageHandlerMethodFactoryAdapter messageHandlerMethodFactory = new MessageHandlerMethodFactoryAdapter(); + private final MessageHandlerMethodFactoryAdapter messageHandlerMethodFactory = + new MessageHandlerMethodFactoryAdapter(); private final JmsListenerEndpointRegistrar registrar = new JmsListenerEndpointRegistrar(); @@ -192,6 +194,10 @@ public class JmsListenerAnnotationBeanPostProcessor } + @Override + public void postProcessMergedBeanDefinition(RootBeanDefinition beanDefinition, Class beanType, String beanName) { + } + @Override public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException { return bean; diff --git a/src/test/java/org/springframework/scheduling/annotation/ScheduledAndTransactionalAnnotationIntegrationTests.java b/src/test/java/org/springframework/scheduling/annotation/ScheduledAndTransactionalAnnotationIntegrationTests.java index 16ec6374574..f0590fcb3d5 100644 --- a/src/test/java/org/springframework/scheduling/annotation/ScheduledAndTransactionalAnnotationIntegrationTests.java +++ b/src/test/java/org/springframework/scheduling/annotation/ScheduledAndTransactionalAnnotationIntegrationTests.java @@ -18,11 +18,14 @@ package org.springframework.scheduling.annotation; import java.util.concurrent.atomic.AtomicInteger; +import org.aspectj.lang.annotation.Aspect; import org.junit.Before; import org.junit.Test; +import org.springframework.aop.aspectj.annotation.AnnotationAwareAspectJAutoProxyCreator; import org.springframework.aop.support.AopUtils; import org.springframework.beans.factory.BeanCreationException; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -46,6 +49,7 @@ import static org.mockito.BDDMockito.*; * as @Transactional or @Async processing. * * @author Chris Beams + * @author Juergen Hoeller * @since 3.1 */ @SuppressWarnings("resource") @@ -76,11 +80,11 @@ public class ScheduledAndTransactionalAnnotationIntegrationTests { ctx.register(Config.class, SubclassProxyTxConfig.class, RepoConfigA.class); ctx.refresh(); - Thread.sleep(100); // allow @Scheduled method to be called several times + Thread.sleep(100); // 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), equalTo(true)); + assertThat("repository is not a proxy", AopUtils.isCglibProxy(repository), equalTo(true)); assertThat("@Scheduled method never called", repository.getInvocationCount(), greaterThan(0)); assertThat("no transactions were committed", txManager.commits, greaterThan(0)); } @@ -91,15 +95,28 @@ public class ScheduledAndTransactionalAnnotationIntegrationTests { ctx.register(Config.class, JdkProxyTxConfig.class, RepoConfigB.class); ctx.refresh(); - Thread.sleep(50); // allow @Scheduled method to be called several times + Thread.sleep(100); // 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("repository is not a proxy", AopUtils.isJdkDynamicProxy(repository), is(true)); assertThat("@Scheduled method never called", repository.getInvocationCount(), greaterThan(0)); assertThat("no transactions were committed", txManager.commits, greaterThan(0)); } + @Test + public void withAspectConfig() throws InterruptedException { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.register(AspectConfig.class, MyRepositoryWithScheduledMethodImpl.class); + ctx.refresh(); + + Thread.sleep(100); // allow @Scheduled method to be called several times + + MyRepositoryWithScheduledMethod repository = ctx.getBean(MyRepositoryWithScheduledMethod.class); + assertThat("repository is not a proxy", AopUtils.isCglibProxy(repository), is(true)); + assertThat("@Scheduled method never called", repository.getInvocationCount(), greaterThan(0)); + } + @Configuration @EnableTransactionManagement @@ -108,7 +125,7 @@ public class ScheduledAndTransactionalAnnotationIntegrationTests { @Configuration - @EnableTransactionManagement(proxyTargetClass=true) + @EnableTransactionManagement(proxyTargetClass = true) static class SubclassProxyTxConfig { } @@ -137,19 +154,49 @@ public class ScheduledAndTransactionalAnnotationIntegrationTests { @EnableScheduling static class Config { + @Bean + public PlatformTransactionManager txManager() { + return new CallCountingTransactionManager(); + } + + @Bean + public PersistenceExceptionTranslator peTranslator() { + return mock(PersistenceExceptionTranslator.class); + } + @Bean public PersistenceExceptionTranslationPostProcessor peTranslationPostProcessor() { return new PersistenceExceptionTranslationPostProcessor(); } + } + + + @Configuration + @EnableScheduling + static class AspectConfig { @Bean - public PlatformTransactionManager txManager() { - return new CallCountingTransactionManager(); + public static AnnotationAwareAspectJAutoProxyCreator autoProxyCreator() { + AnnotationAwareAspectJAutoProxyCreator apc = new AnnotationAwareAspectJAutoProxyCreator(); + apc.setProxyTargetClass(true); + return apc; } @Bean - public PersistenceExceptionTranslator peTranslator() { - return mock(PersistenceExceptionTranslator.class); + public static MyAspect myAspect() { + return new MyAspect(); + } + } + + + @Aspect + public static class MyAspect { + + private final AtomicInteger count = new AtomicInteger(0); + + @org.aspectj.lang.annotation.Before("execution(* scheduled())") + public void checkTransaction() { + this.count.incrementAndGet(); } } @@ -191,6 +238,9 @@ public class ScheduledAndTransactionalAnnotationIntegrationTests { private final AtomicInteger count = new AtomicInteger(0); + @Autowired(required = false) + private MyAspect myAspect; + @Override @Transactional @Scheduled(fixedDelay = 5) @@ -200,6 +250,9 @@ public class ScheduledAndTransactionalAnnotationIntegrationTests { @Override public int getInvocationCount() { + if (this.myAspect != null) { + assertEquals(this.count.get(), this.myAspect.count.get()); + } return this.count.get(); } }