From d291ac6add4578b7872549c518fc5d18eb391569 Mon Sep 17 00:00:00 2001 From: Jean-Pierre Briede Date: Wed, 28 Jul 2021 16:52:09 -0700 Subject: [PATCH 1/4] Sort package vulnerability advisories by severity in descending order --- .../Models/DetailControlModel.cs | 17 +++-------------- .../ViewModels/PackageItemViewModel.cs | 2 +- .../PackageSearchMetadataContextInfo.cs | 2 +- .../Models/V3DetailControlModelTests.cs | 14 ++++++++++++++ 4 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/NuGet.Clients/NuGet.PackageManagement.UI/Models/DetailControlModel.cs b/src/NuGet.Clients/NuGet.PackageManagement.UI/Models/DetailControlModel.cs index 9957dcd1d8b..8f47e5d23a6 100644 --- a/src/NuGet.Clients/NuGet.PackageManagement.UI/Models/DetailControlModel.cs +++ b/src/NuGet.Clients/NuGet.PackageManagement.UI/Models/DetailControlModel.cs @@ -15,6 +15,7 @@ using NuGet.VisualStudio; using NuGet.VisualStudio.Internal.Contracts; using NuGet.VisualStudio.Telemetry; +using static Microsoft.TeamFoundation.Client.CommandLine.Options; using Task = System.Threading.Tasks.Task; namespace NuGet.PackageManagement.UI @@ -415,7 +416,6 @@ public IReadOnlyCollection PackageVulne private set { _packageVulnerabilities = value; - PackageVulnerabilityMaxSeverity = value?.Max(v => v.Severity) ?? -1; OnPropertyChanged(nameof(PackageVulnerabilities)); OnPropertyChanged(nameof(IsPackageVulnerable)); @@ -423,19 +423,9 @@ private set } } - private int _packageVulnerabilityMaxSeverity = -1; public int PackageVulnerabilityMaxSeverity { - get => _packageVulnerabilityMaxSeverity; - private set - { - if (_packageVulnerabilityMaxSeverity != value) - { - _packageVulnerabilityMaxSeverity = value; - - OnPropertyChanged(nameof(PackageVulnerabilityMaxSeverity)); - } - } + get => PackageVulnerabilities?.FirstOrDefault()?.Severity ?? -1; } public bool IsPackageVulnerable @@ -509,8 +499,7 @@ public DetailedPackageMetadata PackageMetadata PackageDeprecationReasons = newDeprecationReasons; PackageDeprecationAlternatePackageText = newAlternatePackageText; - IEnumerable vulnerabilities = _packageMetadata?.Vulnerabilities; - PackageVulnerabilities = vulnerabilities?.ToList(); + PackageVulnerabilities = _packageMetadata?.Vulnerabilities?.ToList(); OnPropertyChanged(nameof(PackageMetadata)); OnPropertyChanged(nameof(IsPackageDeprecated)); diff --git a/src/NuGet.Clients/NuGet.PackageManagement.UI/ViewModels/PackageItemViewModel.cs b/src/NuGet.Clients/NuGet.PackageManagement.UI/ViewModels/PackageItemViewModel.cs index 67e89f52ec8..1d88d951e9a 100644 --- a/src/NuGet.Clients/NuGet.PackageManagement.UI/ViewModels/PackageItemViewModel.cs +++ b/src/NuGet.Clients/NuGet.PackageManagement.UI/ViewModels/PackageItemViewModel.cs @@ -668,7 +668,7 @@ private async Task ReloadPackageMetadataAsync() DeprecationMetadata = deprecationMetadata; IsPackageDeprecated = deprecationMetadata != null; - VulnerabilityMaxSeverity = packageMetadata?.Vulnerabilities?.Max(v => v.Severity) ?? -1; + VulnerabilityMaxSeverity = packageMetadata?.Vulnerabilities?.FirstOrDefault()?.Severity ?? -1; } catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { diff --git a/src/NuGet.Clients/NuGet.VisualStudio.Internal.Contracts/ContextInfos/PackageSearchMetadataContextInfo.cs b/src/NuGet.Clients/NuGet.VisualStudio.Internal.Contracts/ContextInfos/PackageSearchMetadataContextInfo.cs index 1770f5aa035..11cd6267562 100644 --- a/src/NuGet.Clients/NuGet.VisualStudio.Internal.Contracts/ContextInfos/PackageSearchMetadataContextInfo.cs +++ b/src/NuGet.Clients/NuGet.VisualStudio.Internal.Contracts/ContextInfos/PackageSearchMetadataContextInfo.cs @@ -73,7 +73,7 @@ public static PackageSearchMetadataContextInfo Create(IPackageSearchMetadata pac IsListed = packageSearchMetadata.IsListed, DependencySets = packageSearchMetadata.DependencySets?.ToList(), DownloadCount = packageSearchMetadata.DownloadCount, - Vulnerabilities = packageSearchMetadata.Vulnerabilities?.Select(vulnerability => new PackageVulnerabilityMetadataContextInfo(vulnerability.AdvisoryUrl, vulnerability.Severity)).ToArray(), + Vulnerabilities = packageSearchMetadata.Vulnerabilities?.Select(vulnerability => new PackageVulnerabilityMetadataContextInfo(vulnerability.AdvisoryUrl, vulnerability.Severity)).OrderByDescending(v => v.Severity).ToArray(), }; } } diff --git a/test/NuGet.Clients.Tests/NuGet.PackageManagement.UI.Test/Models/V3DetailControlModelTests.cs b/test/NuGet.Clients.Tests/NuGet.PackageManagement.UI.Test/Models/V3DetailControlModelTests.cs index ddec71effe0..561c4a723ec 100644 --- a/test/NuGet.Clients.Tests/NuGet.PackageManagement.UI.Test/Models/V3DetailControlModelTests.cs +++ b/test/NuGet.Clients.Tests/NuGet.PackageManagement.UI.Test/Models/V3DetailControlModelTests.cs @@ -131,6 +131,20 @@ public void VulnerabilityCountWhenMetadataHasVulnerability_Calculated() Assert.Equal(_testInstance.PackageVulnerabilityCount, _testData.TestData.Vulnerabilities.Count()); } + [Fact] + public void PackageVulnerabilitiesWhenMetadataHasVulnerability_OrderedBySeverityDescending() + { + Assert.Collection(_testInstance.PackageVulnerabilities, + item => + { + Assert.Equal(3, item.Severity); + }, + item => + { + Assert.Equal(2, item.Severity); + }); + } + [Fact] public async Task SetCurrentPackageAsync_SortsVersions_ByNuGetVersionDesc() { From 66310d6b754682db570b32d2364e7e60a5eeafa6 Mon Sep 17 00:00:00 2001 From: Jean-Pierre Briede Date: Wed, 28 Jul 2021 17:02:10 -0700 Subject: [PATCH 2/4] Removed unused using, added missing OnPropertyChanged call --- .../NuGet.PackageManagement.UI/Models/DetailControlModel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NuGet.Clients/NuGet.PackageManagement.UI/Models/DetailControlModel.cs b/src/NuGet.Clients/NuGet.PackageManagement.UI/Models/DetailControlModel.cs index 8f47e5d23a6..74e2e5e6b72 100644 --- a/src/NuGet.Clients/NuGet.PackageManagement.UI/Models/DetailControlModel.cs +++ b/src/NuGet.Clients/NuGet.PackageManagement.UI/Models/DetailControlModel.cs @@ -15,7 +15,6 @@ using NuGet.VisualStudio; using NuGet.VisualStudio.Internal.Contracts; using NuGet.VisualStudio.Telemetry; -using static Microsoft.TeamFoundation.Client.CommandLine.Options; using Task = System.Threading.Tasks.Task; namespace NuGet.PackageManagement.UI @@ -418,6 +417,7 @@ private set _packageVulnerabilities = value; OnPropertyChanged(nameof(PackageVulnerabilities)); + OnPropertyChanged(nameof(PackageVulnerabilityMaxSeverity)); OnPropertyChanged(nameof(IsPackageVulnerable)); OnPropertyChanged(nameof(PackageVulnerabilityCount)); } From c28106a529c39f796f73f53807f27b431011402b Mon Sep 17 00:00:00 2001 From: Jean-Pierre Briede Date: Wed, 29 Sep 2021 16:13:38 -0700 Subject: [PATCH 3/4] Addressed PR feedback --- .../PackageSearchMetadataContextInfo.cs | 4 +++- .../Models/V3DetailControlModelTests.cs | 19 +++++++++---------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/NuGet.Clients/NuGet.VisualStudio.Internal.Contracts/ContextInfos/PackageSearchMetadataContextInfo.cs b/src/NuGet.Clients/NuGet.VisualStudio.Internal.Contracts/ContextInfos/PackageSearchMetadataContextInfo.cs index 11cd6267562..5110e2386a9 100644 --- a/src/NuGet.Clients/NuGet.VisualStudio.Internal.Contracts/ContextInfos/PackageSearchMetadataContextInfo.cs +++ b/src/NuGet.Clients/NuGet.VisualStudio.Internal.Contracts/ContextInfos/PackageSearchMetadataContextInfo.cs @@ -73,7 +73,9 @@ public static PackageSearchMetadataContextInfo Create(IPackageSearchMetadata pac IsListed = packageSearchMetadata.IsListed, DependencySets = packageSearchMetadata.DependencySets?.ToList(), DownloadCount = packageSearchMetadata.DownloadCount, - Vulnerabilities = packageSearchMetadata.Vulnerabilities?.Select(vulnerability => new PackageVulnerabilityMetadataContextInfo(vulnerability.AdvisoryUrl, vulnerability.Severity)).OrderByDescending(v => v.Severity).ToArray(), + Vulnerabilities = packageSearchMetadata.Vulnerabilities? + .Select(vulnerability => new PackageVulnerabilityMetadataContextInfo(vulnerability.AdvisoryUrl, vulnerability.Severity)) + .OrderByDescending(v => v.Severity).ToArray(), }; } } diff --git a/test/NuGet.Clients.Tests/NuGet.PackageManagement.UI.Test/Models/V3DetailControlModelTests.cs b/test/NuGet.Clients.Tests/NuGet.PackageManagement.UI.Test/Models/V3DetailControlModelTests.cs index 561c4a723ec..33675d75648 100644 --- a/test/NuGet.Clients.Tests/NuGet.PackageManagement.UI.Test/Models/V3DetailControlModelTests.cs +++ b/test/NuGet.Clients.Tests/NuGet.PackageManagement.UI.Test/Models/V3DetailControlModelTests.cs @@ -132,17 +132,16 @@ public void VulnerabilityCountWhenMetadataHasVulnerability_Calculated() } [Fact] - public void PackageVulnerabilitiesWhenMetadataHasVulnerability_OrderedBySeverityDescending() + public void PackageVulnerabilities_WhenMetadataHasVulnerability_IsOrderedBySeverityDescending() { - Assert.Collection(_testInstance.PackageVulnerabilities, - item => - { - Assert.Equal(3, item.Severity); - }, - item => - { - Assert.Equal(2, item.Severity); - }); + var sortedTestVulnerabilities = + _testData.TestData.Vulnerabilities + .OrderByDescending(v => v.Severity) + .Select(v => new Action( + (item) => Assert.Equal(v.Severity, item.Severity)) + ).ToArray(); + + Assert.Collection(_testInstance.PackageVulnerabilities, sortedTestVulnerabilities); } [Fact] From 96698aba4630d3302161bf35905cd22c86a2d099 Mon Sep 17 00:00:00 2001 From: Jean-Pierre Briede Date: Fri, 1 Oct 2021 16:56:20 -0700 Subject: [PATCH 4/4] Addressed PR feedback --- .../Models/V3DetailControlModelTests.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/test/NuGet.Clients.Tests/NuGet.PackageManagement.UI.Test/Models/V3DetailControlModelTests.cs b/test/NuGet.Clients.Tests/NuGet.PackageManagement.UI.Test/Models/V3DetailControlModelTests.cs index 33675d75648..be5d18e3561 100644 --- a/test/NuGet.Clients.Tests/NuGet.PackageManagement.UI.Test/Models/V3DetailControlModelTests.cs +++ b/test/NuGet.Clients.Tests/NuGet.PackageManagement.UI.Test/Models/V3DetailControlModelTests.cs @@ -134,14 +134,12 @@ public void VulnerabilityCountWhenMetadataHasVulnerability_Calculated() [Fact] public void PackageVulnerabilities_WhenMetadataHasVulnerability_IsOrderedBySeverityDescending() { - var sortedTestVulnerabilities = + IEnumerable sortedTestVulnerabilities = _testData.TestData.Vulnerabilities .OrderByDescending(v => v.Severity) - .Select(v => new Action( - (item) => Assert.Equal(v.Severity, item.Severity)) - ).ToArray(); + .Select(v => new PackageVulnerabilityMetadataContextInfo(v.AdvisoryUrl, v.Severity)); - Assert.Collection(_testInstance.PackageVulnerabilities, sortedTestVulnerabilities); + Assert.Equal(sortedTestVulnerabilities, _testInstance.PackageVulnerabilities); } [Fact]