From e2278adfe767d65d06238fa6bbc7c2d6b53ec4f8 Mon Sep 17 00:00:00 2001
From: Michael Calmer <mc@suse.de>
Date: Fri, 22 Mar 2024 14:46:28 +0100
Subject: [PATCH] refactor due to sonarcloud suggestions

---
 .../rhn/domain/action/ActionFactory.java      | 10 +--
 .../schedule/InProgressSystemsAction.java     |  8 +--
 .../services/SaltServerActionService.java     | 63 +++++++++----------
 3 files changed, 38 insertions(+), 43 deletions(-)

diff --git a/java/code/src/com/redhat/rhn/domain/action/ActionFactory.java b/java/code/src/com/redhat/rhn/domain/action/ActionFactory.java
index 9f97fc43374..ca14b0204fc 100644
--- a/java/code/src/com/redhat/rhn/domain/action/ActionFactory.java
+++ b/java/code/src/com/redhat/rhn/domain/action/ActionFactory.java
@@ -191,8 +191,8 @@ public static int removeActionForSystemSet(long actionId, String setLabel, User
         Set<Server> involvedMinions = MinionServerFactory
                     .lookupByIds(new ArrayList<>(set.getElementValues()))
                     .collect(toSet());
-        Set<Server> involvedSystems = ServerFactory.lookupByIds(
-                new ArrayList<>(set.getElementValues())).stream().collect(toSet());
+        Set<Server> involvedSystems = new HashSet<>(ServerFactory.lookupByIds(
+                new ArrayList<>(set.getElementValues())));
         Action action = ActionFactory.lookupById(actionId);
 
         TASKOMATIC_API.deleteScheduledActions(Collections.singletonMap(action, involvedMinions));
@@ -795,6 +795,7 @@ public static void deleteServerActionsByParent(Set parentActions) {
      * @param serverIn you want to limit the list of Actions to
      * @return List of Action objects
      */
+    @SuppressWarnings("unchecked")
     public static List<Action> listActionsForServer(User user, Server serverIn) {
         Map<String, Object> params = new HashMap<>();
         params.put("orgId", user.getOrg().getId());
@@ -830,7 +831,6 @@ public static List<ServerAction> listServerActionsForServer(Server serverIn, Str
         params.put("server", serverIn);
         params.put("actionType", actionType);
         params.put("date", date);
-
         return singleton.listObjectsByNamedQuery("ServerAction.findByServerAndActionTypeAndCreatedDate", params);
     }
     /**
@@ -839,6 +839,7 @@ public static List<ServerAction> listServerActionsForServer(Server serverIn, Str
      * @param statusList to filter the ServerActoins by
      * @return List of ServerAction objects
      */
+    @SuppressWarnings("unchecked")
     public static List<ServerAction> listServerActionsForServer(Server serverIn,
             List<ActionStatus> statusList) {
         Map<String, Object> params = new HashMap<>();
@@ -855,7 +856,8 @@ public static List<ServerAction> listServerActionsForServer(Server serverIn,
      * @param createdDate to filter the ServerActions by
      * @return List of ServerAction objects
      */
-    public static List listServerActionsForServer(Server serverIn, List<ActionStatus> statusList, Date createdDate) {
+    @SuppressWarnings("unchecked")
+    public static List<ServerAction> listServerActionsForServer(Server serverIn, List<ActionStatus> statusList, Date createdDate) {
         Map<String, Object> params = new HashMap<>();
         params.put("server", serverIn);
         params.put("statusList", statusList);
diff --git a/java/code/src/com/redhat/rhn/frontend/action/schedule/InProgressSystemsAction.java b/java/code/src/com/redhat/rhn/frontend/action/schedule/InProgressSystemsAction.java
index bb30024aaee..eeda8e896d9 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/schedule/InProgressSystemsAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/schedule/InProgressSystemsAction.java
@@ -47,7 +47,7 @@
 public class InProgressSystemsAction extends RhnSetAction {
 
     /** Logger instance */
-    private static Logger log = LogManager.getLogger(InProgressSystemsAction.class);
+    private static final Logger LOG = LogManager.getLogger(InProgressSystemsAction.class);
 
     /**
      * Removes unscheduleaction set from server actions.
@@ -75,7 +75,7 @@ public ActionForward unscheduleAction(ActionMapping mapping,
          */
         int numSystems = updateSet(request).size();
         ActionMessages msgs = new ActionMessages();
-        Map params = makeParamMap(formIn, request);
+        Map<String, Object> params = makeParamMap(formIn, request);
 
         if (numSystems == 0) {
             msgs.add(ActionMessages.GLOBAL_MESSAGE,
@@ -88,7 +88,7 @@ public ActionForward unscheduleAction(ActionMapping mapping,
 
         try {
             ActionFactory.removeActionForSystemSet(aid, "unscheduleaction", user);
-            /**
+            /*
              * If we've unscheduled the action for more than one system, send the pluralized
              * version of the message.
              */
@@ -109,7 +109,7 @@ public ActionForward unscheduleAction(ActionMapping mapping,
             strutsDelegate.saveMessages(request, msgs);
         }
         catch (TaskomaticApiException e) {
-            log.error("Could not unschedule action:", e);
+            LOG.error("Could not unschedule action:", e);
             ActionErrors errors = new ActionErrors();
             strutsDelegate.addError(errors, "taskscheduler.down");
             strutsDelegate.saveMessages(request, errors);
diff --git a/java/code/src/com/suse/manager/webui/services/SaltServerActionService.java b/java/code/src/com/suse/manager/webui/services/SaltServerActionService.java
index 648eb99c0c4..0b5831ef9cf 100644
--- a/java/code/src/com/suse/manager/webui/services/SaltServerActionService.java
+++ b/java/code/src/com/suse/manager/webui/services/SaltServerActionService.java
@@ -2664,19 +2664,16 @@ public void failDependentServerActions(long actionId, String minionId, Optional<
             // set first action to failed if not already in that state
             Action action = ActionFactory.lookupById(actionId);
             Optional.ofNullable(action)
-                    .ifPresent(firstAction ->
-                            firstAction.getServerActions().stream()
-                                    .filter(sa -> sa.getServerId().equals(minion.get().getId()))
-                                    .filter(sa -> !ActionFactory.STATUS_FAILED.equals(sa.getStatus()))
-                                    .filter(sa -> !ActionFactory.STATUS_COMPLETED.equals(sa.getStatus()))
-                                    .findFirst()
-                                    .ifPresent(sa -> sa.fail(message.orElse("Prerequisite failed"))));
+                    .flatMap(firstAction -> firstAction.getServerActions().stream()
+                            .filter(sa -> sa.getServerId().equals(minion.get().getId()))
+                            .filter(sa -> !ActionFactory.STATUS_FAILED.equals(sa.getStatus()))
+                            .filter(sa -> !ActionFactory.STATUS_COMPLETED.equals(sa.getStatus()))
+                            .findFirst()).ifPresent(sa -> sa.fail(message.orElse("Prerequisite failed")));
 
             // walk dependent server actions recursively and set them to failed
             Stack<Long> actionIdsDependencies = new Stack<>();
             actionIdsDependencies.push(actionId);
-            List<ServerAction> serverActions = ActionFactory
-                    .listServerActionsForServer(minion.get(),
+            List<ServerAction> serverActions = ActionFactory.listServerActionsForServer(minion.get(),
                             Arrays.asList(ActionFactory.STATUS_QUEUED, ActionFactory.STATUS_PICKED_UP,
                                     ActionFactory.STATUS_FAILED), action.getCreated());
 
@@ -2846,7 +2843,7 @@ public void handleActionChainResult(
             Map<String, StateApplyResult<Ret<JsonElement>>> actionChainResult,
             Function<StateApplyResult<Ret<JsonElement>>, Boolean> skipFunction) {
         int chunk = 1;
-        Long retActionChainId = Long.valueOf(0);
+        Long retActionChainId = 0L;
         boolean actionChainFailed = false;
         List<Long> failedActionIds = new ArrayList<>();
         for (Map.Entry<String, StateApplyResult<Ret<JsonElement>>> entry : actionChainResult.entrySet()) {
@@ -2904,40 +2901,36 @@ else if (key.contains("schedule_next_chunk")) {
                                     .stream()
                                     .filter(sa -> sa.getServer().equals(minionServer)).findFirst();
 
-                            serverAction.ifPresent(sa -> {
-                                setActionAsPickedUp(sa);
-                            });
+                            serverAction.ifPresent(this::setActionAsPickedUp);
                         }
                     }
                 });
             }
-            else if (!key.contains("schedule_next_chunk")) {
+            else {
                 LOG.warn("Could not find action id in action chain state key: {}", key);
             }
         }
 
-        if (retActionChainId != null) {
-            if (actionChainFailed) {
-                long firstFailedActionId = failedActionIds.stream().min(Long::compare).get();
-                // Set rest of actions as FAILED due to failed prerequisite
-                failDependentServerActions(firstFailedActionId, minionId, Optional.empty());
+        if (actionChainFailed) {
+            long firstFailedActionId = failedActionIds.stream().min(Long::compare).orElseThrow();
+            // Set rest of actions as FAILED due to failed prerequisite
+            failDependentServerActions(firstFailedActionId, minionId, Optional.empty());
+        }
+        // Removing the generated SLS file
+        SaltActionChainGeneratorService.INSTANCE.removeActionChainSLSFiles(
+                retActionChainId, minionId, chunk, actionChainFailed);
+
+        ActionChainFactory.getActionChain(retActionChainId).ifPresent(ac -> {
+            // We need to reload server actions since saltssh will be in
+            // the same db session from when the action was started and
+            // won't see results of non ssh minions otherwise.
+            ac.getEntries().stream()
+                    .flatMap(ace -> ace.getAction().getServerActions().stream())
+                    .forEach(HibernateFactory::reload);
+            if (ac.isDone()) {
+                ActionChainFactory.delete(ac);
             }
-            // Removing the generated SLS file
-            SaltActionChainGeneratorService.INSTANCE.removeActionChainSLSFiles(
-                    retActionChainId, minionId, chunk, actionChainFailed);
-
-            ActionChainFactory.getActionChain(retActionChainId).ifPresent(ac -> {
-                // We need to reload server actions since saltssh will be in
-                // the same db session from when the action was started and
-                // won't see results of non ssh minions otherwise.
-                ac.getEntries().stream()
-                        .flatMap(ace -> ace.getAction().getServerActions().stream())
-                        .forEach(HibernateFactory::reload);
-                if (ac.isDone()) {
-                    ActionChainFactory.delete(ac);
-                }
-            });
-        }
+        });
     }
 
     /**
