From 5c725b33df48164183d84036b647d62edfa53a17 Mon Sep 17 00:00:00 2001 From: Akshay Dubey <38462415+itsAkshayDubey@users.noreply.github.com> Date: Sat, 24 Jun 2023 14:02:29 +0000 Subject: [PATCH 1/5] refactor: #35940 Refactor exception handling --- .../autoconfigure/batch/JobLauncherApplicationRunner.java | 4 ++++ 1 file changed, 4 insertions(+) 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 76b3e955d208..6816d2c96a42 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 @@ -66,6 +66,7 @@ * @author Jean-Pierre Bergamin * @author Mahmoud Ben Hassine * @author Stephane Nicoll + * @author Akshay Dubey * @since 2.3.0 */ public class JobLauncherApplicationRunner implements ApplicationRunner, Ordered, ApplicationEventPublisherAware { @@ -187,6 +188,9 @@ private void executeRegisteredJobs(JobParameters jobParameters) throws JobExecut } catch (NoSuchJobException ex) { logger.debug(LogMessage.format("No job found in registry for job name: %s", jobName)); + if(this.jobExplorer != null && this.jobExplorer.getLastJobInstance(jobName) == null) { + throw ex; + } } } } From 65f2df65035de6136e2035a28207397f6126301f Mon Sep 17 00:00:00 2001 From: Akshay Dubey <38462415+itsAkshayDubey@users.noreply.github.com> Date: Sat, 24 Jun 2023 14:03:05 +0000 Subject: [PATCH 2/5] test: #35940 Add unit tests --- .../batch/BatchAutoConfigurationTests.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) 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 6b1062e41b6e..5418b8a298a9 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 @@ -38,6 +38,7 @@ import org.springframework.batch.core.explore.JobExplorer; import org.springframework.batch.core.job.AbstractJob; import org.springframework.batch.core.launch.JobLauncher; +import org.springframework.batch.core.launch.NoSuchJobException; import org.springframework.batch.core.repository.JobRepository; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; @@ -71,6 +72,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.mock; /** @@ -80,6 +82,7 @@ * @author Stephane Nicoll * @author Vedran Pavic * @author Kazuki Shimizu + * @author Akshay Dubey */ @ExtendWith(OutputCaptureExtension.class) class BatchAutoConfigurationTests { @@ -374,6 +377,21 @@ void whenTheUserDefinesTheirOwnDatabaseInitializerThenTheAutoConfiguredBatchInit .hasBean("customInitializer")); } + @Test + void testExecuteLocalAndNonConfiguredJob() { + this.contextRunner + .withUserConfiguration(NamedJobConfigurationWithLocalJob.class, EmbeddedDataSourceConfiguration.class) + .withPropertyValues("spring.batch.job.names:discreteLocalJob,nonConfiguredJob") + .run((context) -> { + assertThat(context).hasSingleBean(JobLauncher.class); + assertThrows(NoSuchJobException.class,()->{ + context.getBean(JobLauncherApplicationRunner.class).run(); + }); + assertThat(context.getBean(JobRepository.class) + .getLastJobExecution("discreteLocalJob", new JobParameters())).isNotNull(); + }); + } + @Configuration(proxyBeanMethods = false) protected static class BatchDataSourceConfiguration { From b7427c19f45524e4ccf509738f2d002977e5b71e Mon Sep 17 00:00:00 2001 From: Akshay Dubey <38462415+itsAkshayDubey@users.noreply.github.com> Date: Wed, 5 Jul 2023 21:07:09 +0000 Subject: [PATCH 3/5] fix: Add validate method, remove thrown exception --- .../batch/JobLauncherApplicationRunner.java | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 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 6816d2c96a42..9c86e3c1e7cb 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,6 +24,8 @@ import java.util.Map; import java.util.Properties; +import javax.annotation.PostConstruct; + import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -112,6 +114,21 @@ public JobLauncherApplicationRunner(JobLauncher jobLauncher, JobExplorer jobExpl this.jobRepository = jobRepository; } + @PostConstruct + public void validate() throws NoSuchJobException { + if (StringUtils.hasText(this.jobNames)) { + String[] jobsToRun = this.jobNames.split(","); + for (Job job : this.jobs) { + for(String jobName: jobsToRun) { + if (!PatternMatchUtils.simpleMatch(jobName, job.getName()) + && !this.jobRegistry.getJobNames().contains(jobName)) { + throw new NoSuchJobException("Job with name " + jobName + " does not exist."); + } + } + } + } + } + public void setOrder(int order) { this.order = order; } @@ -188,9 +205,6 @@ private void executeRegisteredJobs(JobParameters jobParameters) throws JobExecut } catch (NoSuchJobException ex) { logger.debug(LogMessage.format("No job found in registry for job name: %s", jobName)); - if(this.jobExplorer != null && this.jobExplorer.getLastJobInstance(jobName) == null) { - throw ex; - } } } } From e4ecb9ce628a92067ab2963dcef695acc03cc57d Mon Sep 17 00:00:00 2001 From: Akshay Dubey <38462415+itsAkshayDubey@users.noreply.github.com> Date: Wed, 5 Jul 2023 21:07:38 +0000 Subject: [PATCH 4/5] test: Fix unit test according to validate method --- .../batch/BatchAutoConfigurationTests.java | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) 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 5418b8a298a9..015edde6d16c 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 @@ -38,7 +38,6 @@ import org.springframework.batch.core.explore.JobExplorer; import org.springframework.batch.core.job.AbstractJob; import org.springframework.batch.core.launch.JobLauncher; -import org.springframework.batch.core.launch.NoSuchJobException; import org.springframework.batch.core.repository.JobRepository; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; @@ -72,7 +71,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; -import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.mock; /** @@ -383,12 +382,10 @@ void testExecuteLocalAndNonConfiguredJob() { .withUserConfiguration(NamedJobConfigurationWithLocalJob.class, EmbeddedDataSourceConfiguration.class) .withPropertyValues("spring.batch.job.names:discreteLocalJob,nonConfiguredJob") .run((context) -> { - assertThat(context).hasSingleBean(JobLauncher.class); - assertThrows(NoSuchJobException.class,()->{ - context.getBean(JobLauncherApplicationRunner.class).run(); - }); - assertThat(context.getBean(JobRepository.class) - .getLastJobExecution("discreteLocalJob", new JobParameters())).isNotNull(); + assertThatThrownBy(() -> { + context.getBean(JobLauncherApplicationRunner.class).run(); + } + ).hasRootCauseMessage("Job with name nonConfiguredJob does not exist."); }); } From bee0c228c533318790c1e84ae36ca47feb0daaca Mon Sep 17 00:00:00 2001 From: Akshay Dubey <38462415+itsAkshayDubey@users.noreply.github.com> Date: Fri, 7 Jul 2023 13:54:55 +0000 Subject: [PATCH 5/5] refactor: Remove try catch block --- .../batch/JobLauncherApplicationRunner.java | 29 +++++++++---------- .../batch/BatchAutoConfigurationTests.java | 9 ++---- 2 files changed, 17 insertions(+), 21 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 9c86e3c1e7cb..dd6baf6865c6 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 @@ -115,17 +115,14 @@ public JobLauncherApplicationRunner(JobLauncher jobLauncher, JobExplorer jobExpl } @PostConstruct - public void validate() throws NoSuchJobException { + public void validate() { if (StringUtils.hasText(this.jobNames)) { String[] jobsToRun = this.jobNames.split(","); - for (Job job : this.jobs) { - for(String jobName: jobsToRun) { - if (!PatternMatchUtils.simpleMatch(jobName, job.getName()) - && !this.jobRegistry.getJobNames().contains(jobName)) { - throw new NoSuchJobException("Job with name " + jobName + " does not exist."); - } + for(String jobName: jobsToRun) { + if (!isLocalJob(jobName) && !isRegisteredJob(jobName)) { + throw new IllegalArgumentException("No job instances were found for job name [" + jobName + "]"); } - } + } } } @@ -179,6 +176,14 @@ protected void launchJobFromProperties(Properties properties) throws JobExecutio executeRegisteredJobs(jobParameters); } + private boolean isLocalJob(String jobName) { + return this.jobs.stream().anyMatch((job) -> job.getName().equals(jobName)); + } + + private boolean isRegisteredJob(String jobName) { + return this.jobRegistry != null && this.jobRegistry.getJobNames().contains(jobName); + } + private void executeLocalJobs(JobParameters jobParameters) throws JobExecutionException { for (Job job : this.jobs) { if (StringUtils.hasText(this.jobNames)) { @@ -196,16 +201,10 @@ private void executeRegisteredJobs(JobParameters jobParameters) throws JobExecut if (this.jobRegistry != null && StringUtils.hasText(this.jobNames)) { String[] jobsToRun = this.jobNames.split(","); for (String jobName : jobsToRun) { - try { + if(isRegisteredJob(jobName) && !isLocalJob(jobName)) { Job job = this.jobRegistry.getJob(jobName); - if (this.jobs.contains(job)) { - continue; - } execute(job, jobParameters); } - catch (NoSuchJobException ex) { - logger.debug(LogMessage.format("No job found in registry for job name: %s", jobName)); - } } } } 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 015edde6d16c..a25e7190b5af 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 @@ -71,7 +71,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; -import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.mock; /** @@ -377,15 +376,13 @@ void whenTheUserDefinesTheirOwnDatabaseInitializerThenTheAutoConfiguredBatchInit } @Test - void testExecuteLocalAndNonConfiguredJob() { + void testNonConfiguredJobThrowsException() { this.contextRunner .withUserConfiguration(NamedJobConfigurationWithLocalJob.class, EmbeddedDataSourceConfiguration.class) .withPropertyValues("spring.batch.job.names:discreteLocalJob,nonConfiguredJob") .run((context) -> { - assertThatThrownBy(() -> { - context.getBean(JobLauncherApplicationRunner.class).run(); - } - ).hasRootCauseMessage("Job with name nonConfiguredJob does not exist."); + assertThat(context).hasFailed().getFailure().getRootCause() + .hasMessageContaining("nonConfiguredJob"); }); }