Yarn - Refactor the process of building impact graph

build.gradle

@@ -49,7 +49,7 @@ repositories {
 }
 
 def buildInfoVersion = '2.41.6'
-def idePluginsCommonVersion = '2.3.1'
+def idePluginsCommonVersion = '2.3.2'
 
 dependencies {
     implementation group: 'com.fasterxml.jackson.dataformat', name: 'jackson-dataformat-yaml', version: '2.15.2'

src/main/java/com/jfrog/ide/idea/scan/ScannerBase.java

@@ -61,8 +61,6 @@
  * Created by romang on 4/26/17.
  */
 public abstract class ScannerBase {
-    public static final int IMPACT_PATHS_LIMIT = 50;
-
     private final ServerConfig serverConfig;
     private final ComponentPrefix prefix;
     @Getter
@@ -159,7 +157,7 @@ private void scanAndUpdate(ProgressIndicator indicator) {
                 // No violations/vulnerabilities or no components to scan or an error was thrown
                 return;
             }
-            List<FileTreeNode> fileTreeNodes = walkDepTree(results, depTree);
+            List<FileTreeNode> fileTreeNodes = buildImpactGraph(results, depTree);
             addScanResults(fileTreeNodes);
 
             // Contextual Analysis
@@ -185,9 +183,9 @@ private void scanAndUpdate(ProgressIndicator indicator) {
      * @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.
      */
-    private List<FileTreeNode> walkDepTree(Map<String, DependencyNode> vulnerableDependencies, DepTree depTree) {
+    protected List<FileTreeNode> buildImpactGraph(Map<String, DependencyNode> vulnerableDependencies, DepTree depTree) throws IOException {
         Map<String, DescriptorFileTreeNode> descriptorNodes = new HashMap<>();
-        visitDepTreeNode(vulnerableDependencies, depTree, Collections.singletonList(depTree.getRootId()), descriptorNodes, new ArrayList<>(), new HashMap<>());
+        visitDepTreeNode(vulnerableDependencies, depTree, Collections.singletonList(depTree.rootId()), descriptorNodes, new ArrayList<>(), new HashMap<>());
         return new CopyOnWriteArrayList<>(descriptorNodes.values());
     }
 
@@ -207,7 +205,7 @@ private void visitDepTreeNode(Map<String, DependencyNode> vulnerableDependencies
                                   Map<String, DescriptorFileTreeNode> descriptorNodes, List<String> descriptorPaths,
                                   Map<String, Map<String, DependencyNode>> addedDeps) {
         String compId = path.get(path.size() - 1);
-        DepTreeNode compNode = depTree.getNodes().get(compId);
+        DepTreeNode compNode = depTree.nodes().get(compId);
         List<String> innerDescriptorPaths = descriptorPaths;
         if (compNode.getDescriptorFilePath() != null) {
             innerDescriptorPaths = new ArrayList<>(descriptorPaths);
@@ -220,7 +218,7 @@ private void visitDepTreeNode(Map<String, DependencyNode> vulnerableDependencies
             DepTreeNode parentCompNode = null;
             if (path.size() >= 2) {
                 String parentCompId = path.get(path.size() - 2);
-                parentCompNode = depTree.getNodes().get(parentCompId);
+                parentCompNode = depTree.nodes().get(parentCompId);
             }
             for (String descriptorPath : innerDescriptorPaths) {
                 boolean indirect = parentCompNode != null && !descriptorPath.equals(parentCompNode.getDescriptorFilePath());
@@ -255,13 +253,10 @@ private void visitDepTreeNode(Map<String, DependencyNode> vulnerableDependencies
         }
     }
 
-    private void addImpactPathToDependencyNode(DependencyNode dependencyNode, List<String> path) {
-        if (dependencyNode.getImpactTree() == null) {
-            dependencyNode.setImpactTree(new ImpactTree(new ImpactTreeNode(path.get(0))));
-        }
+    protected void addImpactPathToDependencyNode(DependencyNode dependencyNode, List<String> path) {
+        dependencyNode.setImpactTree(new ImpactTree(new ImpactTreeNode(path.get(0))));
         ImpactTree impactTree = dependencyNode.getImpactTree();
-        impactTree.incImpactPathsCount();
-        if (impactTree.getImpactPathsCount() > IMPACT_PATHS_LIMIT) {
+        if (impactTree.getImpactPathsCount() == ImpactTree.IMPACT_PATHS_LIMIT) {
             return;
         }
         ImpactTreeNode parentImpactTreeNode = impactTree.getRoot();
@@ -272,6 +267,10 @@ private void addImpactPathToDependencyNode(DependencyNode dependencyNode, List<S
             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();
+                }
             }
             parentImpactTreeNode = currImpactTreeNode;
         }

src/main/java/com/jfrog/ide/idea/scan/YarnScanner.java

@@ -7,6 +7,9 @@
 import com.intellij.psi.PsiManager;
 import com.intellij.util.EnvironmentUtil;
 import com.jfrog.ide.common.deptree.DepTree;
+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.common.yarn.YarnTreeBuilder;
@@ -15,9 +18,13 @@
 import com.jfrog.ide.idea.scan.data.PackageManagerType;
 import com.jfrog.ide.idea.ui.ComponentsTree;
 import com.jfrog.ide.idea.ui.menus.filtermanager.ConsistentFilterManager;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
 
 import java.io.IOException;
 import java.nio.file.Paths;
+import java.util.*;
+import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.ExecutorService;
 
 /**
@@ -26,6 +33,7 @@
 public class YarnScanner extends SingleDescriptorScanner {
 
     private final YarnTreeBuilder yarnTreeBuilder;
+
     /**
      * @param project   currently opened IntelliJ project. We'll use this project to retrieve project based services
      *                  like {@link ConsistentFilterManager} and {@link ComponentsTree}.
@@ -36,12 +44,12 @@ public class YarnScanner extends SingleDescriptorScanner {
     YarnScanner(Project project, String basePath, ExecutorService executor, ScanLogic scanLogic) {
         super(project, basePath, ComponentPrefix.NPM, executor, Paths.get(basePath, "package.json").toString(), scanLogic);
         getLog().info("Found yarn project: " + getProjectPath());
-        yarnTreeBuilder = new YarnTreeBuilder(Paths.get(basePath), descriptorFilePath, EnvironmentUtil.getEnvironmentMap());
+        yarnTreeBuilder = new YarnTreeBuilder(Paths.get(basePath), descriptorFilePath, EnvironmentUtil.getEnvironmentMap(), getLog());
     }
 
     @Override
     protected DepTree buildTree() throws IOException {
-        return yarnTreeBuilder.buildTree(getLog());
+        return yarnTreeBuilder.buildTree();
     }
 
     @Override
@@ -63,5 +71,72 @@ protected AbstractInspection getInspectionTool() {
     protected PackageManagerType getPackageManagerType() {
         return PackageManagerType.YARN;
     }
+
+    /**
+     * Builds a map of package name to versions out of a set of <package-name>:<version> Strings.
+     *
+     * @param packages - A set of packages in the format of 'package-name:version'.
+     * @return - A map of package name to a set of versions.
+     */
+    Map<String, Set<String>> getPackageNameToVersionsMap(Set<String> packages) {
+        Map<String, Set<String>> packageNameToVersions = new HashMap<>();
+        for (String fullNamePackage : CollectionUtils.emptyIfNull(packages)) {
+            String[] packageSplit = StringUtils.split(fullNamePackage, ":");
+            if (packageSplit.length != 2) {
+                this.getLog().error("Illegal package name: " + fullNamePackage + ". Skipping package, the dependency tree may be incomplete.");
+                continue;
+            }
+            String packageName = packageSplit[0];
+            String packageVersion = packageSplit[1];
+            packageNameToVersions.putIfAbsent(packageName, new HashSet<>());
+            packageNameToVersions.get(packageName).add(packageVersion);
+        }
+        return packageNameToVersions;
+    }
+
+    private void buildImpactGraphFromPaths(DescriptorFileTreeNode descriptorNode, Map<String, DependencyNode> vulnerableDependencies, Map<String, List<List<String>>> packageVersionsImpactPaths) {
+        for (Map.Entry<String, List<List<String>>> aPackageVersionImpactPaths : packageVersionsImpactPaths.entrySet()) {
+            String packageFullName = aPackageVersionImpactPaths.getKey();
+            List<List<String>> impactPaths = aPackageVersionImpactPaths.getValue();
+            DependencyNode dependencyNode = vulnerableDependencies.get(packageFullName);
+
+            // build the impact graph for each vulnerable dependency out of its impact paths
+            for (List<String> impactPath : impactPaths) {
+                this.addImpactPathToDependencyNode(dependencyNode, impactPath);
+            }
+
+            boolean direct = impactPaths.stream().map(List::size).anyMatch(size -> size == 2);
+
+            dependencyNode.setIndirect(!direct);
+            descriptorNode.addDependency(dependencyNode);
+        }
+    }
+
+    /**
+     * Builds the impact graph for each given vulnerable dependencies.
+     * The impact graph is built by running 'yarn why <package>' command, making it different from other package managers.
+     *
+     * @param vulnerableDependencies - The vulnerable dependencies to build the impact graph for.
+     *                               The key is the package name and version, and the value is the dependency node.
+     * @param depTree                - The whole dependency tree (not just vulnerable dependencies) that was generated earlier.
+     * @return - The impact graph attached to package.json DescriptorFileTreeNode
+     */
+    @Override
+    protected List<FileTreeNode> buildImpactGraph(Map<String, DependencyNode> vulnerableDependencies, DepTree depTree) throws IOException {
+        DescriptorFileTreeNode descriptorNode = new DescriptorFileTreeNode(depTree.getRootNodeDescriptorFilePath());
+        // Build a map of package name to versions, to avoid running 'yarn why' multiple times for the same package.
+        Map<String, Set<String>> packageNameToVersions = this.getPackageNameToVersionsMap(vulnerableDependencies.keySet());
+
+        for (Map.Entry<String, Set<String>> entry : packageNameToVersions.entrySet()) {
+            // Find the impact paths for each package for all its vulnerable versions
+            Map<String, List<List<String>>> packageVersionsImpactPaths = yarnTreeBuilder.findDependencyImpactPaths(depTree.rootId(), entry.getKey(), entry.getValue());
+            // Build the impact graph for each vulnerable dependency out of its impact paths, set Indirect flag and add it to the descriptor node
+            buildImpactGraphFromPaths(descriptorNode, vulnerableDependencies, packageVersionsImpactPaths);
+        }
+
+        // Return a list of one element - the descriptor node for package.json
+        // COW list is used to avoid ConcurrentModificationException in SourceCodeScannerManager
+        return new CopyOnWriteArrayList<>(Collections.singletonList(descriptorNode));
+    }
 }
 

src/main/java/com/jfrog/ide/idea/ui/webview/WebviewObjectConverter.java

@@ -3,7 +3,6 @@
 import com.jfrog.ide.common.nodes.*;
 import com.jfrog.ide.common.nodes.subentities.*;
 import com.jfrog.ide.common.scan.ComponentPrefix;
-import com.jfrog.ide.idea.scan.ScannerBase;
 import com.jfrog.ide.idea.ui.webview.model.Cve;
 import com.jfrog.ide.idea.ui.webview.model.Evidence;
 import com.jfrog.ide.idea.ui.webview.model.License;
@@ -143,7 +142,7 @@ public static DependencyPage convertLicenseToDepPage(LicenseViolationNode licens
     }
 
     private static ImpactGraph convertImpactGraph(ImpactTree impactTree) {
-        return new ImpactGraph(convertImpactGraphNode(impactTree.getRoot()), impactTree.getImpactPathsCount(), ScannerBase.IMPACT_PATHS_LIMIT);
+        return new ImpactGraph(convertImpactGraphNode(impactTree.getRoot()), impactTree.getImpactPathsCount(), ImpactTree.IMPACT_PATHS_LIMIT);
     }
 
     private static ImpactGraphNode convertImpactGraphNode(ImpactTreeNode impactTreeNode) {

src/test/java/com/jfrog/ide/idea/TestUtils.java

@@ -30,7 +30,7 @@ public static PsiElement getNonLeafElement(PsiFile fileDescriptor, Class<? exten
      */
     public static DepTreeNode getAndAssertChild(DepTree depTree, DepTreeNode parent, String childName) {
         Assert.assertTrue(parent.getChildren().contains(childName));
-        DepTreeNode childNode = depTree.getNodes().get(childName);
+        DepTreeNode childNode = depTree.nodes().get(childName);
         Assert.assertNotNull("Couldn't find node '" + childName + "'.", childNode);
         return childNode;
     }

src/test/java/com/jfrog/ide/idea/scan/GradleScannerTest.java

@@ -83,7 +83,7 @@ public void testBuildTree() throws IOException {
         // Run and check scan results
         DepTree results = gradleScanner.buildTree();
         assertNotNull(results);
-        assertEquals(Paths.get(globalProjectDir).getFileName().toString(), results.getRootId());
+        assertEquals(Paths.get(globalProjectDir).getFileName().toString(), results.rootId());
         assertEquals(3, results.getRootNode().getChildren().size());
 
         // Check module dependency

src/test/java/com/jfrog/ide/idea/scan/PypiScannerTest.java

@@ -112,9 +112,9 @@ public void testBuildTree() throws IOException {
 
         // Check root SDK node
         DepTree results = pypiScanner.buildTree();
-        assertEquals(SDK_NAME, results.getRootId());
-        assertEquals(SDK_NAME, results.getRootId());
-        assertEquals(pythonSdk.getHomePath(), results.getRootNode().getDescriptorFilePath());
+        assertEquals(SDK_NAME, results.rootId());
+        assertEquals(SDK_NAME, results.rootId());
+        assertEquals(pythonSdk.getHomePath(), results.getRootNodeDescriptorFilePath());
         assertNotEmpty(results.getRootNode().getChildren());
 
         // Check direct dependency
@@ -124,7 +124,7 @@ public void testBuildTree() throws IOException {
 
         // Check transitive dependency
         String anyTreeDepId = pipGrip.getChildren().stream().filter(childId -> childId.startsWith(TRANSITIVE_DEPENDENCY_NAME + ":")).findFirst().get();
-        DepTreeNode anyTreeDepNode = results.getNodes().get(anyTreeDepId);
+        DepTreeNode anyTreeDepNode = results.nodes().get(anyTreeDepId);
         assertNotNull("Couldn't find node '" + anyTreeDepId + "'.", anyTreeDepNode);
         assertSize(1, anyTreeDepNode.getChildren());
     }
@@ -136,8 +136,8 @@ public void testBuildTreeCircularDependency() throws IOException {
 
             // Check root SDK node
             DepTree results = pypiScanner.buildTree();
-            assertEquals(SDK_NAME, results.getRootId());
-            assertEquals(pythonSdk.getHomePath(), results.getRootNode().getDescriptorFilePath());
+            assertEquals(SDK_NAME, results.rootId());
+            assertEquals(pythonSdk.getHomePath(), results.getRootNodeDescriptorFilePath());
             assertNotEmpty(results.getRootNode().getChildren());
 
             // The expected tree: root -> a-> b -> a