Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> 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) */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Integer> skippedByReason = metric == null ? Map.of() : metric.skippedByReason();
Map<String, String> issueSkipReasons = metric == null ? Map.of() : metric.issueSkipReasons();
Set<String> modifiedFiles = metric == null ? Set.of() : metric.modifiedFiles();

result.put("totalRemediation", total);
Expand All @@ -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));
}

Expand Down Expand Up @@ -232,11 +233,28 @@ public static void putCacheExtras(
result.set(idArrayField, toStringArrayNode(ids));
}

public static ObjectNode toObjectNode(Map<String, Integer> skippedByReason) {
/** Inverts an issueId-to-reason map into reason-to-issueIds, preserving encounter order. */
public static Map<String, List<String>> groupIssueIdsByReason(Map<String, String> issueSkipReasons) {
Map<String, List<String>> 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<String, Integer> skippedByReason, Map<String, String> issueSkipReasons) {
ObjectNode object = JsonHelper.getObjectMapper().createObjectNode();
if (skippedByReason != null) {
skippedByReason.forEach(object::put);
if (skippedByReason == null) {
return object;
}
Map<String, List<String>> 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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -36,7 +38,11 @@ private AuditXmlIssuePruner() {}
* Removes every {@code <Issue>} whose {@code instanceId} is absent from {@code retainIds}.
* Issues with a blank {@code instanceId} are always removed.
* <p>
* 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
Expand All @@ -50,12 +56,15 @@ static int retainOnly(Document auditDoc, Set<String> retainIds) {
Set<String> 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<Element> 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -68,6 +69,36 @@ void aggregateUnfilteredMergesSkippedByReason() {
AviatorRemediationMetricsHelper.formatSkippedReasons(aggregated.skippedByReason()));
}

@Test
void groupIssueIdsByReasonInvertsIssueIdToReasonMap() {
Map<String, String> 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<String, List<String>> 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<String, Integer> skippedByReason = new LinkedHashMap<>();
skippedByReason.put("Source file missing", 2);
Map<String, String> 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"));
Expand Down
Loading