Skip to content

Commit

Permalink
More cleanup. Minimize the number of methods doing exactly the same t…
Browse files Browse the repository at this point in the history
…hing
  • Loading branch information
nkolev92 committed Sep 12, 2023
1 parent 0612ae9 commit 58aadcb
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 73 deletions.
48 changes: 27 additions & 21 deletions src/Common/Common.Test/Common.Test/PackageDependencyGraphTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<string, PackageDependencyGraph> graphs = await PackageDependencyGraph.GenerateAllDependencyGraphsFromAssetsFileAsync(assetsFile, new GraphOptions(generateProjectsOnly: false));
var dependencyGraphSpec = new DependencyGraphSpec();
dependencyGraphSpec.AddProject(assetsFile.PackageSpec);
Dictionary<string, PackageDependencyGraph> graphs = await PackageDependencyGraph.GenerateAllDependencyGraphsFromAssetsFileAsync(assetsFile, dependencyGraphSpec, new GraphOptions(generateProjectsOnly: false), new(), CancellationToken.None);

var net472Graph = graphs["net472"];
net472Graph.Node.Identity.Id.Should().Be("TestProject");
Expand Down Expand Up @@ -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<string, PackageDependencyGraph> graphs = await PackageDependencyGraph.GenerateAllDependencyGraphsFromAssetsFileAsync(assetsFile, dgSpec, new GraphOptions(generateProjectsOnly: false), new(), CancellationToken.None);
graphs.Should().HaveCount(1);
var graph = graphs.Single().Value;

Expand All @@ -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<PackageDependencyGraph> 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<string, PackageDependencyGraph> graphs = await PackageDependencyGraph.GenerateAllDependencyGraphsFromAssetsFileAsync(assetsFile, dependencyGraphSpec, new GraphOptions(generateProjectsOnly: false), new(), CancellationToken.None);
graphs.Should().HaveCount(1);
var graph = graphs.Single().Value;
return graph;
Expand Down
41 changes: 1 addition & 40 deletions src/Common/PackageDependencyGraph.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
using NuGet.ProjectModel;
using NuGet.Versioning;
using System.Diagnostics;
using System.Threading;

namespace Common
{
Expand All @@ -13,13 +12,6 @@ public PackageDependencyGraph(Node<DependencyNodeIdentity, VersionRange> node) :
{
}

/// <summary>
/// Generate a graph given an assets file
/// </summary>
/// <param name="assetsFile">The assets file must not be null.</param>
/// <returns></returns>
/// <exception cref="InvalidOperationException">If the assets file is not valid</exception>
/// <exception cref="ArgumentNullException">If the assets file is null</exception>
public static async Task<Dictionary<string, PackageDependencyGraph>> GenerateAllDependencyGraphsFromAssetsFileAsync(
LockFile assetsFile,
DependencyGraphSpec dependencyGraphSpec,
Expand Down Expand Up @@ -60,37 +52,6 @@ public static async Task<Dictionary<string, PackageDependencyGraph>> GenerateAll
return aliasToDependencyGraph;
}

/// <summary>
/// Generate a graph given an assets file
/// </summary>
/// <param name="assetsFile">The assets file must not be null.</param>
/// <returns></returns>
/// <exception cref="InvalidOperationException">If the assets file is not valid</exception>
/// <exception cref="ArgumentNullException">If the assets file is null</exception>
public static async Task<Dictionary<string, PackageDependencyGraph>> GenerateAllDependencyGraphsFromAssetsFileAsync(LockFile assetsFile, GraphOptions graphOptions)
{
ArgumentNullException.ThrowIfNull(assetsFile);
DependencyNodeIdentity projectIdentity = new(assetsFile.PackageSpec.Name, assetsFile.PackageSpec.Version, DependencyType.Project);

List<LockFileTarget> 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<string, PackageDependencyGraph> 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<PackageDependencyGraph> GenerateGraphForAGivenFramework(
DependencyNodeIdentity projectIdentity,
LockFileTarget framework,
Expand All @@ -104,8 +65,8 @@ private static async Task<PackageDependencyGraph> GenerateGraphForAGivenFramewor
ArgumentNullException.ThrowIfNull(framework);
ArgumentNullException.ThrowIfNull(packageSpec);
ArgumentNullException.ThrowIfNull(projectPathToProjectNameMap);
PackageDependencyGraph graph = new(new PackageDependencyNode(projectIdentity));

PackageDependencyGraph graph = new(new PackageDependencyNode(projectIdentity));
Dictionary<string, PackageDependencyNode> packageIdToNode = GenerateNodesForAllPackagesInGraph(framework, projectsOnly);

packageIdToNode.Add(graph.Node.Identity.Id, (PackageDependencyNode)graph.Node);
Expand Down
9 changes: 5 additions & 4 deletions src/Common/VulnerabilityInfoDecorator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ internal static bool IsPackageVulnerable(

foreach (SourceRepository source in sourceRepositories)
{
Task<GetVulnerabilityInfoResult?> getVulnerabilityInfoResult = GetVulnerabilityInfoAsync(source, sourceCacheContext);
Task<GetVulnerabilityInfoResult?> getVulnerabilityInfoResult = GetVulnerabilityInfoAsync(source, sourceCacheContext, cancellationToken);
if (getVulnerabilityInfoResult != null)
{
results.Add(getVulnerabilityInfoResult);
Expand Down Expand Up @@ -155,15 +155,16 @@ internal static bool IsPackageVulnerable(
: null;
return final;

static async Task<GetVulnerabilityInfoResult?> GetVulnerabilityInfoAsync(SourceRepository source, SourceCacheContext cacheContext)
static async Task<GetVulnerabilityInfoResult?> GetVulnerabilityInfoAsync(SourceRepository source, SourceCacheContext cacheContext, CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();
IVulnerabilityInfoResource vulnerabilityInfoResource =
await source.GetResourceAsync<IVulnerabilityInfoResource>(CancellationToken.None);
await source.GetResourceAsync<IVulnerabilityInfoResource>(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);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, PackageDependencyGraph> graphs = await PackageDependencyGraph.GenerateAllDependencyGraphsFromAssetsFileAsync(assetsFile, dependencyGraphSpec, new GraphOptions(generateProjectsOnly: false), new(), CancellationToken.None);
graphs.Should().HaveCount(2);
var graph = graphs.First().Value;

Expand All @@ -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<string, PackageDependencyGraph> graphs = await PackageDependencyGraph.GenerateAllDependencyGraphsFromAssetsFileAsync(assetsFile, dependencyGraphSpec, new GraphOptions(generateProjectsOnly: false), new(), CancellationToken.None);
graphs.Should().HaveCount(2);
var graph = graphs.First().Value;

Expand Down Expand Up @@ -149,7 +153,9 @@ private async Task<PackageDependencyGraph> 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<string, PackageDependencyGraph> graphs = await PackageDependencyGraph.GenerateAllDependencyGraphsFromAssetsFileAsync(assetsFile, dependencyGraphSpec, new GraphOptions(generateProjectsOnly: false), new(), CancellationToken.None);
graphs.Should().HaveCount(1);
var graph = graphs.Single().Value;
return graph;
Expand Down
Loading

0 comments on commit 58aadcb

Please sign in to comment.