SONAR-20424 - Support issue impact changes for incremental analysis

2 files changed, 102 insertions, 53 deletions

diff --git a/server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorIT.java b/server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorIT.java
index fd2da91f154..9ffc8136d57 100644
--- a/server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorIT.java
+++ b/server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorIT.java
@@ -21,6 +21,7 @@ package org.sonar.ce.task.projectanalysis.issue;
 
 import java.util.Date;
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import org.jetbrains.annotations.NotNull;
@@ -33,8 +34,10 @@ import org.sonar.api.config.internal.MapSettings;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rule.Severity;
 import org.sonar.api.utils.System2;
+import org.sonar.ce.task.projectanalysis.analysis.Analysis;
 import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder;
 import org.sonar.ce.task.projectanalysis.analysis.Branch;
+import org.sonar.ce.task.projectanalysis.analysis.ScannerPlugin;
 import org.sonar.ce.task.projectanalysis.batch.BatchReportReaderRule;
 import org.sonar.ce.task.projectanalysis.component.Component;
 import org.sonar.ce.task.projectanalysis.component.FileStatuses;
@@ -49,7 +52,6 @@ import org.sonar.ce.task.projectanalysis.qualityprofile.ActiveRulesHolderRule;
 import org.sonar.ce.task.projectanalysis.qualityprofile.AlwaysActiveRulesHolderImpl;
 import org.sonar.ce.task.projectanalysis.source.NewLinesRepository;
 import org.sonar.ce.task.projectanalysis.source.SourceLinesHashRepository;
-import org.sonar.ce.task.projectanalysis.source.SourceLinesRepository;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.FieldDiffs;
 import org.sonar.core.issue.IssueChangeContext;
