Post-processors consistently ignore ScopedObject/AopInfrastructureBean

Issue: SPR-17166

spring-aop/src/main/java/org/springframework/aop/framework/AbstractAdvisingBeanPostProcessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -63,7 +63,7 @@ public abstract class AbstractAdvisingBeanPostProcessor extends ProxyProcessorSu
 
 	@Override
 	public Object postProcessAfterInitialization(Object bean, String beanName) {
-		if (bean instanceof AopInfrastructureBean || this.advisor == null) {
+		if (this.advisor == null || bean instanceof AopInfrastructureBean) {
 			// Ignore AOP infrastructure such as scoped proxies.
 			return bean;
 		}
@@ -92,7 +92,7 @@ public abstract class AbstractAdvisingBeanPostProcessor extends ProxyProcessorSu
 			return proxyFactory.getProxy(getProxyClassLoader());
 		}
 
-		// No async proxy needed.
+		// No proxy needed.
 		return bean;
 	}
 
@@ -160,7 +160,7 @@ public abstract class AbstractAdvisingBeanPostProcessor extends ProxyProcessorSu
 	 * Subclasses may choose to implement this: for example,
 	 * to change the interfaces exposed.
 	 * <p>The default implementation is empty.
-	 * @param proxyFactory ProxyFactory that is already configured with
+	 * @param proxyFactory the ProxyFactory that is already configured with
 	 * target, advisor and interfaces and will be used to create the proxy
 	 * immediately after this method returns
 	 * @since 4.2.3

spring-aop/src/main/java/org/springframework/aop/scope/ScopedProxyFactoryBean.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -52,7 +52,8 @@ import org.springframework.util.ClassUtils;
  * @see #setProxyTargetClass
  */
 @SuppressWarnings("serial")
