Use proper formatter for java.time classes

Prevents "java.lang.IllegalArgumentException: Cannot format given Object
as a Date" during persisting of logs.

Add tests with some help from Claude

This closes #884

Author: kwin

accesscontroltool-bundle/src/main/java/biz/netcentric/cq/tools/actool/history/impl/PersistableInstallationLogger.java

@@ -18,11 +18,9 @@
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.io.StringWriter;
-import java.sql.Timestamp;
-import java.text.DateFormat;
 import java.text.NumberFormat;
-import java.text.SimpleDateFormat;
 import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
 import java.util.Collection;
 import java.util.Date;
 import java.util.HashSet;
@@ -79,7 +77,7 @@ public class PersistableInstallationLogger implements InstallationLogger, Instal
 
     private int missingParentPathsForInitialContent = 0;
 
-    private DateFormat timestampFormat = new SimpleDateFormat("HH:mm:ss.SSS");
+    private DateTimeFormatter timestampFormat = DateTimeFormatter.ofPattern("HH:mm:ss.SSS");
 
     private final Collection<BiConsumer<InstallationLogLevel, String>> listeners;
     private final Collection<Consumer<Boolean>> finishListeners;
@@ -265,8 +263,7 @@ private String getMessageString(Set<HistoryEntry> messageHistorySet) {
         StringBuilder sb = new StringBuilder();
         if (!messageHistorySet.isEmpty()) {
             for (HistoryEntry entry : messageHistorySet) {
-                sb.append(EOL + timestampFormat.format(entry.getDate()) + ": "
-                        + entry.getMessage());
+                sb.append(EOL).append((entry.getDate().format(timestampFormat))).append(": ").append(entry.getMessage());
             }
         }
         return sb.toString();

accesscontroltool-bundle/src/test/java/biz/netcentric/cq/tools/actool/history/impl/PersistableInstallationLoggerTest.java

@@ -0,0 +1,121 @@
+package biz.netcentric.cq.tools.actool.history.impl;
+
+/*-
+ * #%L
+ * Access Control Tool Bundle
+ * %%
+ * Copyright (C) 2015 - 2024 Cognizant Netcentric
+ * %%
+ * This program and the accompanying materials are made
+ * available under the terms of the Eclipse Public License 2.0
+ * which is available at https://www.eclipse.org/legal/epl-2.0/
+ * 
+ * SPDX-License-Identifier: EPL-2.0
+ * #L%
+ */
+
+import static org.junit.jupiter.api.Assertions.*;
+
+import java.util.regex.Pattern;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.slf4j.helpers.NOPLogger;
+
+class PersistableInstallationLoggerTest {
+
+    private PersistableInstallationLogger logger;
+
+    @BeforeEach
+    void setUp() {
+        logger = new PersistableInstallationLogger();
+    }
+
+    @Test
+    void testGetVerboseMessageHistoryContainsAllMessageTypes() {
+        logger.addMessage(NOPLogger.NOP_LOGGER, "regular message");
+        logger.addWarning(NOPLogger.NOP_LOGGER, "a warning");
+        logger.addVerboseMessage(NOPLogger.NOP_LOGGER, "verbose detail");
+        logger.addError(NOPLogger.NOP_LOGGER, "an error", null);
+
+        String verbose = logger.getVerboseMessageHistory();
+
+        assertTrue(verbose.contains("regular message"), "verbose history should contain regular messages");
+        assertTrue(verbose.contains("a warning"), "verbose history should contain warnings");
+        assertTrue(verbose.contains("verbose detail"), "verbose history should contain verbose messages");
+        assertTrue(verbose.contains("an error"), "verbose history should contain errors");
+    }
+
+    @Test
+    void testGetVerboseMessageHistoryContainsVerboseMessagesAbsentFromMessageHistory() {
+        logger.addMessage(NOPLogger.NOP_LOGGER, "regular message");
+        logger.addVerboseMessage(NOPLogger.NOP_LOGGER, "verbose only detail");
+
+        String messageHistory = logger.getMessageHistory();
+        String verboseHistory = logger.getVerboseMessageHistory();
+
+        assertFalse(messageHistory.contains("verbose only detail"),
+                "non-verbose message history should NOT contain verbose messages");
+        assertTrue(verboseHistory.contains("verbose only detail"),
+                "verbose message history should contain verbose messages");
+        assertTrue(verboseHistory.contains("regular message"),
+                "verbose message history should also contain regular messages");
+    }
+
+    @Test
+    void testGetVerboseMessageHistoryWithExceptionIncludesStackTrace() {
+        RuntimeException ex = new RuntimeException("something went wrong");
+        logger.addError(NOPLogger.NOP_LOGGER, "error with exception", ex);
+
+        String verboseHistory = logger.getVerboseMessageHistory();
+
+        assertTrue(verboseHistory.contains("error with exception"),
+                "verbose history should contain the error message");
+        assertTrue(verboseHistory.contains("RuntimeException"),
+                "verbose history should contain the exception stack trace");
+    }
+
+    @Test
+    void testGetVerboseMessageHistoryEmptyWhenNoMessages() {
+        String verboseHistory = logger.getVerboseMessageHistory();
+        assertTrue(verboseHistory.isEmpty(), "verbose history should be empty when no messages have been added");
+    }
+
+    @Test
+    void testGetVerboseMessageHistoryTimestampPrefix() {
+        logger.addMessage(NOPLogger.NOP_LOGGER, "timed message");
+        logger.addVerboseMessage(NOPLogger.NOP_LOGGER, "timed verbose");
+
+        String verboseHistory = logger.getVerboseMessageHistory();
+
+        // Each entry is formatted as: \nHH:mm:ss.SSS: <message>
+        Pattern timestampPattern = Pattern.compile("\\d{2}:\\d{2}:\\d{2}\\.\\d{3}: ");
+        assertTrue(timestampPattern.matcher(verboseHistory).find(),
+                "each entry should be prefixed with a timestamp in HH:mm:ss.SSS format");
+
+        // Verify every line that contains a known message has the timestamp prefix
+        for (String line : verboseHistory.split(PersistableInstallationLogger.EOL)) {
+            if (line.isEmpty()) {
+                continue;
+            }
+            assertTrue(timestampPattern.matcher(line).find(),
+                    "line should start with HH:mm:ss.SSS timestamp: " + line);
+        }
+    }
+
+    @Test
+    void testGetVerboseMessageHistoryMessagesAreOrderedByIndex() {
+        logger.addMessage(NOPLogger.NOP_LOGGER, "first");
+        logger.addVerboseMessage(NOPLogger.NOP_LOGGER, "second");
+        logger.addMessage(NOPLogger.NOP_LOGGER, "third");
+
+        String verboseHistory = logger.getVerboseMessageHistory();
+
+        int firstPos = verboseHistory.indexOf("first");
+        int secondPos = verboseHistory.indexOf("second");
+        int thirdPos = verboseHistory.indexOf("third");
+
+        assertTrue(firstPos < secondPos, "first message should appear before second");
+        assertTrue(secondPos < thirdPos, "second message should appear before third");
+    }
+}