Post merge conflict message even if vote is restricted.
If Code-Review +2/-2 is restricted to a certain group, we still want to
notify users that there is a merge conflict.
Change-Id: I5f41bebfb98dc5859b634e081aabb3405d9367d0
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java
index 02f5bc5..d36fa1f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java
@@ -37,6 +37,7 @@
import com.google.gerrit.extensions.events.DraftPublishedListener;
import com.google.gerrit.extensions.events.RevisionCreatedListener;
import com.google.gerrit.extensions.events.TopicEditedListener;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.MergeConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.inject.Inject;
@@ -291,10 +292,19 @@
}
}
reviewInput.labels = labels;
- gApi.changes()
- .id(mdsMergeInput.changeNumber)
- .revision(mdsMergeInput.currentRevision)
- .review(reviewInput);
+ // if this fails, i.e. -2 is restricted, catch it and still post message without a vote.
+ try {
+ gApi.changes()
+ .id(mdsMergeInput.changeNumber)
+ .revision(mdsMergeInput.currentRevision)
+ .review(reviewInput);
+ } catch (AuthException e) {
+ reviewInput.labels = null;
+ gApi.changes()
+ .id(mdsMergeInput.changeNumber)
+ .revision(mdsMergeInput.currentRevision)
+ .review(reviewInput);
+ }
}
/**
@@ -582,7 +592,11 @@
reviewInput.labels = labels;
reviewInput.notify = NotifyHandling.NONE;
reviewInput.tag = AUTOMERGER_TAG;
- gApi.changes().id(change.id).revision(change.currentRevision).review(reviewInput);
+ try {
+ gApi.changes().id(change.id).revision(change.currentRevision).review(reviewInput);
+ } catch (AuthException e) {
+ log.error("Automerger could not set label, but still continuing.", e);
+ }
}
private void updateDownstreamMerge(
diff --git a/src/test/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreatorIT.java b/src/test/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreatorIT.java
index bd37fe2..cc5abc4 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreatorIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreatorIT.java
@@ -26,11 +26,13 @@
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.common.ApprovalInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.testutil.TestTimeUtil;
import java.io.InputStream;
import java.io.InputStreamReader;
@@ -529,7 +531,7 @@
List<ChangeInfo> changesInTopic =
gApi.changes()
.query("topic: " + gApi.changes().id(masterResult.getChangeId()).topic())
- .withOption(ListChangesOption.CURRENT_REVISION)
+ .withOptions(ListChangesOption.CURRENT_REVISION, ListChangesOption.MESSAGES)
.get();
assertThat(changesInTopic).hasSize(2);
List<ChangeInfo> sortedChanges = sortedChanges(changesInTopic);
@@ -546,6 +548,84 @@
assertThat(getVote(masterChange, "Code-Review").value).isEqualTo(-2);
assertThat(getVote(masterChange, "Code-Review").tag).isEqualTo("autogenerated:MergeConflict");
assertThat(masterChangeInfo.branch).isEqualTo("master");
+
+ // Make sure that merge conflict message is still added
+ List<String> messages = new ArrayList<>();
+ for (ChangeMessageInfo cmi : masterChangeInfo.messages) {
+ messages.add(cmi.message);
+ }
+ assertThat(messages).contains("Patch Set 1: Code-Review-2\n\nMerge conflict found on ds_one");
+
+ // Ensure that commit subjects are correct
+ String masterSubject = masterChangeInfo.subject;
+ String shortMasterSha = masterChangeInfo.currentRevision.substring(0, 10);
+ assertThat(masterChangeInfo.subject).doesNotContainMatch("automerger");
+ assertThat(dsTwoChangeInfo.subject)
+ .isEqualTo("[automerger] " + masterSubject + " am: " + shortMasterSha);
+ }
+
+ @Test
+ public void testRestrictedVotePermissions() throws Exception {
+ Project.NameKey manifestNameKey = defaultSetup();
+ // Create initial change
+ PushOneCommit.Result result = createChange("subject", "filename", "echo Hello");
+ // Project name is scoped by test, so we need to get it from our initial change
+ String projectName = result.getChange().project().get();
+ createBranch(new Branch.NameKey(projectName, "ds_one"));
+ createBranch(new Branch.NameKey(projectName, "ds_two"));
+ result.assertOkStatus();
+ merge(result);
+ // Reset to create a sibling
+ ObjectId initial = repo().exactRef("HEAD").getLeaf().getObjectId();
+ testRepo.reset(initial);
+ // Set up a merge conflict between master and ds_one
+ PushOneCommit.Result ds1Result =
+ createChange(
+ testRepo, "ds_one", "subject", "filename", "echo \"Hello asdfsd World\"", "randtopic");
+ ds1Result.assertOkStatus();
+ merge(ds1Result);
+ // Reset to allow our merge conflict to come
+ testRepo.reset(initial);
+ pushConfig("automerger.config", manifestNameKey.get(), projectName, "ds_one", "ds_two");
+
+ // Block Code Review label to test restrictions
+ blockLabel("Code-Review", -2, 2, SystemGroupBackend.CHANGE_OWNER, "refs/heads/*", project);
+
+ // After we upload our config, we upload a new change to create the downstreams
+ PushOneCommit.Result masterResult =
+ pushFactory
+ .create(db, admin.getIdent(), testRepo, "subject", "filename", "echo 'Hello World!'")
+ .to("refs/for/master");
+ masterResult.assertOkStatus();
+
+ // Since there's a conflict with ds_one, there should only be two changes in the topic
+ List<ChangeInfo> changesInTopic =
+ gApi.changes()
+ .query("topic: " + gApi.changes().id(masterResult.getChangeId()).topic())
+ .withOptions(ListChangesOption.CURRENT_REVISION, ListChangesOption.MESSAGES)
+ .get();
+ assertThat(changesInTopic).hasSize(2);
+ List<ChangeInfo> sortedChanges = sortedChanges(changesInTopic);
+
+ ChangeInfo dsTwoChangeInfo = sortedChanges.get(0);
+ assertThat(dsTwoChangeInfo.branch).isEqualTo("ds_two");
+ ChangeApi dsTwoChange = gApi.changes().id(dsTwoChangeInfo._number);
+ assertThat(getVote(dsTwoChange, "Code-Review").value).isEqualTo(0);
+ assertThat(getVote(dsTwoChange, "Code-Review").tag).isEqualTo("autogenerated:Automerger");
+
+ ChangeInfo masterChangeInfo = sortedChanges.get(1);
+ ChangeApi masterChange = gApi.changes().id(masterChangeInfo._number);
+ // This is 0 because the -2 vote on master failed due to permissions
+ assertThat(getVote(masterChange, "Code-Review").value).isEqualTo(0);
+ assertThat(getVote(masterChange, "Code-Review").tag).isEqualTo("autogenerated:MergeConflict");
+ assertThat(masterChangeInfo.branch).isEqualTo("master");
+
+ // Make sure that merge conflict message is still added
+ List<String> messages = new ArrayList<>();
+ for (ChangeMessageInfo cmi : masterChangeInfo.messages) {
+ messages.add(cmi.message);
+ }
+ assertThat(messages).contains("Patch Set 1:\n\nMerge conflict found on ds_one");
// Ensure that commit subjects are correct
String masterSubject = masterChangeInfo.subject;
