GH24992 Admin Logger: Tests, more read commands

appserver/itest-tools/pom.xml

@@ -28,7 +28,7 @@
                     <artifactId>*</artifactId>
                 </exclusion>
             </exclusions>
-       </dependency>
+        </dependency>
 
         <dependency>
             <groupId>org.glassfish.jersey.core</groupId>
@@ -76,6 +76,11 @@
             <artifactId>shrinkwrap-resolver-impl-maven-archive</artifactId>
             <scope>compile</scope>
         </dependency>
+        <dependency>
+            <groupId>org.apache.commons</groupId>
+            <artifactId>commons-lang3</artifactId>
+            <scope>compile</scope>
+        </dependency>
     </dependencies>
 
     <build>

appserver/itest-tools/src/main/java/org/glassfish/main/itest/tools/GlassFishTestEnvironment.java

@@ -16,6 +16,8 @@
 
 package org.glassfish.main.itest.tools;
 
+import static org.glassfish.main.itest.tools.TestUtilities.getConfigBoolean;
+
 import jakarta.ws.rs.client.Client;
 
 import java.io.File;
@@ -82,7 +84,8 @@ public class GlassFishTestEnvironment {
             asadmin.withEnv("AS_START_TIMEOUT", Integer.toString(ASADMIN_START_DOMAIN_TIMEOUT - 5000));
         }
         // This is the absolutely first start - if it fails, all other starts will fail too.
-        assertThat(asadmin.exec(ASADMIN_START_DOMAIN_TIMEOUT, "start-domain", "--debug"), asadminOK());
+        assertThat(asadmin.exec(ASADMIN_START_DOMAIN_TIMEOUT, "start-domain",
+                getConfigBoolean("glassfish.suspend") ? "--suspend" : "--debug"), asadminOK());
     }
 
 

appserver/itest-tools/src/main/java/org/glassfish/main/itest/tools/TestUtilities.java

@@ -65,4 +65,14 @@ public static void delete(final File... files) throws IOException {
         }
         throw failures;
     }
+
+    public static String getConfig(String key) {
+        final String NOT_ALPHA_NUM = "\\P{Alnum}";
+        final String envValue = System.getenv(key.replaceAll(NOT_ALPHA_NUM, "_"));
+        return envValue != null ? envValue : System.getProperty(key);
+    }
+
+    public static boolean getConfigBoolean(String key) {
+        return Boolean.parseBoolean(getConfig(key));
+    }
 }

appserver/itest-tools/src/main/java/org/glassfish/main/itest/tools/asadmin/CollectLogFiles.java

@@ -0,0 +1,70 @@
+/*
+ * Copyright (c) 2024 Eclipse Foundation and/or its affiliates. All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0, which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the
+ * Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
+ * version 2 with the GNU Classpath Exception, which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ */
+package org.glassfish.main.itest.tools.asadmin;
+
+import static org.glassfish.main.itest.tools.GlassFishTestEnvironment.getAsadmin;
+import static org.glassfish.main.itest.tools.asadmin.AsadminResultMatcher.asadminOK;
+import static org.hamcrest.CoreMatchers.endsWith;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipFile;
+import org.apache.commons.lang3.StringUtils;
+
+/**
+ *
+ * @author Ondro Mihalyi
+ */
+public class CollectLogFiles {
+
+    private AsadminResult result;
+
+    private File fileWithLogs;
+
+    public CollectLogFiles collect() throws IOException {
+        result = getAsadmin().exec("collect-log-files");
+        assertThat(result, asadminOK());
+        String path = StringUtils.substringBetween(result.getStdOut(), "Created Zip file under ", ".\n");
+        assertThat("zip file path parsed from " + result.getStdOut(), path, notNullValue());
+        fileWithLogs = new File(path);
+        fileWithLogs.deleteOnExit();
+        assertThat(fileWithLogs.getName(), endsWith(".zip"));
+        return this;
+    }
+
+    public List<String> getServerLogLines() throws IOException {
+        return getLogLines("logs/server/server.log");
+    }
+
+    private List<String> getLogLines(String logFilePath) throws IOException {
+        final ZipFile zipFile = new ZipFile(fileWithLogs);
+        final ZipEntry entry = zipFile.getEntry(logFilePath);
+        final BufferedReader reader = new BufferedReader(new InputStreamReader(zipFile.getInputStream(entry)));
+        List<String> lines = new ArrayList<>();
+        while (reader.ready()) {
+            lines.add(reader.readLine());
+        }
+        return lines;
+    }
+
+}

