Improve error message for a failed submission for FAST_FORWARD_ONLY
There is a specific case for FAST_FORWARD_ONLY submit strategy that
currently fails with NullPointerException: two changes that are
independent but with the same topic and the same destination branch.
It's impossible to perform this submission in a fast-forward way, hence
this should in fact fail. However, this should fail without
NullPointerException.
This change creates a better error message.
Change-Id: I58cb3391176606b257348cb81f1ff00b49b30bfa
diff --git a/java/com/google/gerrit/server/submit/CommitMergeStatus.java b/java/com/google/gerrit/server/submit/CommitMergeStatus.java
index bf8b840..4638bfa 100644
--- a/java/com/google/gerrit/server/submit/CommitMergeStatus.java
+++ b/java/com/google/gerrit/server/submit/CommitMergeStatus.java
@@ -77,7 +77,14 @@
EMPTY_COMMIT(
"Change could not be merged because the commit is empty.\n"
+ "\n"
- + "Project policy requires all commits to contain modifications to at least one file.");
+ + "Project policy requires all commits to contain modifications to at least one file."),
+
+ FAST_FORWARD_INDEPENDENT_CHANGES(
+ "Change could not be merged because the submission has two independent changes "
+ + "with the same destination branch.\n"
+ + "\n"
+ + "Independent changes can't be submitted to the same destination branch with "
+ + "FAST_FORWARD_ONLY submit strategy");
private final String description;
diff --git a/java/com/google/gerrit/server/submit/FastForwardOnly.java b/java/com/google/gerrit/server/submit/FastForwardOnly.java
index 176b063..8a30898 100644
--- a/java/com/google/gerrit/server/submit/FastForwardOnly.java
+++ b/java/com/google/gerrit/server/submit/FastForwardOnly.java
@@ -14,11 +14,15 @@
package com.google.gerrit.server.submit;
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.update.RepoContext;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
public class FastForwardOnly extends SubmitStrategy {
FastForwardOnly(SubmitStrategy.Arguments args) {
@@ -28,6 +32,21 @@
@Override
public List<SubmitStrategyOp> buildOps(Collection<CodeReviewCommit> toMerge) {
List<CodeReviewCommit> sorted = args.mergeUtil.reduceToMinimalMerge(args.mergeSorter, toMerge);
+
+ Map<BranchNameKey, CodeReviewCommit> branchToCommit = new HashMap<>();
+ for (CodeReviewCommit codeReviewCommit : sorted) {
+ BranchNameKey branchNameKey = codeReviewCommit.change().getDest();
+ CodeReviewCommit otherCommitInBranch = branchToCommit.get(branchNameKey);
+ if (otherCommitInBranch == null) {
+ branchToCommit.put(branchNameKey, codeReviewCommit);
+ } else {
+ // we found another change with the same destination branch.
+ codeReviewCommit.setStatusCode(CommitMergeStatus.FAST_FORWARD_INDEPENDENT_CHANGES);
+ otherCommitInBranch.setStatusCode(CommitMergeStatus.FAST_FORWARD_INDEPENDENT_CHANGES);
+ return ImmutableList.of();
+ }
+ }
+
List<SubmitStrategyOp> ops = new ArrayList<>(sorted.size());
CodeReviewCommit newTipCommit =
args.mergeUtil.getFirstFastForward(args.mergeTip.getInitialTip(), args.rw, sorted);
diff --git a/java/com/google/gerrit/server/submit/SubmitStrategyListener.java b/java/com/google/gerrit/server/submit/SubmitStrategyListener.java
index b533bebc..59c6b81 100644
--- a/java/com/google/gerrit/server/submit/SubmitStrategyListener.java
+++ b/java/com/google/gerrit/server/submit/SubmitStrategyListener.java
@@ -134,6 +134,7 @@
case NOT_FAST_FORWARD:
case EMPTY_COMMIT:
case MISSING_DEPENDENCY:
+ case FAST_FORWARD_INDEPENDENT_CHANGES:
// TODO(dborowitz): Reformat these messages to be more appropriate for
// short problem descriptions.
String message = s.getDescription();
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByFastForwardIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByFastForwardIT.java
index 66eb48c..58e48e9 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByFastForwardIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByFastForwardIT.java
@@ -20,6 +20,7 @@
import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
@@ -107,6 +108,39 @@
}
@Test
+ @GerritConfig(name = "change.submitWholeTopic", value = "true")
+ public void submitTwoIndependentChangesWithFastForwardFail() throws Throwable {
+ RevCommit initialHead = projectOperations.project(project).getHead("master");
+ PushOneCommit.Result change1 = createChange("subject1", "file1.txt", "content", "topic");
+
+ testRepo.reset(initialHead);
+ PushOneCommit.Result change2 = createChange("subject2", "file2.txt", "content", "topic");
+
+ approve(change1.getChangeId());
+ approve(change2.getChangeId());
+
+ String fastForwardIndependentChangesError =
+ "Change could not be merged because the submission"
+ + " has two independent changes with the same destination branch. Independent changes can't "
+ + "be submitted to the same destination branch with FAST_FORWARD_ONLY submit strategy";
+
+ submitWithConflict(
+ change2.getChangeId(),
+ String.format(
+ "Failed to submit 2 changes due to the following problems:\n"
+ + "Change %d: %s\nChange %d: %s",
+ change1.getChange().getId().get(),
+ fastForwardIndependentChangesError,
+ change2.getChange().getId().get(),
+ fastForwardIndependentChangesError));
+
+ RevCommit updatedHead = projectOperations.project(project).getHead("master");
+ assertThat(updatedHead.getId()).isEqualTo(initialHead.getId());
+ assertRefUpdatedEvents();
+ assertChangeMergedEvents();
+ }
+
+ @Test
public void submitFastForwardNotPossible_Conflict() throws Throwable {
RevCommit initialHead = projectOperations.project(project).getHead("master");
PushOneCommit.Result change = createChange("Change 1", "a.txt", "content");
