fix(perc-content-explorer): type relationships manager + process monitor (#2446)

docs/ai-generated/code-reviews/issue-2446-dce-relationships-process-monitor-erlang.md

@@ -0,0 +1,23 @@
+# Erlang self-review — issue #2446 PSItemRelationshipsManager + PSProcessMonitor
+
+## Scope
+- Type collections/iterators on `PSItemRelationshipsManager` and pure helpers on `PSProcessMonitor`
+- Call site: `PSActionManager` DT dependency load uses typed `Iterator<PSNode>`
+- Unit tests for pure helpers
+- Module: `modules/DesktopContentExplorer` (`perc-content-explorer`)
+
+## Findings
+- **None (bugs):** Typed iterators match `PSComponentSummaries.iterator()` / `getSummaries()` contracts (`Iterator<PSComponentSummary>`). Label formatting behavior preserved (LinkedHashMap order in tests).
+- **None (paths):** No filesystem I/O changes.
+- **None (tests):** 10 new pure-helper tests; full module suite 64 / 0 failures.
+
+## Residual (module still hot under #2045)
+Compile still reports pre-existing Xlint outside named files, e.g.:
+- `PSContentExplorerStatusDialog` (#2445)
+- `PSDesktopExplorerWindow` (#2444)
+- `PSActionManager` cloning / `PSCommunityMappingsPage.OutputData` unchecked Map
+- `PSWizardDialog` raw Map
+- Cataloger `this-escape` after prior typing batches
+
+## Verdict
+**Approve** for PR under #2446. Named residual classes clean of raw Iterator/cast paths addressed here; module clean install green.

modules/DesktopContentExplorer/src/main/java/com/percussion/cx/PSActionManager.java

@@ -1814,7 +1814,7 @@ public Iterator<PSNode> loadChildren(PSNode node) {
       // Load children using Item Relationship Manager for dependency tree
       // mode
       if (m_applet.getView().equals(PSUiMode.TYPE_VIEW_DT)) {
-        children = asNodes(m_irsManager.loadDependencies(node));
+        children = m_irsManager.loadDependencies(node);
       } else {
         /**
          * If the node type is "Folder" or "SystemFolder" or "SystemSite" load the children using

modules/DesktopContentExplorer/src/main/java/com/percussion/cx/PSItemRelationshipsManager.java

@@ -39,6 +39,7 @@
 import com.percussion.xml.PSXmlDocumentBuilder;
 import java.net.URL;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -51,16 +52,6 @@
 
 /** The manager class to use to get ancestors or descendants of an item for a relationship. */
 public class PSItemRelationshipsManager {
-  /**
-   * Constructs the manager with supplied parameters.
-   *
-   * @param proxy the remote proxy to use to execute a relationship request to the server, may not
-   *     be <code>null</code>.
-   * @param folderMgr Never <code>null</code>.
-   * @param remCataloger the remote cataloger to use to get list of available relationships, may not
-   *     be <code>null</code>
-   * @param docBase applet base Url to make any server requests, may not be <code>null</code>
-   */
   /**
    * Constructs the manager with supplied parameters.
    *
@@ -124,7 +115,7 @@ public PSRelationshipInfoSet getRelationships() throws PSCmsException {
    * @throws PSCmsException if an error happens loading dependencies
    * @throws PSContentExplorerException if an error happens loading details of dependencies
    */
-  public Iterator loadDependencies(PSNode parentNode)
+  public Iterator<PSNode> loadDependencies(PSNode parentNode)
       throws PSCmsException, PSContentExplorerException {
     if (parentNode == null) throw new IllegalArgumentException("parentNode may not be null.");
 
@@ -169,9 +160,9 @@ public Iterator loadDependencies(PSNode parentNode)
 
         // filter dependents for current revision
         summaries = new PSComponentSummaries();
-        Iterator walker = testSummaries.iterator();
+        Iterator<PSComponentSummary> walker = testSummaries.iterator();
         while (walker.hasNext()) {
-          PSComponentSummary summary = (PSComponentSummary) walker.next();
+          PSComponentSummary summary = walker.next();
           filter.setOwner(summary.getCurrentLocator());
           if (!m_proxy.getSummaries(filter, true).isEmpty()) summaries.add(summary);
         }
@@ -189,9 +180,9 @@ public Iterator loadDependencies(PSNode parentNode)
     List<Integer> idList = new ArrayList<>();
     // Set the 'relationship', 'rs_type' and the locator as properties of each
     // node
-    Iterator it = summaries.iterator();
+    Iterator<PSComponentSummary> it = summaries.iterator();
     while (it.hasNext()) {
-      PSComponentSummary dependent = (PSComponentSummary) it.next();
+      PSComponentSummary dependent = it.next();
 
       PSLocator locator = dependent.getCurrentLocator();
       int id = locator.getId();
@@ -238,20 +229,7 @@ public Iterator loadDependencies(PSNode parentNode)
         node.setProperty(IPSConstants.PROPERTY_RELATIONSHIP, relationship);
 
         node.setProperty(PROP_RS_LOOKUP_TYPE, rsType);
-        String label = node.getLabel();
-        Map<String, Object> rowData = node.getRowData();
-        if (rowData != null) {
-          Iterator<Object> values = rowData.values().iterator();
-          int ii = 0;
-          while (values.hasNext()) {
-            if (ii == 0) label += " (";
-            else if (ii > 0) label += " - ";
-            label += values.next().toString();
-            ii++;
-          }
-          if (ii > 0) label += ")";
-          node.setLabel(label);
-        }
+        node.setLabel(formatDependencyLabel(node.getLabel(), node.getRowData()));
       }
       resList.addAll(list);
     }
@@ -260,6 +238,77 @@ public Iterator loadDependencies(PSNode parentNode)
     return resList.iterator();
   }
 
+  /**
+   * Appends display-format row values to a dependency node label. Pure helper for unit tests.
+   *
+   * @param baseLabel the node label before row data, may be <code>null</code>
+   * @param rowData display-format column values, may be <code>null</code> or empty
+   * @return the formatted label, never <code>null</code>
+   */
+  public static String formatDependencyLabel(String baseLabel, Map<String, ?> rowData) {
+    String label = baseLabel == null ? "" : baseLabel;
+    if (rowData == null || rowData.isEmpty()) {
+      return label;
+    }
+    StringBuilder result = new StringBuilder(label);
+    int ii = 0;
+    for (Object value : rowData.values()) {
+      if (ii == 0) {
+        result.append(" (");
+      } else {
+        result.append(" - ");
+      }
+      result.append(value == null ? "null" : value.toString());
+      ii++;
+    }
+    if (ii > 0) {
+      result.append(')');
+    }
+    return result.toString();
+  }
+
+  /**
+   * Returns whether a relationship slot is still allowed (registered template slot or inline).
+   * Pure helper for unit tests.
+   *
+   * @param slotId the relationship <code>sys_slotid</code>, may be <code>null</code>
+   * @param registeredSlots slots registered to the owner content type templates, may be <code>null
+   *     </code>
+   * @param inlineSlots known inline slot ids, may be <code>null</code>
+   * @return <code>true</code> when the slot is registered or inline
+   */
+  public static boolean isAllowedRelationshipSlot(
+      String slotId, Collection<String> registeredSlots, Collection<String> inlineSlots) {
+    if (slotId == null) {
+      return false;
+    }
+    return (registeredSlots != null && registeredSlots.contains(slotId))
+        || (inlineSlots != null && inlineSlots.contains(slotId));
+  }
+
+  /**
+   * Filters component summaries to those whose content id is in the allowed set. Pure helper for
+   * unit tests.
+   *
+   * @param summaries summaries to filter, may be <code>null</code>
+   * @param allowedContentIds content ids as strings, may be <code>null</code>
+   * @return matching summaries in encounter order, never <code>null</code>
+   */
+  public static List<PSComponentSummary> filterSummariesByContentIds(
+      Iterator<PSComponentSummary> summaries, Collection<String> allowedContentIds) {
+    List<PSComponentSummary> result = new ArrayList<>();
+    if (summaries == null || allowedContentIds == null || allowedContentIds.isEmpty()) {
+      return result;
+    }
+    while (summaries.hasNext()) {
+      PSComponentSummary sum = summaries.next();
+      if (sum != null && allowedContentIds.contains(String.valueOf(sum.getContentId()))) {
+        result.add(sum);
+      }
+    }
+    return result;
+  }
+
   /**
    * Filters the relationships based on whether the slots are still registered to the content types
    * through their allowed variants. Gets the related content document and gets the sys_slotid
@@ -309,20 +358,13 @@ private PSComponentSummaries getFilteredRelationshipSummaries(
         Object[] args = new Object[] {e.getLocalizedMessage()};
         throw new PSCmsException(IPSCmsErrors.UNEXPECTED_ERROR, args);
       }
-      if (slotId != null && cid != null) {
-        if (regSlots.contains(slotId) || inlineSlots.contains(slotId)) {
-          contentIds.add(cid);
-        }
+      if (cid != null && isAllowedRelationshipSlot(slotId, regSlots, inlineSlots)) {
+        contentIds.add(cid);
       }
     }
     PSComponentSummaries allsummaries = m_proxy.getSummaries(filter, false);
-    Iterator siter = allsummaries.getSummaries();
-    while (siter.hasNext()) {
-      PSComponentSummary sum = (PSComponentSummary) siter.next();
-      String cid = "" + sum.getContentId();
-      if (contentIds.contains(cid)) {
-        summaries.add(sum);
-      }
+    for (PSComponentSummary sum : filterSummariesByContentIds(allsummaries.getSummaries(), contentIds)) {
+      summaries.add(sum);
     }
     return summaries;
   }

modules/DesktopContentExplorer/src/main/java/com/percussion/cx/PSProcessMonitor.java

@@ -26,28 +26,60 @@
  * thread safe to set or get the status.
  */
 public class PSProcessMonitor {
-  /**
-   * Constructs this object to monitor the processing of supplied number of nodes.
-   *
-   * @param total the total number of nodes to process, may not be <= 0.
-   * @throws IllegalArgumentException if total is invalid.
-   */
   /**
    * Constructs this object to monitor the processing of supplied number of nodes.
    *
    * @param total the total number of nodes to process, may not be <= 0.
    * @param applet the content explorer applet, may not be <code>null</code>.
-   * @throws IllegalArgumentException if total is invalid.
+   * @throws IllegalArgumentException if total is invalid or applet is <code>null</code>.
    */
   public PSProcessMonitor(int total, PSContentExplorerApplet applet) {
-    if (total <= 0) throw new IllegalArgumentException("total may not be <= 0");
+    if (!isValidTotal(total)) throw new IllegalArgumentException("total may not be <= 0");
 
     if (applet == null) throw new IllegalArgumentException("applet must not be null");
     m_applet = applet;
 
     m_total = total;
   }
 
+  /**
+   * Returns whether a total node count is valid for construction. Pure helper for unit tests.
+   *
+   * @param total candidate total
+   * @return <code>true</code> when total is positive
+   */
+  public static boolean isValidTotal(int total) {
+    return total > 0;
+  }
+
+  /**
+   * Returns whether a status constant is one of {@code STATUS_xxx}. Pure helper for unit tests.
+   *
+   * @param status candidate status value
+   * @return <code>true</code> when status is in range
+   */
+  public static boolean isValidStatus(int status) {
+    return status >= STATUS_INIT && status <= STATUS_COMPLETE;
+  }
+
+  /**
+   * Computes dialog percent-complete for the current node index. Pure helper for unit tests.
+   *
+   * @param current 1-based index of the node being processed
+   * @param total total nodes being processed, must be &gt; 0
+   * @return percent in range 0..100
+   * @throws IllegalArgumentException if current/total are invalid
+   */
+  public static int computePercentDone(int current, int total) {
+    if (!isValidTotal(total)) {
+      throw new IllegalArgumentException("total may not be <= 0");
+    }
+    if (current <= 0 || current > total) {
+      throw new IllegalArgumentException("current may not be <= 0 or > actual total");
+    }
+    return (current - 1) * 100 / total;
+  }
+
   /**
    * Sets the dialog to be updated whenever this monitor status gets updated.
    *
@@ -69,8 +101,8 @@ public void setStatusDialog(PSContentExplorerStatusDialog dlg) {
    * @throws IllegalArgumentException if any parameter is invalid
    */
   public void updateStatus(int current, PSNode processingNode) {
-    if (current > m_total || current <= 0)
-      throw new IllegalArgumentException("current may not be <= 0 or > actual total");
+    // validate range via pure helper (throws on invalid)
+    int percentDone = computePercentDone(current, m_total);
 
     if (processingNode == null)
       throw new IllegalArgumentException("processingNode may not be null.");
@@ -82,9 +114,9 @@ public void updateStatus(int current, PSNode processingNode) {
       String msg =
           MessageFormat.format(
               m_applet.getResourceString(getClass(), "Processing the {0} <{1}>"),
-              new String[] {m_curProcessNode.getType(), m_curProcessNode.getLabel()});
+              m_curProcessNode.getType(),
+              m_curProcessNode.getLabel());
 
-      int percentDone = (m_current - 1) * 100 / m_total;
       m_dlg.updateStatus(msg, percentDone);
     }
   }
@@ -109,8 +141,7 @@ public int getStatus() {
    * @throws IllegalArgumentException if any currentStatus is invalid
    */
   public synchronized void setStatus(int currentStatus) {
-    if (currentStatus < STATUS_INIT || currentStatus > STATUS_COMPLETE)
-      throw new IllegalArgumentException("invalid status");
+    if (!isValidStatus(currentStatus)) throw new IllegalArgumentException("invalid status");
 
     synchronized (m_syncObject) {
       m_status = currentStatus;