#2385: Cleanup of IdeLogEntry, IdeLogListener and IdeGuiLogListener

CHANGELOG.adoc

@@ -17,6 +17,7 @@ The full list of changes for this release can be found in https://github.com/dev
 
 Release with new features and bugfixes:
 
+* https://github.com/devonfw/IDEasy/issues/2725[#2725]: Cleanup of IdeLogEntry, IdeLogListener and IdeGuiLogListener
 * https://github.com/devonfw/IDEasy/issues/2585[#2585]: Create npm base commandlet for gemini cli
 * https://github.com/devonfw/IDEasy/issues/2365[#2365]: Reduce plugin reinstallations
 * https://github.com/devonfw/IDEasy/issues/2701[#2701]: Stop exporting UV_PROJECT_ENVIRONMENT which broke uv projects
@@ -60,8 +61,7 @@ The full list of changes for this release can be found in https://github.com/dev
 
 == 2026.09.001
 
-* https://github.com/devonfw/IDEasy/issues/2293[#2293]: Rework spyder to have same features as IdeToolCommandlets
-Release with new features and bugfixes:
+* https://github.com/devonfw/IDEasy/issues/2293[#2293]: Rework spyder to have same features as IdeToolCommandlets Release with new features and bugfixes:
 
 * https://github.com/devonfw/IDEasy/issues/1525[#1525]: Document known issue and workaround for lombok plugin in Eclipse
 * https://github.com/devonfw/IDEasy/issues/1031[#1031]: Added OpenRewrite commandlet

cli/src/main/java/com/devonfw/tools/ide/log/IdeLogEntry.java

