Refine contribution #4944

- Log warnings instead of throwing exceptions when recovering
already recovered job executions
- Update tests
- Update Javadocs

Author: fmbenhassine

spring-batch-core/src/main/java/org/springframework/batch/core/launch/JobOperator.java

@@ -256,8 +256,7 @@ JobExecution startNextInstance(Job job) throws JobRestartException, JobExecution
 	 * for restart by updating its execution context with the flag {@code recovered=true}.
 	 * @param jobExecution the {@link JobExecution} to recover
 	 * @return the {@link JobExecution} after it has been marked as recovered
-	 * @throws UnexpectedJobExecutionException if the job execution is already complete or
-	 * abandoned
+	 * @since 6.0
 	 */
 	JobExecution recover(JobExecution jobExecution);
 

spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/CommandLineJobOperator.java

@@ -37,7 +37,8 @@
 
 /**
  * A command-line utility to operate Spring Batch jobs using the {@link JobOperator}. It
- * allows starting, stopping, restarting, and abandoning jobs from the command line.
+ * allows starting, stopping, restarting, abandoning and recovering jobs from the command
+ * line.
  * <p>
  * This utility requires a Spring application context to be set up with the necessary
  * batch infrastructure, including a {@link JobOperator}, a {@link JobRepository}, and a
@@ -247,6 +248,7 @@ public int recover(long jobExecutionId) {
      *     <li>restart jobExecutionId</li>
      *     <li>stop jobExecutionId</li>
      *     <li>abandon jobExecutionId</li>
+     *     <li>recover jobExecutionId</li>
      * </ul>
 	 * <p>
 	 * and <code>jobParameters</code> are key-value pairs in the form name=value,type,identifying.
@@ -269,6 +271,7 @@ public static void main(String[] args) {
 					 - restart jobExecutionId
 					 - stop jobExecutionId
 					 - abandon jobExecutionId
+					 - recover jobExecutionId
 					and jobParameters are key-value pairs in the form name=value,type,identifying.
 					""";
 			System.err.printf(String.format(usage, CommandLineJobOperator.class.getName()));

spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleJobOperator.java

@@ -395,18 +395,22 @@ public JobExecution abandon(JobExecution jobExecution) throws JobExecutionAlread
 	@Override
 	public JobExecution recover(JobExecution jobExecution) {
 		Assert.notNull(jobExecution, "JobExecution must not be null");
-
 		if (jobExecution.getExecutionContext().containsKey("recovered")) {
-			if (logger.isInfoEnabled()) {
-				logger.info("already recovered job execution: " + jobExecution);
+			if (logger.isWarnEnabled()) {
+				logger.warn("Job execution already recovered: " + jobExecution);
 			}
-			throw new UnexpectedJobExecutionException("JobExecution is already recovered");
+			return jobExecution;
 		}
 
 		BatchStatus jobStatus = jobExecution.getStatus();
-		if (jobStatus == BatchStatus.COMPLETED || jobStatus == BatchStatus.ABANDONED) {
-			throw new UnexpectedJobExecutionException(
-					"JobExecution is already complete or abandoned and therefore cannot be recovered: " + jobExecution);
+		if (jobStatus == BatchStatus.COMPLETED || jobStatus == BatchStatus.ABANDONED
+				|| jobStatus == BatchStatus.UNKNOWN) {
+			if (logger.isWarnEnabled()) {
+				logger.warn(
+						"JobExecution is already complete or abandoned or in an unknown state, and therefore cannot be recovered: "
+								+ jobExecution);
+			}
+			return jobExecution;
 		}
 
 		if (logger.isInfoEnabled()) {
@@ -415,7 +419,7 @@ public JobExecution recover(JobExecution jobExecution) {
 
 		for (StepExecution stepExecution : jobExecution.getStepExecutions()) {
 			BatchStatus stepStatus = stepExecution.getStatus();
-			if (stepStatus.isRunning() || stepStatus == BatchStatus.STOPPING) {
+			if (stepStatus.isRunning()) {
 				stepExecution.setStatus(BatchStatus.FAILED);
 				stepExecution.setEndTime(LocalDateTime.now());
 				stepExecution.getExecutionContext().put("recovered", true);

spring-batch-core/src/test/java/org/springframework/batch/core/launch/support/TaskExecutorJobOperatorTests.java

@@ -32,7 +32,6 @@
 import org.springframework.batch.core.job.JobExecution;
 import org.springframework.batch.core.job.JobExecutionException;
 import org.springframework.batch.core.job.JobInstance;
-import org.springframework.batch.core.job.UnexpectedJobExecutionException;
 import org.springframework.batch.core.job.parameters.JobParameters;
 import org.springframework.batch.core.job.parameters.JobParametersIncrementer;
 import org.springframework.batch.core.step.Step;
@@ -56,14 +55,9 @@
 import org.springframework.batch.support.PropertiesConverter;
 import org.springframework.lang.Nullable;
 
-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.assertTrue;
+import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 
 /**
  * @author Dave Syer
@@ -429,49 +423,51 @@ void testAbortNonStopping() {
 	}
 
 	@Test
-	void testRecover() {
+	void testRecoverStartedJob() {
 		JobInstance jobInstance = new JobInstance(123L, job.getName());
 		JobExecution jobExecution = new JobExecution(jobInstance, 111L, jobParameters);
 		jobExecution.setStatus(BatchStatus.STARTED);
 		jobExecution.createStepExecution("step1").setStatus(BatchStatus.STARTED);
 		when(jobRepository.getJobExecution(111L)).thenReturn(jobExecution);
-		when(jobRepository.getLastJobExecution(jobInstance)).thenReturn(jobExecution);
 		JobExecution recover = jobOperator.recover(jobExecution);
 		assertEquals(BatchStatus.FAILED, recover.getStatus());
+		assertNotNull(recover.getEndTime());
 		assertTrue(recover.getExecutionContext().containsKey("recovered"));
 	}
 
 	@Test
-	void testRecoverStepStopping() {
+	void testRecoverStoppingStep() {
 		JobInstance jobInstance = new JobInstance(123L, job.getName());
 		JobExecution jobExecution = new JobExecution(jobInstance, 111L, jobParameters);
 		jobExecution.setStatus(BatchStatus.STARTED);
 		jobExecution.createStepExecution("step1").setStatus(BatchStatus.STOPPING);
 		when(jobRepository.getJobExecution(111L)).thenReturn(jobExecution);
-		when(jobRepository.getLastJobExecution(jobInstance)).thenReturn(jobExecution);
 		JobExecution recover = jobOperator.recover(jobExecution);
 		assertEquals(BatchStatus.FAILED, recover.getStatus());
+		assertNotNull(recover.getEndTime());
 		assertTrue(recover.getExecutionContext().containsKey("recovered"));
 	}
 
 	@Test
-	void testRecoverJobAbandon() {
+	void testRecoverAbandonedJob() {
 		JobInstance jobInstance = new JobInstance(123L, job.getName());
 		JobExecution jobExecution = new JobExecution(jobInstance, 111L, jobParameters);
 		jobExecution.setStatus(BatchStatus.ABANDONED);
 		when(jobRepository.getJobExecution(111L)).thenReturn(jobExecution);
-		when(jobRepository.getLastJobExecution(jobInstance)).thenReturn(jobExecution);
-		assertThrows(UnexpectedJobExecutionException.class, () -> jobOperator.recover(jobExecution));
+		JobExecution recover = jobOperator.recover(jobExecution);
+		assertSame(recover, jobExecution);
+		verifyNoInteractions(jobRepository);
 	}
 
 	@Test
-	void testRecoverJobCompleted() {
+	void testRecoverCompletedJob() {
 		JobInstance jobInstance = new JobInstance(123L, job.getName());
 		JobExecution jobExecution = new JobExecution(jobInstance, 111L, jobParameters);
 		jobExecution.setStatus(BatchStatus.COMPLETED);
 		when(jobRepository.getJobExecution(111L)).thenReturn(jobExecution);
-		when(jobRepository.getLastJobExecution(jobInstance)).thenReturn(jobExecution);
-		assertThrows(UnexpectedJobExecutionException.class, () -> jobOperator.recover(jobExecution));
+		JobExecution recover = jobOperator.recover(jobExecution);
+		assertSame(recover, jobExecution);
+		verifyNoInteractions(jobRepository);
 	}
 
 	static class MockJob extends AbstractJob {