Merge "If submit fails due to dependency on non-visible change include calling user into message"
diff --git a/java/com/google/gerrit/server/change/MergeabilityCacheImpl.java b/java/com/google/gerrit/server/change/MergeabilityCacheImpl.java
index 1ac558b..d408519 100644
--- a/java/com/google/gerrit/server/change/MergeabilityCacheImpl.java
+++ b/java/com/google/gerrit/server/change/MergeabilityCacheImpl.java
@@ -207,7 +207,7 @@
accepted.add(rw.parseCommit(key.into));
accepted.addAll(Arrays.asList(rw.parseCommit(key.commit).getParents()));
return submitDryRun.run(
- key.submitType, repo, rw, dest, key.into, key.commit, accepted);
+ null, key.submitType, repo, rw, dest, key.into, key.commit, accepted);
}
});
} catch (ExecutionException | UncheckedExecutionException e) {
diff --git a/java/com/google/gerrit/server/query/change/ConflictsPredicate.java b/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
index 7a399f2..dfbc221 100644
--- a/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ConflictsPredicate.java
@@ -129,6 +129,7 @@
CodeReviewRevWalk rw = CodeReviewCommit.newRevWalk(repo)) {
boolean conflicts =
!args.submitDryRun.run(
+ null,
str.type,
repo,
rw,
diff --git a/java/com/google/gerrit/server/submit/CommitMergeStatus.java b/java/com/google/gerrit/server/submit/CommitMergeStatus.java
index 3c7b986..b101898 100644
--- a/java/com/google/gerrit/server/submit/CommitMergeStatus.java
+++ b/java/com/google/gerrit/server/submit/CommitMergeStatus.java
@@ -16,7 +16,9 @@
import static java.util.stream.Collectors.toSet;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gwtorm.server.OrmException;
@@ -88,15 +90,18 @@
}
public static String createMissingDependencyMessage(
- Provider<InternalChangeQuery> queryProvider, String commit, String otherCommit)
+ @Nullable CurrentUser caller,
+ Provider<InternalChangeQuery> queryProvider,
+ String commit,
+ String otherCommit)
throws OrmException {
List<ChangeData> changes = queryProvider.get().enforceVisibility(true).byCommit(otherCommit);
if (changes.isEmpty()) {
return String.format(
"Commit %s depends on commit %s which cannot be merged."
- + " Is the change of this commit not visible or was it deleted?",
- commit, otherCommit);
+ + " Is the change of this commit not visible to '%s' or was it deleted?",
+ commit, otherCommit, caller != null ? caller.getLoggableName() : "<user-not-available>");
} else if (changes.size() == 1) {
ChangeData cd = changes.get(0);
if (cd.currentPatchSet().getRevision().get().equals(otherCommit)) {
diff --git a/java/com/google/gerrit/server/submit/MergeSorter.java b/java/com/google/gerrit/server/submit/MergeSorter.java
index 6dbec32..dbdb0b4 100644
--- a/java/com/google/gerrit/server/submit/MergeSorter.java
+++ b/java/com/google/gerrit/server/submit/MergeSorter.java
@@ -14,6 +14,8 @@
package com.google.gerrit.server.submit;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.query.change.InternalChangeQuery;
@@ -29,6 +31,7 @@
import org.eclipse.jgit.revwalk.RevFlag;
public class MergeSorter {
+ @Nullable private final CurrentUser caller;
private final CodeReviewRevWalk rw;
private final RevFlag canMergeFlag;
private final Set<RevCommit> accepted;
@@ -36,11 +39,13 @@
private final Set<CodeReviewCommit> incoming;
public MergeSorter(
+ @Nullable CurrentUser caller,
CodeReviewRevWalk rw,
Set<RevCommit> alreadyAccepted,
RevFlag canMergeFlag,
Provider<InternalChangeQuery> queryProvider,
Set<CodeReviewCommit> incoming) {
+ this.caller = caller;
this.rw = rw;
this.canMergeFlag = canMergeFlag;
this.accepted = alreadyAccepted;
@@ -70,7 +75,8 @@
//
n.setStatusCode(CommitMergeStatus.MISSING_DEPENDENCY);
n.setStatusMessage(
- CommitMergeStatus.createMissingDependencyMessage(queryProvider, n.name(), c.name()));
+ CommitMergeStatus.createMissingDependencyMessage(
+ caller, queryProvider, n.name(), c.name()));
break;
}
contents.add(c);
diff --git a/java/com/google/gerrit/server/submit/RebaseSorter.java b/java/com/google/gerrit/server/submit/RebaseSorter.java
index 32f3558..1fca330 100644
--- a/java/com/google/gerrit/server/submit/RebaseSorter.java
+++ b/java/com/google/gerrit/server/submit/RebaseSorter.java
@@ -17,6 +17,7 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change.Status;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.query.change.ChangeData;
@@ -37,6 +38,7 @@
public class RebaseSorter {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private final CurrentUser caller;
private final CodeReviewRevWalk rw;
private final RevFlag canMergeFlag;
private final RevCommit initialTip;
@@ -45,12 +47,14 @@
private final Set<CodeReviewCommit> incoming;
public RebaseSorter(
+ CurrentUser caller,
CodeReviewRevWalk rw,
RevCommit initialTip,
Set<RevCommit> alreadyAccepted,
RevFlag canMergeFlag,
Provider<InternalChangeQuery> queryProvider,
Set<CodeReviewCommit> incoming) {
+ this.caller = caller;
this.rw = rw;
this.canMergeFlag = canMergeFlag;
this.initialTip = initialTip;
@@ -85,7 +89,7 @@
n.setStatusCode(CommitMergeStatus.MISSING_DEPENDENCY);
n.setStatusMessage(
CommitMergeStatus.createMissingDependencyMessage(
- queryProvider, n.name(), c.name()));
+ caller, queryProvider, n.name(), c.name()));
}
// Stop RevWalk because c is either a merged commit or a missing
// dependency. Not need to walk further.
diff --git a/java/com/google/gerrit/server/submit/SubmitDryRun.java b/java/com/google/gerrit/server/submit/SubmitDryRun.java
index e273652..391d956 100644
--- a/java/com/google/gerrit/server/submit/SubmitDryRun.java
+++ b/java/com/google/gerrit/server/submit/SubmitDryRun.java
@@ -19,8 +19,10 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Streams;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.reviewdb.client.Branch;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.git.MergeUtil;
@@ -106,6 +108,7 @@
}
public boolean run(
+ @Nullable CurrentUser caller,
SubmitType submitType,
Repository repo,
CodeReviewRevWalk rw,
@@ -124,7 +127,12 @@
rw,
mergeUtilFactory.create(getProject(destBranch)),
new MergeSorter(
- rw, alreadyAccepted, canMerge, queryProvider, ImmutableSet.of(toMergeCommit)));
+ caller,
+ rw,
+ alreadyAccepted,
+ canMerge,
+ queryProvider,
+ ImmutableSet.of(toMergeCommit)));
switch (submitType) {
case CHERRY_PICK:
diff --git a/java/com/google/gerrit/server/submit/SubmitStrategy.java b/java/com/google/gerrit/server/submit/SubmitStrategy.java
index 16c2b27..c9baf99 100644
--- a/java/com/google/gerrit/server/submit/SubmitStrategy.java
+++ b/java/com/google/gerrit/server/submit/SubmitStrategy.java
@@ -203,10 +203,16 @@
projectCache.get(destBranch.getParentKey()),
() -> String.format("project not found: %s", destBranch.getParentKey()));
this.mergeSorter =
- new MergeSorter(rw, alreadyAccepted, canMergeFlag, queryProvider, incoming);
+ new MergeSorter(caller, rw, alreadyAccepted, canMergeFlag, queryProvider, incoming);
this.rebaseSorter =
new RebaseSorter(
- rw, mergeTip.getInitialTip(), alreadyAccepted, canMergeFlag, queryProvider, incoming);
+ caller,
+ rw,
+ mergeTip.getInitialTip(),
+ alreadyAccepted,
+ canMergeFlag,
+ queryProvider,
+ incoming);
this.mergeUtil = mergeUtilFactory.create(project);
this.onSubmitValidatorsFactory = onSubmitValidatorsFactory;
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java
index 94090a6..147abe1 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java
@@ -599,7 +599,9 @@
+ " depends on commit "
+ changeResult.getCommit().name()
+ " which cannot be merged."
- + " Is the change of this commit not visible or was it deleted?");
+ + " Is the change of this commit not visible to '"
+ + admin.username
+ + "' or was it deleted?");
assertRefUpdatedEvents();
assertChangeMergedEvents();
@@ -652,7 +654,9 @@
+ " depends on commit "
+ changeResult.getCommit().name()
+ " which cannot be merged."
- + " Is the change of this commit not visible or was it deleted?");
+ + " Is the change of this commit not visible to '"
+ + user.username
+ + "' or was it deleted?");
assertRefUpdatedEvents();
assertChangeMergedEvents();
