Merge "Use change number instead of triplet."
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/ConfigLoader.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/ConfigLoader.java
index 5d439b6..e4e11da 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/automerger/ConfigLoader.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/ConfigLoader.java
@@ -15,12 +15,14 @@
package com.googlesource.gerrit.plugins.automerger;
import com.google.common.base.Joiner;
+import com.google.common.base.Strings;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.inject.Inject;
@@ -41,9 +43,11 @@
public class ConfigLoader {
private static final Logger log = LoggerFactory.getLogger(ConfigLoader.class);
private static final String BRANCH_DELIMITER = ":";
+ private static final String DEFAULT_CONFLICT_MESSAGE = "Merge conflict found on ${branch}";
private final GerritApi gApi;
private final String pluginName;
+ private final String canonicalWebUrl;
private final AllProjectsName allProjectsName;
private final PluginConfigFactory cfgFactory;
@@ -60,8 +64,10 @@
GerritApi gApi,
AllProjectsName allProjectsName,
@PluginName String pluginName,
+ @CanonicalWebUrl String canonicalWebUrl,
PluginConfigFactory cfgFactory) {
this.gApi = gApi;
+ this.canonicalWebUrl = canonicalWebUrl;
this.pluginName = pluginName;
this.cfgFactory = cfgFactory;
this.allProjectsName = allProjectsName;
@@ -124,6 +130,34 @@
}
/**
+ * Returns the hostName.
+ *
+ * <p>Uses the hostName defined in the configuration if specified. If not, defaults to the
+ * canonicalWebUrl.
+ *
+ * @return Returns the hostname
+ * @throws ConfigInvalidException
+ */
+ public String getHostName() throws ConfigInvalidException {
+ String hostName = getConfig().getString("global", null, "hostName");
+ return hostName != null ? hostName : canonicalWebUrl;
+ }
+
+ /**
+ * Returns a string to append to the end of the merge conflict message.
+ *
+ * @return The message string, or the empty string if nothing is specified.
+ * @throws ConfigInvalidException
+ */
+ public String getConflictMessage() throws ConfigInvalidException {
+ String conflictMessage = getConfig().getString("global", null, "conflictMessage");
+ if (Strings.isNullOrEmpty(conflictMessage)) {
+ conflictMessage = DEFAULT_CONFLICT_MESSAGE;
+ }
+ return conflictMessage;
+ }
+
+ /**
* Get the projects that should be merged for the given pair of branches.
*
* @param fromBranch Branch we are merging from.
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 fcfb82f..f953898 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/DownstreamCreator.java
@@ -45,6 +45,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.TreeMap;
import java.util.UUID;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.slf4j.Logger;
@@ -241,7 +242,7 @@
+ " succeeded!";
reviewInput.notify = NotifyHandling.NONE;
} catch (FailedMergeException e) {
- reviewInput.message = e.displayConflicts();
+ reviewInput.message = e.getDisplayString();
reviewInput.notify = NotifyHandling.ALL;
vote = -1;
}
@@ -264,7 +265,8 @@
*/
public void createDownstreamMerges(MultipleDownstreamMergeInput mdsMergeInput)
throws RestApiException, FailedMergeException, ConfigInvalidException {
- Map<String, String> failedMerges = new HashMap<String, String>();
+ // Map from branch to error message
+ Map<String, String> failedMergeBranchMap = new TreeMap<String, String>();
List<Integer> existingDownstream;
for (String downstreamBranch : mdsMergeInput.dsBranchMap.keySet()) {
@@ -309,16 +311,20 @@
createSingleDownstreamMerge(sdsMergeInput);
}
} catch (MergeConflictException e) {
- log.debug("Merge conflict from {} to {}", mdsMergeInput.currentRevision, downstreamBranch);
- failedMerges.put(downstreamBranch, e.getMessage());
+ failedMergeBranchMap.put(downstreamBranch, e.getMessage());
log.debug("Abandoning downstream of {}", mdsMergeInput.sourceId);
abandonDownstream(
gApi.changes().id(mdsMergeInput.sourceId).info(), mdsMergeInput.currentRevision);
}
}
- if (!failedMerges.keySet().isEmpty()) {
- throw new FailedMergeException(failedMerges);
+ if (!failedMergeBranchMap.isEmpty()) {
+ throw new FailedMergeException(
+ failedMergeBranchMap,
+ mdsMergeInput.currentRevision,
+ config.getHostName(),
+ config.getConflictMessage(),
+ mdsMergeInput.topic);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/FailedMergeException.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/FailedMergeException.java
index b51d52f..6fd7983 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/automerger/FailedMergeException.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/FailedMergeException.java
@@ -14,17 +14,32 @@
package com.googlesource.gerrit.plugins.automerger;
import com.google.common.base.Joiner;
-
+import com.google.gerrit.common.data.ParameterizedString;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
import java.util.Map;
/** Exception class for merge conflicts. */
class FailedMergeException extends Exception {
private static final int MAX_CONFLICT_MESSAGE_LENGTH = 10000;
+ public final String currentRevision;
+ public final String conflictMessage;
+ public final String hostName;
+ public final String topic;
+ public final Map<String, String> failedMergeBranchMap;
- public final Map<String, String> failedMerges;
-
- FailedMergeException(Map<String, String> failedMerges) {
- this.failedMerges = failedMerges;
+ FailedMergeException(
+ Map<String, String> failedMergeBranchMap,
+ String currentRevision,
+ String hostName,
+ String conflictMessage,
+ String topic) {
+ this.failedMergeBranchMap = failedMergeBranchMap;
+ this.currentRevision = currentRevision;
+ this.hostName = hostName;
+ this.conflictMessage = conflictMessage;
+ this.topic = topic;
}
/**
@@ -32,38 +47,36 @@
*
* @return A string representation of the conflicts.
*/
- public String displayConflicts() {
- StringBuilder output = new StringBuilder();
- output.append("Merge conflict found on ");
- output.append(failedMergeKeys());
- output.append(". Please follow instructions at go/resolveconflict ");
- output.append("to resolve this merge conflict.\n\n");
-
- for (Map.Entry<String, String> entry : failedMerges.entrySet()) {
- String branch = entry.getKey();
- String message = entry.getValue();
- String conflictMessage = message;
- boolean truncated = false;
- if (message.length() > MAX_CONFLICT_MESSAGE_LENGTH) {
- conflictMessage = message.substring(0, MAX_CONFLICT_MESSAGE_LENGTH);
- truncated = true;
- }
- output.append(branch);
- output.append(":\n");
- output.append(conflictMessage);
- if (truncated) {
- output.append("...\n\n");
- }
+ public String getDisplayString() {
+ List<String> conflictMessages = new ArrayList<String>();
+ for (Map.Entry<String, String> branchMapEntry : failedMergeBranchMap.entrySet()) {
+ String branch = branchMapEntry.getKey();
+ String mergeConflictMessage = branchMapEntry.getValue();
+ conflictMessages.add(
+ assembleConflictMessage(
+ conflictMessage, getSubstitutionMap(branch, mergeConflictMessage)));
}
- return output.toString();
+
+ return Joiner.on("\n").join(conflictMessages);
}
- /**
- * Get the branches that we failed to merge to.
- *
- * @return The comma-separated branches that we failed to merge to.
- */
- public String failedMergeKeys() {
- return Joiner.on(", ").join(failedMerges.keySet());
+ private String assembleConflictMessage(String errorString, Map<String, String> substitutionMap) {
+ ParameterizedString pattern = new ParameterizedString(errorString);
+
+ String modifiedString = pattern.replace(substitutionMap);
+ if (errorString.length() > MAX_CONFLICT_MESSAGE_LENGTH) {
+ modifiedString = modifiedString.substring(0, MAX_CONFLICT_MESSAGE_LENGTH) + "\n...";
+ }
+ return modifiedString;
+ }
+
+ private Map<String, String> getSubstitutionMap(String branch, String mergeConflictMessage) {
+ Map<String, String> substitutionMap = new HashMap<>();
+ substitutionMap.put("branch", branch);
+ substitutionMap.put("revision", currentRevision);
+ substitutionMap.put("conflict", mergeConflictMessage);
+ substitutionMap.put("hostname", hostName);
+ substitutionMap.put("topic", topic);
+ return substitutionMap;
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/automerger/ConfigLoaderIT.java b/src/test/java/com/googlesource/gerrit/plugins/automerger/ConfigLoaderIT.java
index 86ae78b..c2f9637 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/automerger/ConfigLoaderIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/automerger/ConfigLoaderIT.java
@@ -45,6 +45,7 @@
private ConfigLoader configLoader;
@Inject private AllProjectsName allProjectsName;
@Inject private PluginConfigFactory cfgFactory;
+ @Inject private String canonicalWebUrl;
private Project.NameKey manifestNameKey;
@Test
@@ -174,6 +175,19 @@
loadConfig(resourceName);
}
+ @Test
+ public void getDefaultConflictMessageTest() throws Exception {
+ defaultSetup("automerger.config");
+ assertThat(configLoader.getConflictMessage()).isEqualTo("Merge conflict found on ${branch}");
+ }
+
+ @Test
+ public void getMultilineConflictMessageTest() throws Exception {
+ defaultSetup("alternate.config");
+ assertThat(configLoader.getConflictMessage())
+ .isEqualTo("line1\n" + "line2\n" + "line3 ${branch}\n" + "line4");
+ }
+
private void setupTestRepo(
String resourceName, Project.NameKey projectNameKey, String branchName, String filename)
throws Exception {
@@ -208,6 +222,7 @@
private void loadConfig(String configFilename) throws Exception {
pushConfig(configFilename);
- configLoader = new ConfigLoader(gApi, allProjectsName, "automerger", cfgFactory);
+ configLoader =
+ new ConfigLoader(gApi, allProjectsName, "automerger", canonicalWebUrl, cfgFactory);
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/automerger/FailedMergeExceptionTest.java b/src/test/java/com/googlesource/gerrit/plugins/automerger/FailedMergeExceptionTest.java
new file mode 100644
index 0000000..235272d
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/automerger/FailedMergeExceptionTest.java
@@ -0,0 +1,127 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.automerger;
+
+import java.util.TreeMap;
+import java.util.Map;
+import org.junit.Before;
+import org.junit.Test;
+
+import static com.google.common.truth.Truth.assertThat;
+
+public class FailedMergeExceptionTest {
+ private Map<String, String> failedMergeBranches;
+ private String currentRevision;
+ private String hostName;
+ private String topic;
+
+ @Before
+ public void setUp() throws Exception {
+ failedMergeBranches = new TreeMap<String, String>();
+ failedMergeBranches.put("branch1", "branch1 merge conflict");
+ failedMergeBranches.put("branch2", "branch2 merge conflict");
+ failedMergeBranches.put("branch3", "branch3 merge conflict");
+ currentRevision = "d3adb33f";
+ hostName = "livingbeef.example.com";
+ topic = "testtopic";
+ }
+
+ @Test
+ public void conflictMessageTest() throws Exception {
+ FailedMergeException fme =
+ new FailedMergeException(
+ failedMergeBranches,
+ currentRevision,
+ hostName,
+ "Merge conflict found on ${branch}",
+ topic);
+ String branch1String = "Merge conflict found on branch1";
+ String branch2String = "Merge conflict found on branch2";
+ String branch3String = "Merge conflict found on branch3";
+ assertThat(fme.getDisplayString().split("\\n"))
+ .asList()
+ .containsExactly(branch1String, branch2String, branch3String);
+ }
+
+ @Test
+ public void customConflictMessageTest() throws Exception {
+ FailedMergeException fme =
+ new FailedMergeException(failedMergeBranches, currentRevision, hostName, "asdf", topic);
+ assertThat(fme.getDisplayString().split("\\n"))
+ .asList()
+ .containsExactly("asdf", "asdf", "asdf");
+ }
+
+ @Test
+ public void customConflictMessageWithRevisionSubstitutionTest() throws Exception {
+ FailedMergeException fme =
+ new FailedMergeException(
+ failedMergeBranches, currentRevision, hostName, "${branch} ${revision}", topic);
+ String branch1String = "branch1 " + currentRevision;
+ String branch2String = "branch2 " + currentRevision;
+ String branch3String = "branch3 " + currentRevision;
+ assertThat(fme.getDisplayString().split("\\n"))
+ .asList()
+ .containsExactly(branch1String, branch2String, branch3String);
+ }
+
+ @Test
+ public void customConflictMessageWithConflictSubstitutionTest() throws Exception {
+ FailedMergeException fme =
+ new FailedMergeException(
+ failedMergeBranches, currentRevision, hostName, "${branch}: ${conflict}", topic);
+ String branch1String = "branch1: branch1 merge conflict";
+ String branch2String = "branch2: branch2 merge conflict";
+ String branch3String = "branch3: branch3 merge conflict";
+ assertThat(fme.getDisplayString().split("\\n"))
+ .asList()
+ .containsExactly(branch1String, branch2String, branch3String);
+ }
+
+ @Test
+ public void customConflictMessageWithHostNameTest() throws Exception {
+ FailedMergeException fme =
+ new FailedMergeException(
+ failedMergeBranches, currentRevision, hostName, "${hostname}: ${conflict}", topic);
+ String branch1String = hostName + ": branch1 merge conflict";
+ String branch2String = hostName + ": branch2 merge conflict";
+ String branch3String = hostName + ": branch3 merge conflict";
+ assertThat(fme.getDisplayString().split("\\n"))
+ .asList()
+ .containsExactly(branch1String, branch2String, branch3String);
+ }
+
+ @Test
+ public void customConflictMessageWitTopicTest() throws Exception {
+ FailedMergeException fme =
+ new FailedMergeException(
+ failedMergeBranches, currentRevision, hostName, "i am ${topic}", topic);
+ String conflictMessage = "i am testtopic";
+ assertThat(fme.getDisplayString().split("\\n"))
+ .asList()
+ .containsExactly("i am testtopic", "i am testtopic", "i am testtopic");
+ }
+
+ @Test
+ public void customConflictMessageMultilineTest() throws Exception {
+ FailedMergeException fme =
+ new FailedMergeException(
+ failedMergeBranches, currentRevision, hostName, "${branch}a\nb\nc\nd", topic);
+ assertThat(fme.getDisplayString().split("\\n"))
+ .asList()
+ .containsExactly(
+ "branch1a", "b", "c", "d", "branch2a", "b", "c", "d", "branch3a", "b", "c", "d");
+ }
+}
diff --git a/src/test/resources/com/googlesource/gerrit/plugins/automerger/alternate.config b/src/test/resources/com/googlesource/gerrit/plugins/automerger/alternate.config
index eaaf49f..001a1cf 100644
--- a/src/test/resources/com/googlesource/gerrit/plugins/automerger/alternate.config
+++ b/src/test/resources/com/googlesource/gerrit/plugins/automerger/alternate.config
@@ -12,4 +12,8 @@
blankMerge = .*DO NOT MERGE.*
manifestFile = default.xml
manifestProject = platform/manifest
- ignoreProjects = platform/ignore/me
\ No newline at end of file
+ ignoreProjects = platform/ignore/me
+ conflictMessage = line1\n\
+line2\n\
+line3 ${branch}\n\
+line4
\ No newline at end of file
