#734: Improve Process Result Model (#773)

CHANGELOG.adoc

@@ -31,6 +31,7 @@ Then run the `setup` and all should work fine.
 
 Release with new features and bugfixes:
 
+* https://github.com/devonfw/IDEasy/issues/734[#734]: Improve ProcessResult: get out and err in order
 * https://github.com/devonfw/IDEasy/issues/764[#764]: Fix IDEasy in CMD
 * https://github.com/devonfw/IDEasy/issues/774[#774]: HTTP proxy support not working properly
 * https://github.com/devonfw/IDEasy/issues/792[#792]: Honor new variable IDE_OPTIONS in ide command wrapper

cli/src/main/java/com/devonfw/tools/ide/process/OutputMessage.java

@@ -0,0 +1,11 @@
+package com.devonfw.tools.ide.process;
+
+/**
+ * Represent an output message that can be either from stdout or stderr.
+ *
+ * @param error A boolean flag that indicates whether the output message is from stdout or stderr
+ * @param message A string containing the outout message
+ */
+public record OutputMessage(boolean error, String message) {
+
+}

cli/src/main/java/com/devonfw/tools/ide/process/ProcessContextImpl.java

@@ -13,6 +13,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.stream.Collectors;
 
 import com.devonfw.tools.ide.cli.CliProcessException;
