Refine contribution #3972

Related to #3955

Author: fmbenhassine

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2006-2021 the original author or authors.
+ * Copyright 2006-2022 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.

spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/CommandRunner.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2006-2021 the original author or authors.
+ * Copyright 2006-2022 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.
@@ -19,50 +19,35 @@
 import java.io.IOException;
 
 /**
- * Interface for executing commands. This abstraction is only
- * useful in order to allow classes that make {@link Runtime#exec} calls
- * to be testable, since the invoked command might not be
- * available during tests execution.
+ * Strategy interface for executing commands. This abstraction is useful to decouple the
+ * command execution from the enclosing tasklet so that implementations can be unit tested
+ * in isolation.
  *
  * @author Stefano Cordio
- * @since FIXME
+ * @author Mahmoud Ben Hassine
+ * @since 5.0
  */
 public interface CommandRunner {
 
 	/**
-	 * Executes the specified string command in a separate process with the
-	 * specified environment and working directory.
-	 *
-	 * @param   command   a specified system command.
-	 *
-	 * @param   envp      array of strings, each element of which
-	 *                    has environment variable settings in the format
-	 *                    <i>name</i>=<i>value</i>, or
-	 *                    {@code null} if the subprocess should inherit
-	 *                    the environment of the current process.
-	 *
-	 * @param   dir       the working directory of the subprocess, or
-	 *                    {@code null} if the subprocess should inherit
-	 *                    the working directory of the current process.
-	 *
-	 * @return  A new {@link Process} object for managing the subprocess
-	 *
-	 * @throws  SecurityException
-	 *          If a security manager exists and its
-	 *          {@link SecurityManager#checkExec checkExec}
-	 *          method doesn't allow creation of the subprocess
-	 *
-	 * @throws  IOException
-	 *          If an I/O error occurs
-	 *
-	 * @throws  NullPointerException
-	 *          If {@code command} is {@code null},
-	 *          or one of the elements of {@code envp} is {@code null}
-	 *
-	 * @throws  IllegalArgumentException
-	 *          If {@code command} is empty
-	 *
-	 * @see     Runtime#exec(String, String[], File)
+	 * Executes the specified string command in a separate process with the specified
+	 * environment and working directory.
+	 * @param command a specified system command.
+	 * @param envp array of strings, each element of which has environment variable
+	 * settings in the format <i>name</i>=<i>value</i>, or {@code null} if the subprocess
+	 * should inherit the environment of the current process.
+	 * @param dir the working directory of the subprocess, or {@code null} if the
+	 * subprocess should inherit the working directory of the current process.
+	 * @return A new {@link Process} object for managing the subprocess
+	 * @throws SecurityException If a security manager exists and its
+	 * {@link SecurityManager#checkExec checkExec} method doesn't allow creation of the
+	 * subprocess
+	 * @throws IOException If an I/O error occurs
+	 * @throws NullPointerException If {@code command} is {@code null}, or one of the
+	 * elements of {@code envp} is {@code null}
+	 * @throws IllegalArgumentException If {@code command} is empty
+	 *
+	 * @see Runtime#exec(String, String[], File)
 	 */
 	Process exec(String command, String[] envp, File dir) throws IOException;
 

spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/JvmCommandRunner.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2006-2021 the original author or authors.
+ * Copyright 2006-2022 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.
@@ -20,12 +20,12 @@
 
 /**
  * Implementation of the {@link CommandRunner} interface that calls the standard
- * {@link Runtime#exec} method. It should be noted that there is no unit tests for
- * this class, since there is only one line of actual code, that would only be
- * testable by mocking {@link Runtime}.
+ * {@link Runtime#exec} method. It should be noted that there is no unit tests for this
+ * class, since there is only one line of actual code, that would only be testable by
+ * mocking {@link Runtime}.
  *
  * @author Stefano Cordio
- * @since FIXME
+ * @since 5.0
  */
 public class JvmCommandRunner implements CommandRunner {
 

spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/SystemCommandTasklet.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2006-2021 the original author or authors.
+ * Copyright 2006-2022 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.
@@ -64,7 +64,7 @@ public class SystemCommandTasklet implements StepExecutionListener, StoppableTas
 
 	protected static final Log logger = LogFactory.getLog(SystemCommandTasklet.class);
 
-	private static CommandRunner commandRunner = new JvmCommandRunner();
+	private CommandRunner commandRunner = new JvmCommandRunner();
 
 	private String command;
 
@@ -144,24 +144,14 @@ else if (stopped) {
 		}
 	}
 
-	/**
-	 * Static setter for the {@link CommandRunner} so it can be adjusted before
-	 * dependency injection. Typically overridden by
-	 * {@link #setCommandRunner(CommandRunner)}.
-	 *
-	 * @param commandRunner {@link CommandRunner} instance to be used by SystemCommandTasklet instance.
-	 */
-	public static void presetCommandRunner(CommandRunner commandRunner) {
-		SystemCommandTasklet.commandRunner = commandRunner;
-	}
-
 	/**
 	 * Injection setter for the {@link CommandRunner}.
-	 *
-	 * @param commandRunner {@link CommandRunner} instance to be used by SystemCommandTasklet instance.
+	 * @param commandRunner {@link CommandRunner} instance to be used by
+	 * SystemCommandTasklet instance. Defaults to {@link JvmCommandRunner}.
+	 * @since 5.0
 	 */
 	public void setCommandRunner(CommandRunner commandRunner) {
-		SystemCommandTasklet.commandRunner = commandRunner;
+		this.commandRunner = commandRunner;
 	}
 
 	/**

spring-batch-core/src/test/java/org/springframework/batch/core/step/tasklet/SystemCommandTaskletIntegrationTests.java

@@ -172,27 +172,8 @@ void testInterruption() throws Exception {
 	 */
 	@Test
 	public void testCommandRunnerNotSet() throws Exception {
-		SystemCommandTasklet.presetCommandRunner(null);
-		try {
-			tasklet.afterPropertiesSet();
-			fail();
-		}
-		catch (IllegalArgumentException e) {
-			// expected
-		} finally {
-			SystemCommandTasklet.presetCommandRunner(new JvmCommandRunner());
-		}
-
 		tasklet.setCommandRunner(null);
-		try {
-			tasklet.afterPropertiesSet();
-			fail();
-		}
-		catch (IllegalArgumentException e) {
-			// expected
-		} finally {
-			SystemCommandTasklet.presetCommandRunner(new JvmCommandRunner());
-		}
+		assertThrows(IllegalArgumentException.class, tasklet::afterPropertiesSet);
 	}
 
 	/*
@@ -296,50 +277,42 @@ private boolean isRunningOnWindows() {
 
 	@Test
 	public void testExecuteWithSuccessfulCommandRunnerMockExecution() throws Exception {
-		try {
-			StepContribution stepContribution = stepExecution.createStepContribution();
-			CommandRunner commandRunner = mock(CommandRunner.class);
-			Process process = mock(Process.class);
-			String command = "invalid command";
+		StepContribution stepContribution = stepExecution.createStepContribution();
+		CommandRunner commandRunner = mock(CommandRunner.class);
+		Process process = mock(Process.class);
+		String command = "invalid command";
 
-			when(commandRunner.exec(eq(command), any(), any())).thenReturn(process);
-			when(process.waitFor()).thenReturn(0);
+		when(commandRunner.exec(eq(command), any(), any())).thenReturn(process);
+		when(process.waitFor()).thenReturn(0);
 
-			SystemCommandTasklet.presetCommandRunner(commandRunner);
-			tasklet.setCommand(command);
-			tasklet.afterPropertiesSet();
+		tasklet.setCommandRunner(commandRunner);
+		tasklet.setCommand(command);
+		tasklet.afterPropertiesSet();
 
-			RepeatStatus exitStatus = tasklet.execute(stepContribution, null);
+		RepeatStatus exitStatus = tasklet.execute(stepContribution, null);
 
-			assertEquals(RepeatStatus.FINISHED, exitStatus);
-			assertEquals(ExitStatus.COMPLETED, stepContribution.getExitStatus());
-		} finally {
-			SystemCommandTasklet.presetCommandRunner(new JvmCommandRunner());
-		}
+		assertEquals(RepeatStatus.FINISHED, exitStatus);
+		assertEquals(ExitStatus.COMPLETED, stepContribution.getExitStatus());
 	}
 
 	@Test
 	public void testExecuteWithFailedCommandRunnerMockExecution() throws Exception {
-		try {
-			StepContribution stepContribution = stepExecution.createStepContribution();
-			CommandRunner commandRunner = mock(CommandRunner.class);
-			Process process = mock(Process.class);
-			String command = "invalid command";
+		StepContribution stepContribution = stepExecution.createStepContribution();
+		CommandRunner commandRunner = mock(CommandRunner.class);
+		Process process = mock(Process.class);
+		String command = "invalid command";
 
-			when(commandRunner.exec(eq(command), any(), any())).thenReturn(process);
-			when(process.waitFor()).thenReturn(1);
+		when(commandRunner.exec(eq(command), any(), any())).thenReturn(process);
+		when(process.waitFor()).thenReturn(1);
 
-			SystemCommandTasklet.presetCommandRunner(commandRunner);
-			tasklet.setCommand(command);
-			tasklet.afterPropertiesSet();
+		tasklet.setCommandRunner(commandRunner);
+		tasklet.setCommand(command);
+		tasklet.afterPropertiesSet();
 
-			RepeatStatus exitStatus = tasklet.execute(stepContribution, null);
+		RepeatStatus exitStatus = tasklet.execute(stepContribution, null);
 
-			assertEquals(RepeatStatus.FINISHED, exitStatus);
-			assertEquals(ExitStatus.FAILED, stepContribution.getExitStatus());
-		} finally {
-			SystemCommandTasklet.presetCommandRunner(new JvmCommandRunner());
-		}
+		assertEquals(RepeatStatus.FINISHED, exitStatus);
+		assertEquals(ExitStatus.FAILED, stepContribution.getExitStatus());
 	}
 
 }