Simplify CliListener interface.

apuig · apuig · commit 32ead10b97bb · 2025-03-10T20:57:11.000+01:00
Add CliListener to CLIMethods
diff --git a/core/src/main/java/hudson/cli/CLICommand.java b/core/src/main/java/hudson/cli/CLICommand.java
@@ -257,58 +257,56 @@ public int main(List<String> args, Locale locale, InputStream stdin, PrintStream
             p.parseArgument(args.toArray(new String[0]));
 
             CliListener.fireExecution(correlationId, getName(), argsSize, auth);
-
             int res = run();
-
             CliListener.fireCompleted(correlationId, getName(), argsSize, auth, res);
 
             return res;
-        } catch (CmdLineException e) {
-            fireErrorAndPrintExceptionMessageToCommandStderr(correlationId, argsSize, e);
-            printUsage(stderr, p);
-            return 2;
-        } catch (IllegalStateException e) {
-            fireErrorAndPrintExceptionMessageToCommandStderr(correlationId, argsSize, e);
-            return 4;
-        } catch (IllegalArgumentException e) {
-            fireErrorAndPrintExceptionMessageToCommandStderr(correlationId, argsSize, e);
-            return 3;
-        } catch (AbortException e) {
-            fireErrorAndPrintExceptionMessageToCommandStderr(correlationId, argsSize, e);
-            return 5;
-        } catch (AccessDeniedException e) {
-            fireErrorAndPrintExceptionMessageToCommandStderr(correlationId, argsSize, e);
-            return 6;
-        } catch (BadCredentialsException e) {
-            // to the caller, we can't reveal whether the user didn't exist or the password didn't match.
-            // do that to the server log instead
-            String id = UUID.randomUUID().toString();
-
-            CliListener.fireLoginFailed(correlationId, getName(), argsSize, id, e);
-
-            printError("Bad Credentials. Search the server log for " + id + " for more details.");
-
-            return 7;
         } catch (Throwable e) {
-            String errorMsg = "Unexpected exception occurred while performing " + getName() + " command.";
-
-            CliListener.fireError(correlationId, getName(), argsSize, getTransportAuthentication2(), false, e);
-
-            printError(errorMsg);
-
-            Functions.printStackTrace(e, stderr);
-            return 1;
+            int exitCode = handleException(e, correlationId, p);
+            CliListener.fireError(correlationId, getName(), argsSize, getTransportAuthentication2(), exitCode, e);
+            return exitCode;
         } finally {
             if (sc != null)
                 sc.setAuthentication(old); // restore
         }
     }
 
