From 58aadcb0ab2b48aea6f66181538652c5992438b9 Mon Sep 17 00:00:00 2001 From: Nikolche Kolev Date: Mon, 11 Sep 2023 17:16:45 -0700 Subject: [PATCH] More cleanup. Minimize the number of methods doing exactly the same thing --- .../PackageDependencyGraphTests.cs | 48 +++++++++++-------- src/Common/PackageDependencyGraph.cs | 41 +--------------- src/Common/VulnerabilityInfoDecorator.cs | 9 ++-- .../PackageDependencyVisualizerToolTests.cs | 12 +++-- src/DependencyVisualizerTool/Program.cs | 18 +++++-- 5 files changed, 55 insertions(+), 73 deletions(-) diff --git a/src/Common/Common.Test/Common.Test/PackageDependencyGraphTests.cs b/src/Common/Common.Test/Common.Test/PackageDependencyGraphTests.cs index ceeb6e9..ad7b3a1 100644 --- a/src/Common/Common.Test/Common.Test/PackageDependencyGraphTests.cs +++ b/src/Common/Common.Test/Common.Test/PackageDependencyGraphTests.cs @@ -13,7 +13,9 @@ public async Task FromAssetsFile_WithLargeGraph_ParsesGraphCorrectly() var assetsFileText = TestHelpers.GetResource("Common.Test.compiler.resources.nuget.common.assets.json", GetType()); var assetsFile = new LockFileFormat().Parse(assetsFileText, Path.GetTempPath()); - var graphs = await PackageDependencyGraph.GenerateAllDependencyGraphsFromAssetsFileAsync(assetsFile, new GraphOptions(generateProjectsOnly: false)); + var dependencyGraphSpec = new DependencyGraphSpec(); + dependencyGraphSpec.AddProject(assetsFile.PackageSpec); + var graphs = await PackageDependencyGraph.GenerateAllDependencyGraphsFromAssetsFileAsync(assetsFile, dependencyGraphSpec, new GraphOptions(generateProjectsOnly: false), new(), CancellationToken.None); graphs.Should().HaveCount(2); var graph = graphs.First().Value; @@ -59,9 +61,9 @@ public async Task FromAssetsFile_WithLargeGraph_ParsesGraphCorrectly() // SingleProjectSingleFramework -> Newtonsoft.Json 13.0.1 [Fact] - public void FromAssetsFile_WithSingleFramework_WithSinglePackageReference_ParsesGraphCorrectly() + public async Task FromAssetsFile_WithSingleFramework_WithSinglePackageReference_ParsesGraphCorrectly() { - var graph = GetOnlyDependencyGraph("Common.Test.compiler.resources.singlepackagereference.assets.json"); + var graph = await GetOnlyDependencyGraphAsync("Common.Test.compiler.resources.singlepackagereference.assets.json"); graph.Node.Identity.Id.Should().Be("SingleProjectSingleFramework"); graph.Node.ParentNodes.Should().HaveCount(0); @@ -75,9 +77,9 @@ public void FromAssetsFile_WithSingleFramework_WithSinglePackageReference_Parses // Parent -> Leaf 3.0.0 [Fact] - public void FromAssetsFile_WithSingleFramework_WithSingleProjectReference_ParsesGraphCorrectly() + public async Task FromAssetsFile_WithSingleFramework_WithSingleProjectReference_ParsesGraphCorrectly() { - var graph = GetOnlyDependencyGraph("Common.Test.compiler.resources.singleprojectreference.assets.json"); + var graph = await GetOnlyDependencyGraphAsync("Common.Test.compiler.resources.singleprojectreference.assets.json"); graph.Node.Identity.Id.Should().Be("Parent"); graph.Node.ParentNodes.Should().HaveCount(0); @@ -93,9 +95,9 @@ public void FromAssetsFile_WithSingleFramework_WithSingleProjectReference_Parses // TransitivePackageReference -> NuGet.Common 6.3.0 -> NuGet.Frameworks 6.3.0 [Fact] - public void FromAssetsFile_WithTransitivePackageReference_ParsesGraphCorrectly() + public async Task FromAssetsFile_WithTransitivePackageReference_ParsesGraphCorrectly() { - var graph = GetOnlyDependencyGraph("Common.Test.compiler.resources.transitivepackagereference.assets.json"); + var graph = await GetOnlyDependencyGraphAsync("Common.Test.compiler.resources.transitivepackagereference.assets.json"); graph.Node.Identity.Id.Should().Be("TransitivePackageReference"); graph.Node.ParentNodes.Should().HaveCount(0); @@ -120,9 +122,9 @@ public void FromAssetsFile_WithTransitivePackageReference_ParsesGraphCorrectly() // ParentProject -> LeafProject 1.0.0 -> NuGet.Versioning 6.3.0 [Fact] - public void FromAssetsFile_WithTransitiveProjectReference_ParsesGraphCorrectly() + public async Task FromAssetsFile_WithTransitiveProjectReference_ParsesGraphCorrectly() { - var graph = GetOnlyDependencyGraph("Common.Test.compiler.resources.transitiveprojectreference.assets.json"); + var graph = await GetOnlyDependencyGraphAsync("Common.Test.compiler.resources.transitiveprojectreference.assets.json"); graph.Node.Identity.Id.Should().Be("ParentProject"); graph.Node.ParentNodes.Should().HaveCount(0); @@ -146,9 +148,9 @@ public void FromAssetsFile_WithTransitiveProjectReference_ParsesGraphCorrectly() // Project -> (>= 6.2.5) NuGet.Common 6.3.0 -> NuGet.Frameworks 6.3.0 [Fact] - public void FromAssetsFile_WithPackageReference_AndMinVersionDoesNotMatchSelectedVersion_ParsesGraphCorrectly() + public async Task FromAssetsFile_WithPackageReference_AndMinVersionDoesNotMatchSelectedVersion_ParsesGraphCorrectly() { - var graph = GetOnlyDependencyGraph("Common.Test.compiler.resources.missingpackageversion.assets.json"); + var graph = await GetOnlyDependencyGraphAsync("Common.Test.compiler.resources.missingpackageversion.assets.json"); graph.Node.Identity.Id.Should().Be("Project"); graph.Node.ParentNodes.Should().HaveCount(0); @@ -172,9 +174,9 @@ public void FromAssetsFile_WithPackageReference_AndMinVersionDoesNotMatchSelecte // Project -> NuGet.Common 6.3.0 -> NuGet.Frameworks 6.3.0 // Project -> Newtonsoft.Json.Bson 1.0.2 -> Newtonsoft.Json 12.0.1 [Fact] - public void FromAssetsFile_WithMultipleDirectAndTransitivePackageReferences_ParsesGraphCorrectly() + public async Task FromAssetsFile_WithMultipleDirectAndTransitivePackageReferences_ParsesGraphCorrectly() { - var graph = GetOnlyDependencyGraph("Common.Test.compiler.resources.multipleversions.assets.json"); + var graph = await GetOnlyDependencyGraphAsync("Common.Test.compiler.resources.multipleversions.assets.json"); graph.Node.Identity.Id.Should().Be("Project"); graph.Node.ParentNodes.Should().HaveCount(0); @@ -212,9 +214,9 @@ public void FromAssetsFile_WithMultipleDirectAndTransitivePackageReferences_Pars // TestProject -> A 1.0.0 -> (>= 1.0.0) C 1.1.0 // TestProject -> B 1.0.0 -> (>= 1.1.0) C 1.1.0 [Fact] - public void FromAssetsFile_WithADiamondDependency_ParsesGraphCorrectly() + public async Task FromAssetsFile_WithADiamondDependency_ParsesGraphCorrectly() { - var graph = GetOnlyDependencyGraph("Common.Test.compiler.resources.diamonddependency.assets.json"); + var graph = await GetOnlyDependencyGraphAsync("Common.Test.compiler.resources.diamonddependency.assets.json"); graph.Node.Identity.Id.Should().Be("TestProject"); graph.Node.ParentNodes.Should().HaveCount(0); @@ -255,9 +257,9 @@ public void FromAssetsFile_WithADiamondDependency_ParsesGraphCorrectly() // TestProject -> A 1.0.0 -> (>= 1.0.0) B 2.0.0 // TestProject -> B 2.0.0 [Fact] - public void FromAssetsFile_WithADiamondDependencyAsDirectReference_ParsesGraphCorrectly() + public async Task FromAssetsFile_WithADiamondDependencyAsDirectReference_ParsesGraphCorrectly() { - var graph = GetOnlyDependencyGraph("Common.Test.compiler.resources.diamonddependencywithtoplevel.assets.json"); + var graph = await GetOnlyDependencyGraphAsync("Common.Test.compiler.resources.diamonddependencywithtoplevel.assets.json"); graph.Node.Identity.Id.Should().Be("TestProject"); graph.Node.ParentNodes.Should().HaveCount(0); @@ -296,7 +298,9 @@ public async Task GenerateAllDependencyGraphsFromAssetsFile_WithMultipleFramewor var assetsFileText = TestHelpers.GetResource("Common.Test.compiler.resources.multitargeted.assets.json", GetType()); var assetsFile = new LockFileFormat().Parse(assetsFileText, Path.GetTempPath()); - Dictionary graphs = await PackageDependencyGraph.GenerateAllDependencyGraphsFromAssetsFileAsync(assetsFile, new GraphOptions(generateProjectsOnly: false)); + var dependencyGraphSpec = new DependencyGraphSpec(); + dependencyGraphSpec.AddProject(assetsFile.PackageSpec); + Dictionary graphs = await PackageDependencyGraph.GenerateAllDependencyGraphsFromAssetsFileAsync(assetsFile, dependencyGraphSpec, new GraphOptions(generateProjectsOnly: false), new(), CancellationToken.None); var net472Graph = graphs["net472"]; net472Graph.Node.Identity.Id.Should().Be("TestProject"); @@ -330,7 +334,7 @@ public async Task FromAssetsFile_WithProjectReferenceWithAPackageId_ParsesGraphC var assetsFile = new LockFileFormat().Parse(assetsFileText, Path.GetTempPath()); var dgSpec = DependencyGraphSpec.Load(tempFile.FilePath); - var graphs = await PackageDependencyGraph.GenerateAllDependencyGraphsFromAssetsFileAsync(assetsFile, dgSpec, new GraphOptions(generateProjectsOnly: false), new(), CancellationToken.None); + Dictionary graphs = await PackageDependencyGraph.GenerateAllDependencyGraphsFromAssetsFileAsync(assetsFile, dgSpec, new GraphOptions(generateProjectsOnly: false), new(), CancellationToken.None); graphs.Should().HaveCount(1); var graph = graphs.Single().Value; @@ -352,12 +356,14 @@ public async Task FromAssetsFile_WithProjectReferenceWithAPackageId_ParsesGraphC ValidateBidirectionalEdges(nephewNode.Item1, nugetVersioning, VersionRange.Parse("6.3.0")); } - private PackageDependencyGraph GetOnlyDependencyGraph(string resourceName) + private async Task GetOnlyDependencyGraphAsync(string resourceName) { var assetsFileText = TestHelpers.GetResource(resourceName, GetType()); var assetsFile = new LockFileFormat().Parse(assetsFileText, Path.GetTempPath()); - var graphs = PackageDependencyGraph.GenerateAllDependencyGraphsFromAssetsFileAsync(assetsFile, new GraphOptions(generateProjectsOnly: false)).Result; + var dependencyGraphSpec = new DependencyGraphSpec(); + dependencyGraphSpec.AddProject(assetsFile.PackageSpec); + Dictionary graphs = await PackageDependencyGraph.GenerateAllDependencyGraphsFromAssetsFileAsync(assetsFile, dependencyGraphSpec, new GraphOptions(generateProjectsOnly: false), new(), CancellationToken.None); graphs.Should().HaveCount(1); var graph = graphs.Single().Value; return graph; diff --git a/src/Common/PackageDependencyGraph.cs b/src/Common/PackageDependencyGraph.cs index 5d6a243..2d2a3ca 100644 --- a/src/Common/PackageDependencyGraph.cs +++ b/src/Common/PackageDependencyGraph.cs @@ -2,7 +2,6 @@ using NuGet.ProjectModel; using NuGet.Versioning; using System.Diagnostics; -using System.Threading; namespace Common { @@ -13,13 +12,6 @@ public PackageDependencyGraph(Node node) : { } - /// - /// Generate a graph given an assets file - /// - /// The assets file must not be null. - /// - /// If the assets file is not valid - /// If the assets file is null public static async Task> GenerateAllDependencyGraphsFromAssetsFileAsync( LockFile assetsFile, DependencyGraphSpec dependencyGraphSpec, @@ -60,37 +52,6 @@ public static async Task> GenerateAll return aliasToDependencyGraph; } - /// - /// Generate a graph given an assets file - /// - /// The assets file must not be null. - /// - /// If the assets file is not valid - /// If the assets file is null - public static async Task> GenerateAllDependencyGraphsFromAssetsFileAsync(LockFile assetsFile, GraphOptions graphOptions) - { - ArgumentNullException.ThrowIfNull(assetsFile); - DependencyNodeIdentity projectIdentity = new(assetsFile.PackageSpec.Name, assetsFile.PackageSpec.Version, DependencyType.Project); - - List frameworks = assetsFile.Targets.Where(e => string.IsNullOrEmpty(e.RuntimeIdentifier)).ToList(); - - if (frameworks.Count == 0) - { - throw new InvalidProgramException("There are no valid frameworks to process in the assets file"); - } - - Dictionary aliasToDependencyGraph = new(); - - foreach (var framework in frameworks) - { - var dependenyGraph = await GenerateGraphForAGivenFramework(projectIdentity, framework, assetsFile.PackageSpec, new(), projectsOnly: false, new(), CancellationToken.None); - var alias = assetsFile.PackageSpec.GetTargetFramework(framework.TargetFramework); - aliasToDependencyGraph.Add(alias.TargetAlias, dependenyGraph); - } - - return aliasToDependencyGraph; - } - private static async Task GenerateGraphForAGivenFramework( DependencyNodeIdentity projectIdentity, LockFileTarget framework, @@ -104,8 +65,8 @@ private static async Task GenerateGraphForAGivenFramewor ArgumentNullException.ThrowIfNull(framework); ArgumentNullException.ThrowIfNull(packageSpec); ArgumentNullException.ThrowIfNull(projectPathToProjectNameMap); - PackageDependencyGraph graph = new(new PackageDependencyNode(projectIdentity)); + PackageDependencyGraph graph = new(new PackageDependencyNode(projectIdentity)); Dictionary packageIdToNode = GenerateNodesForAllPackagesInGraph(framework, projectsOnly); packageIdToNode.Add(graph.Node.Identity.Id, (PackageDependencyNode)graph.Node); diff --git a/src/Common/VulnerabilityInfoDecorator.cs b/src/Common/VulnerabilityInfoDecorator.cs index 4c7842e..f9d6c54 100644 --- a/src/Common/VulnerabilityInfoDecorator.cs +++ b/src/Common/VulnerabilityInfoDecorator.cs @@ -108,7 +108,7 @@ internal static bool IsPackageVulnerable( foreach (SourceRepository source in sourceRepositories) { - Task getVulnerabilityInfoResult = GetVulnerabilityInfoAsync(source, sourceCacheContext); + Task getVulnerabilityInfoResult = GetVulnerabilityInfoAsync(source, sourceCacheContext, cancellationToken); if (getVulnerabilityInfoResult != null) { results.Add(getVulnerabilityInfoResult); @@ -155,15 +155,16 @@ internal static bool IsPackageVulnerable( : null; return final; - static async Task GetVulnerabilityInfoAsync(SourceRepository source, SourceCacheContext cacheContext) + static async Task GetVulnerabilityInfoAsync(SourceRepository source, SourceCacheContext cacheContext, CancellationToken cancellationToken) { + cancellationToken.ThrowIfCancellationRequested(); IVulnerabilityInfoResource vulnerabilityInfoResource = - await source.GetResourceAsync(CancellationToken.None); + await source.GetResourceAsync(cancellationToken); if (vulnerabilityInfoResource is null) { return null; } - return await vulnerabilityInfoResource.GetVulnerabilityInfoAsync(cacheContext, NuGet.Common.NullLogger.Instance, CancellationToken.None); + return await vulnerabilityInfoResource.GetVulnerabilityInfoAsync(cacheContext, NuGet.Common.NullLogger.Instance, cancellationToken); } } } diff --git a/src/DependencyVisualizerTool/DependencyVisualizerTool.Test/PackageDependencyVisualizerToolTests.cs b/src/DependencyVisualizerTool/DependencyVisualizerTool.Test/PackageDependencyVisualizerToolTests.cs index fbdeb39..1fb335b 100644 --- a/src/DependencyVisualizerTool/DependencyVisualizerTool.Test/PackageDependencyVisualizerToolTests.cs +++ b/src/DependencyVisualizerTool/DependencyVisualizerTool.Test/PackageDependencyVisualizerToolTests.cs @@ -62,7 +62,9 @@ public async Task TransGraphToDGMLXDocument_multitargeted_CreateDGMLCorrectly() var assetsFileText = TestHelpers.GetResource("DependencyVisualizerTool.Test.compiler.resources.multitargeted.assets.json", GetType()); var assetsFile = new LockFileFormat().Parse(assetsFileText, Path.GetTempPath()); - var graphs = await PackageDependencyGraph.GenerateAllDependencyGraphsFromAssetsFileAsync(assetsFile, new GraphOptions(generateProjectsOnly: false)); + var dependencyGraphSpec = new DependencyGraphSpec(); + dependencyGraphSpec.AddProject(assetsFile.PackageSpec); + Dictionary graphs = await PackageDependencyGraph.GenerateAllDependencyGraphsFromAssetsFileAsync(assetsFile, dependencyGraphSpec, new GraphOptions(generateProjectsOnly: false), new(), CancellationToken.None); graphs.Should().HaveCount(2); var graph = graphs.First().Value; @@ -79,7 +81,9 @@ public async Task TransGraphToDGMLXDocument_nugetcommon_CreateDGMLCorrectly() var assetsFileText = TestHelpers.GetResource("DependencyVisualizerTool.Test.compiler.resources.nuget.common.assets.json", GetType()); var assetsFile = new LockFileFormat().Parse(assetsFileText, Path.GetTempPath()); - var graphs = await PackageDependencyGraph.GenerateAllDependencyGraphsFromAssetsFileAsync(assetsFile, new GraphOptions(generateProjectsOnly: false)); + var dependencyGraphSpec = new DependencyGraphSpec(); + dependencyGraphSpec.AddProject(assetsFile.PackageSpec); + Dictionary graphs = await PackageDependencyGraph.GenerateAllDependencyGraphsFromAssetsFileAsync(assetsFile, dependencyGraphSpec, new GraphOptions(generateProjectsOnly: false), new(), CancellationToken.None); graphs.Should().HaveCount(2); var graph = graphs.First().Value; @@ -149,7 +153,9 @@ private async Task GetOnlyDependencyGraphAsync(string re var assetsFileText = TestHelpers.GetResource(resourceName, GetType()); var assetsFile = new LockFileFormat().Parse(assetsFileText, Path.GetTempPath()); - var graphs = await PackageDependencyGraph.GenerateAllDependencyGraphsFromAssetsFileAsync(assetsFile, new GraphOptions(generateProjectsOnly: false)); + var dependencyGraphSpec = new DependencyGraphSpec(); + dependencyGraphSpec.AddProject(assetsFile.PackageSpec); + Dictionary graphs = await PackageDependencyGraph.GenerateAllDependencyGraphsFromAssetsFileAsync(assetsFile, dependencyGraphSpec, new GraphOptions(generateProjectsOnly: false), new(), CancellationToken.None); graphs.Should().HaveCount(1); var graph = graphs.Single().Value; return graph; diff --git a/src/DependencyVisualizerTool/Program.cs b/src/DependencyVisualizerTool/Program.cs index 9d25040..f6c2e3a 100644 --- a/src/DependencyVisualizerTool/Program.cs +++ b/src/DependencyVisualizerTool/Program.cs @@ -13,8 +13,11 @@ namespace DependencyVisualizerTool { internal class Program { + private static CancellationTokenSource CancellationTokenSource = new CancellationTokenSource(); + public static int Main(string[] args) { + Console.CancelKeyPress += new ConsoleCancelEventHandler(myHandler); var fileArgument = new Argument( name: "projectFilePath", description: "Project file path.", @@ -54,14 +57,14 @@ public static int Main(string[] args) #if DEBUG System.Diagnostics.Debugger.Launch(); #endif - await GenerateGraph(fileArgument, outputOption, checkVulnerabilityOption, projectsOnly); + await GenerateGraph(fileArgument, outputOption, checkVulnerabilityOption, projectsOnly, CancellationTokenSource.Token); }, fileArgument, outputOption, checkVulnerabilityOption, projectsOnlyOption); return rootCommand.InvokeAsync(args).Result; } - private static async Task GenerateGraph(FileInfo projectFile, string? outputFolder, bool? checkVulnerabilities, bool? projectsOnly) + private static async Task GenerateGraph(FileInfo projectFile, string? outputFolder, bool? checkVulnerabilities, bool? projectsOnly, CancellationToken cancellationToken) { MSBuildLocator.RegisterDefaults(); string projectExtensionsPath = GetMSBuildProjectExtensionsPath(projectFile.FullName); @@ -89,7 +92,7 @@ private static async Task GenerateGraph(FileInfo projectFile, string? outpu dgspecFile, new GraphOptions(projectsOnly == true), decorators, - CancellationToken.None); + cancellationToken); var outputFiles = new List(dictGraph.Count); foreach (var keyValuePair in dictGraph) @@ -99,7 +102,7 @@ private static async Task GenerateGraph(FileInfo projectFile, string? outpu string dgmlFileName = Path.Combine(outputFolder, $"{projectName}_{tfm}.dgml"); try { - DGMLDependencyVisualizerTool.TransGraphToDGMLFile(keyValuePair.Value, dgmlFileName, projectsOnly != true); + DGMLDependencyVisualizerTool.TransGraphToDGMLFile(keyValuePair.Value, dgmlFileName, populateCosts: projectsOnly != true); outputFiles.Add(dgmlFileName); } catch (Exception e) @@ -173,7 +176,6 @@ static List GetHTTPSourceRepositories(PackageSpec projectPacka AppLogger.Logger.LogDebug(e.StackTrace); return null; } - } private static DependencyGraphSpec GetDgspecFilePath(string projectExtensionsPath, FileInfo projectFile) @@ -220,6 +222,12 @@ private static void CreateOutputIfNotExists(string outputFolder) } } + protected static void myHandler(object sender, ConsoleCancelEventArgs args) + { + args.Cancel = true; + CancellationTokenSource.Cancel(); + } + } } \ No newline at end of file