From bbf35b79f7f830e50545de2bbbf191382e3e7dd4 Mon Sep 17 00:00:00 2001 From: Marvin Deng Date: Wed, 11 May 2022 23:50:19 +0800 Subject: [PATCH] Make the meaning of running status consistent - A running status is STARTING, STARTED, or STOPPING. - Update related tests. Resolves #1483 --- .../batch/core/BatchStatus.java | 4 +- .../batch/core/JobExecution.java | 5 +- .../launch/support/SimpleJobLauncher.java | 2 +- .../repository/dao/JdbcJobExecutionDao.java | 2 +- .../support/SimpleJobRepository.java | 2 +- .../batch/core/BatchStatusTests.java | 1 + .../batch/core/JobExecutionTests.java | 13 +++-- .../batch/core/job/SimpleJobTests.java | 1 + .../dao/AbstractJobExecutionDaoTests.java | 11 ++++- .../SimpleJobRepositoryIntegrationTests.java | 47 +++++++++++++++++++ .../batch/core/step/job/JobStepTests.java | 1 + .../test/JobRepositoryTestUtilsTests.java | 2 + 12 files changed, 79 insertions(+), 12 deletions(-) diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/BatchStatus.java b/spring-batch-core/src/main/java/org/springframework/batch/core/BatchStatus.java index c5a2798a58..7efe0e573e 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/BatchStatus.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/BatchStatus.java @@ -84,10 +84,10 @@ public static BatchStatus max(BatchStatus status1, BatchStatus status2) { /** * Convenience method to decide if a status indicates that work is in progress. - * @return true if the status is STARTING, STARTED + * @return true if the status is STARTING, STARTED, STOPPING */ public boolean isRunning() { - return this == STARTING || this == STARTED; + return this == STARTING || this == STARTED || this == STOPPING; } /** diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/JobExecution.java b/spring-batch-core/src/main/java/org/springframework/batch/core/JobExecution.java index 7e3d20d69a..1e72406457 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/JobExecution.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/JobExecution.java @@ -256,10 +256,11 @@ public StepExecution createStepExecution(String stepName) { /** * Test if this {@link JobExecution} indicates that it is running. Note that this does * not necessarily mean that it has been persisted. - * @return {@code true} if the end time is null and the start time is not null. + * @return {@code true} if the status is one of the running statuses. + * @see BatchStatus#isRunning() */ public boolean isRunning() { - return startTime != null && endTime == null; + return status.isRunning(); } /** diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleJobLauncher.java b/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleJobLauncher.java index 71d6bf76f2..744e64e427 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleJobLauncher.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleJobLauncher.java @@ -119,7 +119,7 @@ public JobExecution run(final Job job, final JobParameters jobParameters) */ for (StepExecution execution : lastExecution.getStepExecutions()) { BatchStatus status = execution.getStatus(); - if (status.isRunning() || status == BatchStatus.STOPPING) { + if (status.isRunning()) { throw new JobExecutionAlreadyRunningException( "A job execution for this job is already running: " + lastExecution); } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/dao/JdbcJobExecutionDao.java b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/dao/JdbcJobExecutionDao.java index b10ec141cd..78f13523ad 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/dao/JdbcJobExecutionDao.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/dao/JdbcJobExecutionDao.java @@ -88,7 +88,7 @@ public class JdbcJobExecutionDao extends AbstractJdbcBatchMetadataDao implements + " from %PREFIX%JOB_EXECUTION where JOB_EXECUTION_ID = ?"; private static final String GET_RUNNING_EXECUTIONS = "SELECT E.JOB_EXECUTION_ID, E.START_TIME, E.END_TIME, E.STATUS, E.EXIT_CODE, E.EXIT_MESSAGE, E.CREATE_TIME, E.LAST_UPDATED, E.VERSION, " - + "E.JOB_INSTANCE_ID from %PREFIX%JOB_EXECUTION E, %PREFIX%JOB_INSTANCE I where E.JOB_INSTANCE_ID=I.JOB_INSTANCE_ID and I.JOB_NAME=? and E.START_TIME is not NULL and E.END_TIME is NULL order by E.JOB_EXECUTION_ID desc"; + + "E.JOB_INSTANCE_ID from %PREFIX%JOB_EXECUTION E, %PREFIX%JOB_INSTANCE I where E.JOB_INSTANCE_ID=I.JOB_INSTANCE_ID and I.JOB_NAME=? and E.STATUS in ('STARTING', 'STARTED', 'STOPPING') order by E.JOB_EXECUTION_ID desc"; private static final String CURRENT_VERSION_JOB_EXECUTION = "SELECT VERSION FROM %PREFIX%JOB_EXECUTION WHERE JOB_EXECUTION_ID=?"; diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java index 8107106f09..5fd0350466 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/repository/support/SimpleJobRepository.java @@ -136,7 +136,7 @@ public JobExecution createJobExecution(String jobName, JobParameters jobParamete // check for running executions and find the last started for (JobExecution execution : executions) { - if (execution.isRunning() || execution.isStopping()) { + if (execution.isRunning()) { throw new JobExecutionAlreadyRunningException( "A job execution for this job is already running: " + jobInstance); } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/BatchStatusTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/BatchStatusTests.java index 2d37584395..f3aa256e1c 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/BatchStatusTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/BatchStatusTests.java @@ -71,6 +71,7 @@ void testIsRunning() { assertFalse(BatchStatus.COMPLETED.isRunning()); assertTrue(BatchStatus.STARTED.isRunning()); assertTrue(BatchStatus.STARTING.isRunning()); + assertTrue(BatchStatus.STOPPING.isRunning()); } @Test diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/JobExecutionTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/JobExecutionTests.java index 158339d4ca..7aeb709672 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/JobExecutionTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/JobExecutionTests.java @@ -56,14 +56,19 @@ void testGetEndTime() { } /** - * Test method for {@link org.springframework.batch.core.JobExecution#getEndTime()}. + * Test method for {@link org.springframework.batch.core.JobExecution#isRunning()}. */ @Test void testIsRunning() { - LocalDateTime now = LocalDateTime.now(); - execution.setStartTime(now); + execution.setStatus(BatchStatus.STARTING); + assertTrue(execution.isRunning()); + execution.setStatus(BatchStatus.STARTED); assertTrue(execution.isRunning()); - execution.setEndTime(now.plus(10, ChronoUnit.SECONDS)); + execution.setStatus(BatchStatus.STOPPING); + assertTrue(execution.isRunning()); + execution.setStatus(BatchStatus.COMPLETED); + assertFalse(execution.isRunning()); + execution.setStatus(BatchStatus.FAILED); assertFalse(execution.isRunning()); } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/job/SimpleJobTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/job/SimpleJobTests.java index be24fd9be8..b32fdc1f83 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/job/SimpleJobTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/job/SimpleJobTests.java @@ -363,6 +363,7 @@ void testStepAlreadyComplete() throws Exception { stepExecution1.setStatus(BatchStatus.COMPLETED); jobRepository.add(stepExecution1); jobExecution.setEndTime(LocalDateTime.now()); + jobExecution.setStatus(BatchStatus.COMPLETED); jobRepository.update(jobExecution); jobExecution = jobRepository.createJobExecution(job.getName(), jobParameters); job.execute(jobExecution); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/AbstractJobExecutionDaoTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/AbstractJobExecutionDaoTests.java index 1eb1fa4f95..ada9a3894a 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/AbstractJobExecutionDaoTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/AbstractJobExecutionDaoTests.java @@ -204,6 +204,7 @@ void testFindRunningExecutions() { exec.setCreateTime(now); exec.setStartTime(now.plus(1, ChronoUnit.SECONDS)); exec.setEndTime(now.plus(2, ChronoUnit.SECONDS)); + exec.setStatus(BatchStatus.COMPLETED); exec.setLastUpdated(now.plus(3, ChronoUnit.SECONDS)); dao.saveJobExecution(exec); @@ -215,9 +216,17 @@ void testFindRunningExecutions() { exec.setLastUpdated(now.plus(3, ChronoUnit.SECONDS)); dao.saveJobExecution(exec); + // Stopping JobExecution as status is STOPPING + exec = new JobExecution(jobInstance, jobParameters); + exec.setStartTime(now.plus(6, ChronoUnit.SECONDS)); + exec.setStatus(BatchStatus.STOPPING); + exec.setLastUpdated(now.plus(7, ChronoUnit.SECONDS)); + dao.saveJobExecution(exec); + // Running JobExecution as StartTime is populated but EndTime is null exec = new JobExecution(jobInstance, jobParameters); exec.setStartTime(now.plus(2, ChronoUnit.SECONDS)); + exec.setStatus(BatchStatus.STARTED); exec.setLastUpdated(now.plus(3, ChronoUnit.SECONDS)); exec.createStepExecution("step"); dao.saveJobExecution(exec); @@ -231,7 +240,7 @@ void testFindRunningExecutions() { Set values = dao.findRunningJobExecutions(exec.getJobInstance().getJobName()); - assertEquals(1, values.size()); + assertEquals(3, values.size()); JobExecution value = values.iterator().next(); assertEquals(exec, value); assertEquals(now.plus(3, ChronoUnit.SECONDS), value.getLastUpdated()); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/SimpleJobRepositoryIntegrationTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/SimpleJobRepositoryIntegrationTests.java index affd2f695d..98e996f461 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/SimpleJobRepositoryIntegrationTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/SimpleJobRepositoryIntegrationTests.java @@ -37,6 +37,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.fail; /** * Repository tests using JDBC DAOs (rather than mocks). @@ -76,6 +77,7 @@ void testCreateAndFind() throws Exception { assertEquals(job.getName(), firstExecution.getJobInstance().getJobName()); jobRepository.update(firstExecution); + firstExecution.setStatus(BatchStatus.FAILED); firstExecution.setEndTime(LocalDateTime.now()); jobRepository.update(firstExecution); JobExecution secondExecution = jobRepository.createJobExecution(job.getName(), jobParams); @@ -97,6 +99,7 @@ void testCreateAndFindWithNoStartDate() throws Exception { LocalDateTime now = LocalDateTime.now(); firstExecution.setStartTime(now); firstExecution.setEndTime(now.plus(1, ChronoUnit.SECONDS)); + firstExecution.setStatus(BatchStatus.COMPLETED); jobRepository.update(firstExecution); JobExecution secondExecution = jobRepository.createJobExecution(job.getName(), jobParameters); @@ -224,4 +227,48 @@ void testReExecuteWithSameJobParameters() throws Exception { assertNotNull(jobExecution2); } + /* + * When a job execution is running, JobExecutionAlreadyRunningException should be + * thrown if trying to create any other ones with same job parameters. + */ + @Transactional + @Test + public void testReExecuteWithSameJobParametersWhenRunning() throws Exception { + JobParameters jobParameters = new JobParametersBuilder().addString("stringKey", "stringValue") + .toJobParameters(); + + // jobExecution with status STARTING + JobExecution jobExecution = jobRepository.createJobExecution(job.getName(), jobParameters); + try { + jobRepository.createJobExecution(job.getName(), jobParameters); + fail(); + } + catch (JobExecutionAlreadyRunningException e) { + // expected + } + + // jobExecution with status STARTED + jobExecution.setStatus(BatchStatus.STARTED); + jobExecution.setStartTime(LocalDateTime.now()); + jobRepository.update(jobExecution); + try { + jobRepository.createJobExecution(job.getName(), jobParameters); + fail(); + } + catch (JobExecutionAlreadyRunningException e) { + // expected + } + + // jobExecution with status STOPPING + jobExecution.setStatus(BatchStatus.STOPPING); + jobRepository.update(jobExecution); + try { + jobRepository.createJobExecution(job.getName(), jobParameters); + fail(); + } + catch (JobExecutionAlreadyRunningException e) { + // expected + } + } + } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/job/JobStepTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/job/JobStepTests.java index 20b968c0a5..ddf20b469c 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/job/JobStepTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/job/JobStepTests.java @@ -163,6 +163,7 @@ public boolean isRestartable() { assertEquals("FOO", stepExecution.getFailureExceptions().get(0).getMessage()); JobExecution jobExecution = stepExecution.getJobExecution(); jobExecution.setEndTime(LocalDateTime.now()); + jobExecution.setStatus(BatchStatus.FAILED); jobRepository.update(jobExecution); jobExecution = jobRepository.createJobExecution("job", new JobParameters()); diff --git a/spring-batch-test/src/test/java/org/springframework/batch/test/JobRepositoryTestUtilsTests.java b/spring-batch-test/src/test/java/org/springframework/batch/test/JobRepositoryTestUtilsTests.java index 2610773058..e0ed7c7eb7 100644 --- a/spring-batch-test/src/test/java/org/springframework/batch/test/JobRepositoryTestUtilsTests.java +++ b/spring-batch-test/src/test/java/org/springframework/batch/test/JobRepositoryTestUtilsTests.java @@ -24,6 +24,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.springframework.batch.core.BatchStatus; import org.springframework.batch.core.JobExecution; import org.springframework.batch.core.JobParameters; import org.springframework.batch.core.JobParametersBuilder; @@ -84,6 +85,7 @@ void testRemoveJobExecutionsWithSameJobInstance() throws Exception { List list = new ArrayList<>(); JobExecution jobExecution = jobRepository.createJobExecution("job", new JobParameters()); jobExecution.setEndTime(LocalDateTime.now()); + jobExecution.setStatus(BatchStatus.COMPLETED); list.add(jobExecution); jobRepository.update(jobExecution); jobExecution = jobRepository.createJobExecution("job", new JobParameters());