From 13f154f0c15371f760aef415db2f25bf02bb73ae Mon Sep 17 00:00:00 2001 From: fjtirado Date: Fri, 24 Oct 2025 17:08:43 +0200 Subject: [PATCH] [Fix #898] RunShell minor improvements Enable test only in linux (till we implement for other Operating systems) Run asynchrnously Signed-off-by: fjtirado --- .../impl/executors/RunShellExecutor.java | 80 ++++++++----------- .../impl/test/RunShellExecutorTest.java | 6 +- 2 files changed, 37 insertions(+), 49 deletions(-) diff --git a/impl/core/src/main/java/io/serverlessworkflow/impl/executors/RunShellExecutor.java b/impl/core/src/main/java/io/serverlessworkflow/impl/executors/RunShellExecutor.java index 73fdb697..efa611da 100644 --- a/impl/core/src/main/java/io/serverlessworkflow/impl/executors/RunShellExecutor.java +++ b/impl/core/src/main/java/io/serverlessworkflow/impl/executors/RunShellExecutor.java @@ -53,8 +53,8 @@ private interface ProcessBuilderSupplier { public CompletableFuture apply( WorkflowContext workflowContext, TaskContext taskContext, WorkflowModel input) { ProcessBuilder processBuilder = this.processBuilderSupplier.apply(workflowContext, taskContext); - return CompletableFuture.completedFuture( - this.shellResultSupplier.apply(taskContext, input, processBuilder)); + return CompletableFuture.supplyAsync( + () -> this.shellResultSupplier.apply(taskContext, input, processBuilder)); } @Override @@ -69,48 +69,44 @@ public void init(RunShell taskConfiguration, WorkflowDefinition definition) { (workflowContext, taskContext) -> { WorkflowApplication application = definition.application(); - String command = - ExpressionUtils.isExpr(shellCommand) - ? WorkflowUtils.buildStringResolver( - application, shellCommand, taskContext.input().asJavaObject()) - .apply(workflowContext, taskContext, taskContext.input()) - : shellCommand; - - StringBuilder commandBuilder = new StringBuilder(command); + StringBuilder commandBuilder = + new StringBuilder( + ExpressionUtils.isExpr(shellCommand) + ? WorkflowUtils.buildStringResolver( + application, shellCommand, taskContext.input().asJavaObject()) + .apply(workflowContext, taskContext, taskContext.input()) + : shellCommand); if (shell.getArguments() != null && shell.getArguments().getAdditionalProperties() != null) { for (Map.Entry entry : shell.getArguments().getAdditionalProperties().entrySet()) { - - String argKey = - ExpressionUtils.isExpr(entry.getKey()) - ? WorkflowUtils.buildStringResolver( - application, entry.getKey(), taskContext.input().asJavaObject()) - .apply(workflowContext, taskContext, taskContext.input()) - : entry.getKey(); - - if (entry.getValue() == null) { - commandBuilder.append(" ").append(argKey); - continue; + commandBuilder + .append(" ") + .append( + ExpressionUtils.isExpr(entry.getKey()) + ? WorkflowUtils.buildStringResolver( + application, entry.getKey(), taskContext.input().asJavaObject()) + .apply(workflowContext, taskContext, taskContext.input()) + : entry.getKey()); + if (entry.getValue() != null) { + + commandBuilder + .append("=") + .append( + ExpressionUtils.isExpr(entry.getValue()) + ? WorkflowUtils.buildStringResolver( + application, + entry.getValue().toString(), + taskContext.input().asJavaObject()) + .apply(workflowContext, taskContext, taskContext.input()) + : entry.getValue().toString()); } - - String argValue = - ExpressionUtils.isExpr(entry.getValue()) - ? WorkflowUtils.buildStringResolver( - application, - entry.getValue().toString(), - taskContext.input().asJavaObject()) - .apply(workflowContext, taskContext, taskContext.input()) - : entry.getValue().toString(); - - commandBuilder.append(" ").append(argKey).append("=").append(argValue); } } // TODO: support Windows cmd.exe ProcessBuilder builder = new ProcessBuilder("sh", "-c", commandBuilder.toString()); - if (shell.getEnvironment() != null && shell.getEnvironment().getAdditionalProperties() != null) { for (Map.Entry entry : @@ -128,7 +124,6 @@ public void init(RunShell taskConfiguration, WorkflowDefinition definition) { builder.environment().put(entry.getKey(), value); } } - return builder; }; @@ -136,13 +131,9 @@ public void init(RunShell taskConfiguration, WorkflowDefinition definition) { (taskContext, input, processBuilder) -> { try { Process process = processBuilder.start(); - - if (taskConfiguration.isAwait()) { - return buildResultFromProcess(taskConfiguration, definition, process); - } else { - return input; - } - + return taskConfiguration.isAwait() + ? buildResultFromProcess(taskConfiguration, definition, process) + : input; } catch (IOException | InterruptedException e) { throw new WorkflowException(WorkflowError.runtime(taskContext, e).build(), e); } @@ -156,17 +147,12 @@ public void init(RunShell taskConfiguration, WorkflowDefinition definition) { private WorkflowModel buildResultFromProcess( RunShell taskConfiguration, WorkflowDefinition definition, Process process) throws IOException, InterruptedException { - int exitCode = process.waitFor(); - String stdout = new String(process.getInputStream().readAllBytes(), StandardCharsets.UTF_8); String stderr = new String(process.getErrorStream().readAllBytes(), StandardCharsets.UTF_8); - RunTaskConfiguration.ProcessReturnType returnType = taskConfiguration.getReturn(); - WorkflowModelFactory modelFactory = definition.application().modelFactory(); - - return switch (returnType) { + return switch (taskConfiguration.getReturn()) { case ALL -> modelFactory.fromAny(new ProcessResult(exitCode, stdout.trim(), stderr.trim())); case NONE -> modelFactory.fromNull(); case CODE -> modelFactory.from(exitCode); diff --git a/impl/test/src/test/java/io/serverlessworkflow/impl/test/RunShellExecutorTest.java b/impl/test/src/test/java/io/serverlessworkflow/impl/test/RunShellExecutorTest.java index bd96d225..40f8600f 100644 --- a/impl/test/src/test/java/io/serverlessworkflow/impl/test/RunShellExecutorTest.java +++ b/impl/test/src/test/java/io/serverlessworkflow/impl/test/RunShellExecutorTest.java @@ -26,7 +26,10 @@ import java.util.Map; import org.assertj.core.api.SoftAssertions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledOnOs; +import org.junit.jupiter.api.condition.OS; +@EnabledOnOs(value = OS.LINUX) public class RunShellExecutorTest { @Test @@ -152,8 +155,7 @@ void testStderr() throws IOException { softly -> { softly.assertThat(outputModel.asText()).isPresent(); softly.assertThat(outputModel.asText().get()).isNotEmpty(); - softly.assertThat(outputModel.asText().get()).contains("ls: cannot access"); - softly.assertThat(outputModel.asText().get()).contains("No such file or directory"); + softly.assertThat(outputModel.asText().get()).contains("ls:"); }); } }