Updating "ComponentFilePaths" list of Yarn's Workspace Dependencies. (#565)

* Adding PackageJson LocationPath In YarnDetector

* Adding new tests for YarnDetector to check workspace filepath addition

* Update YarnLockDetectorTests.cs

Addressing iteration-2 comments, and using FluentAssertions

* Added 'GetLocationMapKey' util & updated YarnLockDetectorTests's FilePath logic to check Package.json entry existence alone.

* Updating YarnDetectorTests to check filepaths count

* Updating YarnLockcomponentDetector version

---------

Co-authored-by: Sindhusha Yadavalli <Sindhusha.Yadavalli@microsoft.com>

Author: siyadava-sindhu

src/Microsoft.ComponentDetection.Common/DependencyGraph/ComponentRecorder.cs

@@ -173,11 +173,6 @@ public void RegisterUsage(
             }
 
 #if DEBUG
-            if (detectedComponent.FilePaths?.Any() ?? false)
-            {
-                this.logger.LogWarning("Detector should not populate DetectedComponent.FilePaths!");
-            }
-
             if (detectedComponent.DependencyRoots?.Any() ?? false)
             {
                 this.logger.LogWarning("Detector should not populate DetectedComponent.DependencyRoots!");

src/Microsoft.ComponentDetection.Contracts/DetectedComponent.cs

@@ -61,7 +61,10 @@ public DetectedComponent(TypedComponent.TypedComponent component, IComponentDete
     private string DebuggerDisplay => $"{this.Component.DebuggerDisplay}";
 
     /// <summary>Adds a filepath to the FilePaths hashset for this detected component.
-    /// Note: Dependency Graph automatically captures the location where a component is found, no need to call it at all inside package manager detectors.</summary>
+    /// Note:
+    /// (1) Dependency Graph automatically captures the location where a component is found, no need to call it at all inside package manager detectors.</summary>
+    /// (2) Only usecase where Detectors allowed to call this API is, in scenarios where "Detectors(eg:Yarn) further process other config files to get workspace dependencies recursively"
+    /// and detectors need to add WorkspaceDependency found path too.
     /// <param name="filePath">The file path to add to the hashset.</param>
     public void AddComponentFilePath(string filePath)
     {

src/Microsoft.ComponentDetection.Detectors/yarn/YarnEntry.cs

@@ -1,4 +1,4 @@
-namespace Microsoft.ComponentDetection.Detectors.Yarn;
+namespace Microsoft.ComponentDetection.Detectors.Yarn;
 using System.Collections.Generic;
 
 public class YarnEntry
@@ -39,4 +39,9 @@ public class YarnEntry
     /// Gets or sets a value indicating whether or not the component is a dev dependency.
     /// </summary>
     public bool DevDependency { get; set; }
+
+    /// <summary>
+    /// Gets or Sets the location for this yarnentry. Often a file path if not in test circumstances.
+    /// </summary>
+    public string Location { get; set; }
 }

src/Microsoft.ComponentDetection.Detectors/yarn/YarnLockComponentDetector.cs

@@ -1,4 +1,4 @@
-namespace Microsoft.ComponentDetection.Detectors.Yarn;
+namespace Microsoft.ComponentDetection.Detectors.Yarn;
 
 using System;
 using System.Collections.Generic;
@@ -35,7 +35,7 @@ public YarnLockComponentDetector(
 
     public override IEnumerable<ComponentType> SupportedComponentTypes { get; } = new[] { ComponentType.Npm };
 
-    public override int Version => 6;
+    public override int Version => 7;
 
     public override IEnumerable<string> Categories => new[] { Enum.GetName(typeof(DetectorClass), DetectorClass.Npm) };
 
@@ -98,6 +98,12 @@ private void DetectComponents(YarnLockFile file, string location, ISingleFileCom
         foreach (var dependency in yarnRoots)
         {
             var root = new DetectedComponent(new NpmComponent(dependency.Name, dependency.Version));
+
+            if (!string.IsNullOrWhiteSpace(dependency.Location))
+            {
+                root.AddComponentFilePath(dependency.Location);
+            }
+
             this.AddDetectedComponentToGraph(root, null, singleFileComponentRecorder, isRootComponent: true);
         }
 
@@ -207,9 +213,10 @@ private bool TryReadPeerPackageJsonRequestsAsYarnEntries(ISingleFileComponentRec
             return false;
         }
 
+        var workspaceDependencyVsLocationMap = new Dictionary<string, string>();
         if (yarnWorkspaces.Count > 0)
         {
-            this.GetWorkspaceDependencies(yarnWorkspaces, new FileInfo(location).Directory, combinedDependencies);
+            this.GetWorkspaceDependencies(yarnWorkspaces, new FileInfo(location).Directory, combinedDependencies, workspaceDependencyVsLocationMap);
         }
 
         // Convert all of the dependencies we retrieved from package.json
@@ -232,13 +239,19 @@ private bool TryReadPeerPackageJsonRequestsAsYarnEntries(ISingleFileComponentRec
                 entry.DevDependency = version.Value;
 
                 yarnRoots.Add(entry);
+
+                var locationMapDictonaryKey = this.GetLocationMapKey(name, version.Key);
+                if (workspaceDependencyVsLocationMap.ContainsKey(locationMapDictonaryKey))
+                {
+                    entry.Location = workspaceDependencyVsLocationMap[locationMapDictonaryKey];
+                }
             }
         }
 
         return true;
     }
 
-    private void GetWorkspaceDependencies(IList<string> yarnWorkspaces, DirectoryInfo root, IDictionary<string, IDictionary<string, bool>> dependencies)
+    private void GetWorkspaceDependencies(IList<string> yarnWorkspaces, DirectoryInfo root, IDictionary<string, IDictionary<string, bool>> dependencies, IDictionary<string, string> workspaceDependencyVsLocationMap)
     {
         var ignoreCase = RuntimeInformation.IsOSPlatform(OSPlatform.Windows);
 
@@ -263,31 +276,61 @@ private void GetWorkspaceDependencies(IList<string> yarnWorkspaces, DirectoryInf
 
                 foreach (var dependency in combinedDependencies)
                 {
-                    this.ProcessWorkspaceDependency(dependencies, dependency);
+                    this.ProcessWorkspaceDependency(dependencies, dependency, workspaceDependencyVsLocationMap, stream.Location);
                 }
             }
         }
     }
 
-    private void ProcessWorkspaceDependency(IDictionary<string, IDictionary<string, bool>> dependencies, KeyValuePair<string, IDictionary<string, bool>> newDependency)
+    private void ProcessWorkspaceDependency(IDictionary<string, IDictionary<string, bool>> dependencies, KeyValuePair<string, IDictionary<string, bool>> newDependency, IDictionary<string, string> workspaceDependencyVsLocationMap, string streamLocation)
     {
-        if (!dependencies.TryGetValue(newDependency.Key, out var existingDependency))
-        {
-            dependencies.Add(newDependency.Key, newDependency.Value);
-            return;
-        }
-
-        foreach (var item in newDependency.Value)
+        try
         {
-            if (existingDependency.TryGetValue(item.Key, out var wasDev))
+            if (!dependencies.TryGetValue(newDependency.Key, out var existingDependency))
             {
-                existingDependency[item.Key] = wasDev && item.Value;
+                dependencies.Add(newDependency.Key, newDependency.Value);
+                foreach (var item in newDependency.Value)
+                {
+                    // Adding 'Package.json stream's location'(in which workspacedependency of Yarn.lock file was found) as location of respective WorkSpaceDependency.
+                    this.AddLocationInfoToWorkspaceDependency(workspaceDependencyVsLocationMap, streamLocation, newDependency.Key, item.Key);
+                }
+
+                return;
             }
-            else
+
+            foreach (var item in newDependency.Value)
             {
-                existingDependency[item.Key] = item.Value;
+                if (existingDependency.TryGetValue(item.Key, out var wasDev))
+                {
+                    existingDependency[item.Key] = wasDev && item.Value;
+                }
+                else
+                {
+                    existingDependency[item.Key] = item.Value;
+                }
+
+                // Adding 'Package.json stream's location'(in which workspacedependency of Yarn.lock file was found) as location of respective WorkSpaceDependency.
+                this.AddLocationInfoToWorkspaceDependency(workspaceDependencyVsLocationMap, streamLocation, newDependency.Key, item.Key);
             }
         }
+        catch (Exception ex)
+        {
+            this.Logger.LogError(ex, "Could not process workspace dependency from file {PackageJsonStreamLocation}.", streamLocation);
+        }
+    }
+
+    private void AddLocationInfoToWorkspaceDependency(IDictionary<string, string> workspaceDependencyVsLocationMap, string streamLocation, string dependencyName, string dependencyVersion)
+    {
+        var locationMapDictionaryKey = this.GetLocationMapKey(dependencyName, dependencyVersion);
+        if (!workspaceDependencyVsLocationMap.ContainsKey(locationMapDictionaryKey))
+        {
+            workspaceDependencyVsLocationMap[locationMapDictionaryKey] = streamLocation;
+        }
+    }
+
+    private string GetLocationMapKey(string dependencyName, string dependencyVersion)
+    {
+        return $"{dependencyName}-{dependencyVersion}";
     }
 
     private void AddDetectedComponentToGraph(DetectedComponent componentToAdd, DetectedComponent parentComponent, ISingleFileComponentRecorder singleFileComponentRecorder, bool isRootComponent = false, bool? isDevDependency = null)

test/Microsoft.ComponentDetection.Detectors.Tests/YarnLockDetectorTests.cs

@@ -240,6 +240,52 @@ public async Task WellFormedYarnLockV1WithWorkspace_FindsComponentAsync()
             parentComponent => parentComponent.Name == componentA.Name && parentComponent.Version == version0);
     }
 
