@Scheduled reliably applies after other post-processors and shuts down before TaskScheduler

Issue: SPR-14692
Issue: SPR-15067
(cherry picked from commit edc62be)

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")

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<ContextRefreshedEvent>, 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> T resolveSchedulerBean(Class<T> 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<T> 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) {

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());

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;

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();
 		}
 	}