appserver/tests/extras/command-logger/pom.xml

@@ -54,6 +54,10 @@
             <groupId>org.junit.jupiter</groupId>
             <artifactId>junit-jupiter-engine</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-params</artifactId>
+        </dependency>
         <dependency>
             <groupId>org.hamcrest</groupId>
             <artifactId>hamcrest</artifactId>

appserver/tests/extras/command-logger/src/test/java/org/glassfish/main/test/extras/commandlogger/CommandLoggerTest.java

@@ -16,24 +16,19 @@
 
 import static org.glassfish.main.itest.tools.GlassFishTestEnvironment.getAsadmin;
 import static org.glassfish.main.itest.tools.asadmin.AsadminResultMatcher.asadminOK;
+import static org.hamcrest.CoreMatchers.allOf;
 import static org.hamcrest.CoreMatchers.containsString;
-import static org.hamcrest.CoreMatchers.endsWith;
+import static org.hamcrest.CoreMatchers.everyItem;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.hasItem;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.hamcrest.Matchers.not;
 
-import java.io.BufferedReader;
-import java.io.File;
 import java.io.IOException;
-import java.io.InputStreamReader;
-import java.util.ArrayList;
 import java.util.List;
-import java.util.zip.ZipEntry;
-import java.util.zip.ZipFile;
-import org.apache.commons.lang3.StringUtils;
 import org.glassfish.main.itest.tools.asadmin.Asadmin;
