Updated based on code review

Updated to add regression tests for @EnableTask to verify no negative impact when in use.

Author: cppwfs

pom.xml

@@ -141,6 +141,7 @@
 		<spring-batch.version>4.1.0.BUILD-SNAPSHOT</spring-batch.version>
 		<commons-logging.version>1.1</commons-logging.version>
 		<java-ee-api.version>8.0</java-ee-api.version>
+		<junit.version>5.3.1</junit.version>
 
 		<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
 		<sonar.jacoco.reportPath>${project.build.directory}/coverage-reports/jacoco-ut.exec</sonar.jacoco.reportPath>

spring-cloud-task-batch/src/test/java/org/springframework/cloud/task/batch/handler/TaskJobLauncherCommandLineRunnerTests.java

@@ -39,6 +39,8 @@
 import org.springframework.boot.autoconfigure.jdbc.EmbeddedDataSourceConfiguration;
 import org.springframework.cloud.task.batch.configuration.TaskBatchAutoConfiguration;
 import org.springframework.cloud.task.batch.configuration.TaskJobLauncherAutoConfiguration;
+import org.springframework.cloud.task.batch.listener.TaskBatchTest;
+import org.springframework.cloud.task.configuration.EnableTask;
 import org.springframework.cloud.task.configuration.SimpleTaskAutoConfiguration;
 import org.springframework.cloud.task.configuration.SingleTaskConfiguration;
 import org.springframework.cloud.task.repository.TaskExecution;
@@ -81,6 +83,26 @@ public void testTaskJobLauncherCLRSuccessFail() {
 		assertThat(isExceptionThrown).isTrue();
 	}
 
