Skip to content

Commit

Permalink
Optimized impact paths building (#441)
Browse files Browse the repository at this point in the history
* Separated the process of grouping dependencies in file nodes from the process of building impact paths.
* Improved both processes for better performance.
  • Loading branch information
asafgabai authored Nov 30, 2023
1 parent 8e30f09 commit 5a6a4a4
Show file tree
Hide file tree
Showing 17 changed files with 502 additions and 112 deletions.
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ repositories {
}

def buildInfoVersion = '2.41.6'
def idePluginsCommonVersion = '2.3.2'
def idePluginsCommonVersion = '2.3.3'

dependencies {
implementation group: 'com.fasterxml.jackson.dataformat', name: 'jackson-dataformat-yaml', version: '2.15.2'
Expand Down
4 changes: 2 additions & 2 deletions gradle.properties
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
webviewVersion=0.2.8
webviewVersion=0.2.12
sandboxVersion=2022.3.2
webviewChecksum=6b0bb07998ee6d83037f0278e61a210d22c5234f0906892ed5df4831fd5f7ff1
webviewChecksum=cb88f9fd7fe2380a811c7b290a5aa451b972cdcba84fc9c41e5e274706c79a12
currentVersion=2.6.x-SNAPSHOT
60 changes: 60 additions & 0 deletions src/main/java/com/jfrog/ide/idea/scan/MavenScanner.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
import com.intellij.psi.search.GlobalSearchScope;
import com.jfrog.ide.common.deptree.DepTree;
import com.jfrog.ide.common.deptree.DepTreeNode;
import com.jfrog.ide.common.nodes.DependencyNode;
import com.jfrog.ide.common.nodes.DescriptorFileTreeNode;
import com.jfrog.ide.common.nodes.FileTreeNode;
import com.jfrog.ide.common.scan.ComponentPrefix;
import com.jfrog.ide.common.scan.ScanLogic;
import com.jfrog.ide.idea.inspections.AbstractInspection;
Expand All @@ -28,6 +31,7 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.*;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.ExecutorService;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -158,4 +162,60 @@ private void updateChildrenNodes(Map<String, DepTreeNode> nodes, DepTreeNode par
.forEach(childrenArtifactNode -> updateChildrenNodes(nodes, currentNode, childrenArtifactNode, false));
parentNode.getChildren().add(compId);
}

/**
* Groups a collection of {@link DependencyNode}s by the descriptor files of the modules that depend on them.
* The returned DependencyNodes inside the {@link FileTreeNode}s are clones of the ones in depScanResults.
*
* @param depScanResults collection of DependencyNodes
* @param depTree the project's dependency tree
* @param parents a map of components by their IDs and their parents in the dependency tree
* @return a list of FileTreeNodes (that are all DescriptorFileTreeNodes) having the DependencyNodes as their children
*/
@Override
protected List<FileTreeNode> groupDependenciesToDescriptorNodes(Collection<DependencyNode> depScanResults, DepTree depTree, Map<String, Set<String>> parents) {
Map<String, DescriptorFileTreeNode> descriptorMap = new HashMap<>();
Map<String, Set<String>> visitedComponents = new HashMap<>();
for (DependencyNode dependencyNode : depScanResults) {
String vulnerableDepId = dependencyNode.getComponentIdWithoutPrefix();
Set<String> affectedModulesIds = getDependentModules(vulnerableDepId, depTree, parents, visitedComponents);
for (String descriptorId : affectedModulesIds) {
String descriptorPath = depTree.nodes().get(descriptorId).getDescriptorFilePath();
descriptorMap.putIfAbsent(descriptorPath, new DescriptorFileTreeNode(descriptorPath));

// Each dependency might be a child of more than one POM file, but Artifact is a tree node, so it can have only one parent.
// The solution for this is to clone the dependency before adding it as a child of the POM.
DependencyNode clonedDep = (DependencyNode) dependencyNode.clone();
clonedDep.setIndirect(!parents.get(vulnerableDepId).contains(descriptorId));
descriptorMap.get(descriptorPath).addDependency(clonedDep);
}
}
return new CopyOnWriteArrayList<>(descriptorMap.values());
}

/**
* Retrieve component IDs of all modules in the project that are dependent on the specified component.
*
* @param compId the component ID to identify modules depending on it
* @param depTree the project's dependency tree
* @param parents a map of components by their IDs and their parents in the dependency tree
* @param visitedComponents a map of components for which dependent modules have already been found
* @return a set of component IDs representing modules dependent on the specified component
*/
Set<String> getDependentModules(String compId, DepTree depTree, Map<String, Set<String>> parents, Map<String, Set<String>> visitedComponents) {
if (visitedComponents.containsKey(compId)) {
return visitedComponents.get(compId);
}
Set<String> modulesIds = new HashSet<>();
if (depTree.nodes().get(compId).getDescriptorFilePath() != null) {
modulesIds.add(compId);
}
if (parents.containsKey(compId)) {
for (String parentId : parents.get(compId)) {
modulesIds.addAll(getDependentModules(parentId, depTree, parents, visitedComponents));
}
}
visitedComponents.put(compId, modulesIds);
return modulesIds;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
import static com.jfrog.ide.common.utils.Utils.createMapper;
import static com.jfrog.ide.common.utils.Utils.createYAMLMapper;
import static com.jfrog.ide.common.utils.XrayConnectionUtils.createXrayClientBuilder;
import static com.jfrog.ide.idea.scan.ScanUtils.getOSAndArc;
import static com.jfrog.ide.idea.scan.utils.ScanUtils.getOSAndArc;
import static com.jfrog.ide.idea.utils.Utils.HOME_PATH;
import static java.lang.String.join;

Expand Down
122 changes: 28 additions & 94 deletions src/main/java/com/jfrog/ide/idea/scan/ScannerBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,15 @@
import com.jfrog.ide.common.deptree.DepTreeNode;
import com.jfrog.ide.common.log.ProgressIndicator;
import com.jfrog.ide.common.nodes.DependencyNode;
import com.jfrog.ide.common.nodes.DescriptorFileTreeNode;
import com.jfrog.ide.common.nodes.FileTreeNode;
import com.jfrog.ide.common.nodes.subentities.ImpactTree;
import com.jfrog.ide.common.nodes.subentities.ImpactTreeNode;
import com.jfrog.ide.common.scan.ComponentPrefix;
import com.jfrog.ide.common.scan.ScanLogic;
import com.jfrog.ide.idea.configuration.GlobalSettings;
import com.jfrog.ide.idea.inspections.AbstractInspection;
import com.jfrog.ide.idea.log.Logger;
import com.jfrog.ide.idea.log.ProgressIndicatorImpl;
import com.jfrog.ide.idea.scan.data.PackageManagerType;
import com.jfrog.ide.idea.scan.utils.ImpactTreeBuilder;
import com.jfrog.ide.idea.ui.ComponentsTree;
import com.jfrog.ide.idea.ui.LocalComponentsTree;
import com.jfrog.ide.idea.ui.menus.filtermanager.ConsistentFilterManager;
Expand All @@ -50,7 +48,6 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.*;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.atomic.AtomicBoolean;
Expand Down Expand Up @@ -132,6 +129,18 @@ protected void sendUsageReport() {

protected abstract PackageManagerType getPackageManagerType();

/**
* Groups a collection of {@link DependencyNode}s by the descriptor files of the modules that depend on them.
* The returned DependencyNodes inside the {@link FileTreeNode}s might be clones of the ones in depScanResults, but
* it's not guaranteed.
*
* @param depScanResults collection of DependencyNodes
* @param depTree the project's dependency tree
* @param parents a map of components by their IDs and their parents in the dependency tree
* @return a list of FileTreeNodes (that are all DescriptorFileTreeNodes) having the DependencyNodes as their children
*/
protected abstract List<FileTreeNode> groupDependenciesToDescriptorNodes(Collection<DependencyNode> depScanResults, DepTree depTree, Map<String, Set<String>> parents);

/**
* Scan and update dependency components.
*
Expand Down Expand Up @@ -176,104 +185,29 @@ private void scanAndUpdate(ProgressIndicator indicator) {
}
}

/**
* Walks through a {@link DepTree}'s nodes.
* Builds impact paths for {@link DependencyNode} objects and groups them in {@link DescriptorFileTreeNode}s.
*
* @param vulnerableDependencies a map of component IDs and the DependencyNode object matching each of them.
* @param depTree the project's dependency tree to walk through.
*/
protected List<FileTreeNode> buildImpactGraph(Map<String, DependencyNode> vulnerableDependencies, DepTree depTree) throws IOException {
Map<String, DescriptorFileTreeNode> descriptorNodes = new HashMap<>();
visitDepTreeNode(vulnerableDependencies, depTree, Collections.singletonList(depTree.rootId()), descriptorNodes, new ArrayList<>(), new HashMap<>());
return new CopyOnWriteArrayList<>(descriptorNodes.values());
Map<String, Set<String>> parents = getParents(depTree);
ImpactTreeBuilder.populateImpactTrees(vulnerableDependencies, parents, depTree.rootId());
return groupDependenciesToDescriptorNodes(vulnerableDependencies.values(), depTree, parents);
}

/**
* Visit a node in the {@link DepTree} and walk through its children.
* Each impact path to a vulnerable dependency is added in its {@link DependencyNode}.
* Each DependencyNode is added to the relevant {@link DescriptorFileTreeNode}s.
* Find the parents of each node in the given {@link DepTree}.
* Nodes without parents (the root) don't appear in the returned map.
*
* @param vulnerableDependencies a map of {@link DependencyNode}s by their component IDs.
* @param depTree the project's dependency tree.
* @param path a path of nodes (represented by their component IDs) from the root to the current node.
* @param descriptorNodes a map of {@link DescriptorFileTreeNode}s by the descriptor file path. Missing DescriptorFileTreeNodes will be added to this map.
* @param descriptorPaths a list of descriptor file paths that their matching components are in the path to the current node.
* @param addedDeps a map of all {@link DependencyNode}s already grouped to {@link DescriptorFileTreeNode}s. Newly grouped DependencyNodes will be added to this map.
* @param depTree the {@link DepTree} to find the parents of its nodes
* @return a map of nodes from the {@link DepTree} amd each one's parents
*/
private void visitDepTreeNode(Map<String, DependencyNode> vulnerableDependencies, DepTree depTree, List<String> path,
Map<String, DescriptorFileTreeNode> descriptorNodes, List<String> descriptorPaths,
Map<String, Map<String, DependencyNode>> addedDeps) {
String compId = path.get(path.size() - 1);
DepTreeNode compNode = depTree.nodes().get(compId);
List<String> innerDescriptorPaths = descriptorPaths;
if (compNode.getDescriptorFilePath() != null) {
innerDescriptorPaths = new ArrayList<>(descriptorPaths);
innerDescriptorPaths.add(compNode.getDescriptorFilePath());
}
if (vulnerableDependencies.containsKey(compId)) {
DependencyNode dependencyNode = vulnerableDependencies.get(compId);
addImpactPathToDependencyNode(dependencyNode, path);

DepTreeNode parentCompNode = null;
if (path.size() >= 2) {
String parentCompId = path.get(path.size() - 2);
parentCompNode = depTree.nodes().get(parentCompId);
}
for (String descriptorPath : innerDescriptorPaths) {
boolean indirect = parentCompNode != null && !descriptorPath.equals(parentCompNode.getDescriptorFilePath());
if (!descriptorNodes.containsKey(descriptorPath)) {
descriptorNodes.put(descriptorPath, new DescriptorFileTreeNode(descriptorPath));
addedDeps.put(descriptorPath, new HashMap<>());
}
DependencyNode existingDep = addedDeps.get(descriptorPath).get(compId);
if (existingDep != null) {
// If this dependency has any direct path, then it's direct
if (existingDep.isIndirect() && !indirect) {
existingDep.setIndirect(false);
}
continue;
}
// Each dependency might be a child of more than one descriptor, but DependencyNode is a tree node, so it can have only one parent.
// The solution for this is to clone the dependency before adding it as a child of the POM.
DependencyNode clonedDep = (DependencyNode) dependencyNode.clone();
clonedDep.setIndirect(indirect);

descriptorNodes.get(descriptorPath).addDependency(clonedDep);
addedDeps.get(descriptorPath).put(compId, clonedDep);
}
}

for (String childId : compNode.getChildren()) {
List<String> pathToChild = new ArrayList<>(path);
pathToChild.add(childId);
if (!path.contains(childId)) {
visitDepTreeNode(vulnerableDependencies, depTree, pathToChild, descriptorNodes, innerDescriptorPaths, addedDeps);
}
}
}

protected void addImpactPathToDependencyNode(DependencyNode dependencyNode, List<String> path) {
dependencyNode.setImpactTree(new ImpactTree(new ImpactTreeNode(path.get(0))));
ImpactTree impactTree = dependencyNode.getImpactTree();
if (impactTree.getImpactPathsCount() == ImpactTree.IMPACT_PATHS_LIMIT) {
return;
}
ImpactTreeNode parentImpactTreeNode = impactTree.getRoot();
for (int pathNodeIndex = 1; pathNodeIndex < path.size(); pathNodeIndex++) {
String currPathNode = path.get(pathNodeIndex);
// Find a child of parentImpactTreeNode with a name equals to currPathNode
ImpactTreeNode currImpactTreeNode = parentImpactTreeNode.getChildren().stream().filter(impactTreeNode -> impactTreeNode.getName().equals(currPathNode)).findFirst().orElse(null);
if (currImpactTreeNode == null) {
currImpactTreeNode = new ImpactTreeNode(currPathNode);
parentImpactTreeNode.getChildren().add(currImpactTreeNode);
if (pathNodeIndex == path.size() - 1) {
// If a new leaf was added, thus a new impact path was added (impact paths don't collide after they split)
impactTree.incImpactPathsCount();
}
static Map<String, Set<String>> getParents(DepTree depTree) {
Map<String, Set<String>> parents = new HashMap<>();
for (Map.Entry<String, DepTreeNode> node : depTree.nodes().entrySet()) {
String parentId = node.getKey();
for (String childId : node.getValue().getChildren()) {
parents.putIfAbsent(childId, new HashSet<>());
parents.get(childId).add(parentId);
}
parentImpactTreeNode = currImpactTreeNode;
}
return parents;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/jfrog/ide/idea/scan/ScannerFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import java.util.Set;
import java.util.concurrent.ExecutorService;

import static com.jfrog.ide.idea.scan.ScanUtils.createScanPaths;
import static com.jfrog.ide.idea.scan.utils.ScanUtils.createScanPaths;
import static com.jfrog.ide.idea.utils.Utils.getProjectBasePath;

/**
Expand Down
35 changes: 35 additions & 0 deletions src/main/java/com/jfrog/ide/idea/scan/SingleDescriptorScanner.java
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
package com.jfrog.ide.idea.scan;

import com.intellij.openapi.project.Project;
import com.jfrog.ide.common.deptree.DepTree;
import com.jfrog.ide.common.deptree.DepTreeNode;
import com.jfrog.ide.common.nodes.DependencyNode;
import com.jfrog.ide.common.nodes.DescriptorFileTreeNode;
import com.jfrog.ide.common.nodes.FileTreeNode;
import com.jfrog.ide.common.scan.ComponentPrefix;
import com.jfrog.ide.common.scan.ScanLogic;
import com.jfrog.ide.idea.ui.ComponentsTree;
import com.jfrog.ide.idea.ui.menus.filtermanager.ConsistentFilterManager;
import org.jetbrains.annotations.NotNull;
import org.jfrog.build.extractor.scan.DependencyTree;

import java.util.*;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.ExecutorService;

public abstract class SingleDescriptorScanner extends ScannerBase {
Expand Down Expand Up @@ -39,4 +47,31 @@ public abstract class SingleDescriptorScanner extends ScannerBase {
ScanLogic scanLogic) {
this(project, basePath, prefix, executor, "", scanLogic);
}

/**
* Groups a collection of {@link DependencyNode}s by the descriptor files of the modules that depend on them.
* The returned DependencyNodes inside the {@link FileTreeNode}s are references of the ones in depScanResults.
*
* @param depScanResults collection of DependencyNodes
* @param depTree the project's dependency tree
* @param parents a map of components by their IDs and their parents in the dependency tree
* @return a list of FileTreeNodes (that are all DescriptorFileTreeNodes) having the DependencyNodes as their children
*/
@Override
protected List<FileTreeNode> groupDependenciesToDescriptorNodes(Collection<DependencyNode> depScanResults, DepTree depTree, Map<String, Set<String>> parents) {
DescriptorFileTreeNode fileTreeNode = new DescriptorFileTreeNode(descriptorFilePath);
for (DependencyNode dependency : depScanResults) {
boolean directDep = false;
for (String parentId : parents.get(dependency.getComponentIdWithoutPrefix())) {
DepTreeNode parent = depTree.nodes().get(parentId);
if (descriptorFilePath.equals(parent.getDescriptorFilePath())) {
directDep = true;
break;
}
}
dependency.setIndirect(!directDep);
fileTreeNode.addDependency(dependency);
}
return new CopyOnWriteArrayList<>(List.of(fileTreeNode));
}
}
3 changes: 2 additions & 1 deletion src/main/java/com/jfrog/ide/idea/scan/YarnScanner.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import com.jfrog.ide.idea.inspections.AbstractInspection;
import com.jfrog.ide.idea.inspections.YarnInspection;
import com.jfrog.ide.idea.scan.data.PackageManagerType;
import com.jfrog.ide.idea.scan.utils.ImpactTreeBuilder;
import com.jfrog.ide.idea.ui.ComponentsTree;
import com.jfrog.ide.idea.ui.menus.filtermanager.ConsistentFilterManager;
import org.apache.commons.collections4.CollectionUtils;
Expand Down Expand Up @@ -102,7 +103,7 @@ private void buildImpactGraphFromPaths(DescriptorFileTreeNode descriptorNode, Ma

// build the impact graph for each vulnerable dependency out of its impact paths
for (List<String> impactPath : impactPaths) {
this.addImpactPathToDependencyNode(dependencyNode, impactPath);
ImpactTreeBuilder.addImpactPathToDependencyNode(dependencyNode, impactPath);
}

boolean direct = impactPaths.stream().map(List::size).anyMatch(size -> size == 2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import com.fasterxml.jackson.annotation.JsonProperty;
import com.intellij.openapi.project.Project;
import com.jfrog.ide.idea.configuration.GlobalSettings;
import com.jfrog.ide.idea.scan.ScanUtils;
import com.jfrog.ide.idea.scan.utils.ScanUtils;
import lombok.Getter;
import lombok.NoArgsConstructor;

Expand Down
Loading

0 comments on commit 5a6a4a4

Please sign in to comment.