-import org.glassfish.main.itest.tools.asadmin.AsadminResult;
-import org.junit.jupiter.api.Test;
+import org.glassfish.main.itest.tools.asadmin.CollectLogFiles;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 
 /**
  *
@@ -43,24 +38,69 @@ public class CommandLoggerTest {
 
     private static final Asadmin ASADMIN = getAsadmin();
 
-    @Test
-    public void testLogWriteCommands() throws IOException {
-        assertThat(ASADMIN.exec("create-system-properties", "--target=server", "glassfish.commandlogger.logmode=WRITE_COMMANDS"), asadminOK());
+    @ParameterizedTest
+    @CsvSource(useHeadersInDisplayName = true,
+            value = {
+                "LOG_MODE               , LOG_WRITE , LOG_READ , LOG_INTERNAL",
+                "WRITE_COMMANDS         , true      , false    , false       ",
+                "READ_WRITE_COMMANDS    , true      , true     , false       ",
+                "INTERNAL_COMMANDS      , true      , false    , true        ",
+                "ALL_COMMANDS           , true      , true     , true        ",
+                "NO_COMMAND             , false     , false    , false       ",
+                "                       , false     , false    , false       ",}
+    )
+    public void testLogWriteCommands(String logMode, boolean logWriteOp, boolean logReadOp, boolean logInternalOp) throws IOException {
+        if (logMode != null) {
+            assertThat(ASADMIN.exec("create-system-properties", "--target=server", "glassfish.commandlogger.logmode=" + logMode), asadminOK());
+        } else {
+            ASADMIN.exec("delete-system-property", "--target=server", "glassfish.commandlogger.logmode");
+        }
+        clearLogFile();
+
+        // execute some write command, it doesn't have to complete successfully
         ASADMIN.exec("delete-system-property", "X");
-        AsadminResult result = ASADMIN.exec("collect-log-files");
-        assertThat(result, asadminOK());
-        String path = StringUtils.substringBetween(result.getStdOut(), "Created Zip file under ", ".\n");
-        assertNotNull(path, () -> "zip file path parsed from " + result.getStdOut());
-        File file = new File(path);
-        assertThat(file.getName(), endsWith(".zip"));
-        final ZipFile zipFile = new ZipFile(file);
-        final ZipEntry entry = zipFile.getEntry("logs/server/server.log");
-        final BufferedReader reader = new BufferedReader(new InputStreamReader(zipFile.getInputStream(entry)));
-        List<String> lines = new ArrayList<>();
-        while (reader.ready()) {
-            lines.add(reader.readLine());
+        // execute some read command
+        ASADMIN.exec("list-applications", "--long");
+        // exxecute some internal command
+        ASADMIN.exec("__locations");
+
+        final List<String> lines = new CollectLogFiles()
+                .collect()
+                .getServerLogLines();
+
+        if (logWriteOp) {
+            assertCommandLogged(lines, "admin", "delete-system-property X");
+        } else {
+            assertCommandNotLogged(lines, "delete-system-property");
+        }
+
+        if (logReadOp) {
+            assertCommandLogged(lines, "admin", "list-applications --long");
+        } else {
+            assertCommandNotLogged(lines, "list-applications");
+        }
+
+        if (logInternalOp) {
+            assertCommandLogged(lines, "admin", "__locations");
+        } else {
+            assertCommandNotLogged(lines, "__locations");
         }
-        assertThat("log", lines, hasItem(containsString("delete-system-property X")));
+
+    }
+
+    private void clearLogFile() {
+        assertThat(ASADMIN.exec("rotate-log", "--target=server"), asadminOK());
+    }
+
+    private void assertCommandNotLogged(final List<String> lines, String command) {
+        assertThat("log", lines, everyItem(not(containsString(command))));
+    }
+
+    private void assertCommandLogged(final List<String> lines, String user, String fullCommand) {
+        assertThat("server.log", lines, hasItem(allOf(containsString(user),
+                containsString(fullCommand)
+        )
+        ));
     }
 
 }

nucleus/extras/command-logger/src/main/java/org/glassfish/extras/commandlogger/AdminCommandLogger.java

@@ -80,7 +80,7 @@ private String constructCommandLine(String commandName, ParameterMap parameters)
     private static enum LogMode {
         ALL_COMMANDS, INTERNAL_COMMANDS, WRITE_COMMANDS, READ_WRITE_COMMANDS, NO_COMMAND;
 
-        public static final LogMode DEFAULT = LogMode.WRITE_COMMANDS;
+        public static final LogMode DEFAULT = LogMode.NO_COMMAND;
         public static final String PROPERTY_NAME = "glassfish.commandlogger.logmode";
 
         public static LogMode get() {
@@ -120,7 +120,9 @@ private boolean shouldLogCommand(String commandName) {
 
     private boolean isReadCommand(String commandName) {
         return Stream.of(
-                "version", "list(.*)", "get(.*)", "(.*)-list-services", "uptime")
+                "attach", "backup-domain", "collect-log-files", "export(.*)", "generate-jvm-report", "get(.*)",
+                "jms-ping", "list(.*)", "login", "monitor", "ping(.*)", "show(.*)",
+                "uptime", "validate(.*)", "verify(.*)", "version", "(.*)-list-services")
                 .filter(commandName::matches)
                 .findAny().isPresent();
     }

nucleus/parent/pom.xml

@@ -881,6 +881,7 @@
                             <java.util.logging.manager>${test.logManager}</java.util.logging.manager>
                             <java.util.logging.config.useDefaults>true</java.util.logging.config.useDefaults>
                             <java.util.logging.config.defaultLevel>${test.logLevel}</java.util.logging.config.defaultLevel>
+                            <glassfish.suspend>${glassfish.suspend}</glassfish.suspend>
                         </systemPropertyVariables>
                     </configuration>
                 </plugin>

nucleus/pom.xml

@@ -76,6 +76,7 @@
         <module>resources</module>
         <module>featuresets</module>
         <module>xmlbind-annotations</module>
+        <module>extras</module>
     </modules>
 
     <scm>