+	/**
+	 * Verifies that the task will return an exit code other than zero if the
+	 * job fails with the deprecated EnableTask annotation.
+	 */
+	@Test
+	public void testTaskJobLauncherCLRSuccessFailWithAnnotation() {
+		String[] enabledArgs = new String[] {
+				"--spring.cloud.task.batch.failOnJobFailure=true",
+				"--spring.batch.job.enabled=false"};// batch job enabled is turned to false so that we can use Task's JobLauncher
+		boolean isExceptionThrown = false;
+		try {
+			this.applicationContext = SpringApplication
+					.run(new Class[] { TaskJobLauncherCommandLineRunnerTests.JobWithFailureAnnotatedConfiguration.class}, enabledArgs);
+		}
+		catch (IllegalStateException exception) {
+			isExceptionThrown = true;
+		}
+		assertThat(isExceptionThrown).isTrue();
+	}
+
 	@Test
 	public void testTaskJobLauncherPickOneJob() {
 		String[] enabledArgs = new String[] {
@@ -131,13 +153,7 @@ private void validateContext() {
 	}
 
 	@EnableBatchProcessing
-	@ImportAutoConfiguration({
-			PropertyPlaceholderAutoConfiguration.class,
-			BatchAutoConfiguration.class,
-			TaskBatchAutoConfiguration.class,
-			TaskJobLauncherAutoConfiguration.class,
-			SingleTaskConfiguration.class,
-			SimpleTaskAutoConfiguration.class })
+	@TaskBatchTest
 	@Import(EmbeddedDataSourceConfiguration.class)
 	public static class JobConfiguration {
 
@@ -209,4 +225,21 @@ public RepeatStatus execute(StepContribution contribution,
 					.build();
 		}
 	}
+
+	@EnableTask
+	@EnableBatchProcessing
+	@ImportAutoConfiguration({
+			PropertyPlaceholderAutoConfiguration.class,
+			BatchAutoConfiguration.class,
+			TaskBatchAutoConfiguration.class,
+			TaskJobLauncherAutoConfiguration.class,
+			SingleTaskConfiguration.class,
+			SimpleTaskAutoConfiguration.class })
+	@Import(EmbeddedDataSourceConfiguration.class)
+	public static class JobWithFailureAnnotatedConfiguration extends JobWithFailureConfiguration{
+
+	}
+
+
+
 }

spring-cloud-task-batch/src/test/java/org/springframework/cloud/task/batch/listener/PrefixTests.java

@@ -72,11 +72,7 @@ public void testPrefix() {
 
 	@Configuration
 	@EnableBatchProcessing
-	@ImportAutoConfiguration({ PropertyPlaceholderAutoConfiguration.class,
-			BatchAutoConfiguration.class,
-			TaskBatchAutoConfiguration.class,
-			SimpleTaskAutoConfiguration.class,
-			SingleTaskConfiguration.class })
+	@TaskBatchTest
 	public static class JobConfiguration {
 
 		@Autowired

spring-cloud-task-batch/src/test/java/org/springframework/cloud/task/batch/listener/TaskBatchExecutionListenerTests.java

@@ -39,7 +39,6 @@
 import org.springframework.beans.factory.FactoryBean;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.SpringApplication;
-import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
 import org.springframework.boot.autoconfigure.batch.BatchAutoConfiguration;
 import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration;
 import org.springframework.boot.autoconfigure.jdbc.EmbeddedDataSourceConfiguration;
@@ -248,22 +247,14 @@ private TaskBatchExecutionListenerBeanPostProcessor beanPostProcessor(
 	}
 
 	@EnableBatchProcessing
-	@ImportAutoConfiguration({ PropertyPlaceholderAutoConfiguration.class,
-			BatchAutoConfiguration.class,
-			TaskBatchAutoConfiguration.class,
-			SimpleTaskAutoConfiguration.class,
-			SingleTaskConfiguration.class })
+	@TaskBatchTest
 	@Import(EmbeddedDataSourceConfiguration.class)
 	public static class NoJobConfiguration {
 
 	}
 
 	@EnableBatchProcessing
-	@ImportAutoConfiguration({ PropertyPlaceholderAutoConfiguration.class,
-			BatchAutoConfiguration.class,
-			TaskBatchAutoConfiguration.class,
-			SimpleTaskAutoConfiguration.class,
-			SingleTaskConfiguration.class })
+	@TaskBatchTest
 	@Import(EmbeddedDataSourceConfiguration.class)
 	public static class JobConfiguration {
 
@@ -288,11 +279,7 @@ public RepeatStatus execute(StepContribution contribution, ChunkContext chunkCon
 	}
 
 	@EnableBatchProcessing
-	@ImportAutoConfiguration( { PropertyPlaceholderAutoConfiguration.class,
-			BatchAutoConfiguration.class,
-			TaskBatchAutoConfiguration.class,
-			SimpleTaskAutoConfiguration.class,
-			SingleTaskConfiguration.class } )
+	@TaskBatchTest
 	@Import(EmbeddedDataSourceConfiguration.class)
 	public static class JobFactoryBeanConfiguration {
 
@@ -332,11 +319,7 @@ public boolean isSingleton() {
 	}
 
 	@EnableBatchProcessing
-	@ImportAutoConfiguration({ PropertyPlaceholderAutoConfiguration.class,
-			BatchAutoConfiguration.class,
-			TaskBatchAutoConfiguration.class,
-			SimpleTaskAutoConfiguration.class,
-			SingleTaskConfiguration.class })
+	@TaskBatchTest
 	@Import(EmbeddedDataSourceConfiguration.class)
 	public static class JobConfigurationMultipleDataSources {
 
@@ -389,11 +372,7 @@ public DefaultBatchConfigurer batchConfigurer() {
 	}
 
 	@EnableBatchProcessing
-	@ImportAutoConfiguration({ PropertyPlaceholderAutoConfiguration.class,
-			BatchAutoConfiguration.class,
-			TaskBatchAutoConfiguration.class,
-			SimpleTaskAutoConfiguration.class,
-			SingleTaskConfiguration.class })
+	@TaskBatchTest
 	@Import(EmbeddedDataSourceConfiguration.class)
 	public static class MultipleJobConfiguration {
 

spring-cloud-task-batch/src/test/java/org/springframework/cloud/task/batch/listener/TaskBatchTest.java

@@ -0,0 +1,38 @@
+/*
+ *  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.batch.listener;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
+
+/**
+ * Contains the common configurations to run a unit test for the task batch features of
+ * SCT.
+ *
+ * @author Glenn Renfro
+ */
+@Target(ElementType.TYPE)
+@Retention(RetentionPolicy.RUNTIME)
+@Documented
+@ImportAutoConfiguration
+public @interface TaskBatchTest {
+}

spring-cloud-task-batch/src/test/resources/META-INF/spring.factories

@@ -0,0 +1,6 @@
+org.springframework.cloud.task.batch.listener.TaskBatchTest=\
+org.springframework.cloud.task.batch.configuration.TaskBatchAutoConfiguration,\
+org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration,\
+org.springframework.boot.autoconfigure.batch.BatchAutoConfiguration,\
+org.springframework.cloud.task.configuration.SimpleTaskAutoConfiguration,\
+org.springframework.cloud.task.configuration.SingleTaskConfiguration

spring-cloud-task-core/pom.xml

@@ -91,6 +91,12 @@
 			<artifactId>spring-boot-configuration-processor</artifactId>
 			<optional>true</optional>
 		</dependency>
+		<dependency>
+			<groupId>org.junit.jupiter</groupId>
+			<artifactId>junit-jupiter-api</artifactId>
+			<version>${junit.version}</version>
+			<scope>test</scope>
+		</dependency>
 	</dependencies>
 
 </project>

spring-cloud-task-core/src/main/java/org/springframework/cloud/task/configuration/EnableTask.java

@@ -53,6 +53,9 @@
  * </ul>
  *
  * @author Glenn Renfro
+ *
+ * @deprecated The EnableTask annotation is no longer be required to initialize
+ * Spring Cloud Task.  This will be handled by Spring Boot AutoConfiguration.
  */
 @Deprecated
 @Target(ElementType.TYPE)

spring-cloud-task-core/src/main/java/org/springframework/cloud/task/configuration/SimpleTaskAutoConfiguration.java

@@ -33,7 +33,7 @@
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.cloud.task.listener.TaskLifecycleListener;
-import org.springframework.cloud.task.listener.annotation.TaskListenerExecutorObjectProvider;
+import org.springframework.cloud.task.listener.TaskListenerExecutorObjectFactory;
 import org.springframework.cloud.task.repository.TaskExplorer;
 import org.springframework.cloud.task.repository.TaskNameResolver;
 import org.springframework.cloud.task.repository.TaskRepository;
@@ -140,7 +140,7 @@ protected void initialize() throws Exception {
 		this.taskExplorer = taskConfigurer.getTaskExplorer();
 
 		this.taskLifecycleListener = new TaskLifecycleListener(this.taskRepository, taskNameResolver(),
-				this.applicationArguments, taskExplorer, taskProperties, new TaskListenerExecutorObjectProvider(context));
+				this.applicationArguments, taskExplorer, taskProperties, new TaskListenerExecutorObjectFactory(context));
 
 		initialized = true;
 	}

spring-cloud-task-core/src/main/java/org/springframework/cloud/task/listener/TaskLifecycleListener.java

@@ -36,7 +36,6 @@
 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;
@@ -47,6 +46,7 @@
 import org.springframework.context.SmartLifecycle;
 import org.springframework.context.event.ContextRefreshedEvent;
 import org.springframework.util.Assert;
+import org.springframework.util.CollectionUtils;
 import org.springframework.util.StringUtils;
 
 /**
@@ -79,15 +79,13 @@ public class TaskLifecycleListener implements ApplicationListener<ApplicationEve
 
 	private List<TaskExecutionListener> taskExecutionListeners;
 
-	private boolean isTaskExecutionListenersInitialized;
-
 	private final static Log logger = LogFactory.getLog(TaskLifecycleListener.class);
 
 	private final TaskRepository taskRepository;
 
 	private final TaskExplorer taskExplorer;
 
-	private final TaskListenerExecutorObjectProvider taskListenerExecutorObjectProvider;
+	private final TaskListenerExecutorObjectFactory taskListenerExecutorObjectFactory;
 
 	private TaskExecution taskExecution;
 
@@ -116,19 +114,19 @@ public TaskLifecycleListener(TaskRepository taskRepository,
 			TaskNameResolver taskNameResolver,
 			ApplicationArguments applicationArguments, TaskExplorer taskExplorer,
 			TaskProperties taskProperties,
-			TaskListenerExecutorObjectProvider taskListenerExecutorObjectProvider) {
+			TaskListenerExecutorObjectFactory taskListenerExecutorObjectFactory) {
 		Assert.notNull(taskRepository, "A taskRepository is required");
 		Assert.notNull(taskNameResolver, "A taskNameResolver is required");
 		Assert.notNull(taskExplorer, "A taskExplorer is required");
 		Assert.notNull(taskProperties, "TaskProperties is required");
-		Assert.notNull(taskListenerExecutorObjectProvider, "A TaskListenerExecutorObjectFactory is required");
+		Assert.notNull(taskListenerExecutorObjectFactory, "A TaskListenerExecutorObjectFactory is required");
 
 		this.taskRepository = taskRepository;
 		this.taskNameResolver = taskNameResolver;
 		this.applicationArguments = applicationArguments;
 		this.taskExplorer = taskExplorer;
 		this.taskProperties = taskProperties;
-		this.taskListenerExecutorObjectProvider = taskListenerExecutorObjectProvider;
+		this.taskListenerExecutorObjectFactory = taskListenerExecutorObjectFactory;
 	}
 
 	/**
@@ -233,7 +231,13 @@ else if (this.listenerFailed || this.applicationFailedEvent != null) {
 	private void doTaskStart() {
 
 		if(!this.started) {
-			getTaskExecutionListeners();
+			this.taskExecutionListeners = new ArrayList<>();
+			this.taskListenerExecutorObjectFactory.getObject();
+			if(!CollectionUtils.isEmpty(this.taskExecutionListenersFromContext)) {
+				this.taskExecutionListeners.addAll(this.taskExecutionListenersFromContext);
+			}
+			this.taskExecutionListeners.add(this.taskListenerExecutorObjectFactory.getObject());
+
 			List<String> args = new ArrayList<>(0);
 
 			if(this.applicationArguments != null) {
@@ -386,12 +390,4 @@ public void destroy() throws Exception {
 		this.doTaskEnd();
 	}
 
-	private void getTaskExecutionListeners() {
-		this.taskExecutionListeners = new ArrayList<>();
-		this.taskListenerExecutorObjectProvider.getObject();
-		if(this.taskExecutionListenersFromContext != null) {
-			 this.taskExecutionListeners.addAll(this.taskExecutionListenersFromContext);
-		}
-		this.taskExecutionListeners.add(this.taskListenerExecutorObjectProvider.getObject());
-	}
 }