From 6e80ab2c36eb5cf59240b238d31e3cf8aef5d70e Mon Sep 17 00:00:00 2001 From: Georgii Borovinskikh <117642191+georgii-borovinskikh-sonarsource@users.noreply.github.com> Date: Mon, 18 Nov 2024 11:17:37 +0100 Subject: [PATCH] SLVS-1622 Receive server taint vulnerabilities updates via SLCore (#5828) [SLVS-1622](https://sonarsource.atlassian.net/browse/SLVS-1622) [SLVS-1622]: https://sonarsource.atlassian.net/browse/SLVS-1622?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --- src/CFamily.UnitTests/packages.lock.json | 1 + .../SLCore/SLCoreConstantsProviderTests.cs | 2 +- .../packages.lock.json | 1 + ...egration.Vsix_Baseline_WithStrongNames.txt | 5 +- ...ation.Vsix_Baseline_WithoutStrongNames.txt | 5 +- src/Integration.Vsix/packages.lock.json | 1 + .../SLCore/SLCoreConstantsProvider.cs | 4 +- .../Taint/TaintIssuesSynchronizerTests.cs | 2 +- .../Taint/TaintStoreTests.cs | 344 ++++++++++++------ .../Taint/TaintVulnerabilitiesUpdateTests.cs | 78 ++++ .../IssuesStore/IIssuesStore.cs | 6 +- ...TaintIssueToIssueVisualizationConverter.cs | 2 +- .../Taint/TaintIssuesSynchronizer.cs | 4 +- src/IssueViz.Security/Taint/TaintStore.cs | 218 ++++++----- .../packages.lock.json | 1 + .../TaintVulnerabilityListenerTests.cs | 92 ++++- .../packages.lock.json | 34 ++ .../TaintVulnerabilityListener.cs | 29 +- src/SLCore.Listeners/SLCore.Listeners.csproj | 1 + src/SLCore.Listeners/packages.lock.json | 65 ++-- 20 files changed, 614 insertions(+), 281 deletions(-) create mode 100644 src/IssueViz.Security.UnitTests/Taint/TaintVulnerabilitiesUpdateTests.cs diff --git a/src/CFamily.UnitTests/packages.lock.json b/src/CFamily.UnitTests/packages.lock.json index c3623dbf7e..c7bced753d 100644 --- a/src/CFamily.UnitTests/packages.lock.json +++ b/src/CFamily.UnitTests/packages.lock.json @@ -1722,6 +1722,7 @@ "type": "Project", "dependencies": { "SonarLint.VisualStudio.ConnectedMode": "[1.0.0, )", + "SonarLint.VisualStudio.IssueVisualization.Security": "[1.0.0, )", "SonarLint.VisualStudio.SLCore": "[1.0.0, )" } }, diff --git a/src/Integration.UnitTests/SLCore/SLCoreConstantsProviderTests.cs b/src/Integration.UnitTests/SLCore/SLCoreConstantsProviderTests.cs index 75150fc50d..0ee28cef34 100644 --- a/src/Integration.UnitTests/SLCore/SLCoreConstantsProviderTests.cs +++ b/src/Integration.UnitTests/SLCore/SLCoreConstantsProviderTests.cs @@ -66,7 +66,7 @@ public void ClientConstants_ShouldBeExpected() public void FeatureFlags_ShouldBeExpected() { var testSubject = CreateTestSubject(); - var expectedFeatureFlags = new FeatureFlagsDto(true, true, true, true, false, false, true, true, true); + var expectedFeatureFlags = new FeatureFlagsDto(true, true, true, true, true, false, true, true, true); var actual = testSubject.FeatureFlags; actual.Should().BeEquivalentTo(expectedFeatureFlags); diff --git a/src/Integration.Vsix.UnitTests/packages.lock.json b/src/Integration.Vsix.UnitTests/packages.lock.json index 49e623d399..7539c525d8 100644 --- a/src/Integration.Vsix.UnitTests/packages.lock.json +++ b/src/Integration.Vsix.UnitTests/packages.lock.json @@ -1733,6 +1733,7 @@ "type": "Project", "dependencies": { "SonarLint.VisualStudio.ConnectedMode": "[1.0.0, )", + "SonarLint.VisualStudio.IssueVisualization.Security": "[1.0.0, )", "SonarLint.VisualStudio.SLCore": "[1.0.0, )" } }, diff --git a/src/Integration.Vsix/AsmRef_Integration.Vsix_Baseline_WithStrongNames.txt b/src/Integration.Vsix/AsmRef_Integration.Vsix_Baseline_WithStrongNames.txt index 2727296b98..29249016e3 100644 --- a/src/Integration.Vsix/AsmRef_Integration.Vsix_Baseline_WithStrongNames.txt +++ b/src/Integration.Vsix/AsmRef_Integration.Vsix_Baseline_WithStrongNames.txt @@ -1,7 +1,7 @@ --- ################################ # Assembly references report -# Report date/time: 2024-10-23T13:47:06.0805083Z +# Report date/time: 2024-11-12T14:45:18.4864870Z ################################ # # Generated by Devtility CheckAsmRefs v0.11.0.223 @@ -442,11 +442,12 @@ Referenced assemblies: - 'SonarLint.VisualStudio.ConnectedMode, Version=8.7.0.0, Culture=neutral, PublicKeyToken=c5b62af9de6d7244' - 'SonarLint.VisualStudio.Core, Version=8.7.0.0, Culture=neutral, PublicKeyToken=c5b62af9de6d7244' - 'SonarLint.VisualStudio.IssueVisualization, Version=8.7.0.0, Culture=neutral, PublicKeyToken=c5b62af9de6d7244' +- 'SonarLint.VisualStudio.IssueVisualization.Security, Version=8.7.0.0, Culture=neutral, PublicKeyToken=c5b62af9de6d7244' - 'SonarLint.VisualStudio.SLCore, Version=8.7.0.0, Culture=neutral, PublicKeyToken=c5b62af9de6d7244' - 'System, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089' - 'System.ComponentModel.Composition, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089' - 'System.Core, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089' -# Number of references: 8 +# Number of references: 9 --- Assembly: 'SonarQube.Client, Version=8.7.0.0, Culture=neutral, PublicKeyToken=c5b62af9de6d7244' diff --git a/src/Integration.Vsix/AsmRef_Integration.Vsix_Baseline_WithoutStrongNames.txt b/src/Integration.Vsix/AsmRef_Integration.Vsix_Baseline_WithoutStrongNames.txt index c47bd2e14f..dd21a20f5f 100644 --- a/src/Integration.Vsix/AsmRef_Integration.Vsix_Baseline_WithoutStrongNames.txt +++ b/src/Integration.Vsix/AsmRef_Integration.Vsix_Baseline_WithoutStrongNames.txt @@ -1,7 +1,7 @@ --- ################################ # Assembly references report -# Report date/time: 2024-10-23T13:47:06.0805083Z +# Report date/time: 2024-11-12T14:45:18.4864870Z ################################ # # Generated by Devtility CheckAsmRefs v0.11.0.223 @@ -442,11 +442,12 @@ Referenced assemblies: - 'SonarLint.VisualStudio.ConnectedMode, Version=8.7.0.0, Culture=neutral, PublicKeyToken=null' - 'SonarLint.VisualStudio.Core, Version=8.7.0.0, Culture=neutral, PublicKeyToken=null' - 'SonarLint.VisualStudio.IssueVisualization, Version=8.7.0.0, Culture=neutral, PublicKeyToken=null' +- 'SonarLint.VisualStudio.IssueVisualization.Security, Version=8.7.0.0, Culture=neutral, PublicKeyToken=null' - 'SonarLint.VisualStudio.SLCore, Version=8.7.0.0, Culture=neutral, PublicKeyToken=null' - 'System, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089' - 'System.ComponentModel.Composition, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089' - 'System.Core, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089' -# Number of references: 8 +# Number of references: 9 --- Assembly: 'SonarQube.Client, Version=8.7.0.0, Culture=neutral, PublicKeyToken=null' diff --git a/src/Integration.Vsix/packages.lock.json b/src/Integration.Vsix/packages.lock.json index 6a6a9ddf42..e90df397b9 100644 --- a/src/Integration.Vsix/packages.lock.json +++ b/src/Integration.Vsix/packages.lock.json @@ -1657,6 +1657,7 @@ "type": "Project", "dependencies": { "SonarLint.VisualStudio.ConnectedMode": "[1.0.0, )", + "SonarLint.VisualStudio.IssueVisualization.Security": "[1.0.0, )", "SonarLint.VisualStudio.SLCore": "[1.0.0, )" } }, diff --git a/src/Integration/SLCore/SLCoreConstantsProvider.cs b/src/Integration/SLCore/SLCoreConstantsProvider.cs index a72338a417..5cdd32e317 100644 --- a/src/Integration/SLCore/SLCoreConstantsProvider.cs +++ b/src/Integration/SLCore/SLCoreConstantsProvider.cs @@ -42,7 +42,7 @@ public SLCoreConstantsProvider(IVsInfoProvider vsInfoProvider) public ClientConstantsDto ClientConstants => new(vsInfoProvider.Name, $"SonarLint Visual Studio/{VersionHelper.SonarLintVersion}", Process.GetCurrentProcess().Id); - public FeatureFlagsDto FeatureFlags => new(true, true, true, true, false, false, true, true, true); + public FeatureFlagsDto FeatureFlags => new(true, true, true, true, true, false, true, true, true); public TelemetryClientConstantAttributesDto TelemetryConstants => new("visualstudio", "SonarLint Visual Studio", VersionHelper.SonarLintVersion, VisualStudioHelpers.VisualStudioVersion, new Dictionary { @@ -68,7 +68,7 @@ public SLCoreConstantsProvider(IVsInfoProvider vsInfoProvider) Language.CSS, Language.SECRETS ]; - + private static IdeVersionInformation GetVsVersion(IVsVersion vsVersion) { if (vsVersion == null) diff --git a/src/IssueViz.Security.UnitTests/Taint/TaintIssuesSynchronizerTests.cs b/src/IssueViz.Security.UnitTests/Taint/TaintIssuesSynchronizerTests.cs index e31de0d40c..422cbc6fcc 100644 --- a/src/IssueViz.Security.UnitTests/Taint/TaintIssuesSynchronizerTests.cs +++ b/src/IssueViz.Security.UnitTests/Taint/TaintIssuesSynchronizerTests.cs @@ -337,5 +337,5 @@ private void CheckUIContextUpdated(uint expectedCookie, int expectedState) => private void CheckToolWindowServiceIsCalled() => toolWindowService.Received().EnsureToolWindowExists(TaintToolWindow.ToolWindowId); - private void CheckStoreIsCleared() => taintStore.Received(1).Set([], null); + private void CheckStoreIsCleared() => taintStore.Received(1).Reset(); } diff --git a/src/IssueViz.Security.UnitTests/Taint/TaintStoreTests.cs b/src/IssueViz.Security.UnitTests/Taint/TaintStoreTests.cs index bec2492be3..beaa6c5ae6 100644 --- a/src/IssueViz.Security.UnitTests/Taint/TaintStoreTests.cs +++ b/src/IssueViz.Security.UnitTests/Taint/TaintStoreTests.cs @@ -66,7 +66,7 @@ public void GetAll_ReturnsImmutableInstance() testSubject.Set(oldItems, "config scope"); var issuesList1 = testSubject.GetAll(); - testSubject.Add(SetupIssueViz()); + testSubject.Update(new TaintVulnerabilitiesUpdate("config scope", [SetupIssueViz()], [], [])); var issuesList2 = testSubject.GetAll(); issuesList1.Count.Should().Be(2); @@ -76,11 +76,19 @@ public void GetAll_ReturnsImmutableInstance() [TestMethod] public void Set_NullCollection_ArgumentNullException() { - Action act = () => testSubject.Set(null, null); + Action act = () => testSubject.Set(null, "any"); act.Should().Throw().And.ParamName.Should().Be("issueVisualizations"); } + [TestMethod] + public void Set_NullConfigScope_ArgumentNullException() + { + Action act = () => testSubject.Set([], null); + + act.Should().Throw().And.ParamName.Should().Be("newConfigurationScope"); + } + [TestMethod] public void Set_NoSubscribersToIssuesChangedEvent_NoException() { @@ -95,26 +103,34 @@ public void Set_NoPreviousItems_NoNewItems_CollectionChangedAndEventRaised() var eventHandlerMock = Substitute.For>(); testSubject.IssuesChanged += eventHandlerMock; - testSubject.Set([], null); + testSubject.Set([], "some config scope"); testSubject.GetAll().Should().BeEmpty(); eventHandlerMock.ReceivedWithAnyArgs(1).Invoke(default, default); } [TestMethod] - public void Set_NoPreviousItems_HasNewItems_CollectionChangedAndEventRaised() + public void Reset_NoPreviousItems_NoNewItems_CollectionChangedAndEventRaised() { var eventHandlerMock = Substitute.For>(); testSubject.IssuesChanged += eventHandlerMock; + testSubject.Reset(); + + testSubject.GetAll().Should().BeEmpty(); + eventHandlerMock.ReceivedWithAnyArgs(1).Invoke(default, default); + } + + [TestMethod] + public void Set_NoPreviousItems_HasNewItems_CollectionChangedAndEventRaised() + { + var receivedEventGetter = CaptureIssuesChangedEventArgs(); var newItems = new[] { SetupIssueViz(), SetupIssueViz() }; + testSubject.Set(newItems, "some config scope"); testSubject.GetAll().Should().BeEquivalentTo(newItems); - eventHandlerMock.ReceivedWithAnyArgs(1).Invoke(default, default); - var eventArgs = (IssuesChangedEventArgs)eventHandlerMock.ReceivedCalls().Single().GetArguments()[1]!; - eventArgs.RemovedIssues.Should().BeEmpty(); - eventArgs.AddedIssues.Should().BeEquivalentTo(newItems); + receivedEventGetter().Should().BeEquivalentTo(new IssuesChangedEventArgs([], newItems)); } [TestMethod] @@ -122,17 +138,12 @@ public void Set_HasPreviousItems_NoNewItems_CollectionChangedAndEventRaised() { var oldItems = new[] { SetupIssueViz(), SetupIssueViz() }; testSubject.Set(oldItems, "some config scope"); - - var eventHandlerMock = Substitute.For>(); - testSubject.IssuesChanged += eventHandlerMock; + var receivedEventGetter = CaptureIssuesChangedEventArgs(); testSubject.Set([], "some config scope"); testSubject.GetAll().Should().BeEmpty(); - eventHandlerMock.ReceivedWithAnyArgs(1).Invoke(default, default); - var eventArgs = (IssuesChangedEventArgs)eventHandlerMock.ReceivedCalls().Single().GetArguments()[1]!; - eventArgs.RemovedIssues.Should().BeEquivalentTo(oldItems); - eventArgs.AddedIssues.Should().BeEmpty(); + receivedEventGetter().Should().BeEquivalentTo(new IssuesChangedEventArgs(oldItems, [])); } [TestMethod] @@ -140,48 +151,39 @@ public void Set_HasPreviousItems_HasNewItems_CollectionChangedAndEventRaised() { var oldItems = new[] { SetupIssueViz(), SetupIssueViz() }; testSubject.Set(oldItems, "some config scope"); - - var eventHandlerMock = Substitute.For>(); - testSubject.IssuesChanged += eventHandlerMock; + var receivedEventGetter = CaptureIssuesChangedEventArgs(); var newItems = new[] { SetupIssueViz(), SetupIssueViz() }; testSubject.Set(newItems, "some config scope"); testSubject.GetAll().Should().BeEquivalentTo(newItems); - eventHandlerMock.ReceivedWithAnyArgs(1).Invoke(default, default); - var eventArgs = (IssuesChangedEventArgs)eventHandlerMock.ReceivedCalls().Single().GetArguments()[1]!; - eventArgs.RemovedIssues.Should().BeEquivalentTo(oldItems); - eventArgs.AddedIssues.Should().BeEquivalentTo(newItems); + receivedEventGetter().Should().BeEquivalentTo(new IssuesChangedEventArgs(oldItems, newItems)); } [TestMethod] public void Set_HasPreviousItems_HasSomeNewItems_CollectionChangedAndEventRaised() { - var issueViz1 = SetupIssueViz("key1"); - var issueViz2 = SetupIssueViz("key2"); - var issueViz2NewObject = SetupIssueViz("key2"); - var issueViz3 = SetupIssueViz("key3"); + var issueViz1 = SetupIssueViz(); + var issueViz2Id = Guid.NewGuid(); + var issueViz2 = SetupIssueViz(issueViz2Id); + var issueViz2NewObject = SetupIssueViz(issueViz2Id); + var issueViz3 = SetupIssueViz(); var oldItems = new[] { issueViz1, issueViz2 }; testSubject.Set(oldItems, "some config scope"); + var receivedEventGetter = CaptureIssuesChangedEventArgs(); - var eventHandlerMock = Substitute.For>(); - testSubject.IssuesChanged += eventHandlerMock; - - var newItems = new[] { issueViz2NewObject, issueViz3}; + var newItems = new[] { issueViz2NewObject, issueViz3 }; testSubject.Set(newItems, "some config scope"); testSubject.GetAll().Should().BeEquivalentTo(newItems); - eventHandlerMock.ReceivedWithAnyArgs(1).Invoke(default, default); - var eventArgs = (IssuesChangedEventArgs)eventHandlerMock.ReceivedCalls().Single().GetArguments()[1]!; - eventArgs.RemovedIssues.Should().BeEquivalentTo(issueViz1); - eventArgs.AddedIssues.Should().BeEquivalentTo(issueViz3); + receivedEventGetter().Should().BeEquivalentTo(new IssuesChangedEventArgs([issueViz1, issueViz2], [issueViz2NewObject, issueViz3])); } [TestMethod] public void Set_HasItems_NoConfigScope_Throws() { - var issueViz1 = SetupIssueViz("key1"); + var issueViz1 = SetupIssueViz(); var act = () => testSubject.Set([issueViz1], null); @@ -191,7 +193,7 @@ public void Set_HasItems_NoConfigScope_Throws() [TestMethod] public void ConfigScope_NoInformation_ReturnsNull() { - testSubject.Set([], null); + testSubject.Reset(); var result = testSubject.ConfigurationScope; result.Should().BeNull(); @@ -209,152 +211,262 @@ public void ConfigScope_HasInformation_ReturnsInformation() } [TestMethod] - public void Remove_IssueKeyIsNull_ArgumentNullException() + public void Update_NullParameter_Throws() { - Action act = () => testSubject.Remove(null); + var act = () => testSubject.Update(null); - act.Should().Throw().And.ParamName.Should().Be("issueKey"); + act.Should().ThrowExactly().Which.ParamName.Should().Be("taintVulnerabilitiesUpdate"); } [TestMethod] - public void Remove_IssueNotFound_NoIssuesInList_NoEventIsRaised() + public void Update_NoConfigScope_Ignored() { + testSubject.Reset(); var eventHandlerMock = Substitute.For>(); testSubject.IssuesChanged += eventHandlerMock; - testSubject.Remove("some unknown key"); + testSubject.Update(new TaintVulnerabilitiesUpdate("some config scope", [SetupIssueViz()], [], [])); eventHandlerMock.DidNotReceiveWithAnyArgs().Invoke(default, default); - testSubject.GetAll().Should().BeEmpty(); } [TestMethod] - public void Remove_IssueNotFound_NoIssueWithThisId_NoEventIsRaised() + public void Update_ClosedIssues_Removed() { - var existingIssue = SetupIssueViz("key1"); + List analysisIssueVisualizations = [SetupIssueViz(), SetupIssueViz(), SetupIssueViz()]; + testSubject.Set(analysisIssueVisualizations, "some config scope"); + var receivedEventGetter = CaptureIssuesChangedEventArgs(); - testSubject.Set(new[] { existingIssue }, "some config scope"); + testSubject.Update(new TaintVulnerabilitiesUpdate("some config scope", [], [], [analysisIssueVisualizations[0].IssueId!.Value, analysisIssueVisualizations[2].IssueId!.Value])); - var eventHandlerMock = Substitute.For>(); - testSubject.IssuesChanged += eventHandlerMock; + testSubject.GetAll().Should().BeEquivalentTo(analysisIssueVisualizations[1]); + receivedEventGetter().Should().BeEquivalentTo(new IssuesChangedEventArgs([analysisIssueVisualizations[0], analysisIssueVisualizations[2]], [])); + } + + [TestMethod] + public void Update_ClosedIssues_PartiallyPresent_Removed() + { + List analysisIssueVisualizations = [SetupIssueViz(), SetupIssueViz(), SetupIssueViz()]; + testSubject.Set(analysisIssueVisualizations, "some config scope"); + var receivedEventGetter = CaptureIssuesChangedEventArgs(); - testSubject.Remove("some unknown key"); + using (new AssertIgnoreScope()) + { + testSubject.Update(new TaintVulnerabilitiesUpdate("some config scope", [], [], [analysisIssueVisualizations[0].IssueId!.Value, Guid.NewGuid()])); + } - eventHandlerMock.DidNotReceiveWithAnyArgs().Invoke(default, default); - testSubject.GetAll().Should().BeEquivalentTo(existingIssue); + testSubject.GetAll().Should().BeEquivalentTo(analysisIssueVisualizations.Skip(1)); + receivedEventGetter().Should().BeEquivalentTo(new IssuesChangedEventArgs([analysisIssueVisualizations[0]], [])); } [TestMethod] - public void Remove_IssueFound_IssueIsRemovedAndEventIsRaised() + public void Update_ClosedIssues_NotPresent_Ignored() { - var existingIssue1 = SetupIssueViz("key1"); - var existingIssue2 = SetupIssueViz("key2"); - var existingIssue3 = SetupIssueViz("key3"); + List analysisIssueVisualizations = [SetupIssueViz(), SetupIssueViz(), SetupIssueViz()]; + testSubject.Set(analysisIssueVisualizations, "some config scope"); + var eventHandlerMock = CreateEventHandlerMock(); - testSubject.Set([existingIssue1, existingIssue2, existingIssue3], "some config scope"); + using (new AssertIgnoreScope()) + { + testSubject.Update(new TaintVulnerabilitiesUpdate("some config scope", [], [], [Guid.NewGuid()])); + } - var eventHandlerMock = Substitute.For>(); - testSubject.IssuesChanged += eventHandlerMock; + testSubject.GetAll().Should().BeEquivalentTo(analysisIssueVisualizations); + eventHandlerMock.DidNotReceiveWithAnyArgs().Invoke(default, default); + } - testSubject.Remove("key2"); + [TestMethod] + public void Update_UpdatedIssues_Replaced() + { + List analysisIssueVisualizations = [SetupIssueViz(), SetupIssueViz(), SetupIssueViz()]; + var updated1 = SetupIssueViz(analysisIssueVisualizations[0].IssueId); + var updated2 = SetupIssueViz(analysisIssueVisualizations[2].IssueId); + testSubject.Set(analysisIssueVisualizations, "some config scope"); + var receivedEventGetter = CaptureIssuesChangedEventArgs(); - eventHandlerMock.ReceivedWithAnyArgs(1).Invoke(default, default); - var eventArgs = (IssuesChangedEventArgs)eventHandlerMock.ReceivedCalls().Single().GetArguments()[1]!; - eventArgs.RemovedIssues.Should().BeEquivalentTo(existingIssue2); - eventArgs.AddedIssues.Should().BeEmpty(); - testSubject.GetAll().Should().BeEquivalentTo(existingIssue1, existingIssue3); + testSubject.Update(new TaintVulnerabilitiesUpdate("some config scope", [], [updated1, updated2], [])); + + testSubject.GetAll().Should().BeEquivalentTo(updated1, updated2, analysisIssueVisualizations[1]); + receivedEventGetter().Should().BeEquivalentTo(new IssuesChangedEventArgs([analysisIssueVisualizations[0], analysisIssueVisualizations[2]], [updated1, updated2])); } [TestMethod] - public void Remove_MultipleIssuesFoundWithSameId_FirstIssueIsRemovedAndEventIsRaised() + public void Update_UpdatedIssues_PartiallyPresent_Replaced() { - var existingIssue1 = SetupIssueViz("key1"); - var existingIssue2 = SetupIssueViz("key1"); - var existingIssue3 = SetupIssueViz("key1"); + List analysisIssueVisualizations = [SetupIssueViz(), SetupIssueViz(), SetupIssueViz()]; + var updated1 = SetupIssueViz(analysisIssueVisualizations[0].IssueId); + var updated2 = SetupIssueViz(); + testSubject.Set(analysisIssueVisualizations, "some config scope"); + var receivedEventGetter = CaptureIssuesChangedEventArgs(); + + using (new AssertIgnoreScope()) + { + testSubject.Update(new TaintVulnerabilitiesUpdate("some config scope", [], [updated1, updated2], [])); + } + + testSubject.GetAll().Should().BeEquivalentTo(updated1, analysisIssueVisualizations[1], analysisIssueVisualizations[2]); + receivedEventGetter().Should().BeEquivalentTo(new IssuesChangedEventArgs([analysisIssueVisualizations[0]], [updated1])); + } - testSubject.Set([existingIssue1, existingIssue2, existingIssue3], "some config scope"); + [TestMethod] + public void Update_UpdatedIssues_NotPresent_Ignored() + { + List analysisIssueVisualizations = [SetupIssueViz(), SetupIssueViz(), SetupIssueViz()]; + var updated1 = SetupIssueViz(); + var updated2 = SetupIssueViz(); + testSubject.Set(analysisIssueVisualizations, "some config scope"); + var eventHandlerMock = CreateEventHandlerMock(); + + using (new AssertIgnoreScope()) + { + testSubject.Update(new TaintVulnerabilitiesUpdate("some config scope", [], [updated1, updated2], [])); + } + + testSubject.GetAll().Should().BeEquivalentTo(analysisIssueVisualizations); + eventHandlerMock.DidNotReceiveWithAnyArgs().Invoke(default, default); + } - var eventHandlerMock = Substitute.For>(); - testSubject.IssuesChanged += eventHandlerMock; + [TestMethod] + public void Update_AddedIssues_Adds() + { + List analysisIssueVisualizations = [SetupIssueViz(), SetupIssueViz(), SetupIssueViz()]; + var added1 = SetupIssueViz(); + var added2 = SetupIssueViz(); + testSubject.Set(analysisIssueVisualizations, "some config scope"); + var receivedEventGetter = CaptureIssuesChangedEventArgs(); - testSubject.Remove("key1"); + testSubject.Update(new TaintVulnerabilitiesUpdate("some config scope", [added1, added2], [], [])); - eventHandlerMock.ReceivedWithAnyArgs(1).Invoke(default, default); - var eventArgs = (IssuesChangedEventArgs)eventHandlerMock.ReceivedCalls().Single().GetArguments()[1]!; - eventArgs.RemovedIssues.Should().BeEquivalentTo(existingIssue1); - eventArgs.AddedIssues.Should().BeEmpty(); - testSubject.GetAll().Should().BeEquivalentTo(existingIssue2, existingIssue3); + testSubject.GetAll().Should().BeEquivalentTo(analysisIssueVisualizations.Concat([added1, added2])); + receivedEventGetter().Should().BeEquivalentTo(new IssuesChangedEventArgs([], [added1, added2])); } [TestMethod] - public void Add_IssueIsNull_ArgumentNullException() + public void Update_AddedIssues_PartiallyPresent_AddsMissing() { - Action act = () => testSubject.Add(null); + List analysisIssueVisualizations = [SetupIssueViz(), SetupIssueViz(), SetupIssueViz()]; + var added1 = SetupIssueViz(analysisIssueVisualizations[0].IssueId); + var added2 = SetupIssueViz(); + testSubject.Set(analysisIssueVisualizations, "some config scope"); + var receivedEventGetter = CaptureIssuesChangedEventArgs(); + + using (new AssertIgnoreScope()) + { + testSubject.Update(new TaintVulnerabilitiesUpdate("some config scope", [added1, added2], [], [])); + } + + testSubject.GetAll().Should().BeEquivalentTo(analysisIssueVisualizations.Concat([added2])); + receivedEventGetter().Should().BeEquivalentTo(new IssuesChangedEventArgs([], [added2])); + } - act.Should().Throw().And.ParamName.Should().Be("issueVisualization"); + [TestMethod] + public void Update_AddedIssues_AllPresent_Ignored() + { + List analysisIssueVisualizations = [SetupIssueViz(), SetupIssueViz(), SetupIssueViz()]; + var added1 = SetupIssueViz(analysisIssueVisualizations[0].IssueId); + var added2 = SetupIssueViz(analysisIssueVisualizations[2].IssueId); + testSubject.Set(analysisIssueVisualizations, "some config scope"); + var eventHandlerMock = CreateEventHandlerMock(); + + using (new AssertIgnoreScope()) + { + testSubject.Update(new TaintVulnerabilitiesUpdate("some config scope", [added1, added2], [], [])); + } + + testSubject.GetAll().Should().BeEquivalentTo(analysisIssueVisualizations); + eventHandlerMock.DidNotReceiveWithAnyArgs().Invoke(default, default); } [TestMethod] - public void Add_NoConfigScope_IssueIgnoredAndNoEventIsRaised() + public void Update_Complex_RemovesUpdatesAndAdds() { - testSubject.Set([], null); + var added = SetupIssueViz(); + var toUpdate = SetupIssueViz(); + var updated = SetupIssueViz(toUpdate.IssueId); + var toRemove = SetupIssueViz(); + var notTouched = SetupIssueViz(); + List analysisIssueVisualizations = [toUpdate, toRemove, notTouched]; + testSubject.Set(analysisIssueVisualizations, "some config scope"); + var receivedEventGetter = CaptureIssuesChangedEventArgs(); + + testSubject.Update(new TaintVulnerabilitiesUpdate("some config scope", [added], [updated], [toRemove.IssueId!.Value])); + + testSubject.GetAll().Should().BeEquivalentTo(added, updated, notTouched); + receivedEventGetter().Should().BeEquivalentTo(new IssuesChangedEventArgs([toUpdate, toRemove], [added, updated])); + } - var eventHandlerMock = Substitute.For>(); - testSubject.IssuesChanged += eventHandlerMock; + [TestMethod] + public void Update_CloseAndUpdateSameIssue_RemovesAndIgnoresUpdate() + { + var original = SetupIssueViz(); + var updated = SetupIssueViz(original.IssueId); + var remove = original.IssueId!.Value; + testSubject.Set([original], "some config scope"); + var receivedEventGetter = CaptureIssuesChangedEventArgs(); - testSubject.Add(SetupIssueViz()); + using (new AssertIgnoreScope()) + { + testSubject.Update(new TaintVulnerabilitiesUpdate("some config scope", [], [updated], [remove])); + } - eventHandlerMock.DidNotReceiveWithAnyArgs().Invoke(default, default); testSubject.GetAll().Should().BeEmpty(); + receivedEventGetter().Should().BeEquivalentTo(new IssuesChangedEventArgs([original], [])); } [TestMethod] - public void Add_HasConfigScope_IssueAddedAndEventIsRaised() + public void Update_UpdateAndAddSameIssue_UpdatesAndIgnoresAdd() { - var existingIssue = SetupIssueViz("key1"); - - testSubject.Set([existingIssue], "some config scope"); + var original = SetupIssueViz(); + var updated = SetupIssueViz(original.IssueId); + var add = SetupIssueViz(original.IssueId); + testSubject.Set([original], "some config scope"); + var receivedEventGetter = CaptureIssuesChangedEventArgs(); + + using (new AssertIgnoreScope()) + { + testSubject.Update(new TaintVulnerabilitiesUpdate("some config scope", [add], [updated], [])); + } + + testSubject.GetAll().Should().BeEquivalentTo(updated); + receivedEventGetter().Should().BeEquivalentTo(new IssuesChangedEventArgs([original], [updated])); + } - var eventHandlerMock = Substitute.For>(); - testSubject.IssuesChanged += eventHandlerMock; + [TestMethod] + public void Update_RemoveAndAddSameIssue_Updates() + { + var original = SetupIssueViz(); + var remove = original.IssueId!.Value; + var add = SetupIssueViz(original.IssueId); + testSubject.Set([original], "some config scope"); + var receivedEventGetter = CaptureIssuesChangedEventArgs(); - var newIssue = SetupIssueViz(); - testSubject.Add(newIssue); + testSubject.Update(new TaintVulnerabilitiesUpdate("some config scope", [add], [], [remove])); - eventHandlerMock.ReceivedWithAnyArgs(1).Invoke(default, default); - var eventArgs = (IssuesChangedEventArgs)eventHandlerMock.ReceivedCalls().Single().GetArguments()[1]!; - eventArgs.RemovedIssues.Should().BeEmpty(); - eventArgs.AddedIssues.Should().BeEquivalentTo(newIssue); - testSubject.GetAll().Should().BeEquivalentTo(existingIssue, newIssue); + testSubject.GetAll().Should().BeEquivalentTo(add); + receivedEventGetter().Should().BeEquivalentTo(new IssuesChangedEventArgs([original], [add])); } - [TestMethod] - public void Add_DuplicateIssue_IssueIgnoredAndNoEventIsRaised() + private Func CaptureIssuesChangedEventArgs() { - var issueKey = "key1"; - var existingIssue = SetupIssueViz(issueKey); - - testSubject.Set([existingIssue], "some config scope"); + IssuesChangedEventArgs receivedEvent = null; + var eventHandlerMock = CreateEventHandlerMock(); + eventHandlerMock.Invoke(Arg.Any(), Arg.Do(x => receivedEvent = x)); + return () => receivedEvent; + } + private EventHandler CreateEventHandlerMock() + { var eventHandlerMock = Substitute.For>(); testSubject.IssuesChanged += eventHandlerMock; - - var newIssue = SetupIssueViz(issueKey); - testSubject.Add(newIssue); - - eventHandlerMock.DidNotReceiveWithAnyArgs().Invoke(default, default); - testSubject.GetAll().Should().BeEquivalentTo(existingIssue); + return eventHandlerMock; } - private IAnalysisIssueVisualization SetupIssueViz(string issueKey = null) + private IAnalysisIssueVisualization SetupIssueViz(Guid? id = null) { - issueKey ??= Guid.NewGuid().ToString(); - - var taintIssue = Substitute.For(); - taintIssue.IssueKey.Returns(issueKey); + id ??= Guid.NewGuid(); var issueViz = Substitute.For(); - issueViz.Issue.Returns(taintIssue); + issueViz.IssueId.Returns(id.Value); return issueViz; } diff --git a/src/IssueViz.Security.UnitTests/Taint/TaintVulnerabilitiesUpdateTests.cs b/src/IssueViz.Security.UnitTests/Taint/TaintVulnerabilitiesUpdateTests.cs new file mode 100644 index 0000000000..81c704aec0 --- /dev/null +++ b/src/IssueViz.Security.UnitTests/Taint/TaintVulnerabilitiesUpdateTests.cs @@ -0,0 +1,78 @@ +/* + * SonarLint for Visual Studio + * Copyright (C) 2016-2024 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +using SonarLint.VisualStudio.IssueVisualization.Models; +using SonarLint.VisualStudio.IssueVisualization.Security.Taint; + +namespace SonarLint.VisualStudio.IssueVisualization.Security.UnitTests.Taint; + +[TestClass] +public class TaintVulnerabilitiesUpdateTests +{ + [DataTestMethod] + [DataRow(null)] + [DataRow("")] + public void Ctor_ConfigScope_NullOrEmpty_Throws(string configScope) + { + var act = () => new TaintVulnerabilitiesUpdate(configScope, [], [], []); + + act.Should().Throw().Which.ParamName.Should().Be("configurationScope"); + } + + [TestMethod] + public void Ctor_NullAdded_Throws() + { + var act = () => new TaintVulnerabilitiesUpdate("some config scope", null, [], []); + + act.Should().Throw().Which.ParamName.Should().Be("added"); + } + + [TestMethod] + public void Ctor_NullUpdated_Throws() + { + var act = () => new TaintVulnerabilitiesUpdate("some config scope", [], null, []); + + act.Should().Throw().Which.ParamName.Should().Be("updated"); + } + + [TestMethod] + public void Ctor_NullClosed_Throws() + { + var act = () => new TaintVulnerabilitiesUpdate("some config scope", [], [], null); + + act.Should().Throw().Which.ParamName.Should().Be("closed"); + } + + [TestMethod] + public void Ctor_SetsCorrectValues() + { + const string configScope = "some config scope"; + var added = new List(); + var updated = new List(); + var closed = new List(); + + var testSubject = new TaintVulnerabilitiesUpdate(configScope, added, updated, closed); + + testSubject.ConfigurationScope.Should().BeSameAs(configScope); + testSubject.Added.Should().BeSameAs(added); + testSubject.Updated.Should().BeSameAs(updated); + testSubject.Closed.Should().BeSameAs(closed); + } +} diff --git a/src/IssueViz.Security/IssuesStore/IIssuesStore.cs b/src/IssueViz.Security/IssuesStore/IIssuesStore.cs index 71956590cc..1e02a41db5 100644 --- a/src/IssueViz.Security/IssuesStore/IIssuesStore.cs +++ b/src/IssueViz.Security/IssuesStore/IIssuesStore.cs @@ -28,17 +28,17 @@ namespace SonarLint.VisualStudio.IssueVisualization.Security.IssuesStore /// Stores a collection of and /// raises event when the collection is modified. /// - internal interface IIssuesStore + public interface IIssuesStore { IReadOnlyCollection GetAll(); event EventHandler IssuesChanged; } - internal class IssuesChangedEventArgs + public class IssuesChangedEventArgs { public IReadOnlyCollection RemovedIssues { get; } - + public IReadOnlyCollection AddedIssues { get; } public IssuesChangedEventArgs(IReadOnlyCollection removedIssues, IReadOnlyCollection addedIssues) diff --git a/src/IssueViz.Security/Taint/TaintIssueToIssueVisualizationConverter.cs b/src/IssueViz.Security/Taint/TaintIssueToIssueVisualizationConverter.cs index 0b25185ad1..1a49b888b3 100644 --- a/src/IssueViz.Security/Taint/TaintIssueToIssueVisualizationConverter.cs +++ b/src/IssueViz.Security/Taint/TaintIssueToIssueVisualizationConverter.cs @@ -29,7 +29,7 @@ namespace SonarLint.VisualStudio.IssueVisualization.Security.Taint; -internal interface ITaintIssueToIssueVisualizationConverter +public interface ITaintIssueToIssueVisualizationConverter { IAnalysisIssueVisualization Convert(TaintVulnerabilityDto slcoreTaintIssue, string configScopeRoot); } diff --git a/src/IssueViz.Security/Taint/TaintIssuesSynchronizer.cs b/src/IssueViz.Security/Taint/TaintIssuesSynchronizer.cs index 5dccbe2476..d0ba166cfc 100644 --- a/src/IssueViz.Security/Taint/TaintIssuesSynchronizer.cs +++ b/src/IssueViz.Security/Taint/TaintIssuesSynchronizer.cs @@ -172,12 +172,10 @@ private bool IsConnectedModeConfigScope(ConfigurationScope configurationScope) private void HandleNoTaintIssues() { - ClearStore(); + taintStore.Reset(); UpdateTaintIssuesUIContext(false); } - private void ClearStore() => taintStore.Set([], null); - private void UpdateTaintIssuesUIContext(bool hasTaintIssues) => vSServiceOperation.Execute( monitorSelection => diff --git a/src/IssueViz.Security/Taint/TaintStore.cs b/src/IssueViz.Security/Taint/TaintStore.cs index 35db00dd10..014bee7d35 100644 --- a/src/IssueViz.Security/Taint/TaintStore.cs +++ b/src/IssueViz.Security/Taint/TaintStore.cs @@ -21,31 +21,44 @@ using System.ComponentModel.Composition; using SonarLint.VisualStudio.IssueVisualization.Models; using SonarLint.VisualStudio.IssueVisualization.Security.IssuesStore; -using SonarLint.VisualStudio.IssueVisualization.Security.Taint.Models; namespace SonarLint.VisualStudio.IssueVisualization.Security.Taint { - internal interface ITaintStore : IIssuesStore + public interface ITaintStore : IIssuesStore { /// - /// Removes all existing visualizations and initializes the store to the given collection. + /// Removes all existing visualizations and initializes the store to the given collection & configuration scope. /// Can be called multiple times. /// void Set(IReadOnlyCollection issueVisualizations, string newConfigurationScope); - string ConfigurationScope { get; } + /// + /// Removes all existing visualizations and resets the configurations scope. + /// Can be called multiple times. + /// + void Reset(); /// - /// Add the given issue to the existing list of visualizations. - /// If is null, the operation is ignored. + /// Returns current configuration scope id. Null if store is Reset /// - void Add(IAnalysisIssueVisualization issueVisualization); + string ConfigurationScope { get; } /// - /// Removes an issue with the given key from the existing list of visualizations. - /// If no matching issue is found, the operation is ignored. + /// Applies updates to current store. If store is Reset or configuration scope is different, update is ignored. /// - void Remove(string issueKey); + void Update(TaintVulnerabilitiesUpdate taintVulnerabilitiesUpdate); + } + + public class TaintVulnerabilitiesUpdate( + string configurationScope, + IEnumerable added, + IEnumerable updated, + IEnumerable closed) + { + public string ConfigurationScope { get; } = !string.IsNullOrEmpty(configurationScope) ? configurationScope : throw new ArgumentNullException(nameof(configurationScope)); + public IEnumerable Added { get; } = added ?? throw new ArgumentNullException(nameof(added)); + public IEnumerable Updated { get; } = updated ?? throw new ArgumentNullException(nameof(updated)); + public IEnumerable Closed { get; } = closed ?? throw new ArgumentNullException(nameof(closed)); } [Export(typeof(ITaintStore))] @@ -55,157 +68,162 @@ internal sealed class TaintStore : ITaintStore { public event EventHandler IssuesChanged; - private readonly object locker = new object(); + private readonly object locker = new(); private string configurationScope; - private List taintVulnerabilities = new List(); + private Dictionary taintVulnerabilities = new(); - public IReadOnlyCollection GetAll() + + public string ConfigurationScope { - lock (locker) + get { - return taintVulnerabilities.ToList(); + lock (locker) + { + return configurationScope; + } } } - public void Add(IAnalysisIssueVisualization issueVisualization) + public IReadOnlyCollection GetAll() { - if (issueVisualization == null) - { - throw new ArgumentNullException(nameof(issueVisualization)); - } - lock (locker) { - if (configurationScope == null) - { - return; - } - - if (taintVulnerabilities.Contains(issueVisualization, TaintAnalysisIssueVisualizationByIssueKeyEqualityComparer.Instance)) - { - return; - } - - taintVulnerabilities.Add(issueVisualization); - + return taintVulnerabilities.Values.ToList(); } - - NotifyIssuesChanged([], [issueVisualization]); } - public void Remove(string issueKey) + public void Reset() => + SetInternal([], null); + + public void Set(IReadOnlyCollection issueVisualizations, string newConfigurationScope) { - if (issueKey == null) - { - throw new ArgumentNullException(nameof(issueKey)); - } + ValidateSet(issueVisualizations, newConfigurationScope); + SetInternal(issueVisualizations, newConfigurationScope); + } - IAnalysisIssueVisualization valueToRemove; + private void SetInternal(IReadOnlyCollection issueVisualizations, string newConfigurationScope) + { + List diffRemoved = []; + List diffAdded = []; lock (locker) { - if (configurationScope == null) - { - return; - } - - var indexToRemove = - taintVulnerabilities.FindIndex(issueViz => ((ITaintIssue)issueViz.Issue).IssueKey.Equals(issueKey)); - - if (indexToRemove == -1) - { - return; - } - - valueToRemove = taintVulnerabilities[indexToRemove]; - taintVulnerabilities.RemoveAt(indexToRemove); + var oldVulnerabilities = taintVulnerabilities; + taintVulnerabilities = issueVisualizations.ToDictionary(x => x.IssueId!.Value, x => x); + configurationScope = newConfigurationScope; + diffRemoved.AddRange(oldVulnerabilities.Values); + diffAdded.AddRange(taintVulnerabilities.Values); } - NotifyIssuesChanged([valueToRemove], []); + NotifyIssuesChanged(diffRemoved, diffAdded); } - public void Set(IReadOnlyCollection issueVisualizations, string newConfigurationScope) + private static void ValidateSet(IReadOnlyCollection issueVisualizations, string newConfigurationScope) { if (issueVisualizations == null) { throw new ArgumentNullException(nameof(issueVisualizations)); } - if (issueVisualizations.Count > 0 && newConfigurationScope == null) + Debug.Assert(issueVisualizations.All(x => x.IssueId.HasValue)); + + if (string.IsNullOrEmpty(newConfigurationScope)) { throw new ArgumentNullException(nameof(newConfigurationScope)); } + } - IAnalysisIssueVisualization[] removedIssues; - IAnalysisIssueVisualization[] addedIssues; + public void Update(TaintVulnerabilitiesUpdate taintVulnerabilitiesUpdate) + { + ValidateUpdate(taintVulnerabilitiesUpdate); + List diffAdded = []; + List diffRemoved = []; lock (locker) { - var oldIssues = taintVulnerabilities; - taintVulnerabilities = issueVisualizations.ToList(); - configurationScope = newConfigurationScope; - + if (taintVulnerabilitiesUpdate.ConfigurationScope != configurationScope) + { + return; + } - removedIssues = oldIssues.Except(taintVulnerabilities, TaintAnalysisIssueVisualizationByIssueKeyEqualityComparer.Instance).ToArray(); - addedIssues = taintVulnerabilities.Except(oldIssues, TaintAnalysisIssueVisualizationByIssueKeyEqualityComparer.Instance).ToArray(); + HandleClosed(taintVulnerabilitiesUpdate.Closed, diffAdded); + HandleUpdated(taintVulnerabilitiesUpdate.Updated, diffRemoved, diffAdded); + HandleAdded(taintVulnerabilitiesUpdate.Added, diffRemoved); } - NotifyIssuesChanged(removedIssues, addedIssues); + NotifyIfIssuesChanged(diffAdded, diffRemoved); } - public string ConfigurationScope + private void NotifyIfIssuesChanged(List diffAdded, List diffRemoved) { - get + if (diffAdded.Count != 0 || diffRemoved.Count != 0) { - lock (locker) - { - return configurationScope; - } + NotifyIssuesChanged(diffAdded, diffRemoved); } } - private void NotifyIssuesChanged( - IReadOnlyCollection removedIssues, - IReadOnlyCollection addedIssues) + private static void ValidateUpdate(TaintVulnerabilitiesUpdate taintVulnerabilitiesUpdate) { - // Hacky workaround for #4066 - always raise the event, even if - // the set of added/removed files is empty. - // See also #4070. - IssuesChanged?.Invoke(this, new IssuesChangedEventArgs(removedIssues, addedIssues)); + if (taintVulnerabilitiesUpdate == null) + { + throw new ArgumentNullException(nameof(taintVulnerabilitiesUpdate)); + } + + Debug.Assert(taintVulnerabilitiesUpdate.Added.All(x => x.IssueId.HasValue)); + Debug.Assert(taintVulnerabilitiesUpdate.Updated.All(x => x.IssueId.HasValue)); } - private sealed class TaintAnalysisIssueVisualizationByIssueKeyEqualityComparer : IEqualityComparer + private void HandleAdded(IEnumerable added, List diffAdded) { - public static readonly TaintAnalysisIssueVisualizationByIssueKeyEqualityComparer Instance = - new TaintAnalysisIssueVisualizationByIssueKeyEqualityComparer(); - - private TaintAnalysisIssueVisualizationByIssueKeyEqualityComparer(){} - - public bool Equals(IAnalysisIssueVisualization first, IAnalysisIssueVisualization second) + foreach (var addedVulnerability in added) { - if (ReferenceEquals(first, second)) + if (taintVulnerabilities.ContainsKey(addedVulnerability.IssueId!.Value)) { - return true; + Debug.Fail("Taint Update: attempting to add a Vulnerability with the same id that already exists"); + continue; } + taintVulnerabilities[addedVulnerability.IssueId!.Value] = addedVulnerability; + diffAdded.Add(addedVulnerability); + } + } - if (first == null || second == null) + private void HandleUpdated(IEnumerable updated, List diffRemoved, List diffAdded) + { + foreach (var updatedVulnerability in updated) + { + if (!taintVulnerabilities.TryGetValue(updatedVulnerability.IssueId!.Value, out var outdatedVulnerability)) { - return false; + Debug.Fail("Taint Update: attempting to update a non-existent Vulnerability"); + continue; } - - var firstTaintIssue = (ITaintIssue)first.Issue; - var secondTaintIssue = (ITaintIssue)second.Issue; - - return firstTaintIssue.IssueKey.Equals(secondTaintIssue.IssueKey); + taintVulnerabilities[updatedVulnerability.IssueId!.Value] = updatedVulnerability; + diffRemoved.Add(outdatedVulnerability); + diffAdded.Add(updatedVulnerability); } + } - - public int GetHashCode(IAnalysisIssueVisualization obj) + private void HandleClosed(IEnumerable removed, List diffRemoved) + { + foreach (var removedId in removed) { - return (obj.Issue != null ? ((ITaintIssue)obj.Issue).IssueKey.GetHashCode() : 0); + if (!taintVulnerabilities.TryGetValue(removedId, out var removedVulnerability)) + { + Debug.Fail("Taint Update: attempting to remove a non-existent Vulnerability"); + continue; + } + taintVulnerabilities.Remove(removedId); + diffRemoved.Add(removedVulnerability); } } + + private void NotifyIssuesChanged( + IReadOnlyCollection removedIssues, + IReadOnlyCollection addedIssues) => + // Hacky workaround for #4066 - always raise the event, even if + // the set of added/removed files is empty. + // See also #4070. + IssuesChanged?.Invoke(this, new IssuesChangedEventArgs(removedIssues, addedIssues)); } } diff --git a/src/SLCore.IntegrationTests/packages.lock.json b/src/SLCore.IntegrationTests/packages.lock.json index c3623dbf7e..c7bced753d 100644 --- a/src/SLCore.IntegrationTests/packages.lock.json +++ b/src/SLCore.IntegrationTests/packages.lock.json @@ -1722,6 +1722,7 @@ "type": "Project", "dependencies": { "SonarLint.VisualStudio.ConnectedMode": "[1.0.0, )", + "SonarLint.VisualStudio.IssueVisualization.Security": "[1.0.0, )", "SonarLint.VisualStudio.SLCore": "[1.0.0, )" } }, diff --git a/src/SLCore.Listeners.UnitTests/Implementation/TaintVulnerabilityListenerTests.cs b/src/SLCore.Listeners.UnitTests/Implementation/TaintVulnerabilityListenerTests.cs index 4b00ae86ef..cd58a31626 100644 --- a/src/SLCore.Listeners.UnitTests/Implementation/TaintVulnerabilityListenerTests.cs +++ b/src/SLCore.Listeners.UnitTests/Implementation/TaintVulnerabilityListenerTests.cs @@ -18,17 +18,107 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using SonarLint.VisualStudio.IssueVisualization.Models; +using SonarLint.VisualStudio.IssueVisualization.Security.Taint; +using SonarLint.VisualStudio.SLCore.Common.Models; using SonarLint.VisualStudio.SLCore.Core; using SonarLint.VisualStudio.SLCore.Listener.Taint; +using SonarLint.VisualStudio.SLCore.Service.Rules.Models; +using SonarLint.VisualStudio.SLCore.State; namespace SonarLint.VisualStudio.SLCore.Listeners.UnitTests.Implementation; [TestClass] public class TaintVulnerabilityListenerTests { + private TaintVulnerabilityListener testSubject; + private ITaintStore taintStore; + private ITaintIssueToIssueVisualizationConverter converter; + private IActiveConfigScopeTracker configScopeTracker; + [TestMethod] - public void MefCtor_CheckIsExported() => MefTestHelpers.CheckTypeCanBeImported(); + public void MefCtor_CheckIsExported() => + MefTestHelpers.CheckTypeCanBeImported( + MefTestHelpers.CreateExport(), + MefTestHelpers.CreateExport(), + MefTestHelpers.CreateExport()); [TestMethod] public void MefCtor_CheckIsSingleton() => MefTestHelpers.CheckIsSingletonMefComponent(); + + [TestInitialize] + public void TestInitialize() + { + taintStore = Substitute.For(); + converter = Substitute.For(); + configScopeTracker = Substitute.For(); + testSubject = new TaintVulnerabilityListener(taintStore, converter, configScopeTracker); + } + + [TestMethod] + public void DidChangeTaintVulnerabilities_MismatchingConfiguration_Ignores() + { + configScopeTracker.Current.Returns(new ConfigurationScope("Some Config Scope")); + + testSubject.DidChangeTaintVulnerabilities(new ("Some Other Config Scope", [], [], [])); + + taintStore.ReceivedCalls().Should().BeEmpty(); + } + + [TestMethod] + public void DidChangeTaintVulnerabilities_CorrectConfiguration_CallsTaintStore() + { + const string rootPath = "some/root"; + const string configurationScopeId = "Some Config Scope"; + configScopeTracker.Current.Returns(new ConfigurationScope(configurationScopeId, RootPath: rootPath)); + var (addedDto1, addedVisualization1) = SetUpDtoConversion(rootPath); + var (addedDto2, addedVisualization2) = SetUpDtoConversion(rootPath); + var (updatedDto1, updatedVisualization1) = SetUpDtoConversion(rootPath); + var closedId = Guid.NewGuid(); + + testSubject.DidChangeTaintVulnerabilities(new (configurationScopeId, [closedId], [addedDto1, addedDto2], [updatedDto1])); + + _ = configScopeTracker.Received(1).Current; + taintStore.Received(1).Update(Arg.Any()); + taintStore + .ReceivedCalls() + .Single() + .GetArguments()[0] + .Should() + .BeOfType() + .Which + .Should() + .BeEquivalentTo(new TaintVulnerabilitiesUpdate(configurationScopeId, [addedVisualization1, addedVisualization2], [updatedVisualization1], [closedId]), + options => options.ComparingByMembers()); + } + + private (TaintVulnerabilityDto dto, IAnalysisIssueVisualization visualization) SetUpDtoConversion(string rootPath) + { + var dto = CreateDefaultTaintDto(); + var visualization = CreateAnalysisIssueVisualization(); + converter.Convert(dto, rootPath).Returns(visualization); + return (dto, visualization); + } + + private static IAnalysisIssueVisualization CreateAnalysisIssueVisualization() + { + var mock = Substitute.For(); + mock.IssueId.Returns(Guid.NewGuid()); + return mock; + } + + private static TaintVulnerabilityDto CreateDefaultTaintDto() => + new( + Guid.Parse("efa697a2-9cfd-4faf-ba21-71b378667a81"), + "serverkey", + true, + "rulekey:S123", + "message1", + "file\\path\\1", + DateTimeOffset.Now, + new StandardModeDetails(IssueSeverity.MINOR, RuleType.VULNERABILITY), + [], + new TextRangeWithHashDto(1, 2, 3, 4, "hash1"), + "rulecontext", + false); } diff --git a/src/SLCore.Listeners.UnitTests/packages.lock.json b/src/SLCore.Listeners.UnitTests/packages.lock.json index 5d36b29a65..bfd1128f2b 100644 --- a/src/SLCore.Listeners.UnitTests/packages.lock.json +++ b/src/SLCore.Listeners.UnitTests/packages.lock.json @@ -315,6 +315,23 @@ "resolved": "1.1.3", "contentHash": "3Wrmi0kJDzClwAC+iBdUBpEKmEle8FQNsCs77fkiOIw/9oYA07bL1EZNX0kQ2OMN3xpwvl0vAtOCYY3ndDNlhQ==" }, + "Microsoft.Owin": { + "type": "Transitive", + "resolved": "4.2.2", + "contentHash": "jt410l/8dvCIguRdU7dupYdm4kGLepVdD8EOTKU4nYZcLRrn6kQYqI6pbJOTJp7Vlm/T2WUF5bzyKK2z29xtjg==", + "dependencies": { + "Owin": "1.0.0" + } + }, + "Microsoft.Owin.Host.HttpListener": { + "type": "Transitive", + "resolved": "4.2.2", + "contentHash": "Kl1A0sBzfMD3qvX6XcGU0FopN6POFFRpIEQnKIAbvsShadIG9/UxgDdHVlX/IzFHXwIHfY59Ae4RGDVKYNvIqQ==", + "dependencies": { + "Microsoft.Owin": "4.2.2", + "Owin": "1.0.0" + } + }, "Microsoft.ServiceHub.Analyzers": { "type": "Transitive", "resolved": "3.0.3078", @@ -1049,6 +1066,11 @@ "resolved": "3.3.0", "contentHash": "ZVBJ43/IxaAl5iX+OuuDLQ9tSNJaRlP85SUMAdiGoqEbyK/ZPfQmPfqSMZMFpucRlf8HgsEK/hR73DwpCmqcLA==" }, + "Owin": { + "type": "Transitive", + "resolved": "1.0.0", + "contentHash": "OseTFniKmyp76mEzOBwIKGBRS5eMoYNkMKaMXOpxx9jv88+b6mh1rSaw43vjBOItNhaLFG3d0a20PfHyibH5sw==" + }, "stdole": { "type": "Transitive", "resolved": "17.0.31902.203", @@ -1588,6 +1610,17 @@ "SonarQube.Client": "[1.0.0, )" } }, + "SonarLint.VisualStudio.IssueVisualization.Security": { + "type": "Project", + "dependencies": { + "Microsoft.Owin": "[4.2.2, )", + "Microsoft.Owin.Host.HttpListener": "[4.2.2, )", + "Microsoft.VisualStudio.Sdk": "[17.0.31902.203, )", + "SonarLint.VisualStudio.ConnectedMode": "[1.0.0, )", + "SonarLint.VisualStudio.Core": "[1.0.0, )", + "SonarLint.VisualStudio.IssueVisualization": "[1.0.0, )" + } + }, "SonarLint.VisualStudio.Progress": { "type": "Project" }, @@ -1609,6 +1642,7 @@ "type": "Project", "dependencies": { "SonarLint.VisualStudio.ConnectedMode": "[1.0.0, )", + "SonarLint.VisualStudio.IssueVisualization.Security": "[1.0.0, )", "SonarLint.VisualStudio.SLCore": "[1.0.0, )" } }, diff --git a/src/SLCore.Listeners/Implementation/TaintVulnerabilityListener.cs b/src/SLCore.Listeners/Implementation/TaintVulnerabilityListener.cs index f983ae055f..b6de28297a 100644 --- a/src/SLCore.Listeners/Implementation/TaintVulnerabilityListener.cs +++ b/src/SLCore.Listeners/Implementation/TaintVulnerabilityListener.cs @@ -19,25 +19,34 @@ */ using System.ComponentModel.Composition; -using Newtonsoft.Json; -using SonarLint.VisualStudio.Core; +using SonarLint.VisualStudio.IssueVisualization.Security.Taint; using SonarLint.VisualStudio.SLCore.Core; using SonarLint.VisualStudio.SLCore.Listener.Taint; +using SonarLint.VisualStudio.SLCore.State; namespace SonarLint.VisualStudio.SLCore.Listeners.Implementation; [Export(typeof(ISLCoreListener))] [PartCreationPolicy(CreationPolicy.Shared)] -public class TaintVulnerabilityListener : ITaintVulnerabilityListener +[method: ImportingConstructor] +public class TaintVulnerabilityListener( + ITaintStore taintStore, + ITaintIssueToIssueVisualizationConverter converter, + IActiveConfigScopeTracker activeConfigScopeTracker) + : ITaintVulnerabilityListener { - - [ImportingConstructor] - public TaintVulnerabilityListener() - { - } - public void DidChangeTaintVulnerabilities(DidChangeTaintVulnerabilitiesParams parameters) { - // todo https://sonarsource.atlassian.net/browse/SLVS-1592 + var configurationScope = activeConfigScopeTracker.Current; + if (configurationScope is not {Id: {} currentId, RootPath: {} currentRootPath} || currentId != parameters.configurationScopeId) + { + return; + } + + taintStore.Update( + new TaintVulnerabilitiesUpdate(currentId, + parameters.addedTaintVulnerabilities.Select(x => converter.Convert(x, currentRootPath)), + parameters.updatedTaintVulnerabilities.Select(x => converter.Convert(x, currentRootPath)), + parameters.closedTaintVulnerabilityIds)); } } diff --git a/src/SLCore.Listeners/SLCore.Listeners.csproj b/src/SLCore.Listeners/SLCore.Listeners.csproj index 61a49635b0..adcdc92ee2 100644 --- a/src/SLCore.Listeners/SLCore.Listeners.csproj +++ b/src/SLCore.Listeners/SLCore.Listeners.csproj @@ -9,6 +9,7 @@ + diff --git a/src/SLCore.Listeners/packages.lock.json b/src/SLCore.Listeners/packages.lock.json index 65262be2c2..f8b478d983 100644 --- a/src/SLCore.Listeners/packages.lock.json +++ b/src/SLCore.Listeners/packages.lock.json @@ -226,6 +226,23 @@ "resolved": "1.1.3", "contentHash": "3Wrmi0kJDzClwAC+iBdUBpEKmEle8FQNsCs77fkiOIw/9oYA07bL1EZNX0kQ2OMN3xpwvl0vAtOCYY3ndDNlhQ==" }, + "Microsoft.Owin": { + "type": "Transitive", + "resolved": "4.2.2", + "contentHash": "jt410l/8dvCIguRdU7dupYdm4kGLepVdD8EOTKU4nYZcLRrn6kQYqI6pbJOTJp7Vlm/T2WUF5bzyKK2z29xtjg==", + "dependencies": { + "Owin": "1.0.0" + } + }, + "Microsoft.Owin.Host.HttpListener": { + "type": "Transitive", + "resolved": "4.2.2", + "contentHash": "Kl1A0sBzfMD3qvX6XcGU0FopN6POFFRpIEQnKIAbvsShadIG9/UxgDdHVlX/IzFHXwIHfY59Ae4RGDVKYNvIqQ==", + "dependencies": { + "Microsoft.Owin": "4.2.2", + "Owin": "1.0.0" + } + }, "Microsoft.ServiceHub.Analyzers": { "type": "Transitive", "resolved": "3.0.3078", @@ -910,11 +927,6 @@ "resolved": "12.0.0", "contentHash": "HeuaZh8+wNVdwx7VF8guFGH2Z2zH+FYxWBsRNp+FjjlmrhCfM7GUQV5azaTv/bN5TPaK8ALJoP9UX5o1FB5k1A==" }, - "Microsoft.Web.Xdt": { - "type": "Transitive", - "resolved": "2.1.0", - "contentHash": "/ieJ02r4MEJM21Eyl+c5kwoJWPhy+qEEcN68JaqDoamabgxJI1jGi/kNLuKvHUCl6tU7E0rMvaR6FmEnDWtS4A==" - }, "Microsoft.Win32.Primitives": { "type": "Transitive", "resolved": "4.3.0", @@ -947,18 +959,10 @@ "resolved": "13.0.3", "contentHash": "HrC5BXdl00IP9zeV+0Z848QWPAoCr9P3bDEZguI+gkLcBKAOxix/tLEAAHC+UvDNPv4a2d18lOReHMOagPa+zQ==" }, - "NuGet.Core": { - "type": "Transitive", - "resolved": "2.12.0", - "contentHash": "kSDD1VFIq7BBrimjMRk9HeeQlZteyknbHgz3AT4AUVUCZ8BZaCSzO1A5UTVPLLbSHryjozPcEmNYSds/IDAIjw==", - "dependencies": { - "Microsoft.Web.Xdt": "2.1.0" - } - }, - "NuGet.VisualStudio": { + "Owin": { "type": "Transitive", - "resolved": "3.3.0", - "contentHash": "ZVBJ43/IxaAl5iX+OuuDLQ9tSNJaRlP85SUMAdiGoqEbyK/ZPfQmPfqSMZMFpucRlf8HgsEK/hR73DwpCmqcLA==" + "resolved": "1.0.0", + "contentHash": "OseTFniKmyp76mEzOBwIKGBRS5eMoYNkMKaMXOpxx9jv88+b6mh1rSaw43vjBOItNhaLFG3d0a20PfHyibH5sw==" }, "stdole": { "type": "Transitive", @@ -1470,24 +1474,6 @@ "SonarLint.VisualStudio.Core": "[1.0.0, )" } }, - "SonarLint.VisualStudio.Integration": { - "type": "Project", - "dependencies": { - "Microsoft.Alm.Authentication": "[4.0.0.1, )", - "Microsoft.VisualStudio.Sdk": "[17.0.31902.203, )", - "Newtonsoft.Json": "[13.0.3, )", - "NuGet.Core": "[2.12.0, )", - "NuGet.VisualStudio": "[3.3.0, )", - "SonarLint.VisualStudio.ConnectedMode": "[1.0.0, )", - "SonarLint.VisualStudio.Core": "[1.0.0, )", - "SonarLint.VisualStudio.Infrastructure.VS": "[1.0.0, )", - "SonarLint.VisualStudio.Progress": "[1.0.0, )", - "SonarLint.VisualStudio.ProgressVS": "[1.0.0, )", - "SonarQube.Client": "[1.0.0, )", - "StrongNamer": "[0.0.8, )", - "System.IO.Abstractions": "[9.0.4, )" - } - }, "SonarLint.VisualStudio.IssueVisualization": { "type": "Project", "dependencies": { @@ -1498,14 +1484,15 @@ "SonarQube.Client": "[1.0.0, )" } }, - "SonarLint.VisualStudio.Progress": { - "type": "Project" - }, - "SonarLint.VisualStudio.ProgressVS": { + "SonarLint.VisualStudio.IssueVisualization.Security": { "type": "Project", "dependencies": { + "Microsoft.Owin": "[4.2.2, )", + "Microsoft.Owin.Host.HttpListener": "[4.2.2, )", "Microsoft.VisualStudio.Sdk": "[17.0.31902.203, )", - "SonarLint.VisualStudio.Progress": "[1.0.0, )" + "SonarLint.VisualStudio.ConnectedMode": "[1.0.0, )", + "SonarLint.VisualStudio.Core": "[1.0.0, )", + "SonarLint.VisualStudio.IssueVisualization": "[1.0.0, )" } }, "SonarLint.VisualStudio.SLCore": {