@@ -71,10 +73,12 @@ import org.sonar.server.issue.workflow.IssueWorkflow;
 import static com.google.common.collect.Lists.newArrayList;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.core.api.Assertions.entry;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -126,16 +130,12 @@ public class IntegrateIssuesVisitorIT {
   private final SourceLinesHashRepository sourceLinesHash = mock(SourceLinesHashRepository.class);
   private final NewLinesRepository newLinesRepository = mock(NewLinesRepository.class);
   private final TargetBranchComponentUuids targetBranchComponentUuids = mock(TargetBranchComponentUuids.class);
-  private final SourceLinesRepository sourceLinesRepository = mock(SourceLinesRepository.class);
   private final FileStatuses fileStatuses = mock(FileStatuses.class);
+  private TrackerRawInputFactory rawInputFactory;
   private ArgumentCaptor<DefaultIssue> defaultIssueCaptor;
 
   private final ComponentIssuesLoader issuesLoader = new ComponentIssuesLoader(dbTester.getDbClient(), ruleRepositoryRule, activeRulesHolderRule, new MapSettings().asConfig(),
     System2.INSTANCE, mock(IssueChangesToDeleteRepository.class));
-  private IssueTrackingDelegator trackingDelegator;
-  private TrackerExecution tracker;
-  private PullRequestTrackerExecution prBranchTracker;
-  private ReferenceBranchTrackerExecution mergeBranchTracker;
   private final ActiveRulesHolder activeRulesHolder = new AlwaysActiveRulesHolderImpl();
   private ProtoIssueCache protoIssueCache;
 
@@ -149,22 +149,22 @@ public class IntegrateIssuesVisitorIT {
     when(movedFilesRepository.getOriginalFile(any(Component.class))).thenReturn(Optional.empty());
 
     DbClient dbClient = dbTester.getDbClient();
-    TrackerRawInputFactory rawInputFactory = new TrackerRawInputFactory(treeRootHolder, reportReader, sourceLinesHash, issueFilter,
-      ruleRepositoryRule, activeRulesHolder);
+    rawInputFactory = spy(new TrackerRawInputFactory(treeRootHolder, reportReader, sourceLinesHash, issueFilter,
+      ruleRepositoryRule, activeRulesHolder));
     TrackerBaseInputFactory baseInputFactory = new TrackerBaseInputFactory(issuesLoader, dbClient, movedFilesRepository);
     TrackerTargetBranchInputFactory targetInputFactory = new TrackerTargetBranchInputFactory(issuesLoader, targetBranchComponentUuids, dbClient, movedFilesRepository);
     TrackerReferenceBranchInputFactory mergeInputFactory = new TrackerReferenceBranchInputFactory(issuesLoader, mergeBranchComponentsUuids, dbClient);
     ClosedIssuesInputFactory closedIssuesInputFactory = new ClosedIssuesInputFactory(issuesLoader, dbClient, movedFilesRepository);
-    tracker = new TrackerExecution(baseInputFactory, closedIssuesInputFactory, new Tracker<>(), issuesLoader, analysisMetadataHolder);
-    mergeBranchTracker = new ReferenceBranchTrackerExecution(mergeInputFactory, new Tracker<>());
-    prBranchTracker = new PullRequestTrackerExecution(baseInputFactory, targetInputFactory, new Tracker<>(), newLinesRepository);
-    trackingDelegator = new IssueTrackingDelegator(prBranchTracker, mergeBranchTracker, tracker, analysisMetadataHolder);
+    TrackerExecution tracker = new TrackerExecution(baseInputFactory, closedIssuesInputFactory, new Tracker<>(), issuesLoader, analysisMetadataHolder);
+    ReferenceBranchTrackerExecution mergeBranchTracker = new ReferenceBranchTrackerExecution(mergeInputFactory, new Tracker<>());
+    PullRequestTrackerExecution prBranchTracker = new PullRequestTrackerExecution(baseInputFactory, targetInputFactory, new Tracker<>(), newLinesRepository);
+    IssueTrackingDelegator trackingDelegator = new IssueTrackingDelegator(prBranchTracker, mergeBranchTracker, tracker, analysisMetadataHolder);
     treeRootHolder.setRoot(PROJECT);
     protoIssueCache = new ProtoIssueCache(temp.newFile(), System2.INSTANCE);
     when(issueFilter.accept(any(DefaultIssue.class), eq(FILE))).thenReturn(true);
     when(issueChangeContext.date()).thenReturn(new Date());
     underTest = new IntegrateIssuesVisitor(protoIssueCache, rawInputFactory, baseInputFactory, issueLifecycle, issueVisitors, trackingDelegator, issueStatusCopier,
-      referenceBranchComponentUuids, mock(PullRequestSourceBranchMerger.class), fileStatuses);
+      referenceBranchComponentUuids, mock(PullRequestSourceBranchMerger.class), fileStatuses, analysisMetadataHolder);
   }
 
   @Test
@@ -242,15 +242,6 @@ public class IntegrateIssuesVisitorIT {
   }
 
   @Test
