From 31a61e9549e84f8eb3fd8693dcafa3af8ee93ccc Mon Sep 17 00:00:00 2001 From: Umadevi Santhanam Date: Thu, 8 Oct 2026 18:44:32 +0530 Subject: [PATCH] add instanceids --- .../RemediationsApplyHelper.java | 24 +++++++++++++- .../util/AviatorRemediationMetricsHelper.java | 26 +++++++++++++--- .../fpr/processor/AuditXmlIssuePruner.java | 21 +++++++++---- .../AviatorRemediationMetricsHelperTest.java | 31 +++++++++++++++++++ 4 files changed, 91 insertions(+), 11 deletions(-) diff --git a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/_common/remediations_cache/RemediationsApplyHelper.java b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/_common/remediations_cache/RemediationsApplyHelper.java index 7b590e69001..0ea1c0b051d 100644 --- a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/_common/remediations_cache/RemediationsApplyHelper.java +++ b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/_common/remediations_cache/RemediationsApplyHelper.java @@ -71,7 +71,29 @@ public static ApplyResult apply( } return true; }); - return acc.toResult(); + ApplyResult result = acc.toResult(); + logFinalSkippedSummary(result, issueIdFilter, options.isPreviewMode(), logger); + return result; + } + + /** + * Logs a final, aggregated summary of every skipped instance ID grouped by skip reason, so + * troubleshooting which source context/file is missing doesn't require scraping per-FPR log lines. + */ + private static void logFinalSkippedSummary( + ApplyResult result, Set issueIdFilter, boolean previewMode, IAviatorLogger logger) { + if (result.metrics().isEmpty()) { + return; + } + RemediationExecutionMode mode = previewMode ? RemediationExecutionMode.PREVIEW : RemediationExecutionMode.APPLY; + RemediationMetric aggregated = AviatorRemediationMetricsHelper.aggregateMetrics(issueIdFilter, result.metrics(), mode); + if (aggregated.skippedByReason().isEmpty()) { + return; + } + logger.info("Skipped remediation summary: {}", + AviatorRemediationMetricsHelper.formatSkippedReasons(aggregated.skippedByReason())); + AviatorRemediationMetricsHelper.groupIssueIdsByReason(aggregated.issueSkipReasons()) + .forEach((reason, issueIds) -> logger.info(" {} ({}): {}", reason, issueIds.size(), issueIds)); } /** @see AviatorRemediationMetricsHelper#actionLabel(RemediationMetric) */ diff --git a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/_common/util/AviatorRemediationMetricsHelper.java b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/_common/util/AviatorRemediationMetricsHelper.java index ae37ea496e4..6062236efad 100644 --- a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/_common/util/AviatorRemediationMetricsHelper.java +++ b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/_common/util/AviatorRemediationMetricsHelper.java @@ -189,6 +189,7 @@ public static void putRemediationMetricFields(ObjectNode result, RemediationMetr int skipped = metric == null ? 0 : metric.skippedRemediations(); String appliedFieldName = metric != null && metric.isPreview() ? "availableRemediation" : "appliedRemediation"; Map skippedByReason = metric == null ? Map.of() : metric.skippedByReason(); + Map issueSkipReasons = metric == null ? Map.of() : metric.issueSkipReasons(); Set modifiedFiles = metric == null ? Set.of() : metric.modifiedFiles(); result.put("totalRemediation", total); @@ -198,7 +199,7 @@ public static void putRemediationMetricFields(ObjectNode result, RemediationMetr result.put("possiblyRemediatedRemediation", metric == null ? 0 : metric.possiblyRemediatedRemediations()); result.put("skippedRemediation", skipped); result.put("skippedReasons", formatSkippedReasons(skippedByReason)); - result.set("skippedByReason", toObjectNode(skippedByReason)); + result.set("skippedByReason", toSkippedByReasonNode(skippedByReason, issueSkipReasons)); result.set("modifiedFiles", toArrayNode(modifiedFiles)); } @@ -232,11 +233,28 @@ public static void putCacheExtras( result.set(idArrayField, toStringArrayNode(ids)); } - public static ObjectNode toObjectNode(Map skippedByReason) { + /** Inverts an issueId-to-reason map into reason-to-issueIds, preserving encounter order. */ + public static Map> groupIssueIdsByReason(Map issueSkipReasons) { + Map> issueIdsByReason = new LinkedHashMap<>(); + if (issueSkipReasons != null) { + issueSkipReasons.forEach((issueId, reason) -> + issueIdsByReason.computeIfAbsent(reason, r -> new ArrayList<>()).add(issueId)); + } + return issueIdsByReason; + } + + /** Per-reason {@code {count, issueIds}} object, so troubleshooting a reason doesn't require cross-referencing a separate list. */ + public static ObjectNode toSkippedByReasonNode(Map skippedByReason, Map issueSkipReasons) { ObjectNode object = JsonHelper.getObjectMapper().createObjectNode(); - if (skippedByReason != null) { - skippedByReason.forEach(object::put); + if (skippedByReason == null) { + return object; } + Map> issueIdsByReason = groupIssueIdsByReason(issueSkipReasons); + skippedByReason.forEach((reason, count) -> { + ObjectNode reasonNode = object.putObject(reason); + reasonNode.put("count", count); + reasonNode.set("issueIds", toStringArrayNode(issueIdsByReason.getOrDefault(reason, List.of()))); + }); return object; } diff --git a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/AuditXmlIssuePruner.java b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/AuditXmlIssuePruner.java index aa1ebfe96eb..f9a1bf7bddd 100644 --- a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/AuditXmlIssuePruner.java +++ b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/AuditXmlIssuePruner.java @@ -12,6 +12,8 @@ */ package com.fortify.cli.aviator.fpr.processor; +import java.util.ArrayList; +import java.util.List; import java.util.Set; import org.slf4j.Logger; @@ -36,7 +38,11 @@ private AuditXmlIssuePruner() {} * Removes every {@code } whose {@code instanceId} is absent from {@code retainIds}. * Issues with a blank {@code instanceId} are always removed. *

- * Performance: one reverse pass over the live {@link NodeList} with O(1) set lookups. + * Performance: the {@link NodeList} from {@code getElementsByTagNameNS} is live and backed by + * a forward-caching traversal (e.g. Xerces' {@code DeepNodeListImpl}), so indexed access is + * only cheap when walked in increasing order; a decreasing-index pass re-walks the tree from + * the start on every call and turns into O(n^2). Snapshot into a plain list with one forward + * pass first, then remove from that snapshot (order no longer matters once it's not live). * * @param auditDoc in-memory audit.xml document (mutated in place); no-op if null * @param retainIds instance IDs written in the current save; treated as empty if null @@ -50,12 +56,15 @@ static int retainOnly(Document auditDoc, Set retainIds) { Set safeRetainIds = retainIds == null ? Set.of() : retainIds; NodeList issueNodes = auditDoc.getElementsByTagNameNS(AUDIT_NAMESPACE_URI, "Issue"); int beforeCount = issueNodes.getLength(); - int removedCount = 0; - - for (int i = beforeCount - 1; i >= 0; i--) { - if (!(issueNodes.item(i) instanceof Element issueElement)) { - continue; + List issueElements = new ArrayList<>(beforeCount); + for (int i = 0; i < beforeCount; i++) { + if (issueNodes.item(i) instanceof Element issueElement) { + issueElements.add(issueElement); } + } + + int removedCount = 0; + for (Element issueElement : issueElements) { String instanceId = issueElement.getAttribute("instanceId"); if (instanceId == null || instanceId.isBlank() || !safeRetainIds.contains(instanceId)) { Node parent = issueElement.getParentNode(); diff --git a/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/_common/util/AviatorRemediationMetricsHelperTest.java b/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/_common/util/AviatorRemediationMetricsHelperTest.java index 4aec3beb464..ef53da2d4ca 100644 --- a/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/_common/util/AviatorRemediationMetricsHelperTest.java +++ b/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/_common/util/AviatorRemediationMetricsHelperTest.java @@ -23,6 +23,7 @@ import org.junit.jupiter.api.Test; +import com.fasterxml.jackson.databind.node.ObjectNode; import com.fortify.cli.aviator.fpr.remediation.RemediationExecutionMode; import com.fortify.cli.aviator.fpr.remediation.model.RemediationMetric; import com.fortify.cli.aviator.fpr.remediation.preview.PreviewDetail; @@ -68,6 +69,36 @@ void aggregateUnfilteredMergesSkippedByReason() { AviatorRemediationMetricsHelper.formatSkippedReasons(aggregated.skippedByReason())); } + @Test + void groupIssueIdsByReasonInvertsIssueIdToReasonMap() { + Map issueSkipReasons = new LinkedHashMap<>(); + issueSkipReasons.put("ISSUE-1", "Source file missing"); + issueSkipReasons.put("ISSUE-2", "Source file missing"); + issueSkipReasons.put("ISSUE-3", "No file changes found"); + + Map> grouped = AviatorRemediationMetricsHelper.groupIssueIdsByReason(issueSkipReasons); + + assertEquals(List.of("ISSUE-1", "ISSUE-2"), grouped.get("Source file missing")); + assertEquals(List.of("ISSUE-3"), grouped.get("No file changes found")); + } + + @Test + void skippedByReasonNodeIncludesCountAndIssueIdsPerReason() { + Map skippedByReason = new LinkedHashMap<>(); + skippedByReason.put("Source file missing", 2); + Map issueSkipReasons = new LinkedHashMap<>(); + issueSkipReasons.put("ISSUE-1", "Source file missing"); + issueSkipReasons.put("ISSUE-2", "Source file missing"); + + ObjectNode node = AviatorRemediationMetricsHelper.toSkippedByReasonNode(skippedByReason, issueSkipReasons); + + ObjectNode reasonNode = (ObjectNode) node.get("Source file missing"); + assertEquals(2, reasonNode.get("count").asInt()); + assertEquals(2, reasonNode.get("issueIds").size()); + assertEquals("ISSUE-1", reasonNode.get("issueIds").get(0).asText()); + assertEquals("ISSUE-2", reasonNode.get("issueIds").get(1).asText()); + } + @Test void remainingIssueIdsDropsAlreadyApplied() { RemediationMetric metric = filteredMetric(Set.of("ISSUE-1", "ISSUE-2"), Set.of("ISSUE-1"), Set.of("A.java"));