Polish "Fail fast if job name does not exist"

See gh-36060

Author: snicoll

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/JobLauncherApplicationRunner.java

@@ -24,8 +24,6 @@
 import java.util.Map;
 import java.util.Properties;
 
-import javax.annotation.PostConstruct;
-
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
@@ -43,11 +41,11 @@
 import org.springframework.batch.core.explore.JobExplorer;
 import org.springframework.batch.core.launch.JobLauncher;
 import org.springframework.batch.core.launch.JobParametersNotFoundException;
-import org.springframework.batch.core.launch.NoSuchJobException;
 import org.springframework.batch.core.repository.JobExecutionAlreadyRunningException;
 import org.springframework.batch.core.repository.JobInstanceAlreadyCompleteException;
 import org.springframework.batch.core.repository.JobRepository;
 import org.springframework.batch.core.repository.JobRestartException;
+import org.springframework.beans.factory.InitializingBean;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.ApplicationArguments;
 import org.springframework.boot.ApplicationRunner;
@@ -71,7 +69,8 @@
  * @author Akshay Dubey
  * @since 2.3.0
  */
-public class JobLauncherApplicationRunner implements ApplicationRunner, Ordered, ApplicationEventPublisherAware {
+public class JobLauncherApplicationRunner
+		implements ApplicationRunner, InitializingBean, Ordered, ApplicationEventPublisherAware {
 
 	/**
 	 * The default order for the command line runner.
@@ -114,15 +113,14 @@ public JobLauncherApplicationRunner(JobLauncher jobLauncher, JobExplorer jobExpl
 		this.jobRepository = jobRepository;
 	}
 
-	@PostConstruct
-	public void validate() {
+	@Override
+	public void afterPropertiesSet() {
 		if (StringUtils.hasText(this.jobNames)) {
-			String[] jobsToRun = this.jobNames.split(",");
-			for(String jobName: jobsToRun) {
+			for (String jobName : jobsToRun()) {
 				if (!isLocalJob(jobName) && !isRegisteredJob(jobName)) {
-						throw new IllegalArgumentException("No job instances were found for job name [" + jobName + "]");
+					throw new IllegalArgumentException("No job found with name '" + jobName + "'");
 				}
-			}	
+			}
 		}
 	}
 
@@ -187,7 +185,7 @@ private boolean isRegisteredJob(String jobName) {
 	private void executeLocalJobs(JobParameters jobParameters) throws JobExecutionException {
 		for (Job job : this.jobs) {
 			if (StringUtils.hasText(this.jobNames)) {
-				String[] jobsToRun = this.jobNames.split(",");
+				String[] jobsToRun = jobsToRun();
 				if (!PatternMatchUtils.simpleMatch(jobsToRun, job.getName())) {
 					logger.debug(LogMessage.format("Skipped job: %s", job.getName()));
 					continue;
@@ -199,9 +197,9 @@ private void executeLocalJobs(JobParameters jobParameters) throws JobExecutionEx
 
 	private void executeRegisteredJobs(JobParameters jobParameters) throws JobExecutionException {
 		if (this.jobRegistry != null && StringUtils.hasText(this.jobNames)) {
-			String[] jobsToRun = this.jobNames.split(",");
+			String[] jobsToRun = jobsToRun();
 			for (String jobName : jobsToRun) {
-				if(isRegisteredJob(jobName) && !isLocalJob(jobName)) {
+				if (!isLocalJob(jobName)) {
 					Job job = this.jobRegistry.getJob(jobName);
 					execute(job, jobParameters);
 				}
@@ -263,4 +261,8 @@ private JobParameters merge(JobParameters parameters, JobParameters additionals)
 		return new JobParameters(merged);
 	}
 
+	private String[] jobsToRun() {
+		return this.jobNames.split(",");
+	}
+
 }

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfigurationTests.java

@@ -16,6 +16,7 @@
 
 package org.springframework.boot.autoconfigure.batch;
 
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 
@@ -71,6 +72,8 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
+import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
 
 /**
@@ -80,7 +83,6 @@
  * @author Stephane Nicoll
  * @author Vedran Pavic
  * @author Kazuki Shimizu
- * @author Akshay Dubey
  */
 @ExtendWith(OutputCaptureExtension.class)
 class BatchAutoConfigurationTests {
@@ -376,14 +378,50 @@ void whenTheUserDefinesTheirOwnDatabaseInitializerThenTheAutoConfiguredBatchInit
 	}
 
 	@Test
-	void testNonConfiguredJobThrowsException() {
-		this.contextRunner
-		.withUserConfiguration(NamedJobConfigurationWithLocalJob.class, EmbeddedDataSourceConfiguration.class)
-		.withPropertyValues("spring.batch.job.names:discreteLocalJob,nonConfiguredJob")
-		.run((context) -> {
-			assertThat(context).hasFailed().getFailure().getRootCause()
-			.hasMessageContaining("nonConfiguredJob");	
-		});
+	void whenTheUserDefinesAJobNameAsJobInstanceValidates() {
+		JobLauncherApplicationRunner runner = createInstance("another");
+		runner.setJobs(Collections.singletonList(mockJob("test")));
+		runner.setJobNames("test");
+		runner.afterPropertiesSet();
+	}
+
+	@Test
+	void whenTheUserDefinesAJobNameAsRegisteredJobValidates() {
+		JobLauncherApplicationRunner runner = createInstance("test");
+		runner.setJobNames("test");
+		runner.afterPropertiesSet();
+	}
+
+	@Test
+	void whenTheUserDefinesAJobNameThatDoesNotExistWithJobInstancesFailsFast() {
+		JobLauncherApplicationRunner runner = createInstance();
+		runner.setJobs(Arrays.asList(mockJob("one"), mockJob("two")));
+		runner.setJobNames("three");
+		assertThatIllegalArgumentException().isThrownBy(runner::afterPropertiesSet)
+			.withMessage("No job found with name 'three'");
+	}
+
+	@Test
+	void whenTheUserDefinesAJobNameThatDoesNotExistWithRegisteredJobFailsFast() {
+		JobLauncherApplicationRunner runner = createInstance("one", "two");
+		runner.setJobNames("three");
+		assertThatIllegalArgumentException().isThrownBy(runner::afterPropertiesSet)
+			.withMessage("No job found with name 'three'");
+	}
+
+	private JobLauncherApplicationRunner createInstance(String... registeredJobNames) {
+		JobLauncherApplicationRunner runner = new JobLauncherApplicationRunner(mock(JobLauncher.class),
+				mock(JobExplorer.class), mock(JobRepository.class));
+		JobRegistry jobRegistry = mock(JobRegistry.class);
+		given(jobRegistry.getJobNames()).willReturn(Arrays.asList(registeredJobNames));
+		runner.setJobRegistry(jobRegistry);
+		return runner;
+	}
+
+	private Job mockJob(String name) {
+		Job job = mock(Job.class);
+		given(job.getName()).willReturn(name);
+		return job;
 	}
 
 	@Configuration(proxyBeanMethods = false)