From 3f0ffe9d73d75508598e990e102b52300de2ec85 Mon Sep 17 00:00:00 2001 From: Janos Gyerik Date: Tue, 20 Mar 2018 12:04:52 +0100 Subject: [PATCH] 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 --- .../scan/branch/ProjectPullRequests.java | 16 ++-- .../scanner/scan/branch/PullRequestInfo.java | 16 ++-- .../scan/branch/ProjectPullRequestsTest.java | 79 +++++++++++++++++++ 3 files changed, 101 insertions(+), 10 deletions(-) create mode 100644 sonar-scanner-engine/src/test/java/org/sonar/scanner/scan/branch/ProjectPullRequestsTest.java 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 a9f4c2c3d07..f236faff0e0 100644 --- 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 pullRequestsById; + private final Map pullRequestsByBranchName; - public ProjectPullRequests(List pullRequestsById) { - this.pullRequestsById = pullRequestsById.stream().collect(Collectors.toMap(PullRequestInfo::getBranch, Function.identity())); + public ProjectPullRequests(List pullRequestInfos) { + BinaryOperator mergeFunction = pickMostRecentAnalysis(); + this.pullRequestsByBranchName = pullRequestInfos.stream().collect(Collectors.toMap(PullRequestInfo::getBranch, Function.identity(), mergeFunction)); + } + + private static BinaryOperator 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 6ccc3656fb5..ecbc48ec5de 100644 --- 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 index 00000000000..45cffb1bcfa --- /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); + } + +} -- 2.39.5