Revert "Fix GetRelated if multiple changes exist for the same commit"
This reverts commit 90598e8208e5f792ce1b16e72ddc817791dd8708.
Reason for revert: This change broke relation chain for merge changes across branches.
Bug: Issue 16894
Change-Id: I53f85cdebd6fed2f6067520d3aebeb3b576fab94
diff --git a/java/com/google/gerrit/server/change/GetRelatedChangesUtil.java b/java/com/google/gerrit/server/change/GetRelatedChangesUtil.java
index 9a75469..834a623 100644
--- a/java/com/google/gerrit/server/change/GetRelatedChangesUtil.java
+++ b/java/com/google/gerrit/server/change/GetRelatedChangesUtil.java
@@ -99,8 +99,8 @@
}
List<ChangeData> cds =
- InternalChangeQuery.byBranchGroups(
- queryProvider, indexConfig, changeData.change().getDest(), groups);
+ InternalChangeQuery.byProjectGroups(
+ queryProvider, indexConfig, changeData.project(), groups);
if (cds.isEmpty()) {
return Collections.emptyList();
}
diff --git a/java/com/google/gerrit/server/events/EventFactory.java b/java/com/google/gerrit/server/events/EventFactory.java
index 14aadf47..678f4d0 100644
--- a/java/com/google/gerrit/server/events/EventFactory.java
+++ b/java/com/google/gerrit/server/events/EventFactory.java
@@ -294,8 +294,8 @@
// Find changes in the same related group as this patch set, having a patch
// set whose parent matches this patch set's revision.
for (ChangeData cd :
- InternalChangeQuery.byBranchGroups(
- queryProvider, indexConfig, change.getDest(), currentPs.groups())) {
+ InternalChangeQuery.byProjectGroups(
+ queryProvider, indexConfig, change.getProject(), currentPs.groups())) {
PATCH_SETS:
for (PatchSet ps : cd.patchSets()) {
RevCommit commit = rw.parseCommit(ps.commitId());
diff --git a/java/com/google/gerrit/server/query/change/InternalChangeQuery.java b/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
index 3edad69..62c070c 100644
--- a/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
+++ b/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
@@ -264,21 +264,21 @@
return query(ChangePredicates.submissionId(cs));
}
- private static Predicate<ChangeData> byBranchGroupsPredicate(
- IndexConfig indexConfig, BranchNameKey branchAndProject, Collection<String> groups) {
- int n = indexConfig.maxTerms() - 2;
+ private static Predicate<ChangeData> byProjectGroupsPredicate(
+ IndexConfig indexConfig, Project.NameKey project, Collection<String> groups) {
+ int n = indexConfig.maxTerms() - 1;
checkArgument(groups.size() <= n, "cannot exceed %s groups", n);
List<GroupPredicate> groupPredicates = new ArrayList<>(groups.size());
for (String g : groups) {
groupPredicates.add(new GroupPredicate(g));
}
- return and(project(branchAndProject.project()), ref(branchAndProject), or(groupPredicates));
+ return and(project(project), or(groupPredicates));
}
- public static ImmutableList<ChangeData> byBranchGroups(
+ public static ImmutableList<ChangeData> byProjectGroups(
Provider<InternalChangeQuery> queryProvider,
IndexConfig indexConfig,
- BranchNameKey branchAndProject,
+ Project.NameKey project,
Collection<String> groups) {
// These queries may be complex along multiple dimensions:
// * Many groups per change, if there are very many patch sets. This requires partitioning the
@@ -289,17 +289,16 @@
// InternalChangeQuery is single-use.
Supplier<InternalChangeQuery> querySupplier = () -> queryProvider.get().enforceVisibility(true);
- int batchSize = indexConfig.maxTerms() - 2;
+ int batchSize = indexConfig.maxTerms() - 1;
if (groups.size() <= batchSize) {
return queryExhaustively(
- querySupplier, byBranchGroupsPredicate(indexConfig, branchAndProject, groups));
+ querySupplier, byProjectGroupsPredicate(indexConfig, project, groups));
}
Set<Change.Id> seen = new HashSet<>();
ImmutableList.Builder<ChangeData> result = ImmutableList.builder();
for (List<String> part : Iterables.partition(groups, batchSize)) {
for (ChangeData cd :
- queryExhaustively(
- querySupplier, byBranchGroupsPredicate(indexConfig, branchAndProject, part))) {
+ queryExhaustively(querySupplier, byProjectGroupsPredicate(indexConfig, project, part))) {
if (!seen.add(cd.getId())) {
result.add(cd);
}
diff --git a/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java b/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java
index 3e03b2a..e15ed08 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/GetRelatedIT.java
@@ -40,13 +40,11 @@
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AccountGroup;
-import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.api.changes.GetRelatedOption;
import com.google.gerrit.extensions.api.changes.RelatedChangeAndCommitInfo;
import com.google.gerrit.extensions.api.changes.ReviewInput;
-import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.index.IndexConfig;
@@ -71,8 +69,6 @@
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.transport.PushResult;
-import org.eclipse.jgit.transport.RemoteRefUpdate;
import org.junit.Test;
@NoHttpd
@@ -670,71 +666,6 @@
}
}
- @Test
- public void getRelatedLinearSameCommitPushedTwice() throws Exception {
- RevCommit base = projectOperations.project(project).getHead("master");
-
- // 1,1---2,1 on master
- PushOneCommit.Result r1 =
- createChange(
- testRepo,
- "master",
- "subject: 1",
- "a.txt",
- "1",
- /** topic= */
- null);
- RevCommit c1_1 = r1.getCommit();
- PatchSet.Id ps1_1 = r1.getPatchSetId();
-
- PushOneCommit.Result r2 =
- createChange(
- testRepo,
- "master",
- "subject: 2",
- "b.txt",
- "2",
- /** topic= */
- null);
- RevCommit c2_1 = r2.getCommit();
- PatchSet.Id ps2_1 = r2.getPatchSetId();
-
- // 3,1---4,1 on stable
- gApi.projects().name(project.get()).branch("stable").create(new BranchInput());
- testRepo.reset(c1_1);
- PushResult r3 = pushHead(testRepo, "refs/for/stable%base=" + base.getName());
- assertThat(r3.getRemoteUpdate("refs/for/stable%base=" + base.getName()).getStatus())
- .isEqualTo(RemoteRefUpdate.Status.OK);
- ChangeData change3 =
- Iterables.getOnlyElement(
- queryProvider
- .get()
- .byBranchCommit(BranchNameKey.create(project, "stable"), c1_1.getName()));
- assertThat(change3.currentPatchSet().commitId()).isEqualTo(c1_1);
- RevCommit c3_1 = c1_1;
- PatchSet.Id ps3_1 = change3.currentPatchSet().id();
-
- PushOneCommit.Result r4 =
- createChange(
- testRepo,
- "stable",
- "subject: 4",
- "d.txt",
- "4",
- /** topic= */
- null);
- RevCommit c4_1 = r4.getCommit();
- PatchSet.Id ps4_1 = r4.getPatchSetId();
-
- for (PatchSet.Id ps : ImmutableList.of(ps2_1, ps1_1)) {
- assertRelated(ps, changeAndCommit(ps2_1, c2_1, 1), changeAndCommit(ps1_1, c1_1, 1));
- }
-
- for (PatchSet.Id ps : ImmutableList.of(ps4_1, ps3_1)) {
- assertRelated(ps, changeAndCommit(ps4_1, c4_1, 1), changeAndCommit(ps3_1, c3_1, 1));
- }
- }
-
private static Correspondence<RelatedChangeAndCommitInfo, String>
getRelatedChangeToStatusCorrespondence() {
return Correspondence.transforming(