@@ -168,17 +169,16 @@ public ProcessResult run(ProcessMode processMode) {
 
       this.processBuilder.command(args);
 
-      List<String> out = null;
-      List<String> err = null;
+      ConcurrentLinkedQueue<OutputMessage> output = new ConcurrentLinkedQueue<>();
 
       Process process = this.processBuilder.start();
 
       try {
         if (processMode == ProcessMode.DEFAULT_CAPTURE) {
-          CompletableFuture<List<String>> outFut = readInputStream(process.getInputStream());
-          CompletableFuture<List<String>> errFut = readInputStream(process.getErrorStream());
-          out = outFut.get();
-          err = errFut.get();
+          CompletableFuture<Void> outFut = readInputStream(process.getInputStream(), false, output);
+          CompletableFuture<Void> errFut = readInputStream(process.getErrorStream(), true, output);
+          outFut.get();
+          errFut.get();
         }
 
         int exitCode;
@@ -189,7 +189,8 @@ public ProcessResult run(ProcessMode processMode) {
           exitCode = process.waitFor();
         }
 
-        ProcessResult result = new ProcessResultImpl(this.executable.getFileName().toString(), command, exitCode, out, err);
+        List<OutputMessage> finalOutput = new ArrayList<>(output);
+        ProcessResult result = new ProcessResultImpl(this.executable.getFileName().toString(), command, exitCode, finalOutput);
 
         performLogging(result, exitCode, interpreter);
 
@@ -218,14 +219,22 @@ public ProcessResult run(ProcessMode processMode) {
    * "https://stackoverflow.com/questions/14165517/processbuilder-forwarding-stdout-and-stderr-of-started-processes-without-blocki/57483714#57483714">StackOverflow</a>
    *
    * @param is {@link InputStream}.
+   * @param errorStream to identify if the output came from stdout or stderr
    * @return {@link CompletableFuture}.
    */
-  private static CompletableFuture<List<String>> readInputStream(InputStream is) {
+  private static CompletableFuture<Void> readInputStream(InputStream is, boolean errorStream, ConcurrentLinkedQueue<OutputMessage> outputMessages) {
 
     return CompletableFuture.supplyAsync(() -> {
 
       try (InputStreamReader isr = new InputStreamReader(is); BufferedReader br = new BufferedReader(isr)) {
-        return br.lines().toList();
+
+        String line;
+        while ((line = br.readLine()) != null) {
+          OutputMessage outputMessage = new OutputMessage(errorStream, line);
+          outputMessages.add(outputMessage);
+        }
+
+        return null;
       } catch (Throwable e) {
         throw new RuntimeException("There was a problem while executing the program", e);
       }

cli/src/main/java/com/devonfw/tools/ide/process/ProcessResult.java

@@ -76,6 +76,12 @@ default boolean isSuccessful() {
    */
   List<String> getErr();
 
+  /**
+   * @return the {@link List} with {@link OutputMessage} that captured on standard out and standard error lines. Will be {@code null} if not captured but
+   *     redirected.
+   */
+  List<OutputMessage> getOutputMessages();
+
   /**
    * Logs output and error messages on the provided log level.
    *

cli/src/main/java/com/devonfw/tools/ide/process/ProcessResultImpl.java

@@ -19,27 +19,23 @@ public class ProcessResultImpl implements ProcessResult {
 
   private final int exitCode;
 
-  private final List<String> out;
-
-  private final List<String> err;
+  private final List<OutputMessage> outputMessages;
 
   /**
    * The constructor.
    *
    * @param executable the {@link #getExecutable() executable}.
    * @param command the {@link #getCommand() command}.
    * @param exitCode the {@link #getExitCode() exit code}.
-   * @param out the {@link #getOut() out}.
-   * @param err the {@link #getErr() err}.
+   * @param outputMessages {@link #getOutputMessages() output Messages}.
    */
-  public ProcessResultImpl(String executable, String command, int exitCode, List<String> out, List<String> err) {
+  public ProcessResultImpl(String executable, String command, int exitCode, List<OutputMessage> outputMessages) {
 
     super();
     this.executable = executable;
     this.command = command;
     this.exitCode = exitCode;
-    this.out = Objects.requireNonNullElse(out, Collections.emptyList());
-    this.err = Objects.requireNonNullElse(err, Collections.emptyList());
+    this.outputMessages = Objects.requireNonNullElse(outputMessages, Collections.emptyList());
   }
 
   @Override
@@ -63,13 +59,20 @@ public int getExitCode() {
   @Override
   public List<String> getOut() {
 
-    return this.out;
+    return this.outputMessages.stream().filter(outputMessage -> !outputMessage.error()).map(OutputMessage::message).toList();
   }
 
   @Override
   public List<String> getErr() {
 
-    return this.err;
+    return this.outputMessages.stream().filter(OutputMessage::error).map(OutputMessage::message).toList();
+  }
+
+  @Override
+  public List<OutputMessage> getOutputMessages() {
+
+    return this.outputMessages;
+
   }
 
   @Override
@@ -80,22 +83,23 @@ public void log(IdeLogLevel level, IdeContext context) {
   @Override
   public void log(IdeLogLevel outLevel, IdeContext context, IdeLogLevel errorLevel) {
 
-    if (!this.out.isEmpty()) {
-      doLog(outLevel, this.out, context);
-    }
-    if (!this.err.isEmpty()) {
-      doLog(errorLevel, this.err, context);
+    if (!this.outputMessages.isEmpty()) {
+      for (OutputMessage outputMessage : this.outputMessages) {
+        if (outputMessage.error()) {
+          doLog(errorLevel, outputMessage.message(), context);
+        } else {
+          doLog(outLevel, outputMessage.message(), context);
+        }
+      }
     }
   }
 
-  private void doLog(IdeLogLevel level, List<String> lines, IdeContext context) {
-    for (String line : lines) {
-      // remove !MESSAGE from log message
-      if (line.startsWith("!MESSAGE ")) {
-        line = line.substring(9);
-      }
-      context.level(level).log(line);
+  private void doLog(IdeLogLevel level, String message, IdeContext context) {
+    // remove !MESSAGE from log message
+    if (message.startsWith("!MESSAGE ")) {
+      message = message.substring(9);
     }
+    context.level(level).log(message);
   }
 
   @Override

cli/src/test/java/com/devonfw/tools/ide/context/ProcessContextGitMock.java

@@ -8,6 +8,7 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import com.devonfw.tools.ide.process.OutputMessage;
 import com.devonfw.tools.ide.process.ProcessContext;
 import com.devonfw.tools.ide.process.ProcessErrorHandling;
 import com.devonfw.tools.ide.process.ProcessMode;
@@ -21,59 +22,25 @@ public class ProcessContextGitMock implements ProcessContext {
 
   private final List<String> arguments;
 
-  private final List<String> errors;
-
-  private final List<String> outs;
-
   private final LocalDateTime now;
 
-  private int exitCode;
-
   private final Path directory;
 
+  private List<OutputMessage> outputMessages;
+
   /**
    * @param directory the {@link Path} to the git repository.
    */
   public ProcessContextGitMock(Path directory) {
 
     this.arguments = new ArrayList<>();
-    this.errors = new ArrayList<>();
-    this.outs = new ArrayList<>();
-    this.exitCode = ProcessResult.SUCCESS;
     this.directory = directory;
     this.now = LocalDateTime.now();
+    this.outputMessages = new ArrayList<OutputMessage>();
   }
 
-  /**
-   * @return the mocked {@link ProcessResult#getExitCode() exit code}.
-   */
-  public int getExitCode() {
-
-    return this.exitCode;
-  }
-
-  /**
-   * @param exitCode the {@link #getExitCode() exit code}.
-   */
-  public void setExitCode(int exitCode) {
-
-    this.exitCode = exitCode;
-  }
-
-  /**
-   * @return the {@link List} of mocked error messages.
-   */
-  public List<String> getErrors() {
-
-    return errors;
-  }
-
-  /**
-   * @return the {@link List} of mocked out messages.
-   */
-  public List<String> getOuts() {
-
-    return outs;
+  public void addOutputMessage(OutputMessage message) {
+    this.outputMessages.add(message);
   }
 
   @Override
@@ -121,6 +88,7 @@ public ProcessContext withPathEntry(Path path) {
   @Override
   public ProcessResult run(ProcessMode processMode) {
 
+    int exitCode = ProcessResult.SUCCESS;
     StringBuilder command = new StringBuilder("git");
     for (String arg : this.arguments) {
       command.append(' ');
@@ -131,16 +99,15 @@ public ProcessResult run(ProcessMode processMode) {
     if (this.arguments.contains("clean")) {
       try {
         Files.deleteIfExists(this.directory.resolve("new-folder"));
-        this.exitCode = 0;
       } catch (IOException e) {
         throw new RuntimeException(e);
       }
     }
     // part of git cleanup checks if a new directory 'new-folder' exists
     if (this.arguments.contains("ls-files")) {
       if (Files.exists(this.directory.resolve("new-folder"))) {
-        this.outs.add("new-folder");
-        this.exitCode = 0;
+        OutputMessage outputMessage = new OutputMessage(false, "new-folder");
+        this.outputMessages.add(outputMessage);
       }
     }
     if (this.arguments.contains("clone")) {
@@ -149,14 +116,13 @@ public ProcessResult run(ProcessMode processMode) {
         Path newFile = Files.createFile(gitFolderPath.resolve("url"));
         // 3rd argument = repository Url
         Files.writeString(newFile, this.arguments.get(2));
-        this.exitCode = 0;
       } catch (IOException e) {
         throw new RuntimeException(e);
       }
     }
     // always consider that files were changed
     if (this.arguments.contains("diff-index")) {
-      this.exitCode = 1;
+      exitCode = 1;
     }
     // changes file back to initial state (uses reference file in .git folder)
     if (this.arguments.contains("reset")) {
@@ -165,7 +131,7 @@ public ProcessResult run(ProcessMode processMode) {
           Files.copy(gitFolderPath.resolve("objects").resolve("referenceFile"), this.directory.resolve("trackedFile"),
               StandardCopyOption.REPLACE_EXISTING);
         }
-        this.exitCode = 0;
+        exitCode = ProcessResult.SUCCESS;
       } catch (IOException e) {
         throw new RuntimeException(e);
       }
@@ -175,13 +141,13 @@ public ProcessResult run(ProcessMode processMode) {
         Files.createDirectories(gitFolderPath);
         Path newFile = Files.createFile(gitFolderPath.resolve("update"));
         Files.writeString(newFile, this.now.toString());
-        this.exitCode = 0;
+        exitCode = ProcessResult.SUCCESS;
       } catch (IOException e) {
         throw new RuntimeException(e);
       }
     }
     this.arguments.clear();
-    return new ProcessResultImpl("git", command.toString(), this.exitCode, this.outs, this.errors);
+    return new ProcessResultImpl("git", command.toString(), exitCode, this.outputMessages);
   }
 
 }

cli/src/test/java/com/devonfw/tools/ide/git/GitContextTest.java

@@ -19,6 +19,7 @@
 import com.devonfw.tools.ide.context.ProcessContextGitMock;
 import com.devonfw.tools.ide.io.FileAccess;
 import com.devonfw.tools.ide.io.FileAccessImpl;
+import com.devonfw.tools.ide.process.OutputMessage;
 
 /**
  * Test of {@link GitContext}.
@@ -71,7 +72,8 @@ public void testRunGitClone(@TempDir Path tempDir) {
     // arrange
     String gitRepoUrl = "https://github.com/test";
     IdeTestContext context = newGitContext(tempDir);
-    this.processContext.getOuts().add("test-remote");
+    OutputMessage outputMessage = new OutputMessage(false, "test-remote");
+    this.processContext.addOutputMessage(outputMessage);
     // act
     context.getGitContext().pullOrClone(GitUrl.of(gitRepoUrl), tempDir);
     // assert
@@ -89,7 +91,8 @@ public void testRunGitPullWithoutForce(@TempDir Path tempDir) {
     // arrange
     String gitRepoUrl = "https://github.com/test";
     IdeTestContext context = newGitContext(tempDir);
-    this.processContext.getOuts().add("test-remote");
+    OutputMessage outputMessage = new OutputMessage(false, "test-remote");
+    this.processContext.addOutputMessage(outputMessage);
     FileAccess fileAccess = new FileAccessImpl(context);
     Path gitFolderPath = tempDir.resolve(".git");
     fileAccess.mkdirs(gitFolderPath);
@@ -129,7 +132,8 @@ public void testRunGitPullWithForceStartsReset(@TempDir Path tempDir) {
       throw new RuntimeException(e);
     }
     IdeTestContext context = newGitContext(tempDir);
-    this.processContext.getOuts().add("test-remote");
+    OutputMessage outputMessage = new OutputMessage(false, "test-remote");
+    this.processContext.addOutputMessage(outputMessage);
     // act
     context.getGitContext().pullOrCloneAndResetIfNeeded(new GitUrl(gitRepoUrl, "master"), tempDir, "origin");
     // assert
@@ -147,7 +151,8 @@ public void testRunGitPullWithForceStartsCleanup(@TempDir Path tempDir) {
     // arrange
     String gitRepoUrl = "https://github.com/test";
     IdeTestContext context = newGitContext(tempDir);
-    this.processContext.getOuts().add("test-remote");
+    OutputMessage outputMessage = new OutputMessage(false, "test-remote");
+    this.processContext.addOutputMessage(outputMessage);
     GitContext gitContext = context.getGitContext();
     FileAccess fileAccess = context.getFileAccess();
     Path gitFolderPath = tempDir.resolve(".git");

cli/src/test/java/com/devonfw/tools/ide/process/ProcessContextImplTest.java

@@ -221,6 +221,18 @@ public void enablingCaptureShouldRedirectAndCaptureStreamsWithErrorsCorrectly()
     assertThat(context).log(IdeLogLevel.ERROR).hasMessageContaining("another error message to stderr");
   }
 
+  @Test
+  public void defaultCaptureShouldCaptureStreamsWithCorrectOrder() {
+    // arrange
+    IdeTestContext context = newContext(PROJECT_BASIC, null, false);
+
+    // act
+    context.newProcess().executable(TEST_RESOURCES.resolve("process-context").resolve("log-order.sh")).run();
+
+    // assert
+    assertThat(context).log(IdeLogLevel.INFO).hasEntries("out1", "err1", "out2", "err2");
+  }
+
   private IdeLogLevel convertToIdeLogLevel(ProcessErrorHandling processErrorHandling) {
 
     return switch (processErrorHandling) {