SONAR-21259 Increase range of issue status tracked as issue fixed and refactor code

server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorIT.java

@@ -55,6 +55,7 @@ import org.sonar.ce.task.projectanalysis.source.SourceLinesHashRepository;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.FieldDiffs;
 import org.sonar.core.issue.IssueChangeContext;
+import org.sonar.core.issue.tracking.Input;
 import org.sonar.core.issue.tracking.Tracker;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbTester;
@@ -157,14 +158,14 @@ public class IntegrateIssuesVisitorIT {
     ClosedIssuesInputFactory closedIssuesInputFactory = new ClosedIssuesInputFactory(issuesLoader, dbClient, movedFilesRepository);
     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);
+    PullRequestTrackerExecution prBranchTracker = new PullRequestTrackerExecution(baseInputFactory, 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, analysisMetadataHolder);
+      referenceBranchComponentUuids, mock(PullRequestSourceBranchMerger.class), fileStatuses, analysisMetadataHolder, targetInputFactory);
   }
 
   @Test
@@ -229,6 +230,28 @@ public class IntegrateIssuesVisitorIT {
     assertThat(defaultIssueCaptor.getValue().ruleKey().rule()).isEqualTo("x1");
   }
 
+  @Test
+  public void visitAny_whenIsPullRequest_shouldCallExpectedVisitorsRawIssues() {
+    when(analysisMetadataHolder.isPullRequest()).thenReturn(true);
+    when(targetBranchComponentUuids.hasTargetBranchAnalysis()).thenReturn(true);
+
+    ruleRepositoryRule.add(RuleTesting.XOO_X1);
+    ScannerReport.Issue reportIssue = getReportIssue(RuleTesting.XOO_X1);
+    reportReader.putIssues(FILE_REF, singletonList(reportIssue));
+
+    IssueDto otherBranchIssueDto = addBaseIssueOnBranch(RuleTesting.XOO_X1);
+    when(targetBranchComponentUuids.getTargetBranchComponentUuid(FILE.getKey())).thenReturn(otherBranchIssueDto.getComponentUuid());
+
+    underTest.visitAny(FILE);
+
+    ArgumentCaptor<Input<DefaultIssue>> targetInputCaptor = ArgumentCaptor.forClass(Input.class);
+    ArgumentCaptor<Input<DefaultIssue>> rawInputCaptor = ArgumentCaptor.forClass(Input.class);
+    verify(issueVisitor).onRawIssues(eq(FILE), rawInputCaptor.capture(), targetInputCaptor.capture());
+    assertThat(rawInputCaptor.getValue().getIssues()).extracting(i -> i.ruleKey().rule()).containsExactly("x1");
+    assertThat(targetInputCaptor.getValue().getIssues()).extracting(DefaultIssue::key).containsExactly(otherBranchIssueDto.getKee());
+
+  }
+
   @Test
   public void close_unmatched_base_issue() {
     RuleKey ruleKey = RuleTesting.XOO_X1;
@@ -334,7 +357,7 @@ public class IntegrateIssuesVisitorIT {
     dbTester.getSession().commit();
   }
 
-  private void addBaseIssueOnBranch(RuleKey ruleKey) {
+  private IssueDto addBaseIssueOnBranch(RuleKey ruleKey) {
     ComponentDto project = ComponentTesting.newPrivateProjectDto(PROJECT_UUID_ON_BRANCH).setKey(PROJECT_KEY);
     ComponentDto file = ComponentTesting.newFileDto(project, null, FILE_UUID_ON_BRANCH).setKey(FILE_KEY);
     dbTester.components().insertComponents(project, file);
@@ -349,6 +372,7 @@ public class IntegrateIssuesVisitorIT {
       .setChecksum(null);
     dbTester.getDbClient().issueDao().insert(dbTester.getSession(), issue);
     dbTester.getSession().commit();
+    return issue;
   }
 
   @NotNull

server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitor.java

@@ -26,6 +26,8 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.stream.Stream;
+import javax.annotation.CheckForNull;
+import javax.annotation.Nullable;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.ce.task.projectanalysis.analysis.Analysis;
 import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder;
@@ -54,6 +56,8 @@ public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter {
   private final PullRequestSourceBranchMerger pullRequestSourceBranchMerger;
   private final FileStatuses fileStatuses;
   private final AnalysisMetadataHolder analysisMetadataHolder;
+  private final TrackerTargetBranchInputFactory targetInputFactory;
+
 
   public IntegrateIssuesVisitor(
     ProtoIssueCache protoIssueCache,
@@ -66,7 +70,9 @@ public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter {
     ReferenceBranchComponentUuids referenceBranchComponentUuids,
     PullRequestSourceBranchMerger pullRequestSourceBranchMerger,
     FileStatuses fileStatuses,
-    AnalysisMetadataHolder analysisMetadataHolder) {
+    AnalysisMetadataHolder analysisMetadataHolder,
+    TrackerTargetBranchInputFactory targetInputFactory
+  ) {
 
     super(CrawlerDepthLimit.FILE, POST_ORDER);
     this.protoIssueCache = protoIssueCache;
@@ -80,6 +86,7 @@ public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter {
     this.pullRequestSourceBranchMerger = pullRequestSourceBranchMerger;
     this.fileStatuses = fileStatuses;
     this.analysisMetadataHolder = analysisMetadataHolder;
+    this.targetInputFactory = targetInputFactory;
   }
 
   @Override
@@ -87,9 +94,10 @@ public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter {
     try (CacheAppender<DefaultIssue> cacheAppender = protoIssueCache.newAppender()) {
       issueVisitors.beforeComponent(component);
       Input<DefaultIssue> rawInput = rawInputFactory.create(component);
-      List<DefaultIssue> issues = getIssues(rawInput, component);
+      Input<DefaultIssue> targetInput = createTargetInputIfExist(component);
+      List<DefaultIssue> issues = getIssues(rawInput, targetInput, component);
 
-      issueVisitors.onRawIssues(component, rawInput);
+      issueVisitors.onRawIssues(component, rawInput, targetInput);
       processIssues(component, issues);
 
       issueVisitors.beforeCaching(component);
@@ -100,21 +108,29 @@ public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter {
     }
   }
 
-  private List<DefaultIssue> getIssues(Input<DefaultIssue> rawInput, Component component) {
+  @CheckForNull
+  private Input<DefaultIssue> createTargetInputIfExist(Component component) {
+    if (targetInputFactory.hasTargetBranchAnalysis()) {
+      return targetInputFactory.createForTargetBranch(component);
+    }
+    return null;
+  }
+
+  private List<DefaultIssue> getIssues(Input<DefaultIssue> rawInput, @Nullable Input<DefaultIssue> targetInput, Component component) {
     if (fileStatuses.isDataUnchanged(component)) {
       // we assume there's a previous analysis of the same branch
-      return getIssuesForUnchangedFile(rawInput, component);
+      return getIssuesForUnchangedFile(rawInput, targetInput, component);
     } else {
-      return getRawIssues(rawInput, component);
+      return getRawIssues(rawInput, targetInput, component);
     }
   }
 
-  private List<DefaultIssue> getIssuesForUnchangedFile(Input<DefaultIssue> rawInput, Component component) {
+  private List<DefaultIssue> getIssuesForUnchangedFile(Input<DefaultIssue> rawInput, @Nullable Input<DefaultIssue> targetInput, 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(rawInput, component)
+      ? getRawIssues(rawInput, targetInput, component)
       : new LinkedList<>(issues);
   }
 
@@ -134,8 +150,8 @@ public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter {
       .toList();
   }
 
-  private List<DefaultIssue> getRawIssues(Input<DefaultIssue> rawInput, Component component) {
-    TrackingResult tracking = issueTracking.track(component, rawInput);
+  private List<DefaultIssue> getRawIssues(Input<DefaultIssue> rawInput, @Nullable Input<DefaultIssue> targetInput, Component component) {
+    TrackingResult tracking = issueTracking.track(component, rawInput, targetInput);
     var newOpenIssues = fillNewOpenIssues(component, tracking.newIssues(), rawInput);
     var existingOpenIssues = fillExistingOpenIssues(tracking.issuesToMerge());
     var closedIssues = closeIssues(tracking.issuesToClose());

server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueTrackingDelegator.java

@@ -19,6 +19,7 @@
  */
 package org.sonar.ce.task.projectanalysis.issue;
 
+import javax.annotation.Nullable;
 import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder;
 import org.sonar.ce.task.projectanalysis.component.Component;
 import org.sonar.core.issue.DefaultIssue;
@@ -42,9 +43,9 @@ public class IssueTrackingDelegator {
     this.analysisMetadataHolder = analysisMetadataHolder;
   }
 
-  public TrackingResult track(Component component, Input<DefaultIssue> rawInput) {
+  public TrackingResult track(Component component, Input<DefaultIssue> rawInput, @Nullable Input<DefaultIssue> targetInput) {
     if (analysisMetadataHolder.isPullRequest()) {
-      return standardResult(pullRequestTracker.track(component, rawInput));
+      return standardResult(pullRequestTracker.track(component, rawInput, targetInput));
     }
 
     if (isFirstAnalysisSecondaryBranch()) {

server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueVisitor.java

@@ -19,6 +19,7 @@
  */
 package org.sonar.ce.task.projectanalysis.issue;
 
+import javax.annotation.Nullable;
 import org.sonar.ce.task.projectanalysis.component.Component;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.tracking.Input;
@@ -48,7 +49,7 @@ public abstract class IssueVisitor {
   /**
    * This method is called for all raw issues of a component before tracking is done.
    */
-  public void onRawIssues(Component component, Input<DefaultIssue> rawIssues) {
+  public void onRawIssues(Component component, Input<DefaultIssue> rawIssues, @Nullable Input<DefaultIssue> baseIssues) {
 
   }
 

server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueVisitors.java

@@ -19,6 +19,7 @@
  */
 package org.sonar.ce.task.projectanalysis.issue;
 
+import javax.annotation.Nullable;
 import org.sonar.ce.task.projectanalysis.component.Component;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.tracking.Input;
@@ -55,9 +56,9 @@ public class IssueVisitors {
     }
   }
 
-  public void onRawIssues(Component component, Input<DefaultIssue> rawIssues) {
+  public void onRawIssues(Component component, Input<DefaultIssue> rawIssues, @Nullable Input<DefaultIssue> targetIssues) {
     for (IssueVisitor visitor : visitors) {
-      visitor.onRawIssues(component, rawIssues);
+      visitor.onRawIssues(component, rawIssues, targetIssues);
     }
   }
 

server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/PullRequestTrackerExecution.java

@@ -24,6 +24,7 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
+import javax.annotation.Nullable;
 import org.sonar.api.issue.Issue;
 import org.sonar.ce.task.projectanalysis.component.Component;
 import org.sonar.ce.task.projectanalysis.source.NewLinesRepository;
@@ -36,25 +37,22 @@ public class PullRequestTrackerExecution {
   private final TrackerBaseInputFactory baseInputFactory;
   private final Tracker<DefaultIssue, DefaultIssue> tracker;
   private final NewLinesRepository newLinesRepository;
-  private final TrackerTargetBranchInputFactory targetInputFactory;
 
-  public PullRequestTrackerExecution(TrackerBaseInputFactory baseInputFactory, TrackerTargetBranchInputFactory targetInputFactory,
+  public PullRequestTrackerExecution(TrackerBaseInputFactory baseInputFactory,
     Tracker<DefaultIssue, DefaultIssue> tracker, NewLinesRepository newLinesRepository) {
     this.baseInputFactory = baseInputFactory;
-    this.targetInputFactory = targetInputFactory;
     this.tracker = tracker;
     this.newLinesRepository = newLinesRepository;
   }
 
-  public Tracking<DefaultIssue, DefaultIssue> track(Component component, Input<DefaultIssue> rawInput) {
+  public Tracking<DefaultIssue, DefaultIssue> track(Component component, Input<DefaultIssue> rawInput, @Nullable Input<DefaultIssue> targetInput) {
     // Step 1: only keep issues on changed lines
     List<DefaultIssue> filteredRaws = keepIssuesHavingAtLeastOneLocationOnChangedLines(component, rawInput.getIssues());
     Input<DefaultIssue> unmatchedRawsAfterChangedLineFiltering = createInput(rawInput, filteredRaws);
 
     // Step 2: remove issues that are resolved in the target branch
     Input<DefaultIssue> unmatchedRawsAfterTargetResolvedTracking;
-    if (targetInputFactory.hasTargetBranchAnalysis()) {
-      Input<DefaultIssue> targetInput = targetInputFactory.createForTargetBranch(component);
+    if (targetInput != null) {
       List<DefaultIssue> resolvedTargetIssues = targetInput.getIssues().stream().filter(i -> Issue.STATUS_RESOLVED.equals(i.status())).toList();
       Input<DefaultIssue> resolvedTargetInput = createInput(targetInput, resolvedTargetIssues);
       Tracking<DefaultIssue, DefaultIssue> prResolvedTracking = tracker.trackNonClosed(unmatchedRawsAfterChangedLineFiltering, resolvedTargetInput);

server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueTrackingDelegatorTest.java

@@ -52,6 +52,9 @@ public class IssueTrackingDelegatorTest {
   @Mock
   private Input<DefaultIssue> rawInput;
 
+  @Mock
+  private Input<DefaultIssue> targetInput;
+
   private IssueTrackingDelegator underTest;
 
   @Before
@@ -60,14 +63,14 @@ public class IssueTrackingDelegatorTest {
     underTest = new IssueTrackingDelegator(prBranchTracker, mergeBranchTracker, tracker, analysisMetadataHolder);
     when(tracker.track(component, rawInput)).thenReturn(trackingResult);
     when(mergeBranchTracker.track(component, rawInput)).thenReturn(trackingResult);
-    when(prBranchTracker.track(component, rawInput)).thenReturn(trackingResult);
+    when(prBranchTracker.track(component, rawInput, targetInput)).thenReturn(trackingResult);
   }
 
   @Test
   public void delegate_regular_tracker() {
     when(analysisMetadataHolder.getBranch()).thenReturn(mock(Branch.class));
 
-    underTest.track(component, rawInput);
+    underTest.track(component, rawInput, targetInput);
 
     verify(tracker).track(component, rawInput);
     verifyNoInteractions(prBranchTracker);
@@ -82,7 +85,7 @@ public class IssueTrackingDelegatorTest {
     when(analysisMetadataHolder.getBranch()).thenReturn(branch);
     when(analysisMetadataHolder.isFirstAnalysis()).thenReturn(true);
 
-    underTest.track(component, rawInput);
+    underTest.track(component, rawInput, targetInput);
 
     verify(mergeBranchTracker).track(component, rawInput);
     verifyNoInteractions(tracker);
@@ -97,9 +100,9 @@ public class IssueTrackingDelegatorTest {
     when(analysisMetadataHolder.getBranch()).thenReturn(mock(Branch.class));
     when(analysisMetadataHolder.isPullRequest()).thenReturn(true);
 
-    underTest.track(component, rawInput);
+    underTest.track(component, rawInput, targetInput);
 
-    verify(prBranchTracker).track(component, rawInput);
+    verify(prBranchTracker).track(component, rawInput, targetInput);
     verifyNoInteractions(tracker);
     verifyNoInteractions(mergeBranchTracker);
   }

server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/PullRequestTrackerExecutionTest.java

@@ -65,15 +65,13 @@ public class PullRequestTrackerExecutionTest {
 
   private PullRequestTrackerExecution underTest;
 
-  private TrackerTargetBranchInputFactory targetFactory = mock(TrackerTargetBranchInputFactory.class);
 
   @Before
   public void setUp() {
     when(baseFactory.create(FILE)).thenReturn(createInput(baseIssues));
-    when(targetFactory.createForTargetBranch(FILE)).thenReturn(createInput(targetIssues));
 
     Tracker<DefaultIssue, DefaultIssue> tracker = new Tracker<>();
-    underTest = new PullRequestTrackerExecution(baseFactory, targetFactory, tracker, newLinesRepository);
+    underTest = new PullRequestTrackerExecution(baseFactory, tracker, newLinesRepository);
   }
 
   @Test
@@ -84,7 +82,7 @@ public class PullRequestTrackerExecutionTest {
 
     when(newLinesRepository.getNewLines(FILE)).thenReturn(Optional.of(new HashSet<>(Arrays.asList(1, 3))));
 
-    Tracking<DefaultIssue, DefaultIssue> tracking = underTest.track(FILE, createInput(rawIssues));
+    Tracking<DefaultIssue, DefaultIssue> tracking = underTest.track(FILE, createInput(rawIssues), createInput(targetIssues));
 
     assertThat(tracking.getUnmatchedBases()).isEmpty();
     assertThat(tracking.getMatchedRaws()).isEmpty();
@@ -120,7 +118,7 @@ public class PullRequestTrackerExecutionTest {
 
     when(newLinesRepository.getNewLines(FILE)).thenReturn(Optional.of(new HashSet<>(Arrays.asList(7, 10))));
 
-    Tracking<DefaultIssue, DefaultIssue> tracking = underTest.track(FILE, createInput(rawIssues));
+    Tracking<DefaultIssue, DefaultIssue> tracking = underTest.track(FILE, createInput(rawIssues), createInput(targetIssues));
 
     assertThat(tracking.getUnmatchedBases()).isEmpty();
     assertThat(tracking.getMatchedRaws()).isEmpty();
@@ -135,7 +133,6 @@ public class PullRequestTrackerExecutionTest {
     rawIssues.add(createIssue(2, RuleTesting.XOO_X2));
     rawIssues.add(createIssue(3, RuleTesting.XOO_X3));
 
-    when(targetFactory.hasTargetBranchAnalysis()).thenReturn(true);
     DefaultIssue resolvedIssue = createIssue(1, RuleTesting.XOO_X1).setStatus(Issue.STATUS_RESOLVED).setResolution(Issue.RESOLUTION_FALSE_POSITIVE);
     // will cause rawIssue0 to be ignored
     targetIssues.add(resolvedIssue);
@@ -146,7 +143,7 @@ public class PullRequestTrackerExecutionTest {
     // should be matched
     baseIssues.add(rawIssues.get(1));
 
-    Tracking<DefaultIssue, DefaultIssue> tracking = underTest.track(FILE, createInput(rawIssues));
+    Tracking<DefaultIssue, DefaultIssue> tracking = underTest.track(FILE, createInput(rawIssues), createInput(targetIssues));
     assertThat(tracking.getMatchedRaws()).isEqualTo(Collections.singletonMap(rawIssues.get(1), rawIssues.get(1)));
     assertThat(tracking.getUnmatchedRaws()).containsOnly(rawIssues.get(2));
   }
@@ -160,7 +157,7 @@ public class PullRequestTrackerExecutionTest {
     rawIssues.add(createIssue(3, RuleTesting.XOO_X3));
     baseIssues.add(rawIssues.get(0));
 
-    Tracking<DefaultIssue, DefaultIssue> tracking = underTest.track(FILE, createInput(rawIssues));
+    Tracking<DefaultIssue, DefaultIssue> tracking = underTest.track(FILE, createInput(rawIssues), createInput(targetIssues));
     assertThat(tracking.getMatchedRaws()).isEqualTo(Collections.singletonMap(rawIssues.get(0), rawIssues.get(0)));
     assertThat(tracking.getUnmatchedRaws()).containsOnly(rawIssues.get(2));
   }