Refine contribution #3967

Related to #752

Author: fmbenhassine

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

@@ -32,7 +32,7 @@ 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 command a specified system command and its arguments.
 	 * @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.
@@ -49,6 +49,6 @@ public interface CommandRunner {
 	 *
 	 * @see Runtime#exec(String, String[], File)
 	 */
-	Process exec(String command, String[] envp, File dir) throws IOException;
+	Process exec(String command[], String[] envp, File dir) throws IOException;
 
 }

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

@@ -25,17 +25,18 @@
  * mocking {@link Runtime}.
  *
  * @author Stefano Cordio
+ * @author Mahmoud Ben Hassine
  * @since 5.0
  */
 public class JvmCommandRunner implements CommandRunner {
 
 	/**
 	 * Delegate call to {@link Runtime#exec} with the arguments provided.
 	 *
-	 * @see CommandRunner#exec(String, String[], File)
+	 * @see CommandRunner#exec(String[], String[], File)
 	 */
 	@Override
-	public Process exec(String command, String[] envp, File dir) throws IOException {
+	public Process exec(String command[], String[] envp, File dir) throws IOException {
 		return Runtime.getRuntime().exec(command, envp, dir);
 	}
 

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

@@ -102,14 +102,8 @@ public RepeatStatus execute(StepContribution contribution, ChunkContext chunkCon
 
 			@Override
 			public Integer call() throws Exception {
-				if (cmdArray.length == 1) {
-					String command = cmdArray[0];
-					Process process = commandRunner.exec(command, environmentParams, workingDirectory);
-					return process.waitFor();
-				} else {
-					Process process = Runtime.getRuntime().exec(cmdArray, environmentParams, workingDirectory);
-					return process.waitFor();
-				}
+				Process process = commandRunner.exec(cmdArray, environmentParams, workingDirectory);
+				return process.waitFor();
 			}
 
 		});
@@ -162,17 +156,13 @@ public void setCommandRunner(CommandRunner commandRunner) {
 	}
 
 	/**
-	 * @param command command to be executed in a separate system process. Either a single command can be supplied
-	 *                to be tokenized with a space delimiter, or the command and its arguments are supplied as multiple
-	 *                strings that are not tokenized.
-	 * <p>
-	 * <p>Possible calls to setCommand:
-     *
-	 * <pre> {@code setCommand("myCommand myArg1 myArg2");}</pre>
-	 * <pre> {@code setCommand("myCommand", "myArg1", "myArg2 'args for myArg2'");}</pre>
+	 * Set the command to execute along with its arguments. For example:
+	 *
+	 * <pre>setCommand("myCommand", "myArg1", "myArg2");</pre>
+	 * @param command command to be executed in a separate system process.
 	 */
 	public void setCommand(String... command) {
-		this.cmdArray = command ;
+		this.cmdArray = command;
 	}
 
 	/**
@@ -201,9 +191,9 @@ public void setWorkingDirectory(String dir) {
 	@Override
 	public void afterPropertiesSet() throws Exception {
 		Assert.notNull(commandRunner, "CommandRunner must be set");
-		Assert.notNull(cmdArray, "'cmdArray' property value is required with at least 1 element");
+		Assert.notNull(cmdArray, "'cmdArray' property value must not be null");
 		Assert.notEmpty(cmdArray, "'cmdArray' property value is required with at least 1 element");
-		Assert.noNullElements(cmdArray, "'cmdArray' property value is required with at least 1 element");
+		Assert.noNullElements(cmdArray, "'cmdArray' property value must not contain be null elements");
 		Assert.hasLength(cmdArray[0], "'cmdArray' property value is required with at least 1 element");
 		Assert.notNull(systemProcessExitCodeMapper, "SystemProcessExitCodeMapper must be set");
 		Assert.isTrue(timeout > 0, "timeout value must be greater than zero");

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

@@ -103,11 +103,11 @@ public void testExecuteWithSeparateArgument() throws Exception {
 	 */
 	@Test
 	void testExecute() throws Exception {
-		String command = getJavaCommand() + " --version";
+		String[] command = new String[] { getJavaCommand(), "--version" };
 		tasklet.setCommand(command);
 		tasklet.afterPropertiesSet();
 
-		log.info("Executing command: " + command);
+		log.info("Executing command: " + String.join(" ", command));
 		RepeatStatus exitStatus = tasklet.execute(stepExecution.createStepContribution(), null);
 
 		assertEquals(RepeatStatus.FINISHED, exitStatus);
@@ -118,21 +118,21 @@ void testExecute() throws Exception {
 	 */
 	@Test
 	void testExecuteFailure() throws Exception {
-		String command = getJavaCommand() + " org.springframework.batch.sample.tasklet.UnknownClass";
+		String[] command = new String[] { getJavaCommand() + " org.springframework.batch.sample.tasklet.UnknownClass" };
 		tasklet.setCommand(command);
 		tasklet.setTimeout(200L);
 		tasklet.afterPropertiesSet();
 
-		log.info("Executing command: " + command);
+		log.info("Executing command: " + String.join(" ", command));
 		try {
 			StepContribution contribution = stepExecution.createStepContribution();
 			RepeatStatus exitStatus = tasklet.execute(contribution, null);
 			assertEquals(RepeatStatus.FINISHED, exitStatus);
 			assertEquals(ExitStatus.FAILED, contribution.getExitStatus());
 		}
-		catch (RuntimeException e) {
+		catch (Exception e) {
 			// on some platforms the system call does not return
-			assertEquals("Execution of system command did not finish within the timeout", e.getMessage());
+			assertTrue(e.getMessage().contains("Cannot run program"));
 		}
 	}
 
@@ -141,7 +141,7 @@ void testExecuteFailure() throws Exception {
 	 */
 	@Test
 	void testExecuteException() throws Exception {
-		String command = "non-sense-that-should-cause-exception-when-attempted-to-execute";
+		String[] command = new String[] { "non-sense-that-should-cause-exception-when-attempted-to-execute" };
 		tasklet.setCommand(command);
 		tasklet.afterPropertiesSet();
 
@@ -153,12 +153,12 @@ void testExecuteException() throws Exception {
 	 */
 	@Test
 	void testExecuteTimeout() throws Exception {
-		String command = isRunningOnWindows() ? "ping 127.0.0.1" : "sleep 3";
+		String[] command = isRunningOnWindows() ? new String[] { "ping", "127.0.0.1" } : new String[] { "sleep", "3" };
 		tasklet.setCommand(command);
 		tasklet.setTimeout(10);
 		tasklet.afterPropertiesSet();
 
-		log.info("Executing command: " + command);
+		log.info("Executing command: " + String.join(" ", command));
 		Exception exception = assertThrows(SystemCommandException.class, () -> tasklet.execute(null, null));
 		assertTrue(exception.getMessage().contains("did not finish within the timeout"));
 	}
@@ -168,7 +168,7 @@ void testExecuteTimeout() throws Exception {
 	 */
 	@Test
 	void testInterruption() throws Exception {
-		String command = isRunningOnWindows() ? "ping 127.0.0.1" : "sleep 5";
+		String[] command = isRunningOnWindows() ? new String[] { "ping", "127.0.0.1" } : new String[] { "sleep", "5" };
 		tasklet.setCommand(command);
 		tasklet.setTerminationCheckInterval(10);
 		tasklet.afterPropertiesSet();
@@ -178,7 +178,7 @@ void testInterruption() throws Exception {
 		String message = exception.getMessage();
 		System.out.println(message);
 		assertTrue(message.contains("Job interrupted while executing system command"));
-		assertTrue(message.contains(command));
+		assertTrue(message.contains(command[0]));
 	}
 
 	/*
@@ -255,7 +255,8 @@ void testStopped() throws Exception {
 		when(jobExplorer.getJobExecution(1L)).thenReturn(stepExecution.getJobExecution(),
 				stepExecution.getJobExecution(), stoppedJobExecution);
 
-		String command = isRunningOnWindows() ? "ping 127.0.0.1 -n 5" : "sleep 15";
+		String[] command = isRunningOnWindows() ? new String[] { "ping", "127.0.0.1", "-n", "5" }
+				: new String[] { "sleep", "15" };
 		tasklet.setCommand(command);
 		tasklet.setTerminationCheckInterval(10);
 		tasklet.afterPropertiesSet();
@@ -294,7 +295,7 @@ public void testExecuteWithSuccessfulCommandRunnerMockExecution() throws Excepti
 		StepContribution stepContribution = stepExecution.createStepContribution();
 		CommandRunner commandRunner = mock(CommandRunner.class);
 		Process process = mock(Process.class);
-		String command = "invalid command";
+		String[] command = new String[] { "invalid command" };
 
 		when(commandRunner.exec(eq(command), any(), any())).thenReturn(process);
 		when(process.waitFor()).thenReturn(0);
@@ -314,7 +315,7 @@ public void testExecuteWithFailedCommandRunnerMockExecution() throws Exception {
 		StepContribution stepContribution = stepExecution.createStepContribution();
 		CommandRunner commandRunner = mock(CommandRunner.class);
 		Process process = mock(Process.class);
-		String command = "invalid command";
+		String[] command = new String[] { "invalid command" };
 
 		when(commandRunner.exec(eq(command), any(), any())).thenReturn(process);
 		when(process.waitFor()).thenReturn(1);