Skip to content
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.

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

#734: Improve Process Result Model #773

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
11 changes: 11 additions & 0 deletions cli/src/main/java/com/devonfw/tools/ide/process/LogEvent.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.devonfw.tools.ide.process;

/**
* Represent a log event.
*
* @param error A boolean flag that indicates whether the log event represents and error or standard output
* @param message A string containing the log message
*/
public record LogEvent(boolean error, String message) {

}
Copy link
Member Author

Choose a reason for hiding this comment

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

Should it be located in the process or log package or elsewhere?

Original file line number Diff line number Diff line change
Expand Up @@ -167,14 +167,15 @@ public ProcessResult run(ProcessMode processMode) {

this.processBuilder.command(args);

List<LogEvent> logs = new ArrayList<>();
List<String> out = null;
List<String> err = null;

Process process = this.processBuilder.start();

if (processMode == ProcessMode.DEFAULT_CAPTURE) {
CompletableFuture<List<String>> outFut = readInputStream(process.getInputStream());
CompletableFuture<List<String>> errFut = readInputStream(process.getErrorStream());
CompletableFuture<List<String>> outFut = readInputStream(process.getInputStream(), false, logs);
CompletableFuture<List<String>> errFut = readInputStream(process.getErrorStream(), true, logs);
out = outFut.get();
err = errFut.get();
Copy link
Member Author

Choose a reason for hiding this comment

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

Should we remove out and err and keep all the logs in a single list of LogEvent? If so, we will need to refactor ProcessResult as well, right?

}
Expand All @@ -187,10 +188,9 @@ public ProcessResult run(ProcessMode processMode) {
exitCode = process.waitFor();
}

ProcessResult result = new ProcessResultImpl(exitCode, out, err);
ProcessResult result = new ProcessResultImpl(exitCode, out, err, logs);

performLogging(result, exitCode, interpreter);

return result;

} catch (CliProcessException | IllegalStateException e) {
Expand All @@ -214,11 +214,19 @@ public ProcessResult run(ProcessMode processMode) {
* @param is {@link InputStream}.
* @return {@link CompletableFuture}.
*/
private static CompletableFuture<List<String>> readInputStream(InputStream is) {
private static CompletableFuture<List<String>> readInputStream(InputStream is, boolean errorStream, List<LogEvent> logs) {

return CompletableFuture.supplyAsync(() -> {

try (InputStreamReader isr = new InputStreamReader(is); BufferedReader br = new BufferedReader(isr)) {

String line;
while ((line = br.readLine()) != null) {
synchronized (logs) {
LogEvent logEvent = new LogEvent(errorStream, line);
logs.add(logEvent);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I am not sure, if I am using LogEvent correctly. In this implementation, a single LogEvent instance represents one line from either stout or sterr. I don't see another solution to capture the entire log while keeping the order of the log streams.

}
return br.lines().toList();
} catch (Throwable e) {
throw new RuntimeException("There was a problem while executing the program", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,23 @@ public class ProcessResultImpl implements ProcessResult {

private final List<String> err;

private final List<LogEvent> logEvents;

/**
* The constructor.
*
* @param exitCode the {@link #getExitCode() exit code}.
* @param out the {@link #getOut() out}.
* @param err the {@link #getErr() err}.
* @param logEvents the {@link #getLogEvents()} () logEvents}.
*/
public ProcessResultImpl(int exitCode, List<String> out, List<String> err) {
public ProcessResultImpl(int exitCode, List<String> out, List<String> err, List<LogEvent> logEvents) {

super();
this.exitCode = exitCode;
this.out = Objects.requireNonNullElse(out, Collections.emptyList());
this.err = Objects.requireNonNullElse(err, Collections.emptyList());
this.logEvents = Objects.requireNonNullElse(logEvents, Collections.emptyList());
}

@Override
Expand All @@ -51,6 +55,11 @@ public List<String> getErr() {
return this.err;
}

public List<LogEvent> getLogEvents() {

return this.logEvents;
}

@Override
public void log(IdeLogLevel level, IdeContext context) {
log(level, context, level);
Expand Down
Loading