Skip to content

[Fix #898] RunShell minor improvements #910

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 24, 2025
+37 −49
Merged

[Fix #898] RunShell minor improvements #910

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 33 additions & 47 deletions impl/core/src/main/java/io/serverlessworkflow/impl/executors/RunShellExecutor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ private interface ProcessBuilderSupplier {
public CompletableFuture<WorkflowModel> 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(
Copy link
Contributor

@mcruzdev mcruzdev Oct 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot about that, nice

() -> this.shellResultSupplier.apply(taskContext, input, processBuilder));
}

@Override
Expand All @@ -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 =
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just saving a variable declaration

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<String, Object> 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
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to avoid the use of continue, which in this case was possible

.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<String, Object> entry :
Expand All @@ -128,21 +124,16 @@ public void init(RunShell taskConfiguration, WorkflowDefinition definition) {
builder.environment().put(entry.getKey(), value);
}
}

return builder;
};

this.shellResultSupplier =
(taskContext, input, processBuilder) -> {
try {
Process process = processBuilder.start();

if (taskConfiguration.isAwait()) {
return buildResultFromProcess(taskConfiguration, definition, process);
} else {
return input;
}

return taskConfiguration.isAwait()
Copy link
Collaborator Author

@fjtirado fjtirado Oct 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, this one was not needed, but I love ternary operator and could not refrain, sorry ;)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😆

? buildResultFromProcess(taskConfiguration, definition, process)
: input;
} catch (IOException | InterruptedException e) {
throw new WorkflowException(WorkflowError.runtime(taskContext, e).build(), e);
}
Expand All @@ -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);
Expand Down
6 changes: 4 additions & 2 deletions impl/test/src/test/java/io/serverlessworkflow/impl/test/RunShellExecutorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:");
});
}
}
Expand Down