opendev/gerrit
Code Issues Proposed changes

MergeUtil: Sort topics in commit message lexicographically

Browse Source 
The LinkedHashSet implementation is stable, but it uses the order in
which topics are encountered while walking commits in order, which is
fairly arbitrary. While we're in there, convert to streams.

Change-Id: Ieb0a262c2f56bc3e4de698ca56d6b341263ec5c2
This commit is contained in:
Dave Borowitz
2019-05-24 09:45:47 -07:00
committed by David Pursehouse
parent 4fb7a0b340
commit f77ddd6282
2 changed files with 25 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
java/com/google/gerrit/server/git/MergeUtil.java
View File

@@ -17,15 +17,17 @@ package com.google.gerrit.server.git;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.collect.ImmutableSortedSet.toImmutableSortedSet;
import static com.google.gerrit.git.ObjectIds.abbreviateName;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Comparator.naturalOrder;
import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.joining;
import com.google.common.base.Joiner;
import com.google.common.base.Strings;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
@@ -68,7 +70,6 @@ import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@@ -847,26 +848,22 @@ public class MergeUtil {
return String.format("Merge \"%s\"", c.getShortMessage());
}
LinkedHashSet<String> topics = new LinkedHashSet<>(4);
for (CodeReviewCommit c : merged) {
if (!Strings.isNullOrEmpty(c.change().getTopic())) {
topics.add(c.change().getTopic());
}
}
ImmutableSortedSet<String> topics =
merged.stream()
.map(c -> c.change().getTopic())
.filter(t -> !Strings.isNullOrEmpty(t))
.map(t -> "\"" + t + "\"")
.collect(toImmutableSortedSet(naturalOrder()));
if (topics.size() == 1) {
return String.format("Merge changes from topic \"%s\"", Iterables.getFirst(topics, null));
} else if (topics.size() > 1) {
return String.format("Merge changes from topics \"%s\"", Joiner.on("\", \"").join(topics));
} else {
if (!topics.isEmpty()) {
return String.format(
"Merge changes %s%s",
FluentIterable.from(merged)
.limit(5)
.transform(c -> c.change().getKey().abbreviate())
.join(Joiner.on(',')),
merged.size() > 5 ? ", ..." : "");
"Merge changes from topic%s %s",
topics.size() > 1 ? "s" : "", topics.stream().collect(joining(", ")));
}
return merged.stream()
.limit(5)
.map(c -> c.change().getKey().abbreviate())
.collect(joining(",", "Merge changes ", merged.size() > 5 ? ", ..." : ""));
}
public ThreeWayMerger newThreeWayMerger(ObjectInserter inserter, Config repoConfig) {

16
javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
View File

@@ -506,10 +506,10 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
assume().that(isSubmitWholeTopicEnabled()).isTrue();
String topic1 = "test-topic-1";
String topic2 = "test-topic-2";
PushOneCommit.Result change1 = createChange("Change 1", "a.txt", "content", topic2);
PushOneCommit.Result change2 = createChange("Change 2", "b.txt", "content", topic2);
PushOneCommit.Result change3 = createChange("Change 3", "c.txt", "content", topic1);
PushOneCommit.Result change4 = createChange("Change 4", "d.txt", "content", topic1);
PushOneCommit.Result change1 = createChange("Change 1", "a.txt", "content", topic1);
PushOneCommit.Result change2 = createChange("Change 2", "b.txt", "content", topic1);
PushOneCommit.Result change3 = createChange("Change 3", "c.txt", "content", topic2);
PushOneCommit.Result change4 = createChange("Change 4", "d.txt", "content", topic2);
approve(change1.getChangeId());
approve(change2.getChangeId());
approve(change3.getChangeId());
@@ -518,12 +518,12 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
String expectedTopic1 = name(topic1);
String expectedTopic2 = name(topic2);
if (getSubmitType() == SubmitType.CHERRY_PICK) {
change1.assertChange(Change.Status.NEW, expectedTopic2, admin);
change2.assertChange(Change.Status.NEW, expectedTopic2, admin);
change1.assertChange(Change.Status.NEW, expectedTopic1, admin);
change2.assertChange(Change.Status.NEW, expectedTopic1, admin);
} else {
change1.assertChange(Change.Status.MERGED, expectedTopic2, admin);
change2.assertChange(Change.Status.MERGED, expectedTopic2, admin);
change1.assertChange(Change.Status.MERGED, expectedTopic1, admin);
change2.assertChange(Change.Status.MERGED, expectedTopic1, admin);
}
// Check for the exact change to have the correct submitter.