From 854c162966cd5606f91951e7a42a4093f82302df Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Mon, 24 Jul 2023 15:42:32 +0200 Subject: [PATCH] Polish "Fail fast if job name does not exist" See gh-36060 --- .../batch/JobLauncherApplicationRunner.java | 28 +++++----- .../batch/BatchAutoConfigurationTests.java | 56 ++++++++++++++++--- 2 files changed, 62 insertions(+), 22 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/JobLauncherApplicationRunner.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/JobLauncherApplicationRunner.java index dd6baf6865..ae28d64a7a 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/JobLauncherApplicationRunner.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/batch/JobLauncherApplicationRunner.java @@ -24,8 +24,6 @@ import java.util.LinkedHashMap; 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.converter.JobParametersConverter; 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 @@ import org.springframework.util.StringUtils; * @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 class JobLauncherApplicationRunner implements ApplicationRunner, Ordered, 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 @@ public class JobLauncherApplicationRunner implements ApplicationRunner, Ordered, 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 @@ public class JobLauncherApplicationRunner implements ApplicationRunner, Ordered, 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 @@ public class JobLauncherApplicationRunner implements ApplicationRunner, Ordered, return new JobParameters(merged); } + private String[] jobsToRun() { + return this.jobNames.split(","); + } + } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfigurationTests.java index a25e7190b5..6da759a819 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/batch/BatchAutoConfigurationTests.java +++ b/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 org.springframework.transaction.PlatformTransactionManager; 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 @@ import static org.mockito.Mockito.mock; * @author Stephane Nicoll * @author Vedran Pavic * @author Kazuki Shimizu - * @author Akshay Dubey */ @ExtendWith(OutputCaptureExtension.class) class BatchAutoConfigurationTests { @@ -376,14 +378,50 @@ class BatchAutoConfigurationTests { } @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)