On update taint with changed id, fallback to match by server key

SonarSource · Nov 22, 2024 · 939fed7 · 939fed7
1 parent 89a7efa
commit 939fed7
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 4 deletions.
diff --git a/src/IssueViz.Security.UnitTests/Taint/TaintStoreTests.cs b/src/IssueViz.Security.UnitTests/Taint/TaintStoreTests.cs
@@ -20,11 +20,11 @@
 
 using System.ComponentModel.Composition;
 using System.ComponentModel.Composition.Hosting;
-using SonarLint.VisualStudio.TestInfrastructure;
 using SonarLint.VisualStudio.IssueVisualization.Models;
 using SonarLint.VisualStudio.IssueVisualization.Security.IssuesStore;
 using SonarLint.VisualStudio.IssueVisualization.Security.Taint;
 using SonarLint.VisualStudio.IssueVisualization.Security.Taint.Models;
+using SonarLint.VisualStudio.TestInfrastructure;
 
 namespace SonarLint.VisualStudio.IssueVisualization.Security.UnitTests.Taint;
 
@@ -326,6 +326,26 @@ public void Update_UpdatedIssues_NotPresent_Ignored()
         eventHandlerMock.DidNotReceiveWithAnyArgs().Invoke(default, default);
     }
 
+    [TestMethod]
+    public void Update_UpdatedIssues_ChangedId_MatchedByIssueKeyAndReplaced()
+    {
+        const string serverKey = "taint-1";
+        var taintWithChangedId = SetupIssueViz(issueKey: serverKey);
+        List<IAnalysisIssueVisualization> analysisIssueVisualizations = [taintWithChangedId, SetupIssueViz(), SetupIssueViz()];
+        var updated1 = SetupIssueViz(issueKey: serverKey);
+        testSubject.Set(analysisIssueVisualizations, "some config scope");
+        var receivedEventGetter = CaptureIssuesChangedEventArgs();
+
+        using (new AssertIgnoreScope())
+        {
+            testSubject.Update(new TaintVulnerabilitiesUpdate("some config scope", [], [updated1], []));
+        }
+
+        testSubject.GetAll().Should().NotContain(taintWithChangedId);
+        testSubject.GetAll().Should().Contain(updated1);
+        receivedEventGetter().Should().BeEquivalentTo(new IssuesChangedEventArgs([updated1], [taintWithChangedId]));
+    }
+
     [TestMethod]
     public void Update_AddedIssues_Adds()
     {
@@ -461,12 +481,16 @@ private EventHandler<IssuesChangedEventArgs> CreateEventHandlerMock()
         return eventHandlerMock;
     }
 
-    private IAnalysisIssueVisualization SetupIssueViz(Guid? id = null)
+    private static IAnalysisIssueVisualization SetupIssueViz(Guid? id = null, string issueKey = null)
     {
         id ??= Guid.NewGuid();
+        issueKey ??= Guid.NewGuid().ToString();
 
         var issueViz = Substitute.For<IAnalysisIssueVisualization>();
         issueViz.IssueId.Returns(id.Value);
+        var taintIssue = Substitute.For<ITaintIssue>();
+        taintIssue.IssueKey.Returns(issueKey);
+        issueViz.Issue.Returns(taintIssue);
 
         return issueViz;
     }

diff --git a/src/IssueViz.Security/Taint/TaintStore.cs b/src/IssueViz.Security/Taint/TaintStore.cs
@@ -21,6 +21,7 @@
 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
 {
@@ -193,9 +194,9 @@ private void HandleUpdated(IEnumerable<IAnalysisIssueVisualization> updated, Lis
         {
             foreach (var updatedVulnerability in updated)
             {
-                if (!taintVulnerabilities.TryGetValue(updatedVulnerability.IssueId!.Value, out var outdatedVulnerability))
+                var outdatedVulnerability = MatchToCached(updatedVulnerability);
+                if (outdatedVulnerability == null)
                 {
-                    Debug.Fail("Taint Update: attempting to update a non-existent Vulnerability");
                     continue;
                 }
                 taintVulnerabilities[updatedVulnerability.IssueId!.Value] = updatedVulnerability;
@@ -225,5 +226,32 @@ private void NotifyIssuesChanged(
             // the set of added/removed files is empty.
             // See also #4070.
             IssuesChanged?.Invoke(this, new IssuesChangedEventArgs(removedIssues, addedIssues));
+
+        private IAnalysisIssueVisualization MatchToCached(IAnalysisIssueVisualization taintVulnerability)
+        {
+            if (taintVulnerabilities.TryGetValue(taintVulnerability.IssueId!.Value, out var outdatedVulnerabilityByIssueId))
+            {
+                return outdatedVulnerabilityByIssueId;
+            }
+
+            var outdatedVulnerabilityByServerKey = MatchByServerKey(taintVulnerability);
+            if (outdatedVulnerabilityByServerKey != null)
+            {
+                // Taint was not found by issue id, but was found by server key
+                // This is a workaround for situations when the issue id randomly changes
+                // In this case, remove the cached non-existing one to re-align the cache with the new one
+                taintVulnerabilities.Remove(outdatedVulnerabilityByServerKey.IssueId!.Value);
+                return outdatedVulnerabilityByServerKey;
+            }
+
+            Debug.Fail("Taint Update: attempting to update a non-existent Vulnerability");
+            return null;
+        }
+
+        private IAnalysisIssueVisualization MatchByServerKey(IAnalysisIssueVisualization taintVulnerability) =>
+            taintVulnerabilities
+                .Where(x => ((ITaintIssue) x.Value.Issue).IssueKey == ((ITaintIssue) taintVulnerability.Issue).IssueKey)
+                .Select(x => x.Value)
+                .FirstOrDefault();
     }
 }