BRANCH-55 Fix bug with multiple pr with same branch name (#8)

* Add PullRequestInfo.analysisDate

* When multiple PR by branch name, pick by the latest analysis date

* Do not obfuscate ProjectPullRequestsLoaderImpl

* Add test for pull request on pull request based on a non-main long branch

diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/ProjectPullRequests.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/ProjectPullRequests.java
index a9f4c2c3d0711f839a5788705c247ec490ebc7d0..f236faff0e0c13829a748e79972037b0bc1c977a 100644 (file)
--- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/ProjectPullRequests.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/ProjectPullRequests.java
@@ -21,6 +21,7 @@ package org.sonar.scanner.scan.branch;
 
 import java.util.List;
 import java.util.Map;
+import java.util.function.BinaryOperator;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 import javax.annotation.CheckForNull;
@@ -32,18 +33,23 @@ import javax.annotation.concurrent.Immutable;
 @Immutable
 public class ProjectPullRequests {
 
-  private final Map<String, PullRequestInfo> pullRequestsById;
+  private final Map<String, PullRequestInfo> pullRequestsByBranchName;
 
-  public ProjectPullRequests(List<PullRequestInfo> pullRequestsById) {
-    this.pullRequestsById = pullRequestsById.stream().collect(Collectors.toMap(PullRequestInfo::getBranch, Function.identity()));
+  public ProjectPullRequests(List<PullRequestInfo> pullRequestInfos) {
+    BinaryOperator<PullRequestInfo> mergeFunction = pickMostRecentAnalysis();
+    this.pullRequestsByBranchName = pullRequestInfos.stream().collect(Collectors.toMap(PullRequestInfo::getBranch, Function.identity(), mergeFunction));
+  }
+
+  private static BinaryOperator<PullRequestInfo> pickMostRecentAnalysis() {
+    return (a, b) -> a.getAnalysisDate() < b.getAnalysisDate() ? b : a;
   }
 
   @CheckForNull
   public PullRequestInfo get(String branch) {
-    return pullRequestsById.get(branch);
+    return pullRequestsByBranchName.get(branch);
   }
 
   public boolean isEmpty() {
-    return pullRequestsById.isEmpty();
+    return pullRequestsByBranchName.isEmpty();
   }
 }

diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/PullRequestInfo.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/PullRequestInfo.java
index 6ccc3656fb59ef490d6248730c6f32a81b39c773..ecbc48ec5de8cab948693c045d5620315bbe173a 100644 (file)
--- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/PullRequestInfo.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/branch/PullRequestInfo.java
@@ -28,18 +28,20 @@ import javax.annotation.concurrent.Immutable;
  */
 @Immutable
 public class PullRequestInfo {
-  private final String id;
+  private final String key;
   private final String branch;
   private final String base;
+  private final long analysisDate;
 
-  public PullRequestInfo(String id, String branch, @Nullable String base) {
-    this.id = id;
+  public PullRequestInfo(String key, String branch, @Nullable String base, long analysisDate) {
+    this.key = key;
     this.branch = branch;
     this.base = base;
+    this.analysisDate = analysisDate;
   }
 
-  public String getId() {
-    return id;
+  public String getKey() {
+    return key;
   }
 
   public String getBranch() {
@@ -50,4 +52,8 @@ public class PullRequestInfo {
   public String getBase() {
     return base;
   }
+
+  public long getAnalysisDate() {
+    return analysisDate;
+  }
 }

diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/branch/ProjectPullRequestsTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/branch/ProjectPullRequestsTest.java
new file mode 100644 (file)
index 0000000..45cffb1
--- /dev/null
+++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/branch/ProjectPullRequestsTest.java
@@ -0,0 +1,79 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2018 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.
+ */
+package org.sonar.scanner.scan.branch;
+
+import java.util.Arrays;
+import java.util.Date;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.junit.*;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class ProjectPullRequestsTest {
+
+  private static AtomicInteger counter = new AtomicInteger();
+
+  @Test
+  public void should_pick_the_latest_branch_by_analysis_date() {
+    String branchName = "dummyBranch";
+    long date = 1000;
+
+    PullRequestInfo pr1 = newPullRequestInfo(branchName, date - 2);
+    PullRequestInfo pr2 = newPullRequestInfo(branchName, date - 1);
+    PullRequestInfo latest = newPullRequestInfo(branchName, date);
+
+    PullRequestInfo pullRequestInfo = getPullRequestInfo(branchName, pr1, pr2, latest);
+    assertThat(pullRequestInfo.getKey()).isEqualTo(latest.getKey());
+    assertThat(pullRequestInfo.getAnalysisDate()).isEqualTo(date);
+  }
+
+  @Test
+  public void should_not_crash_when_cannot_pick_a_unique_branch() {
+    String branchName = "dummyBranch";
+    long date = 1000;
+
+    PullRequestInfo pr1 = newPullRequestInfo(branchName, date);
+    PullRequestInfo pr2 = newPullRequestInfo(branchName, date);
+
+    PullRequestInfo pullRequestInfo = getPullRequestInfo(branchName, pr1, pr2);
+    assertThat(pullRequestInfo.getAnalysisDate()).isEqualTo(date);
+  }
+
+  @Test
+  public void should_get_correct_branch() {
+    long date = 1000;
+
+    PullRequestInfo foo = newPullRequestInfo("foo", date);
+    PullRequestInfo bar = newPullRequestInfo("bar", date);
+
+    assertThat(getPullRequestInfo("foo", foo, bar).getBranch()).isEqualTo("foo");
+    assertThat(getPullRequestInfo("bar", foo, bar).getBranch()).isEqualTo("bar");
+  }
+
+  private PullRequestInfo newPullRequestInfo(String branchName, long date) {
+    return new PullRequestInfo("pr" + counter.incrementAndGet(), branchName, null, date);
+  }
+
+  private PullRequestInfo getPullRequestInfo(String branchName, PullRequestInfo ...prs) {
+    return new ProjectPullRequests(Arrays.asList(prs)).get(branchName);
+  }
+
+}