-  public void remove_uuid_of_original_file_from_componentsWithUnprocessedIssues_if_component_has_one() {
-    String originalFileUuid = "original file uuid";
-    when(movedFilesRepository.getOriginalFile(FILE))
-      .thenReturn(Optional.of(new MovedFilesRepository.OriginalFile(originalFileUuid, "original file key")));
-
-    underTest.visitAny(FILE);
-  }
-
-  @Test
   public void reuse_issues_when_data_unchanged() {
     RuleKey ruleKey = RuleTesting.XOO_X1;
     // Issue from db has severity major
@@ -303,6 +294,30 @@ public class IntegrateIssuesVisitorIT {
     assertThat(issues.get(0).changes().get(0).diffs()).contains(entry(IssueFieldsSetter.FROM_BRANCH, new FieldDiffs.Diff<>("master", null)));
   }
 
+  @Test
+  public void visitAny_whenCacheFileNotFound_shouldThrowException() {
+    temp.delete();
+
+    assertThatThrownBy(() -> underTest.visitAny(FILE))
+      .isInstanceOf(IllegalStateException.class)
+      .hasMessage("Fail to process issues of component 'FILE_KEY'");
+  }
+
+  @Test
+  public void visitAny_whenPluginChangedSinceLastAnalysis_shouldNotExecuteIncrementalAnalysis() {
+    RuleKey ruleKey = RuleTesting.XOO_X1;
+    addBaseIssue(ruleKey);
+    when(fileStatuses.isDataUnchanged(FILE)).thenReturn(true);
+    Analysis analysis = mock(Analysis.class);
+    when(analysis.getCreatedAt()).thenReturn(1L);
+    when(analysisMetadataHolder.getBaseAnalysis()).thenReturn(analysis);
+    when(analysisMetadataHolder.getScannerPluginsByKey()).thenReturn(Map.of("xoo", new ScannerPlugin("xoo", "base", 2L)));
+
+    underTest.visitAny(FILE);
+
+    verify(rawInputFactory).create(FILE);
+  }
+
   private void addBaseIssue(RuleKey ruleKey) {
     ComponentDto project = ComponentTesting.newPrivateProjectDto(PROJECT_UUID).setKey(PROJECT_KEY);
     ComponentDto file = ComponentTesting.newFileDto(project, null, FILE_UUID).setKey(FILE_KEY);
diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitor.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitor.java
index 6f647656169..369ecc8c0dd 100644
--- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitor.java
+++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitor.java
@@ -23,7 +23,13 @@ import java.util.Collection;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
 import java.util.stream.Stream;
+import org.sonar.api.rule.RuleKey;
+import org.sonar.ce.task.projectanalysis.analysis.Analysis;
+import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder;
+import org.sonar.ce.task.projectanalysis.analysis.ScannerPlugin;
 import org.sonar.ce.task.projectanalysis.component.Component;
 import org.sonar.ce.task.projectanalysis.component.CrawlerDepthLimit;
 import org.sonar.ce.task.projectanalysis.component.FileStatuses;
@@ -47,6 +53,7 @@ public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter {
   private final ReferenceBranchComponentUuids referenceBranchComponentUuids;
   private final PullRequestSourceBranchMerger pullRequestSourceBranchMerger;
   private final FileStatuses fileStatuses;
+  private final AnalysisMetadataHolder analysisMetadataHolder;
 
   public IntegrateIssuesVisitor(
     ProtoIssueCache protoIssueCache,
@@ -58,7 +65,9 @@ public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter {
     SiblingsIssueMerger issueStatusCopier,
     ReferenceBranchComponentUuids referenceBranchComponentUuids,
     PullRequestSourceBranchMerger pullRequestSourceBranchMerger,
-    FileStatuses fileStatuses) {
+    FileStatuses fileStatuses,
+    AnalysisMetadataHolder analysisMetadataHolder) {
+
     super(CrawlerDepthLimit.FILE, POST_ORDER);
     this.protoIssueCache = protoIssueCache;
     this.rawInputFactory = rawInputFactory;
@@ -70,54 +79,79 @@ public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter {
     this.referenceBranchComponentUuids = referenceBranchComponentUuids;
     this.pullRequestSourceBranchMerger = pullRequestSourceBranchMerger;
     this.fileStatuses = fileStatuses;
+    this.analysisMetadataHolder = analysisMetadataHolder;
   }
 
   @Override
   public void visitAny(Component component) {
     try (CacheAppender<DefaultIssue> cacheAppender = protoIssueCache.newAppender()) {
       issueVisitors.beforeComponent(component);
-
-      if (fileStatuses.isDataUnchanged(component)) {
-        // we assume there's a previous analysis of the same branch
-        Input<DefaultIssue> baseIssues = baseInputFactory.create(component);
-        var issues = new LinkedList<>(baseIssues.getIssues());
-        processIssues(component, issues);
-        issueVisitors.beforeCaching(component);
-        appendIssuesToCache(cacheAppender, issues);
-      } else {
-        Input<DefaultIssue> rawInput = rawInputFactory.create(component);
-        TrackingResult tracking = issueTracking.track(component, rawInput);
-        var newOpenIssues = fillNewOpenIssues(component, tracking.newIssues(), rawInput);
-        var existingOpenIssues = fillExistingOpenIssues(tracking.issuesToMerge());
-        var closedIssues = closeIssues(tracking.issuesToClose());
-        var copiedIssues = copyIssues(tracking.issuesToCopy());
-
-        var issues = Stream.of(newOpenIssues, existingOpenIssues, closedIssues, copiedIssues)
-          .flatMap(Collection::stream)
-          .toList();
-        processIssues(component, issues);
-        issueVisitors.beforeCaching(component);
-        appendIssuesToCache(cacheAppender, issues);
-      }
+      List<DefaultIssue> issues = getIssues(component);
+      processIssues(component, issues);
+      issueVisitors.beforeCaching(component);
+      appendIssuesToCache(cacheAppender, issues);
       issueVisitors.afterComponent(component);
-    } catch (Exception e) {
-      throw new IllegalStateException(String.format("Fail to process issues of component '%s'", component.getKey()), e);
+    } catch (Exception exception) {
+      throw new IllegalStateException(String.format("Fail to process issues of component '%s'", component.getKey()), exception);
+    }
+  }
+
+  private List<DefaultIssue> getIssues(Component component) {
+    if (fileStatuses.isDataUnchanged(component)) {
+      // we assume there's a previous analysis of the same branch
+      return getIssuesForUnchangedFile(component);
+    } else {
+      return getRawIssues(component);
     }
   }
 
+  private List<DefaultIssue> getIssuesForUnchangedFile(Component component) {
+    Input<DefaultIssue> baseIssues = baseInputFactory.create(component);
+    Collection<DefaultIssue> issues = baseIssues.getIssues();
+    //In case of plugin update, issue impacts are potentially updated. We want to avoid incremental analysis in this case.
+    return hasAnyInvolvedPluginChangedSinceLastAnalysis(issues)
+      ? getRawIssues(component)
+      : new LinkedList<>(issues);
+  }
+
+  private boolean hasAnyInvolvedPluginChangedSinceLastAnalysis(Collection<DefaultIssue> issues) {
+    long lastAnalysisDate = Optional.ofNullable(analysisMetadataHolder.getBaseAnalysis()).map(Analysis::getCreatedAt).orElse(0L);
+    return getInvolvedPlugins(issues).stream()
+      .anyMatch(scannerPlugin -> lastAnalysisDate < scannerPlugin.getUpdatedAt());
+  }
+
+  private List<ScannerPlugin> getInvolvedPlugins(Collection<DefaultIssue> issues) {
+    Map<String, ScannerPlugin> scannerPluginsByKey = analysisMetadataHolder.getScannerPluginsByKey();
+    return issues.stream()
+      .map(DefaultIssue::getRuleKey)
+      .map(RuleKey::repository)
+      .map(scannerPluginsByKey::get)
+      .filter(Objects::nonNull)
+      .toList();
+  }
+
+  private List<DefaultIssue> getRawIssues(Component component) {
+    Input<DefaultIssue> rawInput = rawInputFactory.create(component);
+    TrackingResult tracking = issueTracking.track(component, rawInput);
+    var newOpenIssues = fillNewOpenIssues(component, tracking.newIssues(), rawInput);
+    var existingOpenIssues = fillExistingOpenIssues(tracking.issuesToMerge());
+    var closedIssues = closeIssues(tracking.issuesToClose());
+    var copiedIssues = copyIssues(tracking.issuesToCopy());
+    return Stream.of(newOpenIssues, existingOpenIssues, closedIssues, copiedIssues)
+      .flatMap(Collection::stream)
+      .toList();
+  }
+
   private void processIssues(Component component, Collection<DefaultIssue> issues) {
     issues.forEach(issue -> processIssue(component, issue));
   }
 
   private List<DefaultIssue> fillNewOpenIssues(Component component, Stream<DefaultIssue> newIssues, Input<DefaultIssue> rawInput) {
-    List<DefaultIssue> newIssuesList = newIssues
-      .peek(issueLifecycle::initNewOpenIssue)
-      .toList();
-
+    List<DefaultIssue> newIssuesList = newIssues.toList();
     if (newIssuesList.isEmpty()) {
       return newIssuesList;
     }
-
+    newIssuesList.forEach(issueLifecycle::initNewOpenIssue);
     pullRequestSourceBranchMerger.tryMergeIssuesFromSourceBranchOfPullRequest(component, newIssuesList, rawInput);
     issueStatusCopier.tryMerge(component, newIssuesList);
     return newIssuesList;