-public class ScopedProxyFactoryBean extends ProxyConfig implements FactoryBean<Object>, BeanFactoryAware {
+public class ScopedProxyFactoryBean extends ProxyConfig
+		implements FactoryBean<Object>, BeanFactoryAware, AopInfrastructureBean {
 
 	/** The TargetSource that manages scoping */
 	private final SimpleBeanTargetSource scopedTargetSource = new SimpleBeanTargetSource();

spring-context/src/main/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessor.java

@@ -33,6 +33,7 @@ import java.util.concurrent.ScheduledExecutorService;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import org.springframework.aop.framework.AopInfrastructureBean;
 import org.springframework.aop.framework.AopProxyUtils;
 import org.springframework.aop.support.AopUtils;
 import org.springframework.beans.factory.BeanFactory;
@@ -312,7 +313,12 @@ public class ScheduledAnnotationBeanPostProcessor
 	}
 
 	@Override
-	public Object postProcessAfterInitialization(final Object bean, String beanName) {
+	public Object postProcessAfterInitialization(Object bean, String beanName) {
+		if (bean instanceof AopInfrastructureBean) {
+			// Ignore AOP infrastructure such as scoped proxies.
+			return bean;
+		}
+
 		Class<?> targetClass = AopProxyUtils.ultimateTargetClass(bean);
 		if (!this.nonAnnotatedClasses.contains(targetClass)) {
 			Map<Method, Set<Scheduled>> annotatedMethods = MethodIntrospector.selectMethods(targetClass,

spring-context/src/test/java/org/springframework/scheduling/annotation/EnableSchedulingTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -27,8 +27,6 @@ import org.springframework.context.annotation.AnnotationConfigApplicationContext
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.scheduling.TaskScheduler;
-import org.springframework.scheduling.Trigger;
-import org.springframework.scheduling.TriggerContext;
 import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler;
 import org.springframework.scheduling.config.IntervalTask;
 import org.springframework.scheduling.config.ScheduledTaskHolder;
@@ -457,19 +455,8 @@ public class EnableSchedulingTests {
 		public TaskScheduler scheduler() {
 			ThreadPoolTaskScheduler scheduler = new ThreadPoolTaskScheduler();
 			scheduler.initialize();
-			scheduler.schedule(
-				new Runnable() {
-					@Override
-					public void run() {
-						counter().incrementAndGet();
-					}
-				},
-				new Trigger() {
-					@Override
-					public Date nextExecutionTime(TriggerContext triggerContext) {
-						return new Date(new Date().getTime()+10);
-					}
-				});
+			scheduler.schedule(() -> counter().incrementAndGet(),
+					triggerContext -> new Date(new Date().getTime()+10));
 			return scheduler;
 		}
 	}

spring-context/src/test/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessorTests.java

@@ -34,11 +34,15 @@ import org.junit.After;
 import org.junit.Test;
 
 import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.aop.scope.ScopedProxyUtils;
 import org.springframework.beans.DirectFieldAccessor;
 import org.springframework.beans.factory.BeanCreationException;
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.config.PropertyPlaceholderConfigurer;
 import org.springframework.beans.factory.support.RootBeanDefinition;
+import org.springframework.context.annotation.AnnotatedBeanDefinitionReader;
+import org.springframework.context.annotation.Scope;
+import org.springframework.context.annotation.ScopedProxyMode;
 import org.springframework.context.support.StaticApplicationContext;
 import org.springframework.core.annotation.AliasFor;
 import org.springframework.scheduling.Trigger;
@@ -50,8 +54,7 @@ import org.springframework.scheduling.config.ScheduledTaskRegistrar;
 import org.springframework.scheduling.support.CronTrigger;
 import org.springframework.scheduling.support.ScheduledMethodRunnable;
 import org.springframework.scheduling.support.SimpleTriggerContext;
-import org.springframework.tests.Assume;
-import org.springframework.tests.TestGroup;
+import org.springframework.stereotype.Component;
 import org.springframework.validation.annotation.Validated;
 import org.springframework.validation.beanvalidation.MethodValidationPostProcessor;
 
@@ -231,9 +234,7 @@ public class ScheduledAnnotationBeanPostProcessorTests {
 	}
 
 	@Test
-	public void cronTask() throws InterruptedException {
-		Assume.group(TestGroup.LONG_RUNNING);
-
+	public void cronTask() {
 		BeanDefinition processorDefinition = new RootBeanDefinition(ScheduledAnnotationBeanPostProcessor.class);
 		BeanDefinition targetDefinition = new RootBeanDefinition(CronTestBean.class);
 		context.registerBeanDefinition("postProcessor", processorDefinition);
@@ -257,13 +258,10 @@ public class ScheduledAnnotationBeanPostProcessorTests {
 		assertEquals(target, targetObject);
 		assertEquals("cron", targetMethod.getName());
 		assertEquals("*/7 * * * * ?", task.getExpression());
-		Thread.sleep(10000);
 	}
 
 	@Test
-	public void cronTaskWithZone() throws InterruptedException {
-		Assume.group(TestGroup.LONG_RUNNING);
-
+	public void cronTaskWithZone() {
 		BeanDefinition processorDefinition = new RootBeanDefinition(ScheduledAnnotationBeanPostProcessor.class);
 		BeanDefinition targetDefinition = new RootBeanDefinition(CronWithTimezoneTestBean.class);
 		context.registerBeanDefinition("postProcessor", processorDefinition);
@@ -293,30 +291,26 @@ public class ScheduledAnnotationBeanPostProcessorTests {
 		CronTrigger cronTrigger = (CronTrigger) trigger;
 		Calendar cal = Calendar.getInstance(TimeZone.getTimeZone("GMT+10"));
 		cal.clear();
-		cal.set(2013, 3, 15, 4, 0); // 15-04-2013 4:00 GMT+10
+		cal.set(2013, 3, 15, 4, 0);  // 15-04-2013 4:00 GMT+10
 		Date lastScheduledExecutionTime = cal.getTime();
 		Date lastActualExecutionTime = cal.getTime();
-		cal.add(Calendar.MINUTE, 30); // 4:30
+		cal.add(Calendar.MINUTE, 30);  // 4:30
 		Date lastCompletionTime = cal.getTime();
 		TriggerContext triggerContext = new SimpleTriggerContext(
 				lastScheduledExecutionTime, lastActualExecutionTime, lastCompletionTime);
 		cal.add(Calendar.MINUTE, 30);
-		cal.add(Calendar.HOUR_OF_DAY, 1); // 6:00
+		cal.add(Calendar.HOUR_OF_DAY, 1);  // 6:00
 		Date nextExecutionTime = cronTrigger.nextExecutionTime(triggerContext);
-		assertEquals(cal.getTime(), nextExecutionTime); // assert that 6:00 is next execution time
-		Thread.sleep(10000);
+		assertEquals(cal.getTime(), nextExecutionTime);  // assert that 6:00 is next execution time
 	}
 
 	@Test(expected = BeanCreationException.class)
-	public void cronTaskWithInvalidZone() throws InterruptedException {
-		Assume.group(TestGroup.LONG_RUNNING);
-
+	public void cronTaskWithInvalidZone() {
 		BeanDefinition processorDefinition = new RootBeanDefinition(ScheduledAnnotationBeanPostProcessor.class);
 		BeanDefinition targetDefinition = new RootBeanDefinition(CronWithInvalidTimezoneTestBean.class);
 		context.registerBeanDefinition("postProcessor", processorDefinition);
 		context.registerBeanDefinition("target", targetDefinition);
 		context.refresh();
-		Thread.sleep(10000);
 	}
 
 	@Test(expected = BeanCreationException.class)
@@ -330,6 +324,31 @@ public class ScheduledAnnotationBeanPostProcessorTests {
 		context.refresh();
 	}
 
+	@Test
+	public void cronTaskWithScopedProxy() {
+		BeanDefinition processorDefinition = new RootBeanDefinition(ScheduledAnnotationBeanPostProcessor.class);
+		context.registerBeanDefinition("postProcessor", processorDefinition);
+		new AnnotatedBeanDefinitionReader(context).register(ProxiedCronTestBean.class, ProxiedCronTestBeanDependent.class);
+		context.refresh();
+
+		ScheduledTaskHolder postProcessor = context.getBean("postProcessor", ScheduledTaskHolder.class);
+		assertEquals(1, postProcessor.getScheduledTasks().size());
+
+		ScheduledTaskRegistrar registrar = (ScheduledTaskRegistrar)
+				new DirectFieldAccessor(postProcessor).getPropertyValue("registrar");
+		@SuppressWarnings("unchecked")
+		List<CronTask> cronTasks = (List<CronTask>)
+				new DirectFieldAccessor(registrar).getPropertyValue("cronTasks");
+		assertEquals(1, cronTasks.size());
+		CronTask task = cronTasks.get(0);
+		ScheduledMethodRunnable runnable = (ScheduledMethodRunnable) task.getRunnable();
+		Object targetObject = runnable.getTarget();
+		Method targetMethod = runnable.getMethod();
+		assertEquals(context.getBean(ScopedProxyUtils.getTargetBeanName("target")), targetObject);
+		assertEquals("cron", targetMethod.getName());
+		assertEquals("*/7 * * * * ?", task.getExpression());
+	}
+
 	@Test
 	public void metaAnnotationWithFixedRate() {
 		BeanDefinition processorDefinition = new RootBeanDefinition(ScheduledAnnotationBeanPostProcessor.class);
@@ -753,6 +772,24 @@ public class ScheduledAnnotationBeanPostProcessorTests {
 	}
 
 
+	@Component("target")
+	@Scope(proxyMode = ScopedProxyMode.TARGET_CLASS)
+	static class ProxiedCronTestBean {
+
+		@Scheduled(cron = "*/7 * * * * ?")
+		public void cron() throws IOException {
+			throw new IOException("no no no");
+		}
+	}
+
+
+	static class ProxiedCronTestBeanDependent {
+
+		public ProxiedCronTestBeanDependent(ProxiedCronTestBean testBean) {
+		}
+	}
+
+
 	static class NonVoidReturnTypeTestBean {
 
 		@Scheduled(cron = "0 0 9-17 * * MON-FRI")