+    [TestMethod]
+    public async Task WellFormedYarnLockV1WithWorkspace_CheckFilePathsAsync()
+    {
+        var directory = new DirectoryInfo(Path.GetTempPath());
+
+        var version0 = NewRandomVersion();
+        var componentA = new YarnTestComponentDefinition
+        {
+            ActualVersion = version0,
+            RequestedVersion = $"^{version0}",
+            ResolvedVersion = "https://resolved0/a/resolved",
+            Name = Guid.NewGuid().ToString("N"),
+        };
+
+        var componentStream = YarnTestUtilities.GetMockedYarnLockStream("yarn.lock", this.CreateYarnLockV1FileContent(new List<YarnTestComponentDefinition> { componentA }));
+
+        var workspaceJson = new
+        {
+            name = "testworkspace",
+            version = "1.0.0",
+            @private = true,
+            workspaces = new[] { "workspace" },
+        };
+        var str = JsonConvert.SerializeObject(workspaceJson);
+        var workspaceJsonComponentStream = new ComponentStream { Location = directory.ToString(), Pattern = "package.json", Stream = str.ToStream() };
+
+        var packageStream = NpmTestUtilities.GetPackageJsonOneRootComponentStream(componentA.Name, componentA.RequestedVersion);
+
+        var (scanResult, componentRecorder) = await this.DetectorTestUtility
+            .WithFile("yarn.lock", componentStream.Stream)
+            .WithFile("package.json", workspaceJsonComponentStream.Stream, new[] { "package.json" }, Path.Combine(Path.GetTempPath(), "package.json"))
+            .WithFile("package.json", packageStream.Stream, new[] { "package.json" }, Path.Combine(Path.GetTempPath(), "workspace", "package.json"))
+            .ExecuteDetectorAsync();
+
+        scanResult.ResultCode.Should().Be(ProcessingResultCode.Success);
+
+        var detectedComponents = componentRecorder.GetDetectedComponents();
+        detectedComponents.Should().HaveCount(1);
+
+        // checking if workspace's "package.json FilePath entry" is added or not.
+        var detectedFilePaths = detectedComponents.First().FilePaths;
+        detectedFilePaths.Should().HaveCount(1);
+        var expectedWorkSpacePackageJsonPath = Path.Combine(Path.GetTempPath(), "workspace", "package.json");
+        detectedComponents.First().FilePaths.Contains(expectedWorkSpacePackageJsonPath).Should().Be(true);
+    }
+
     [TestMethod]
     public async Task WellFormedYarnLockV2WithWorkspace_FindsComponentAsync()
     {

test/Microsoft.ComponentDetection.TestsUtilities/DetectorTestUtilityBuilder.cs

@@ -1,5 +1,6 @@
 namespace Microsoft.ComponentDetection.TestsUtilities;
 
+using System;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
@@ -200,6 +201,26 @@ private void InitializeFileMocks()
                                     fileToSend.Location,
                                     fileToSend.Contents)).Select(pr => pr.ComponentStream);
                     });
+
+            this.mockComponentStreamEnumerableFactory.Setup(x =>
+                    x.GetComponentStreams(
+                        It.IsAny<DirectoryInfo>(),
+                        It.IsAny<Func<FileInfo, bool>>(),
+                        It.IsAny<ExcludeDirectoryPredicate>(),
+                        It.IsAny<bool>()))
+                .Returns<DirectoryInfo, Func<FileInfo, bool>, ExcludeDirectoryPredicate, bool>(
+                    (directoryInfo, fileMatchingPredicate, _, recurse) =>
+                    {
+                        return filesToSend
+                            .Where(fileToSend => fileMatchingPredicate(new FileInfo(fileToSend.Location)))
+                            .Select(fileToSend =>
+                                this.CreateProcessRequest(
+                                    FindMatchingPattern(
+                                        fileToSend.Name,
+                                        searchPatterns),
+                                    fileToSend.Location,
+                                    fileToSend.Contents)).Select(pr => pr.ComponentStream);
+                    });
         }
     }
 }