From 219aa2741f6b373ff247ebf147c4f52c7c6f1ae1 Mon Sep 17 00:00:00 2001 From: Glenn Renfro Date: Mon, 1 Oct 2018 16:40:58 -0400 Subject: [PATCH] TaskLifecycleListener triggers early, and possibly cyclical, bean initialisation Replaced TaskListenerExecutorFactory with TaskListenerExecutorObjectProviderTests. This delays the need to acquire a bean till it is needed vs at application event time. resolves #448 --- .../SimpleTaskConfiguration.java | 17 +- .../task/listener/TaskLifecycleListener.java | 34 +++- ...> TaskListenerExecutorObjectProvider.java} | 57 ++++--- .../listener/TaskExecutionListenerTests.java | 25 --- ...skListenerExecutorObjectProviderTests.java | 157 ++++++++++++++++++ .../task/util/TestDefaultConfiguration.java | 8 +- 6 files changed, 229 insertions(+), 69 deletions(-) rename spring-cloud-task-core/src/main/java/org/springframework/cloud/task/listener/annotation/{TaskListenerExecutorFactoryBean.java => TaskListenerExecutorObjectProvider.java} (75%) create mode 100644 spring-cloud-task-core/src/test/java/org/springframework/cloud/task/listener/annotation/TaskListenerExecutorObjectProviderTests.java diff --git a/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/configuration/SimpleTaskConfiguration.java b/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/configuration/SimpleTaskConfiguration.java index e5815a52..9c989cb2 100644 --- a/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/configuration/SimpleTaskConfiguration.java +++ b/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/configuration/SimpleTaskConfiguration.java @@ -29,9 +29,10 @@ import org.apache.commons.logging.LogFactory; import org.springframework.aop.scope.ScopedProxyUtils; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.ApplicationArguments; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.task.listener.TaskLifecycleListener; -import org.springframework.cloud.task.listener.annotation.TaskListenerExecutorFactoryBean; +import org.springframework.cloud.task.listener.annotation.TaskListenerExecutorObjectProvider; import org.springframework.cloud.task.repository.TaskExplorer; import org.springframework.cloud.task.repository.TaskNameResolver; import org.springframework.cloud.task.repository.TaskRepository; @@ -77,8 +78,6 @@ public class SimpleTaskConfiguration { private TaskLifecycleListener taskLifecycleListener; - private TaskListenerExecutorFactoryBean taskListenerExecutorFactoryBean; - private PlatformTransactionManager platformTransactionManager; private TaskExplorer taskExplorer; @@ -94,13 +93,8 @@ public class SimpleTaskConfiguration { } @Bean - public TaskListenerExecutorFactoryBean taskListenerExecutor() - throws Exception { - return this.taskListenerExecutorFactoryBean; - } - - @Bean - public PlatformTransactionManager transactionManager() { + @ConditionalOnMissingBean + public PlatformTransactionManager transactionManager() { return this.platformTransactionManager; } @@ -140,12 +134,11 @@ public class SimpleTaskConfiguration { taskConfigurer.getClass().getName())); this.taskRepository = taskConfigurer.getTaskRepository(); - this.taskListenerExecutorFactoryBean = new TaskListenerExecutorFactoryBean(context); this.platformTransactionManager = taskConfigurer.getTransactionManager(); this.taskExplorer = taskConfigurer.getTaskExplorer(); this.taskLifecycleListener = new TaskLifecycleListener(this.taskRepository, taskNameResolver(), - this.applicationArguments, taskExplorer, taskProperties); + this.applicationArguments, taskExplorer, taskProperties, new TaskListenerExecutorObjectProvider(context)); initialized = true; } diff --git a/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/listener/TaskLifecycleListener.java b/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/listener/TaskLifecycleListener.java index 9a426ed3..dbabb483 100644 --- a/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/listener/TaskLifecycleListener.java +++ b/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/listener/TaskLifecycleListener.java @@ -36,6 +36,7 @@ import org.springframework.boot.ExitCodeGenerator; import org.springframework.boot.context.event.ApplicationFailedEvent; import org.springframework.boot.context.event.ApplicationReadyEvent; import org.springframework.cloud.task.configuration.TaskProperties; +import org.springframework.cloud.task.listener.annotation.TaskListenerExecutorObjectProvider; import org.springframework.cloud.task.repository.TaskExecution; import org.springframework.cloud.task.repository.TaskExplorer; import org.springframework.cloud.task.repository.TaskNameResolver; @@ -74,7 +75,11 @@ public class TaskLifecycleListener implements ApplicationListener taskExecutionListeners; + private Collection taskExecutionListenersFromContext; + + private List taskExecutionListeners; + + private boolean isTaskExecutionListenersInitialized; private final static Log logger = LogFactory.getLog(TaskLifecycleListener.class); @@ -82,6 +87,8 @@ public class TaskLifecycleListener implements ApplicationListener args = new ArrayList<>(0); if(this.applicationArguments != null) { @@ -258,11 +269,11 @@ public class TaskLifecycleListener implements ApplicationListener startupListenerList = new ArrayList<>(this.taskExecutionListeners); + if (startupListenerList != null) { try { - List starterList = new ArrayList<>(taskExecutionListeners); - Collections.reverse(starterList); - for (TaskExecutionListener taskExecutionListener : starterList) { + Collections.reverse(startupListenerList); + for (TaskExecutionListener taskExecutionListener : startupListenerList) { taskExecutionListener.onTaskStartup(listenerTaskExecution); } } @@ -300,7 +311,7 @@ public class TaskLifecycleListener implements ApplicationListener(); + this.taskListenerExecutorObjectProvider.getObject(); + if(this.taskExecutionListenersFromContext != null) { + this.taskExecutionListeners.addAll(this.taskExecutionListenersFromContext); + } + this.taskExecutionListeners.add(this.taskListenerExecutorObjectProvider.getObject()); + } } diff --git a/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/listener/annotation/TaskListenerExecutorFactoryBean.java b/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/listener/annotation/TaskListenerExecutorObjectProvider.java similarity index 75% rename from spring-cloud-task-core/src/main/java/org/springframework/cloud/task/listener/annotation/TaskListenerExecutorFactoryBean.java rename to spring-cloud-task-core/src/main/java/org/springframework/cloud/task/listener/annotation/TaskListenerExecutorObjectProvider.java index 6ea8265a..c3fdc23d 100644 --- a/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/listener/annotation/TaskListenerExecutorFactoryBean.java +++ b/spring-cloud-task-core/src/main/java/org/springframework/cloud/task/listener/annotation/TaskListenerExecutorObjectProvider.java @@ -1,17 +1,17 @@ /* - * Copyright 2016 the original author or authors. + * Copyright 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. - * You may obtain a copy of the License at + * 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 + * 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. + * 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.cloud.task.listener.annotation; @@ -30,8 +30,9 @@ import org.apache.commons.logging.LogFactory; import org.springframework.aop.framework.autoproxy.AutoProxyUtils; import org.springframework.aop.scope.ScopedObject; import org.springframework.aop.scope.ScopedProxyUtils; +import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanInitializationException; -import org.springframework.beans.factory.FactoryBean; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.cloud.task.listener.TaskExecutionListener; import org.springframework.context.ConfigurableApplicationContext; @@ -39,9 +40,12 @@ import org.springframework.core.MethodIntrospector; import org.springframework.core.annotation.AnnotationUtils; /** + * Initializes TaskListenerExecutor for a task. + * * @author Glenn Renfro + * @since 2.1.0 */ -public class TaskListenerExecutorFactoryBean implements FactoryBean { +public class TaskListenerExecutorObjectProvider implements ObjectProvider { private final static Log logger = LogFactory.getLog(TaskListenerExecutor.class); @@ -56,12 +60,12 @@ public class TaskListenerExecutorFactoryBean implements FactoryBean failedTaskInstances; - public TaskListenerExecutorFactoryBean(ConfigurableApplicationContext context){ + public TaskListenerExecutorObjectProvider(ConfigurableApplicationContext context){ this.context = context; } @Override - public TaskListenerExecutor getObject() throws Exception { + public TaskListenerExecutor getObject() { beforeTaskInstances = new HashMap<>(); afterTaskInstances = new HashMap<>(); failedTaskInstances = new HashMap<>(); @@ -69,16 +73,6 @@ public class TaskListenerExecutorFactoryBean implements FactoryBean getObjectType() { - return TaskListenerExecutor.class; - } - - @Override - public boolean isSingleton() { - return false; - } - private void initializeExecutor( ) { ConfigurableListableBeanFactory factory = context.getBeanFactory(); for( String beanName : context.getBeanDefinitionNames()) { @@ -151,6 +145,21 @@ public class TaskListenerExecutorFactoryBean implements FactoryBean { public Map getMethods(final Class type, final Class annotationClass){ return MethodIntrospector.selectMethods(type, diff --git a/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/listener/TaskExecutionListenerTests.java b/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/listener/TaskExecutionListenerTests.java index a1f4d25c..8543c1f1 100644 --- a/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/listener/TaskExecutionListenerTests.java +++ b/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/listener/TaskExecutionListenerTests.java @@ -29,11 +29,9 @@ import org.springframework.boot.context.event.ApplicationReadyEvent; import org.springframework.cloud.task.listener.annotation.AfterTask; import org.springframework.cloud.task.listener.annotation.BeforeTask; import org.springframework.cloud.task.listener.annotation.FailedTask; -import org.springframework.cloud.task.listener.annotation.TaskListenerExecutorFactoryBean; import org.springframework.cloud.task.repository.TaskExecution; import org.springframework.cloud.task.util.TestDefaultConfiguration; import org.springframework.cloud.task.util.TestListener; -import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -290,12 +288,6 @@ public class TaskExecutionListenerTests { return new AnnotatedTaskListener(); } - @Bean - public TaskListenerExecutorFactoryBean taskListenerExecutor(ConfigurableApplicationContext context) throws Exception - { - return new TaskListenerExecutorFactoryBean(context); - } - public static class AnnotatedTaskListener extends TestListener { @BeforeTask @@ -331,12 +323,6 @@ public class TaskExecutionListenerTests { return new AnnotatedTaskListener(); } - @Bean - public TaskListenerExecutorFactoryBean taskListenerExecutor(ConfigurableApplicationContext context) throws Exception - { - return new TaskListenerExecutorFactoryBean(context); - } - public static class AnnotatedTaskListener { @BeforeTask @@ -365,11 +351,6 @@ public class TaskExecutionListenerTests { return new AnnotatedTaskListener(); } - @Bean - public TaskListenerExecutorFactoryBean taskListenerExecutor(ConfigurableApplicationContext context) throws Exception - { - return new TaskListenerExecutorFactoryBean(context); - } public static class AnnotatedTaskListener { @@ -400,12 +381,6 @@ public class TaskExecutionListenerTests { return new AnnotatedTaskListener(); } - @Bean - public TaskListenerExecutorFactoryBean taskListenerExecutor(ConfigurableApplicationContext context) throws Exception - { - return new TaskListenerExecutorFactoryBean(context); - } - public static class AnnotatedTaskListener extends TestListener{ @BeforeTask diff --git a/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/listener/annotation/TaskListenerExecutorObjectProviderTests.java b/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/listener/annotation/TaskListenerExecutorObjectProviderTests.java new file mode 100644 index 00000000..7cb888ed --- /dev/null +++ b/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/listener/annotation/TaskListenerExecutorObjectProviderTests.java @@ -0,0 +1,157 @@ +/* + * Copyright 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. + * 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.cloud.task.listener.annotation; + +import java.util.ArrayList; +import java.util.List; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.cloud.task.repository.TaskExecution; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringRunner; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Verifies that the {@link TaskListenerExecutorObjectProvider} retrieves the + * {@link TaskListenerExecutor}. + * + * @author Glenn Renfro + * @since 2.1.0 + */ +@RunWith(SpringRunner.class) +@ContextConfiguration(classes = { TaskListenerExecutorObjectProviderTests.TaskExecutionListenerConfiguration.class }) +public class TaskListenerExecutorObjectProviderTests { + + public static final String BEFORE_LISTENER = "BEFORE LISTENER"; + + public static final String AFTER_LISTENER = "AFTER LISTENER"; + + public static final String FAIL_LISTENER = "FAIL LISTENER"; + + public static List taskExecutionListenerResults = new ArrayList<>(3); + + @Autowired + private ConfigurableApplicationContext context; + + private TaskListenerExecutor taskListenerExecutor; + + private TaskListenerExecutorObjectProvider taskListenerExecutorObjectProvider; + + @Before + public void setup() { + taskExecutionListenerResults.clear(); + this.taskListenerExecutorObjectProvider = new TaskListenerExecutorObjectProvider(this.context); + this.taskListenerExecutor = this.taskListenerExecutorObjectProvider.getObject(); + } + + @Test + public void verifyTaskStartupListener() { + this.taskListenerExecutor.onTaskStartup(createSampleTaskExecution(BEFORE_LISTENER)); + validateSingleEntry(BEFORE_LISTENER); + } + + @Test + public void verifyTaskFailedListener() { + this.taskListenerExecutor.onTaskFailed(createSampleTaskExecution(FAIL_LISTENER), + new IllegalStateException("oops")); + validateSingleEntry(FAIL_LISTENER); + } + + @Test + public void verifyTaskEndListener() { + this.taskListenerExecutor.onTaskEnd(createSampleTaskExecution(AFTER_LISTENER)); + validateSingleEntry(AFTER_LISTENER); + } + + @Test + public void verifyAllListener() { + this.taskListenerExecutor.onTaskStartup(createSampleTaskExecution(BEFORE_LISTENER)); + this.taskListenerExecutor.onTaskFailed(createSampleTaskExecution(FAIL_LISTENER), + new IllegalStateException("oops")); + this.taskListenerExecutor.onTaskEnd(createSampleTaskExecution(AFTER_LISTENER)); + assertThat(taskExecutionListenerResults.size()).isEqualTo(3); + assertThat(taskExecutionListenerResults.get(0).getTaskName()).isEqualTo(BEFORE_LISTENER); + assertThat(taskExecutionListenerResults.get(1).getTaskName()).isEqualTo(FAIL_LISTENER); + assertThat(taskExecutionListenerResults.get(2).getTaskName()).isEqualTo(AFTER_LISTENER); + } + + @Test(expected = UnsupportedOperationException.class) + public void verifyGetObjectArgs() { + this.taskListenerExecutorObjectProvider.getObject("foo"); + } + + @Test(expected = UnsupportedOperationException.class) + public void verifyGetIfAvailable() { + this.taskListenerExecutorObjectProvider.getIfAvailable(); + } + + @Test(expected = UnsupportedOperationException.class) + public void verifyGetIfUnique() { + this.taskListenerExecutorObjectProvider.getIfUnique(); + } + + @Test(expected = UnsupportedOperationException.class) + public void verifyGetIfUniqueParameter() { + this.taskListenerExecutorObjectProvider.getIfUnique(null); + } + + private TaskExecution createSampleTaskExecution(String taskName) { + TaskExecution taskExecution = new TaskExecution(); + taskExecution.setTaskName(taskName); + return taskExecution; + } + + private void validateSingleEntry(String event) { + assertThat(taskExecutionListenerResults.size()).isEqualTo(1); + assertThat(taskExecutionListenerResults.get(0).getTaskName()).isEqualTo(event); + } + + @Configuration + public static class TaskExecutionListenerConfiguration { + + @Bean + public TaskRunComponent taskRunComponent() { + return new TaskRunComponent(); + } + } + + public static class TaskRunComponent { + + @BeforeTask + public void initBeforeListener(TaskExecution taskExecution) { + TaskListenerExecutorObjectProviderTests.taskExecutionListenerResults.add(taskExecution); + } + + @AfterTask + public void initAfterListener(TaskExecution taskExecution) { + TaskListenerExecutorObjectProviderTests.taskExecutionListenerResults.add(taskExecution); + } + + @FailedTask + public void initFailedListener(TaskExecution taskExecution, Throwable exception) { + TaskListenerExecutorObjectProviderTests.taskExecutionListenerResults.add(taskExecution); + } + } +} diff --git a/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/util/TestDefaultConfiguration.java b/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/util/TestDefaultConfiguration.java index a171a07b..7b7708ab 100644 --- a/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/util/TestDefaultConfiguration.java +++ b/spring-cloud-task-core/src/test/java/org/springframework/cloud/task/util/TestDefaultConfiguration.java @@ -24,6 +24,7 @@ import org.springframework.boot.ApplicationArguments; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.task.configuration.TaskProperties; import org.springframework.cloud.task.listener.TaskLifecycleListener; +import org.springframework.cloud.task.listener.annotation.TaskListenerExecutorObjectProvider; import org.springframework.cloud.task.repository.TaskExplorer; import org.springframework.cloud.task.repository.TaskNameResolver; import org.springframework.cloud.task.repository.TaskRepository; @@ -74,10 +75,15 @@ public class TestDefaultConfiguration implements InitializingBean { return new SimpleTaskNameResolver(); } + @Bean + public TaskListenerExecutorObjectProvider taskListenerExecutorObjectProvider(ConfigurableApplicationContext context) { + return new TaskListenerExecutorObjectProvider(context); + } + @Bean public TaskLifecycleListener taskHandler(TaskExplorer taskExplorer){ return new TaskLifecycleListener(taskRepository(), taskNameResolver(), - applicationArguments, taskExplorer, taskProperties); + applicationArguments, taskExplorer, taskProperties, taskListenerExecutorObjectProvider(context)); } @Override