From 26c39cbbae55cbc0b7b0b61bea4091f26dc19666 Mon Sep 17 00:00:00 2001 From: Mahmoud Ben Hassine Date: Wed, 12 Oct 2022 17:23:39 +0200 Subject: [PATCH] Refine contribution #3967 Related to #752 --- .../core/step/tasklet/CommandRunner.java | 4 +-- .../core/step/tasklet/JvmCommandRunner.java | 5 ++-- .../step/tasklet/SystemCommandTasklet.java | 28 ++++++------------ .../SystemCommandTaskletIntegrationTests.java | 29 ++++++++++--------- 4 files changed, 29 insertions(+), 37 deletions(-) diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/CommandRunner.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/CommandRunner.java index fbaa6195e..7b0c4a7e7 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/CommandRunner.java +++ b/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 name=value, 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; } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/JvmCommandRunner.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/JvmCommandRunner.java index 710f4590b..a16bd6fce 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/JvmCommandRunner.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/JvmCommandRunner.java @@ -25,6 +25,7 @@ import java.io.IOException; * mocking {@link Runtime}. * * @author Stefano Cordio + * @author Mahmoud Ben Hassine * @since 5.0 */ public class JvmCommandRunner implements CommandRunner { @@ -32,10 +33,10 @@ 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); } diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/SystemCommandTasklet.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/SystemCommandTasklet.java index 84ad3e1e6..cd1ceb819 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/SystemCommandTasklet.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/SystemCommandTasklet.java @@ -102,14 +102,8 @@ public class SystemCommandTasklet implements StepExecutionListener, StoppableTas @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 class SystemCommandTasklet implements StepExecutionListener, StoppableTas } /** - * @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. - *

- *

Possible calls to setCommand: - * - * 

 {@code setCommand("myCommand myArg1 myArg2");}
- * 
 {@code setCommand("myCommand", "myArg1", "myArg2 'args for myArg2'");}
+ * Set the command to execute along with its arguments. For example: + * + * 
setCommand("myCommand", "myArg1", "myArg2");
+ * @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 class SystemCommandTasklet implements StepExecutionListener, StoppableTas @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"); diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/tasklet/SystemCommandTaskletIntegrationTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/tasklet/SystemCommandTaskletIntegrationTests.java index d467a1f6b..185de0f20 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/tasklet/SystemCommandTaskletIntegrationTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/tasklet/SystemCommandTaskletIntegrationTests.java @@ -103,11 +103,11 @@ class SystemCommandTaskletIntegrationTests { */ @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 @@ class SystemCommandTaskletIntegrationTests { */ @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 @@ class SystemCommandTaskletIntegrationTests { */ @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 @@ class SystemCommandTaskletIntegrationTests { */ @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 @@ class SystemCommandTaskletIntegrationTests { */ @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 @@ class SystemCommandTaskletIntegrationTests { 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 @@ class SystemCommandTaskletIntegrationTests { 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 @@ class SystemCommandTaskletIntegrationTests { 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 @@ class SystemCommandTaskletIntegrationTests { 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);