SONAR-13579 Ensure issue tracking between new file on Pull Request and original file on target branch

8 files changed, 96 insertions, 21 deletions

diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/filemove/MovedFilesRepository.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/filemove/MovedFilesRepository.java
index 22eb662a997..b541d750090 100644
--- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/filemove/MovedFilesRepository.java
+++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/filemove/MovedFilesRepository.java
@@ -34,6 +34,14 @@ public interface MovedFilesRepository {
    */
   Optional<OriginalFile> getOriginalFile(Component file);
 
+  /**
+   * The original file for the specified component if it was registered as a moved file inside the scope of a Pull Request.
+   * <p>
+   * Calling this method with a Component which is not a file, will always return {@link Optional#empty()}.
+   * </p>
+   */
+  Optional<OriginalFile> getOriginalPullRequestFile(Component file);
+
   final class OriginalFile {
     private final String uuid;
     private final String key;
diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/filemove/MutableMovedFilesRepository.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/filemove/MutableMovedFilesRepository.java
index f357bdf3ea5..3be81900373 100644
--- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/filemove/MutableMovedFilesRepository.java
+++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/filemove/MutableMovedFilesRepository.java
@@ -29,4 +29,12 @@ public interface MutableMovedFilesRepository extends MovedFilesRepository {
    * @throws IllegalStateException if {@code file} already has an original file
    */
   void setOriginalFile(Component file, OriginalFile originalFile);
+
+  /**
+   * Registers the original file for the specified file that has been renamed/moved inside the scope of a Pull Request.
+   *
+   * @throws IllegalArgumentException if {@code file} type is not {@link Component.Type#FILE}
+   * @throws IllegalStateException if {@code file} already has an original file
+   */
+  void setOriginalPullRequestFile(Component file, OriginalFile originalFile);
 }
diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/filemove/MutableMovedFilesRepositoryImpl.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/filemove/MutableMovedFilesRepositoryImpl.java
index b05ba83cf86..b93e7e26e19 100644
--- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/filemove/MutableMovedFilesRepositoryImpl.java
+++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/filemove/MutableMovedFilesRepositoryImpl.java
@@ -30,24 +30,46 @@ import static java.util.Objects.requireNonNull;
 
 public class MutableMovedFilesRepositoryImpl implements MutableMovedFilesRepository {
   private final Map<String, OriginalFile> originalFiles = new HashMap<>();
+  private final Map<String, OriginalFile> originalPullRequestFiles = new HashMap<>();
 
   @Override
   public void setOriginalFile(Component file, OriginalFile originalFile) {
+    storeOriginalFileInCache(originalFiles, file, originalFile);
+  }
+
+  @Override
+  public void setOriginalPullRequestFile(Component file, OriginalFile originalFile) {
+    storeOriginalFileInCache(originalPullRequestFiles, file, originalFile);
+  }
+
+  @Override
+  public Optional<OriginalFile> getOriginalFile(Component file) {
+    return retrieveOriginalFileFromCache(originalFiles, file);
+  }
+
+  @Override
+  public Optional<OriginalFile> getOriginalPullRequestFile(Component file) {
+    return retrieveOriginalFileFromCache(originalPullRequestFiles, file);
+  }
+
+  private void storeOriginalFileInCache(Map<String, OriginalFile> originalFiles, Component file, OriginalFile originalFile) {
     requireNonNull(file, "file can't be null");
     requireNonNull(originalFile, "originalFile can't be null");
     checkArgument(file.getType() == Component.Type.FILE, "file must be of type FILE");
 
     OriginalFile existingOriginalFile = originalFiles.get(file.getKey());
+
     checkState(existingOriginalFile == null || existingOriginalFile.equals(originalFile),
       "Original file %s already registered for file %s. Unable to register %s.", existingOriginalFile, file, originalFile);
+
     if (existingOriginalFile == null) {
       originalFiles.put(file.getKey(), originalFile);
     }
   }
 
-  @Override
-  public Optional<OriginalFile> getOriginalFile(Component file) {
+  private Optional<OriginalFile> retrieveOriginalFileFromCache(Map<String, OriginalFile> originalFiles, Component file) {
     requireNonNull(file, "file can't be null");
+
     if (file.getType() != Component.Type.FILE) {
       return Optional.empty();
     }
diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/filemove/PullRequestFileMoveDetectionStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/filemove/PullRequestFileMoveDetectionStep.java
index 77f172f8763..3a3197fdf3f 100644
--- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/filemove/PullRequestFileMoveDetectionStep.java
+++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/filemove/PullRequestFileMoveDetectionStep.java
@@ -26,7 +26,9 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
+import java.util.function.Consumer;
 import java.util.function.Function;
 import javax.annotation.concurrent.Immutable;
 import org.apache.ibatis.session.ResultHandler;
@@ -39,6 +41,7 @@ import org.sonar.ce.task.projectanalysis.component.CrawlerDepthLimit;
 import org.sonar.ce.task.projectanalysis.component.DepthTraversalTypeAwareCrawler;
 import org.sonar.ce.task.projectanalysis.component.TreeRootHolder;
 import org.sonar.ce.task.projectanalysis.component.TypeAwareVisitorAdapter;
+import org.sonar.ce.task.projectanalysis.filemove.MovedFilesRepository.OriginalFile;
 import org.sonar.ce.task.step.ComputationStep;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
@@ -102,17 +105,18 @@ public class PullRequestFileMoveDetectionStep implements ComputationStep {
     Map<String, Component> newlyAddedFilesByUuid = getNewlyAddedFilesByUuid(reportFilesByUuid, targetBranchDbFilesByUuid);
     context.getStatistics().add("addedFiles", newlyAddedFilesByUuid.size());
 
-    // Do we need to register the moved file in the moved files repo and use the data in the related steps/visitors?
-//    registerMovedFiles(movedFilesByUuid, targetBranchDbFilesByUuid);
+    registerMovedFiles(movedFilesByUuid.values(), targetBranchDbFilesByUuid.values());
     registerNewlyAddedFiles(newlyAddedFilesByUuid);
   }
 
-  private void registerMovedFiles(Map<String, Component> movedFilesByUuid, Map<String, DbComponent> dbFilesByUuid) {
-    movedFilesByUuid
-      .forEach((movedFileUuid, movedFile) -> {
-        DbComponent oldFile = getOldFile(dbFilesByUuid.values(), movedFile.getOldName());
-        movedFilesRepository.setOriginalFile(movedFile, toOriginalFile(oldFile));
-      });
+  private void registerMovedFiles(Collection<Component> movedFiles, Collection<DbComponent> dbFiles) {
+    movedFiles
+      .forEach(registerMovedFile(dbFiles));
+  }
+
+  private Consumer<Component> registerMovedFile(Collection<DbComponent> dbFiles) {
+    return movedFile -> retrieveDbFile(dbFiles, movedFile)
+        .ifPresent(dbFile -> movedFilesRepository.setOriginalPullRequestFile(movedFile, toOriginalFile(dbFile)));
   }
 
   private void registerNewlyAddedFiles(Map<String, Component> newAddedFilesByUuid) {
@@ -139,12 +143,11 @@ public class PullRequestFileMoveDetectionStep implements ComputationStep {
       .collect(toMap(Component::getUuid, Function.identity()));
   }
 
-  private DbComponent getOldFile(Collection<DbComponent> dbFiles, String oldFilePath) {
+  private Optional<DbComponent> retrieveDbFile(Collection<DbComponent> dbFiles, Component file) {
     return dbFiles
       .stream()
-      .filter(file -> file.getPath().equals(oldFilePath))
-      .findFirst()
-      .get();
+      .filter(dbFile -> dbFile.getPath().equals(file.getOldName()))
+      .findFirst();
   }
 
   public Set<String> difference(Set<String> set1, Set<String> set2) {
@@ -166,7 +169,7 @@ public class PullRequestFileMoveDetectionStep implements ComputationStep {
   }
 
   private List<DbComponent> getTargetBranchDbFiles(DbSession dbSession, String targetBranchUuid) {
-     List<DbComponent> files = new LinkedList();
+     List<DbComponent> files = new LinkedList<>();
 
     ResultHandler<FileMoveRowDto> storeFileMove = resultContext -> {
       FileMoveRowDto row = resultContext.getResultObject();
@@ -198,8 +201,8 @@ public class PullRequestFileMoveDetectionStep implements ComputationStep {
     return builder.build();
   }
 
-  private static MovedFilesRepository.OriginalFile toOriginalFile(DbComponent dbComponent) {
-    return new MovedFilesRepository.OriginalFile(dbComponent.getUuid(), dbComponent.getKey());
+  private static OriginalFile toOriginalFile(DbComponent dbComponent) {
+    return new OriginalFile(dbComponent.getUuid(), dbComponent.getKey());
   }
 
   @Immutable
diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerTargetBranchInputFactory.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerTargetBranchInputFactory.java
index 1e28a40b382..a1f513c8371 100644
--- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerTargetBranchInputFactory.java
+++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerTargetBranchInputFactory.java
@@ -21,8 +21,11 @@ package org.sonar.ce.task.projectanalysis.issue;
 
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 import javax.annotation.Nullable;
 import org.sonar.ce.task.projectanalysis.component.Component;
+import org.sonar.ce.task.projectanalysis.filemove.MovedFilesRepository;
+import org.sonar.ce.task.projectanalysis.filemove.MovedFilesRepository.OriginalFile;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.tracking.Input;
 import org.sonar.core.issue.tracking.LazyInput;
@@ -36,12 +39,14 @@ public class TrackerTargetBranchInputFactory {
   private final ComponentIssuesLoader componentIssuesLoader;
   private final DbClient dbClient;
   private final TargetBranchComponentUuids targetBranchComponentUuids;
+  private final MovedFilesRepository movedFilesRepository;
 
   public TrackerTargetBranchInputFactory(ComponentIssuesLoader componentIssuesLoader, TargetBranchComponentUuids targetBranchComponentUuids,
-    DbClient dbClient) {
+    DbClient dbClient, MovedFilesRepository movedFilesRepository) {
     this.componentIssuesLoader = componentIssuesLoader;
     this.targetBranchComponentUuids = targetBranchComponentUuids;
     this.dbClient = dbClient;
+    this.movedFilesRepository = movedFilesRepository;
     // TODO detect file moves?
   }
 
@@ -50,10 +55,26 @@ public class TrackerTargetBranchInputFactory {
   }
 
   public Input<DefaultIssue> createForTargetBranch(Component component) {
-    String targetBranchComponentUuid = targetBranchComponentUuids.getTargetBranchComponentUuid(component.getKey());
+    String targetBranchComponentUuid = getTargetBranchComponentUuid(component);
     return new TargetLazyInput(component.getType(), targetBranchComponentUuid);
   }
 
+  private String getTargetBranchComponentUuid(Component component) {
+    Optional<String> targetBranchOriginalComponentUuid = getOriginalComponentUuid(component);
+
+    if (targetBranchOriginalComponentUuid.isPresent()) {
+      return targetBranchComponentUuids.getTargetBranchComponentUuid(targetBranchOriginalComponentUuid.get());
+    }
+
+    return targetBranchComponentUuids.getTargetBranchComponentUuid(component.getKey());
+  }
+
+  private Optional<String> getOriginalComponentUuid(Component component) {
+    return movedFilesRepository
+      .getOriginalPullRequestFile(component)
+      .map(OriginalFile::getKey);
+  }
+
   private class TargetLazyInput extends LazyInput<DefaultIssue> {
     private final Component.Type type;
     private final String targetBranchComponentUuid;
diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/filemove/MutableMovedFilesRepositoryRule.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/filemove/MutableMovedFilesRepositoryRule.java
index 0a3831289ea..de726f775bc 100644
--- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/filemove/MutableMovedFilesRepositoryRule.java
+++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/filemove/MutableMovedFilesRepositoryRule.java
@@ -50,10 +50,21 @@ public class MutableMovedFilesRepositoryRule extends ExternalResource implements
   }
 
   @Override
+  public void setOriginalPullRequestFile(Component file, OriginalFile originalFile) {
+    this.delegate.setOriginalPullRequestFile(file, originalFile);
+    this.componentsWithOriginal.add(file);
+  }
+
+  @Override
   public Optional<OriginalFile> getOriginalFile(Component file) {
     return this.delegate.getOriginalFile(file);
   }
 
+  @Override
+  public Optional<OriginalFile> getOriginalPullRequestFile(Component file) {
+    return this.delegate.getOriginalPullRequestFile(file);
+  }
+
   public Set<Component> getComponentsWithOriginal() {
     return componentsWithOriginal;
   }
diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java
index e4eb4bb9037..cbc7508b836 100644
--- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java
+++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java
@@ -154,7 +154,7 @@ public class IntegrateIssuesVisitorTest {
       ruleRepositoryRule, activeRulesHolder);
     TrackerBaseInputFactory baseInputFactory = new TrackerBaseInputFactory(issuesLoader, dbClient, movedFilesRepository, mock(ReportModulesPath.class), analysisMetadataHolder,
       new IssueFieldsSetter(), mock(ComponentsWithUnprocessedIssues.class));
-    TrackerTargetBranchInputFactory targetInputFactory = new TrackerTargetBranchInputFactory(issuesLoader, targetBranchComponentUuids, dbClient);
+    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);
diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerTargetBranchInputFactoryTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerTargetBranchInputFactoryTest.java
index 5c678fee52a..4e5a5f05dd0 100644
--- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerTargetBranchInputFactoryTest.java
+++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerTargetBranchInputFactoryTest.java
@@ -24,6 +24,7 @@ import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.sonar.ce.task.projectanalysis.component.Component;
+import org.sonar.ce.task.projectanalysis.filemove.MovedFilesRepository;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.tracking.Input;
 import org.sonar.db.DbTester;
@@ -43,11 +44,12 @@ public class TrackerTargetBranchInputFactoryTest {
 
   private final ComponentIssuesLoader componentIssuesLoader = mock(ComponentIssuesLoader.class);
   private final TargetBranchComponentUuids targetBranchComponentUuids = mock(TargetBranchComponentUuids.class);
+  private final MovedFilesRepository movedFilesRepository = mock(MovedFilesRepository.class);
   private TrackerTargetBranchInputFactory underTest;
 
   @Before
   public void setUp() {
-    underTest = new TrackerTargetBranchInputFactory(componentIssuesLoader, targetBranchComponentUuids, db.getDbClient());
+    underTest = new TrackerTargetBranchInputFactory(componentIssuesLoader, targetBranchComponentUuids, db.getDbClient(), movedFilesRepository);
   }
 
   @Test