-    private void fireErrorAndPrintExceptionMessageToCommandStderr(String correlationId, int argsSize, Throwable e) {
-        CliListener.fireError(correlationId, getName(), argsSize, getTransportAuthentication2(), true, e);
-        printError(e.getMessage());
+    /**
+     * Determines command stderr output and return the exit code as described on {@link #main(List, Locale, InputStream, PrintStream, PrintStream)}
+     * */
+    protected int handleException(Throwable e, String correlationId, CmdLineParser p) {
+        int exitCode;
+        if (e instanceof CmdLineException) {
+            exitCode = 2;
+            printError(e.getMessage());
+            printUsage(stderr, p);
+        } else if (e instanceof IllegalArgumentException) {
+            exitCode = 3;
+            printError(e.getMessage());
+        } else if (e instanceof IllegalStateException) {
+            exitCode = 4;
+            printError(e.getMessage());
+        } else if (e instanceof AbortException) {
+            exitCode = 5;
+            printError(e.getMessage());
+        } else if (e instanceof AccessDeniedException) {
+            exitCode = 6;
+            printError(e.getMessage());
+        } else if (e instanceof BadCredentialsException) {
+            exitCode = 7;
+            // to the caller, we can't reveal whether the user didn't exist or the password didn't match.
+            // do that to the server log instead
+            printError("Bad Credentials. Search the server log for " + correlationId + " for more details.");
+        } else {
+            exitCode = 1;
+            printError("Unexpected exception occurred while performing " + getName() + " command.");
+            Functions.printStackTrace(e, stderr);
+        }
+        return exitCode;
     }
 
+
+
     private void printError(String errorMessage) {
         this.stderr.println();
         this.stderr.println("ERROR: " + errorMessage);
diff --git a/core/src/main/java/hudson/cli/declarative/CLIRegisterer.java b/core/src/main/java/hudson/cli/declarative/CLIRegisterer.java
@@ -27,14 +27,13 @@
 import static java.util.logging.Level.SEVERE;
 
 import edu.umd.cs.findbugs.annotations.NonNull;
-import hudson.AbortException;
 import hudson.Extension;
 import hudson.ExtensionComponent;
 import hudson.ExtensionFinder;
-import hudson.Functions;
 import hudson.Util;
 import hudson.cli.CLICommand;
 import hudson.cli.CloneableCLICommand;
+import hudson.cli.listeners.CliListener;
 import hudson.model.Hudson;
 import java.io.IOException;
 import java.io.InputStream;
@@ -50,18 +49,14 @@
 import java.util.MissingResourceException;
 import java.util.Stack;
 import java.util.UUID;
-import java.util.logging.Level;
 import java.util.logging.Logger;
 import jenkins.ExtensionComponentSet;
 import jenkins.ExtensionRefreshException;
 import jenkins.model.Jenkins;
 import org.jvnet.hudson.annotation_indexer.Index;
 import org.jvnet.localizer.ResourceBundleHolder;
-import org.kohsuke.args4j.CmdLineException;
 import org.kohsuke.args4j.CmdLineParser;
 import org.kohsuke.args4j.ParserProperties;
-import org.springframework.security.access.AccessDeniedException;
-import org.springframework.security.authentication.BadCredentialsException;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.context.SecurityContext;
 import org.springframework.security.core.context.SecurityContextHolder;
@@ -202,6 +197,9 @@ public int main(List<String> args, Locale locale, InputStream stdin, PrintStream
 
                             List<MethodBinder> binders = new ArrayList<>();
 
+                            final String correlationId = UUID.randomUUID().toString();
+                            final int argsSize = args.size();
+
                             CmdLineParser parser = bindMethod(binders);
                             try {
                                 // TODO this could probably use ACL.as; why is it calling SecurityContext.setAuthentication rather than SecurityContextHolder.setContext?
@@ -215,15 +213,16 @@ public int main(List<String> args, Locale locale, InputStream stdin, PrintStream
                                     sc.setAuthentication(auth); // run the CLI with the right credential
                                     jenkins.checkPermission(Jenkins.READ);
 
+                                    CliListener.fireExecution(correlationId, getName(), argsSize, auth);
+
                                     // resolve them
                                     Object instance = null;
                                     for (MethodBinder binder : binders)
                                         instance = binder.call(instance);
 
-                                    if (instance instanceof Integer)
-                                        return (Integer) instance;
-                                    else
-                                        return 0;
+                                    Integer exitCode = (instance instanceof Integer) ? (Integer) instance : 0;
+                                    CliListener.fireCompleted(correlationId, getName(), argsSize, auth, exitCode);
+                                    return exitCode;
                                 } catch (InvocationTargetException e) {
                                     Throwable t = e.getTargetException();
                                     if (t instanceof Exception)
@@ -232,47 +231,13 @@ public int main(List<String> args, Locale locale, InputStream stdin, PrintStream
                                 } finally {
                                     sc.setAuthentication(old); // restore
                                 }
-                            } catch (CmdLineException e) {
-                                printError(e.getMessage());
-                                printUsage(stderr, parser);
-                                return 2;
-                            } catch (IllegalStateException e) {
-                                printError(e.getMessage());
-                                return 4;
-                            } catch (IllegalArgumentException e) {
-                                printError(e.getMessage());
-                                return 3;
-                            } catch (AbortException e) {
-                                printError(e.getMessage());
-                                return 5;
-                            } catch (AccessDeniedException e) {
-                                printError(e.getMessage());
-                                return 6;
-                            } catch (BadCredentialsException e) {
-                                // to the caller, we can't reveal whether the user didn't exist or the password didn't match.
-                                // do that to the server log instead
-                                String id = UUID.randomUUID().toString();
-                                logAndPrintError(e, "Bad Credentials. Search the server log for " + id + " for more details.",
-                                        "CLI login attempt failed: " + id, Level.INFO);
-                                return 7;
                             } catch (Throwable e) {
-                                final String errorMsg = "Unexpected exception occurred while performing " + getName() + " command.";
-                                logAndPrintError(e, errorMsg, errorMsg, Level.WARNING);
-                                Functions.printStackTrace(e, stderr);
-                                return 1;
+                            int exitCode = handleException(e, correlationId, parser);
+                            CliListener.fireError(correlationId, getName(), argsSize, getTransportAuthentication2(), exitCode, e);
+                            return exitCode;
                             }
                         }
 
-                        private void printError(String errorMessage) {
-                            this.stderr.println();
-                            this.stderr.println("ERROR: " + errorMessage);
-                        }
-
-                        private void logAndPrintError(Throwable e, String errorMessage, String logMessage, Level logLevel) {
-                            LOGGER.log(logLevel, logMessage, e);
-                            printError(errorMessage);
-                        }
-
                         @Override
                         protected int run() throws Exception {
                             throw new UnsupportedOperationException();
diff --git a/core/src/main/java/hudson/cli/listeners/CliListener.java b/core/src/main/java/hudson/cli/listeners/CliListener.java
@@ -29,7 +29,6 @@
 import hudson.ExtensionPoint;
 import hudson.cli.CLICommand;
 import jenkins.util.Listeners;
-import org.springframework.security.authentication.BadCredentialsException;
 import org.springframework.security.core.Authentication;
 
 /**
@@ -74,33 +73,17 @@ default void onCompleted(
      * @param command The executed command.
      * @param argsSize Number of arguments passed to the command.
      * @param auth Authenticated user executing the command
-     * @param expected True when caller is handling the error to return an specific exitCode.
+     * @param exitCode `run` returned exit code.
      * @param t Any error during the execution of the command.
      * */
     default void onError(
             @NonNull String correlationId,
             @NonNull String command,
             int argsSize,
             @CheckForNull Authentication auth,
-            boolean expected,
+            int exitCode,
             @NonNull Throwable t) {}
 
-    /**
-     * Catch bad credentials.
-     *
-     * @param correlationId This value is used to correlate this command event to other, related command events.
-     * @param command The executed command.
-     * @param argsSize Number of arguments passed to the command.
-     * @param opaqueId ID used on the command output to indicate an authentication error.
-     * @param authError Authentication error.
-     * */
-    default void onLoginFailed(
-            @NonNull String correlationId,
-            @NonNull String command,
-            int argsSize,
-            @NonNull String opaqueId,
-            @NonNull BadCredentialsException authError) {}
-
     /**
      * Fires the {@link #onExecution} event.
      *
@@ -144,40 +127,19 @@ static void fireCompleted(
      * @param command The executed command.
      * @param argsSize Number of arguments passed to the command.
      * @param auth Authenticated user executing the command
-     * @param expected True when caller is handling the error to return an specific exitCode.
+     * @param exitCode `run` returned exit code.
      * @param t Any error during the execution of the command.
      * */
     static void fireError(
             @NonNull String correlationId,
             @NonNull String command,
             int argsSize,
             @CheckForNull Authentication auth,
-            boolean expected,
+            int exitCode,
             @NonNull Throwable t) {
         Listeners.notify(
                 CliListener.class,
                 true,
-                listener -> listener.onError(correlationId, command, argsSize, auth, expected, t));
-    }
-
-    /**
-     * Fires the {@link #onLoginFailed} event.
-     *
-     * @param correlationId This value is used to correlate this command event to other, related command events.
-     * @param command The executed command.
-     * @param argsSize Number of arguments passed to the command.
-     * @param opaqueId ID used on the command output to indicate an authentication error.
-     * @param authError Authentication error.
-     * */
-    static void fireLoginFailed(
-            @NonNull String correlationId,
-            @NonNull String command,
-            int argsSize,
-            @NonNull String opaqueId,
-            @NonNull BadCredentialsException authError) {
-        Listeners.notify(
-                CliListener.class,
-                true,
-                listener -> listener.onLoginFailed(correlationId, command, argsSize, opaqueId, authError));
+                listener -> listener.onError(correlationId, command, argsSize, auth, exitCode, t));
     }
 }
diff --git a/core/src/main/java/hudson/cli/listeners/DefaultCliListener.java b/core/src/main/java/hudson/cli/listeners/DefaultCliListener.java
@@ -29,7 +29,6 @@
 import java.util.logging.Logger;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
-import org.springframework.security.authentication.BadCredentialsException;
 import org.springframework.security.core.Authentication;
 
 /**
@@ -57,23 +56,18 @@ public void onCompleted(String correlationId, String command, int argsSize, Auth
 
     @Override
     public void onError(
-            String correlationId, String command, int argsSize, Authentication auth, boolean expected, Throwable t) {
-        if (expected) {
+            String correlationId, String command, int argsSize, Authentication auth, int exitCode, Throwable t) {
+        if (exitCode == 1) {
+            LOGGER.log(Level.WARNING, "Unexpected exception occurred while performing " + command + " command.", t);
+        } else if (exitCode == 7) {
+            LOGGER.log(Level.INFO, "CLI login attempt failed: " + correlationId, t);
+        } else {
             LOGGER.log(
                     Level.FINE,
                     String.format(
                             "Failed call to CLI command %s, with %d arguments, as user %s.",
                             command, argsSize, auth != null ? auth.getName() : "<unknown>"),
                     t);
-
-        } else {
-            LOGGER.log(Level.WARNING, "Unexpected exception occurred while performing " + command + " command.", t);
         }
     }
-
-    @Override
-    public void onLoginFailed(
-            String correlationId, String command, int argsSize, String opaqueId, BadCredentialsException authError) {
-        LOGGER.log(Level.INFO, "CLI login attempt failed: " + opaqueId, authError);
-    }
 }
diff --git a/test/src/test/java/hudson/cli/CliListenerTest.java b/test/src/test/java/hudson/cli/CliListenerTest.java
