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(