From 8bf8092740b439496cd9507fa0c8ccaf5b2e97df Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Sat, 11 Aug 2018 22:20:07 +0200 Subject: [PATCH] Post-processors consistently ignore ScopedObject/AopInfrastructureBean ScheduledAnnotationBeanPostProcessor also ignores TaskScheduler and ScheduledExecutorService, avoiding unnecessary annotation introspection on framework classes. Issue: SPR-17166 Issue: SPR-16933 --- .../AbstractAdvisingBeanPostProcessor.java | 3 +- .../aop/scope/ScopedProxyFactoryBean.java | 3 +- .../ScheduledAnnotationBeanPostProcessor.java | 10 ++- .../annotation/EnableSchedulingTests.java | 19 +---- ...duledAnnotationBeanPostProcessorTests.java | 73 ++++++++++++++----- 5 files changed, 71 insertions(+), 37 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/AbstractAdvisingBeanPostProcessor.java b/spring-aop/src/main/java/org/springframework/aop/framework/AbstractAdvisingBeanPostProcessor.java index 21c6e8c52f2..50bdb3a75cd 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/AbstractAdvisingBeanPostProcessor.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/AbstractAdvisingBeanPostProcessor.java @@ -20,6 +20,7 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import org.springframework.aop.Advisor; +import org.springframework.aop.scope.ScopedObject; import org.springframework.aop.support.AopUtils; import org.springframework.beans.factory.config.BeanPostProcessor; import org.springframework.lang.Nullable; @@ -63,7 +64,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 ScopedObject || bean instanceof AopInfrastructureBean) { // Ignore AOP infrastructure such as scoped proxies. return bean; } diff --git a/spring-aop/src/main/java/org/springframework/aop/scope/ScopedProxyFactoryBean.java b/spring-aop/src/main/java/org/springframework/aop/scope/ScopedProxyFactoryBean.java index 19c35e9774e..c2e6f14bb4e 100644 --- a/spring-aop/src/main/java/org/springframework/aop/scope/ScopedProxyFactoryBean.java +++ b/spring-aop/src/main/java/org/springframework/aop/scope/ScopedProxyFactoryBean.java @@ -52,7 +52,8 @@ import org.springframework.util.ClassUtils; * @see #setProxyTargetClass */ @SuppressWarnings("serial") -public class ScopedProxyFactoryBean extends ProxyConfig implements FactoryBean, BeanFactoryAware { +public class ScopedProxyFactoryBean extends ProxyConfig + implements FactoryBean, BeanFactoryAware, AopInfrastructureBean { /** The TargetSource that manages scoping. */ private final SimpleBeanTargetSource scopedTargetSource = new SimpleBeanTargetSource(); 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 d012fd43b7a..6496ddcf6fc 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,7 +33,9 @@ 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.scope.ScopedObject; import org.springframework.aop.support.AopUtils; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; @@ -331,7 +333,13 @@ public class ScheduledAnnotationBeanPostProcessor } @Override - public Object postProcessAfterInitialization(final Object bean, String beanName) { + public Object postProcessAfterInitialization(Object bean, String beanName) { + // Only process scoped target instances, not scoped proxies... + if (bean instanceof ScopedObject || bean instanceof AopInfrastructureBean || + bean instanceof TaskScheduler || bean instanceof ScheduledExecutorService) { + return bean; + } + Class targetClass = AopProxyUtils.ultimateTargetClass(bean); if (!this.nonAnnotatedClasses.contains(targetClass)) { Map> annotatedMethods = MethodIntrospector.selectMethods(targetClass, 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 183ff810b03..a26ad65612a 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 @@ -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; } } diff --git a/spring-context/src/test/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessorTests.java b/spring-context/src/test/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessorTests.java index 46077cb781c..5caeaeafced 100644 --- a/spring-context/src/test/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessorTests.java +++ b/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 cronTasks = (List) + 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); @@ -771,6 +790,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")