From b28238e4b5e8633430f9a8c5f0579fa9f9d132ae Mon Sep 17 00:00:00 2001 From: Dimitrios Liapis Date: Sun, 28 Oct 2018 18:15:48 +0100 Subject: [PATCH] Explicitly considering Start Time not being null for running Job Executions This is to exclude from capturing erroneous Job Executions. An example is whenever a TaskRejectedException is thrown after submitting to the taskExecutor in SimpleJobLauncher#run(), the JobExecution is left without a Start or End Time. Also related tests are fixed. Resolves BATCH-2675 --- .../repository/JdbcJobRepositoryTests.java | 8 +++++++- .../batch/core/JobExecution.java | 7 ++++--- .../repository/dao/JdbcJobExecutionDao.java | 5 +++-- .../batch/core/JobExecutionTests.java | 5 ++++- .../MapJobExplorerFactoryBeanTests.java | 16 ++++++++++++---- .../core/jsr/launch/JsrJobOperatorTests.java | 7 ++++++- .../dao/AbstractJobExecutionDaoTests.java | 19 ++++++++++++++++--- .../dao/OptimisticLockingFailureTests.java | 4 +++- .../MapJobRepositoryFactoryBeanTests.java | 11 +++++++++-- .../SimpleJobRepositoryIntegrationTests.java | 8 ++++++-- 10 files changed, 70 insertions(+), 20 deletions(-) diff --git a/spring-batch-core-tests/src/test/java/org/springframework/batch/core/test/repository/JdbcJobRepositoryTests.java b/spring-batch-core-tests/src/test/java/org/springframework/batch/core/test/repository/JdbcJobRepositoryTests.java index 8085c4ed7..c5414f39a 100644 --- a/spring-batch-core-tests/src/test/java/org/springframework/batch/core/test/repository/JdbcJobRepositoryTests.java +++ b/spring-batch-core-tests/src/test/java/org/springframework/batch/core/test/repository/JdbcJobRepositoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2013 the original author or authors. + * Copyright 2006-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. @@ -23,6 +23,7 @@ import static org.junit.Assert.fail; import java.io.Serializable; import java.sql.Timestamp; import java.util.ArrayList; +import java.util.Date; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -202,6 +203,11 @@ public class JdbcJobRepositoryTests { try { JobExecution execution = repository.createJobExecution(job.getName(), new JobParameters()); + + //simulate running execution + execution.setStartTime(new Date()); + repository.update(execution); + cacheJobIds(execution); list.add(execution); Thread.sleep(1000); 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 71b352294..0890f2c57 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 @@ -1,5 +1,5 @@ /* - * Copyright 2006-2013 the original author or authors. + * Copyright 2006-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. @@ -35,6 +35,7 @@ import java.util.concurrent.CopyOnWriteArraySet; * * @author Lucas Ward * @author Michael Minella + * @author Dimitrios Liapis * */ @SuppressWarnings("serial") @@ -224,10 +225,10 @@ public class JobExecution extends Entity { * Test if this {@link JobExecution} indicates that it is running. It should * be noted that this does not necessarily mean that it has been persisted * as such yet. - * @return true if the end time is null + * @return true if the end time is null and the start time is not null */ public boolean isRunning() { - return endTime == null; + return startTime != null && endTime == null; } /** 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 13e62b937..e2964561d 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 @@ -1,5 +1,5 @@ /* - * Copyright 2006-2013 the original author or authors. + * Copyright 2006-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. @@ -58,6 +58,7 @@ import org.springframework.util.Assert; * @author Dave Syer * @author Robert Kasanicky * @author Michael Minella + * @author Dimitrios Liapis */ public class JdbcJobExecutionDao extends AbstractJdbcBatchMetadataDao implements JobExecutionDao, InitializingBean { @@ -83,7 +84,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, E.JOB_CONFIGURATION_LOCATION from %PREFIX%JOB_EXECUTION E, %PREFIX%JOB_INSTANCE I where E.JOB_INSTANCE_ID=I.JOB_INSTANCE_ID and I.JOB_NAME=? and E.END_TIME is NULL order by E.JOB_EXECUTION_ID desc"; + + "E.JOB_INSTANCE_ID, E.JOB_CONFIGURATION_LOCATION 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"; 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/test/java/org/springframework/batch/core/JobExecutionTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/JobExecutionTests.java index 0805b8bb3..418847520 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 @@ -1,5 +1,5 @@ /* - * Copyright 2006-2014 the original author or authors. + * Copyright 2006-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. @@ -30,6 +30,7 @@ import org.springframework.util.SerializationUtils; /** * @author Dave Syer + * @author Dimitrios Liapis * */ public class JobExecutionTests { @@ -65,6 +66,7 @@ public class JobExecutionTests { */ @Test public void testIsRunning() { + execution.setStartTime(new Date()); assertTrue(execution.isRunning()); execution.setEndTime(new Date(100L)); assertFalse(execution.isRunning()); @@ -76,6 +78,7 @@ public class JobExecutionTests { */ @Test public void testIsRunningWithStoppedExecution() { + execution.setStartTime(new Date()); assertTrue(execution.isRunning()); execution.stop(); assertTrue(execution.isRunning()); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/explore/support/MapJobExplorerFactoryBeanTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/explore/support/MapJobExplorerFactoryBeanTests.java index c565cab08..4b16899b6 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/explore/support/MapJobExplorerFactoryBeanTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/explore/support/MapJobExplorerFactoryBeanTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2010-2014 the original author or authors. + * Copyright 2010-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. @@ -18,11 +18,14 @@ package org.springframework.batch.core.explore.support; import static org.junit.Assert.assertEquals; import org.junit.Test; +import org.springframework.batch.core.JobExecution; import org.springframework.batch.core.JobParameters; import org.springframework.batch.core.explore.JobExplorer; -import org.springframework.batch.core.explore.support.MapJobExplorerFactoryBean; +import org.springframework.batch.core.repository.JobRepository; import org.springframework.batch.core.repository.support.MapJobRepositoryFactoryBean; +import java.util.Date; + /** * Tests for {@link MapJobExplorerFactoryBean}. */ @@ -36,8 +39,13 @@ public class MapJobExplorerFactoryBeanTests { public void testCreateExplorer() throws Exception { MapJobRepositoryFactoryBean repositoryFactory = new MapJobRepositoryFactoryBean(); - repositoryFactory.getObject().createJobExecution("foo", new JobParameters()); - + JobRepository jobRepository = repositoryFactory.getObject(); + JobExecution jobExecution = jobRepository.createJobExecution("foo", new JobParameters()); + + //simulating a running job execution + jobExecution.setStartTime(new Date()); + jobRepository.update(jobExecution); + MapJobExplorerFactoryBean tested = new MapJobExplorerFactoryBean(repositoryFactory); tested.afterPropertiesSet(); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/jsr/launch/JsrJobOperatorTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/jsr/launch/JsrJobOperatorTests.java index c0ac82d24..dc038a0b3 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/jsr/launch/JsrJobOperatorTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/jsr/launch/JsrJobOperatorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013 the original author or authors. + * Copyright 2013-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. @@ -79,6 +79,9 @@ import org.springframework.core.task.SyncTaskExecutor; import org.springframework.test.util.ReflectionTestUtils; import org.springframework.transaction.PlatformTransactionManager; +/** + * Tests for {@link JsrJobOperator}. + */ public class JsrJobOperatorTests extends AbstractJsrTestCase { private JobOperator jsrJobOperator; @@ -225,6 +228,8 @@ public class JsrJobOperatorTests extends AbstractJsrTestCase { @Test(expected=JobExecutionIsRunningException.class) public void testAbandonJobRunning() throws Exception { JobExecution jobExecution = new JobExecution(5L); + jobExecution.setStartTime(new Date(1L)); + when(jobExplorer.getJobExecution(5L)).thenReturn(jobExecution); jsrJobOperator.abandon(5L); 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 2aee364ba..c31b1072b 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 @@ -1,5 +1,5 @@ /* - * Copyright 2008-2014 the original author or authors. + * Copyright 2008-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. @@ -38,6 +38,9 @@ import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.transaction.annotation.Transactional; import org.springframework.util.Assert; +/** + * Parent Test Class for {@link JdbcJobExecutionDao} and {@link MapJobExecutionDao}. + */ public abstract class AbstractJobExecutionDaoTests { protected JobExecutionDao dao; @@ -193,15 +196,25 @@ public abstract class AbstractJobExecutionDaoTests { @Transactional @Test public void testFindRunningExecutions() { - + //Normally completed JobExecution as EndTime is populated JobExecution exec = new JobExecution(jobInstance, jobParameters); exec.setCreateTime(new Date(0)); - exec.setEndTime(new Date(1L)); + exec.setStartTime(new Date(1L)); + exec.setEndTime(new Date(2L)); exec.setLastUpdated(new Date(5L)); dao.saveJobExecution(exec); + //BATCH-2675 + //Abnormal JobExecution as both StartTime and EndTime are null + //This can occur when SimpleJobLauncher#run() submission to taskExecutor throws a TaskRejectedException exec = new JobExecution(jobInstance, jobParameters); exec.setLastUpdated(new Date(5L)); + dao.saveJobExecution(exec); + + //Running JobExecution as StartTime is populated but EndTime is null + exec = new JobExecution(jobInstance, jobParameters); + exec.setStartTime(new Date(2L)); + exec.setLastUpdated(new Date(5L)); exec.createStepExecution("step"); dao.saveJobExecution(exec); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/OptimisticLockingFailureTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/OptimisticLockingFailureTests.java index 2bb4de2be..c98a5d250 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/OptimisticLockingFailureTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/dao/OptimisticLockingFailureTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2014 the original author or authors. + * Copyright 2014-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. @@ -72,6 +72,8 @@ public class OptimisticLockingFailureTests { .addLong("test", 1L) .toJobParameters()); + Thread.sleep(1000); + while(restartJobExecution.isRunning()) { // wait for async launched job to complete execution } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/MapJobRepositoryFactoryBeanTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/MapJobRepositoryFactoryBeanTests.java index 3508491a1..d7bc4bbe1 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/MapJobRepositoryFactoryBeanTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/MapJobRepositoryFactoryBeanTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2014 the original author or authors. + * Copyright 2008-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. @@ -17,11 +17,14 @@ package org.springframework.batch.core.repository.support; import org.junit.Test; import org.springframework.batch.core.Job; +import org.springframework.batch.core.JobExecution; import org.springframework.batch.core.JobParameters; import org.springframework.batch.core.job.JobSupport; import org.springframework.batch.core.repository.JobExecutionAlreadyRunningException; import org.springframework.batch.core.repository.JobRepository; +import java.util.Date; + import static org.junit.Assert.fail; /** @@ -42,7 +45,11 @@ public class MapJobRepositoryFactoryBeanTests { Job job = new JobSupport("jobName"); JobParameters jobParameters = new JobParameters(); - repository.createJobExecution(job.getName(), jobParameters); + JobExecution jobExecution = repository.createJobExecution(job.getName(), jobParameters); + + // simulate a running execution + jobExecution.setStartTime(new Date()); + repository.update(jobExecution); try { repository.createJobExecution(job.getName(), jobParameters); 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 88b006ac5..cb6d0e514 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 @@ -1,5 +1,5 @@ /* - * Copyright 2008-2014 the original author or authors. + * Copyright 2008-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. @@ -43,6 +43,7 @@ import static org.junit.Assert.fail; * Repository tests using JDBC DAOs (rather than mocks). * * @author Robert Kasanicky + * @author Dimitrios Liapis */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration(locations = "/org/springframework/batch/core/repository/dao/sql-dao-test.xml") @@ -180,8 +181,11 @@ public class SimpleJobRepositoryIntegrationTests { @Test public void testOnlyOneJobExecutionAllowedRunning() throws Exception { job.setRestartable(true); - jobRepository.createJobExecution(job.getName(), jobParameters); + JobExecution jobExecution = jobRepository.createJobExecution(job.getName(), jobParameters); + //simulating a running job execution + jobExecution.setStartTime(new Date()); + jobRepository.update(jobExecution); try { jobRepository.createJobExecution(job.getName(), jobParameters); fail();