From f8f8a02a4d0ea111b638f9d48f3b3fae84fee200 Mon Sep 17 00:00:00 2001 From: Dimitrios Liapis Date: Thu, 18 Oct 2018 22:51:04 +0200 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 | 5 +++-- .../repository/dao/JdbcJobExecutionDao.java | 3 ++- .../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 | 9 +++++++-- .../support/SimpleJobRepositoryTests.java | 4 +++- 11 files changed, 72 insertions(+), 19 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 63874d038..d3ea566a6 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. @@ -18,6 +18,7 @@ package org.springframework.batch.core.test.repository; 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; @@ -204,6 +205,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 0aeb021ac..243ecda3a 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 @@ -37,6 +37,7 @@ import org.springframework.lang.Nullable; * @author Lucas Ward * @author Michael Minella * @author Mahmoud Ben Hassine + * @author Dimitrios Liapis * */ @SuppressWarnings("serial") @@ -234,10 +235,10 @@ public class JobExecution extends Entity { * 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 7b87b98e3..a2c8a964b 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 @@ -60,6 +60,7 @@ import org.springframework.util.Assert; * @author Robert Kasanicky * @author Michael Minella * @author Mahmoud Ben Hassine + * @author Dimitrios Liapis */ public class JdbcJobExecutionDao extends AbstractJdbcBatchMetadataDao implements JobExecutionDao, InitializingBean { @@ -85,7 +86,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 b55ed6bf1..be3902a47 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-2017 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. @@ -74,6 +74,9 @@ import static org.junit.Assert.fail; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +/** + * Tests for {@link JsrJobOperator}. + */ public class JsrJobOperatorTests extends AbstractJsrTestCase { private JobOperator jsrJobOperator; @@ -210,6 +213,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 e5658dff9..46bd82e34 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. @@ -39,6 +39,9 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +/** + * Parent Test Class for {@link JdbcJobExecutionDao} and {@link MapJobExecutionDao}. + */ public abstract class AbstractJobExecutionDaoTests { protected JobExecutionDao dao; @@ -194,15 +197,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 a563720b1..f8e845197 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. @@ -73,6 +73,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..86e1f5134 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,7 +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); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/SimpleJobRepositoryTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/SimpleJobRepositoryTests.java index 5370c1bdf..f4a40b081 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/SimpleJobRepositoryTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/repository/support/SimpleJobRepositoryTests.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. @@ -55,6 +55,7 @@ import org.springframework.batch.core.step.StepSupport; * * @author Lucas Ward * @author Will Schipp + * @author Dimitrios Liapis * */ public class SimpleJobRepositoryTests { @@ -240,6 +241,7 @@ public class SimpleJobRepositoryTests { @Test(expected = JobExecutionAlreadyRunningException.class) public void testCreateJobExecutionAlreadyRunning() throws Exception { jobExecution.setStatus(BatchStatus.STARTED); + jobExecution.setStartTime(new Date()); jobExecution.setEndTime(null); when(jobInstanceDao.getJobInstance("foo", new JobParameters())).thenReturn(jobInstance);