@@ -14,23 +14,20 @@
  * @param error the {@link IdeSubLogger#log(Throwable, String) optional error that was logged}.
  * @param contains - {@code true} if the {@link IdeLogEntry} to create is used as sub-string pattern for {@link #matches(IdeLogEntry) matching},
  *     {@code false} otherwise.
+ * @param timestamp the {@link Instant} when the {@link IdeLogEntry} was created.
  */
-public record IdeLogEntry(IdeLogLevel level, String message, String rawMessage, Object[] args, Throwable error, boolean contains, Long timeStamp) {
+public record IdeLogEntry(IdeLogLevel level, String message, String rawMessage, Object[] args, Throwable error, boolean contains, Instant timestamp) {
 
-  /**
-   * The constructor.
-   *
-   * @param level - {@link #level()}.
-   * @param message - {@link #message()}.
-   * @param rawMessage - {@link #rawMessage()}.
-   * @param args - {@link #args()}.
-   * @param error - {@link #error()}.
-   * @param contains - {@link #contains()}.
-   */
-  public IdeLogEntry {
+  /** The {@link DateTimeFormatter} used by {@link #displayValue()}. */
+  private static final DateTimeFormatter TIME_FORMATTER = DateTimeFormatter.ofPattern("HH:mm:ss.SSS").withZone(ZoneId.systemDefault());
 
+  public IdeLogEntry {
     // validation to be used by all constructors...
-    if ((message == null) && (rawMessage == null)) {
+    if (level == null) {
+      throw new NullPointerException();
+    }
+
+    if ((message == null) && (rawMessage == null) && (error == null)) {
       throw new IllegalStateException();
     }
   }
@@ -41,7 +38,7 @@ public record IdeLogEntry(IdeLogLevel level, String message, String rawMessage,
    */
   public IdeLogEntry(IdeLogLevel level, String message) {
 
-    this(level, message, null, null, null, false, System.currentTimeMillis());
+    this(level, message, null, null, null, false, Instant.now());
   }
 
   /**
@@ -51,7 +48,7 @@ public IdeLogEntry(IdeLogLevel level, String message) {
    */
   public IdeLogEntry(IdeLogLevel level, String message, boolean contains) {
 
-    this(level, message, null, null, null, contains, System.currentTimeMillis());
+    this(level, message, null, null, null, contains, Instant.now());
   }
 
   /**
@@ -62,7 +59,7 @@ public IdeLogEntry(IdeLogLevel level, String message, boolean contains) {
    */
   public IdeLogEntry(IdeLogLevel level, String message, String rawMessage, Object[] args) {
 
-    this(level, message, rawMessage, args, null, false, System.currentTimeMillis());
+    this(level, message, rawMessage, args, null, false, Instant.now());
   }
 
   /**
@@ -74,7 +71,7 @@ public IdeLogEntry(IdeLogLevel level, String message, String rawMessage, Object[
    */
   public IdeLogEntry(IdeLogLevel level, String message, String rawMessage, Object[] args, Throwable error) {
 
-    this(level, message, rawMessage, args, error, false, System.currentTimeMillis());
+    this(level, message, rawMessage, args, error, false, Instant.now());
   }
 
   /**
@@ -86,7 +83,7 @@ public IdeLogEntry(IdeLogLevel level, String message, String rawMessage, Object[
    */
   public IdeLogEntry(IdeLogLevel level, String message, String rawMessage, Object[] args, Throwable error, boolean contains) {
 
-    this(level, message, rawMessage, args, error, contains, System.currentTimeMillis());
+    this(level, message, rawMessage, args, error, contains, Instant.now());
   }
 
   /**
@@ -196,8 +193,6 @@ public static IdeLogEntry ofProcessable(String message) {
   /// @return A human-readable, formatted string of the log entry following the structure: `timestamp | [LogLevel] message`
   public String displayValue() {
 
-    DateTimeFormatter DATETIME_FORMAT = DateTimeFormatter.ofPattern("HH:mm:ss.SSS").withZone(ZoneId.systemDefault());
-    String timeStampReadable = DATETIME_FORMAT.format(Instant.ofEpochMilli(this.timeStamp));
-    return String.format("%s | [%s] %s", timeStampReadable, this.level(), this.message());
+    return String.format("%s | [%s] %s", IdeLogEntry.TIME_FORMATTER.format(this.timestamp()), this.level(), this.message());
   }
 }

cli/src/main/java/com/devonfw/tools/ide/log/IdeLogListener.java

@@ -7,13 +7,9 @@
 public interface IdeLogListener {
 
   /**
-   * @param level the {@link IdeLogLevel}.
-   * @param message the actual message to be logged.
-   * @param rawMessage the raw message logged (without args filled in)
-   * @param args the optional message arguments.
-   * @param error the optional error.
+   * @param entry the {@link IdeLogEntry} to log.
    * @return {@code true} to accept this log event (default), {@code false} otherwise to reject further processing and suppress the log message.
    */
-  boolean onLog(IdeLogLevel level, String message, String rawMessage, Object[] args, Throwable error);
+  boolean onLog(IdeLogEntry entry);
 
 }

cli/src/main/java/com/devonfw/tools/ide/log/IdeLogListenerBuffer.java

@@ -36,10 +36,10 @@ public IdeLogListenerBuffer() {
   }
 
   @Override
-  public boolean onLog(IdeLogLevel level, String message, String rawMessage, Object[] args, Throwable error) {
+  public boolean onLog(IdeLogEntry entry) {
     if (this.buffering) {
-      if (level.ordinal() >= threshold.ordinal()) {
-        this.buffer.add(new IdeLogEntry(level, message, rawMessage, args, error));
+      if (entry.level().ordinal() >= threshold.ordinal()) {
+        this.buffer.add(entry);
       }
       return false;
     }

cli/src/main/java/com/devonfw/tools/ide/log/IdeLogListenerNone.java

@@ -13,7 +13,7 @@ private IdeLogListenerNone() {
   }
 
   @Override
-  public boolean onLog(IdeLogLevel level, String message, String rawMessage, Object[] args, Throwable error) {
+  public boolean onLog(IdeLogEntry entry) {
 
     return true;
   }

cli/src/main/java/com/devonfw/tools/ide/log/Slf4jLoggerAdapter.java

@@ -133,7 +133,10 @@ protected void handleNormalizedLoggingCall(Level level, Marker marker, String me
       argFormatter = startContext.getArgFormatter();
     }
     String composedMessage = compose(argFormatter, message, args);
-    boolean accept = listener.onLog(ideLevel, composedMessage, message, args, error);
+    boolean accept = true;
+    if ((composedMessage != null) || (message != null) || (error != null)) {
+      accept = listener.onLog(new IdeLogEntry(ideLevel, composedMessage, message, args, error));
+    }
     if (accept) {
       java.util.logging.Level julLevel = ideLevel.getJulLevel();
       this.julLogger.log(julLevel, composedMessage, error);

cli/src/test/java/com/devonfw/tools/ide/log/IdeLogListenerCollector.java

@@ -27,10 +27,10 @@ public List<IdeLogEntry> getEntries() {
   }
 
   @Override
-  public boolean onLog(IdeLogLevel level, String message, String rawMessage, Object[] args, Throwable error) {
-    boolean accept = super.onLog(level, message, rawMessage, args, error);
+  public boolean onLog(IdeLogEntry entry) {
+    boolean accept = super.onLog(entry);
     if (accept) {
-      this.entries.add(new IdeLogEntry(level, message, rawMessage, args, error));
+      this.entries.add(entry);
     }
     return accept;
   }

gui/src/main/java/com/devonfw/ide/gui/core/context/IdeGuiLogListener.java

@@ -6,7 +6,6 @@
 import com.devonfw.ide.gui.core.event.GuiEventBus;
 import com.devonfw.ide.gui.core.event.console.LogEvent;
 import com.devonfw.tools.ide.log.IdeLogEntry;
-import com.devonfw.tools.ide.log.IdeLogLevel;
 import com.devonfw.tools.ide.log.IdeLogListenerBuffer;
 
 /// Listener class that listens to internal ideasy output, e.g. output from commandlets that are run.
@@ -22,20 +21,13 @@ public IdeGuiLogListener(GuiEventBus eventBus) {
   }
 
   @Override
-  public boolean onLog(IdeLogLevel level, String message, String rawMessage, Object[] args, Throwable error) {
+  public boolean onLog(IdeLogEntry entry) {
 
-    //If we are in buffer mode, pass responsibility to superclass
-    if (this.isBuffering()) {
-      super.onLog(level, message, rawMessage, args, error);
+    boolean accept = super.onLog(entry);
+    if (accept) {
+      this.eventBus.sendEvent(new LogEvent(entry));
     }
-
-    if (this.eventBus != null && message != null) {
-      this.eventBus.sendEvent(new LogEvent(new IdeLogEntry(level, message)));
-      if (error != null) {
-        this.eventBus.sendEvent(new LogEvent(new IdeLogEntry(IdeLogLevel.ERROR, "  Error: " + error.getMessage())));
-      }
-    }
-    return true; // continue processing (also log to standard output if needed)
+    return accept;
   }
 
 }

gui/src/main/java/com/devonfw/ide/gui/core/service/CommandletService.java

@@ -15,11 +15,9 @@
 import com.devonfw.ide.gui.core.context.IdeGuiContext;
 import com.devonfw.ide.gui.core.context.IdeGuiLogListener;
 import com.devonfw.ide.gui.core.event.GuiEventBus;
-import com.devonfw.ide.gui.core.event.console.LogEvent;
 import com.devonfw.ide.gui.core.modal.IdeDialog;
 import com.devonfw.ide.gui.core.progress.ProgressBarTask;
 import com.devonfw.tools.ide.context.IdeStartContextImpl;
-import com.devonfw.tools.ide.log.IdeLogEntry;
 import com.devonfw.tools.ide.log.IdeLogLevel;
 
 /**
@@ -104,11 +102,9 @@ protected Void call() {
 
             LOG.info("Commandlet {} completed successfully.", commandlet);
           } catch (Exception e) {
+            guiLogListener.flushAndEndBuffering();
             LOG.error("Failed to run commandlet {}: {}", commandlet, e.getMessage(), e);
 
-            eventBus.sendEvent(new LogEvent(
-                new IdeLogEntry(IdeLogLevel.ERROR, "[ERROR] Failed to launch " + commandlet + ": " + e.getMessage())
-            ));
           }
           return null;
         }

gui/src/test/java/com/devonfw/ide/gui/core/context/IdeGuiLogListenerTest.java

@@ -0,0 +1,117 @@
+package com.devonfw.ide.gui.core.context;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.devonfw.ide.gui.core.event.GuiEventBus;
+import com.devonfw.ide.gui.core.event.console.LogEvent;
+import com.devonfw.tools.ide.context.IdeStartContextImpl;
+import com.devonfw.tools.ide.log.IdeLogEntry;
+import com.devonfw.tools.ide.log.IdeLogLevel;
+import com.devonfw.tools.ide.log.IdeLogListenerNone;
+
+import io.github.mmm.event.EventListener;
+
+/**
+ * Tests for {@link IdeGuiLogListener}.
+ */
+public class IdeGuiLogListenerTest extends Assertions {
+
+  /**
+   * An {@link EventListener} for {@link LogEvent} that records every event published on the {@link GuiEventBus} instead of touching the JavaFX toolkit, so the
+   * number of times {@link IdeGuiLogListener#onLog(IdeLogEntry)} sends a message can be counted.
+   */
+  private static final class RecordingLogListener implements EventListener<LogEvent> {
+
+    private final List<IdeLogEntry> entries = new ArrayList<>();
+
+    @Override
+    public void onEvent(LogEvent event) {
+
+      this.entries.add(event.logEntry());
+    }
+
+    List<IdeLogEntry> getEntries() {
+      return this.entries;
+    }
+  }
+
+  /**
+   * {@link IdeStartContextImpl} holds its state in a process-wide static field that is (re)assigned by every construction and offers no reset hook. This test
+   * installs a buffering {@link IdeGuiLogListener} into that static; without cleanup it would leak into every other gui test running in the same surefire JVM,
+   * routing their SLF4J log events into the leftover buffering listener. Restore a neutral, non-buffering no-op context afterwards.
+   */
+  @AfterEach
+  void resetStartContext() {
+
+    new IdeStartContextImpl(IdeLogLevel.INFO, IdeLogListenerNone.INSTANCE);
+  }
+
+  @Test
+  void testMessageOnlySendOnce() {
+
+    GuiEventBus eventBus = new GuiEventBus();
+    RecordingLogListener recorder = new RecordingLogListener();
+    eventBus.addListener(LogEvent.class, recorder);
+    IdeGuiLogListener listener = new IdeGuiLogListener(eventBus);
+    listener.flushAndEndBuffering();
+
+    String message = "TestMessage";
+    IdeLogEntry entry = new IdeLogEntry(IdeLogLevel.INFO, message);
+    boolean accepted = listener.onLog(entry);
+
+    assertThat(accepted).as("onLog() should accept the entry").isTrue();
+    assertThat(recorder.getEntries()).as("the entry must be forwarded to the console exactly once").hasSize(1);
+    assertThat(recorder.getEntries().get(0).message()).isEqualTo(message);
+  }
+
+
+  @Test
+  void testBufferedMessageShownOnceAfterFlush() {
+
+    Logger log = LoggerFactory.getLogger(IdeGuiLogListenerTest.class);
+    GuiEventBus eventBus = new GuiEventBus();
+    RecordingLogListener recorder = new RecordingLogListener();
+    eventBus.addListener(LogEvent.class, recorder);
+    IdeGuiLogListener listener = new IdeGuiLogListener(eventBus);
+    new IdeStartContextImpl(IdeLogLevel.INFO, listener);
+
+    log.warn("TestMessage");
+    assertThat(recorder.getEntries()).as("nothing is shown while buffering").isEmpty();
+
+    listener.flushAndEndBuffering();
+    assertThat(recorder.getEntries()).extracting(IdeLogEntry::message).containsExactly("TestMessage");
+  }
+
+  @Test
+  void testNullMessage() {
+
+    Logger log = LoggerFactory.getLogger(IdeGuiLogListenerTest.class);
+    assertThatCode(() -> log.warn((String) null)).doesNotThrowAnyException();
+
+  }
+
+  @Test
+  void testEntryWithErrorAndNullMessageIsForwardedUnchanged() {
+
+    GuiEventBus eventBus = new GuiEventBus();
+    RecordingLogListener recorder = new RecordingLogListener();
+    eventBus.addListener(LogEvent.class, recorder);
+    IdeGuiLogListener listener = new IdeGuiLogListener(eventBus);
+    listener.flushAndEndBuffering();
+
+    RuntimeException error = new NullPointerException();
+    IdeLogEntry entry = new IdeLogEntry(IdeLogLevel.ERROR, null, null, null, error);
+    listener.onLog(entry);
+
+    assertThat(recorder.getEntries()).first().isSameAs(entry);
+    assertThat(recorder.getEntries().get(0).error()).isSameAs(error);
+  }
+
+}

gui/src/test/java/com/devonfw/ide/gui/core/mainwindow/console/ConsolePanelTest.java

@@ -24,6 +24,7 @@
 import com.devonfw.ide.gui.core.service.NlsService;
 import com.devonfw.tools.ide.context.IdeTestContext;
 import com.devonfw.tools.ide.environment.IdeSystemTestImpl;
+import com.devonfw.tools.ide.log.IdeLogEntry;
 import com.devonfw.tools.ide.log.IdeLogLevel;
 
 class ConsolePanelTest extends UIBasedApplicationTest {
@@ -155,16 +156,19 @@ void testLogEventArrivingOverTheEventBusIsShown() {
 
     // Forward logs over the event bus - the path used since the rework instead of a direct appendOutput call
     IdeGuiLogListener listener = new IdeGuiLogListener(eventBus);
+    // The listener buffers by default; end buffering first so the following onLog() calls forward their entries over the event bus.
+    listener.flushAndEndBuffering();
     Platform.runLater(() -> {
-      listener.onLog(IdeLogLevel.INFO, "Info via EventBus", null, null, null);
-      listener.onLog(IdeLogLevel.ERROR, "Error via EventBus", null, null, new RuntimeException("boom"));
+      listener.onLog(new IdeLogEntry(IdeLogLevel.INFO, "Info via EventBus", null, null, null));
+      listener.onLog(new IdeLogEntry(IdeLogLevel.ERROR, "Error via EventBus", null, null, new RuntimeException("boom")));
     });
     waitForFxEvents();
 
     List<String> snapshot = consoleViewModel.getConsoleOutputSnapshot();
     assertThat(snapshot).anyMatch(s -> s.contains("[INFO] Info via EventBus"));
     assertThat(snapshot).anyMatch(s -> s.contains("[ERROR] Error via EventBus"));
-    assertThat(snapshot).anyMatch(s -> s.contains("[ERROR]   Error: boom"));
+    assertThat(snapshot).as("the error is kept on the entry, not printed as an extra line").hasSize(2);
+
   }
 
   /**
@@ -174,8 +178,11 @@ void testLogEventArrivingOverTheEventBusIsShown() {
   @Test
   void testLogEventArrivingOnOffFxThreadIsShown() throws InterruptedException {
 
+    IdeGuiLogListener listener = new IdeGuiLogListener(eventBus);
+    // The listener buffers by default; end buffering first so the onLog() call below forwards its entry over the event bus.
+    listener.flushAndEndBuffering();
     Thread worker = new Thread(() -> {
-      new IdeGuiLogListener(eventBus).onLog(IdeLogLevel.INFO, "Off FX", null, null, null);
+      listener.onLog(new IdeLogEntry(IdeLogLevel.INFO, "Off FX", null, null, null));
     });
     worker.start();
     worker.join(); // ensure the log was emitted before asserting