From 5972e2443cbca9a4978a2b063482f63f22db757d Mon Sep 17 00:00:00 2001 From: Bruce Zu Date: Tue, 28 May 2013 11:12:16 +0800 Subject: [PATCH] Fix: push bypass review cannot close change from UI Push bypass review cannot close the change from UI automatically when this change's current patchset is also the current patchset of another change on the same branch of the same project, and the other change has already been closed, e.g. Abandoned. The reason is in ReceiveCommits.changeRefsById() the patchset's revison is used as key of the result(HashMap), when the key has more than one value(change ref) only the last one is kept, the others are covered. This commit fix it by replacing the HashMap with HashMultimap. Bug: Issue 1933 Change-Id: I6fb37bb9b2b6ec45bde0342c503d95277a94768a --- .../gerrit/server/git/ReceiveCommits.java | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 0a52782ddc..7f3f2fd369 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -30,11 +30,13 @@ import com.google.common.base.Function; import com.google.common.base.Predicate; import com.google.common.base.Splitter; import com.google.common.base.Strings; +import com.google.common.collect.HashMultimap; import com.google.common.collect.Iterables; import com.google.common.collect.LinkedListMultimap; import com.google.common.collect.ListMultimap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; +import com.google.common.collect.SetMultimap; import com.google.common.collect.Sets; import com.google.common.util.concurrent.CheckedFuture; import com.google.common.util.concurrent.Futures; @@ -276,7 +278,7 @@ public class ReceiveCommits { new HashMap(); private final Set validCommits = new HashSet(); - private Map refsById; + private SetMultimap refsById; private Map allRefs; private final SubmoduleOp.Factory subOpFactory; @@ -2011,20 +2013,22 @@ public class ReceiveCommits { rw.markUninteresting(rw.parseCommit(cmd.getOldId())); } - final Map byCommit = changeRefsById(); + final SetMultimap byCommit = changeRefsById(); final Map byKey = openChangesByKey( new Branch.NameKey(project.getNameKey(), cmd.getRefName())); final List toClose = new ArrayList(); RevCommit c; while ((c = rw.next()) != null) { - final Ref ref = byCommit.get(c.copy()); - if (ref != null) { - rw.parseBody(c); - Change.Key closedChange = - closeChange(cmd, PatchSet.Id.fromRef(ref.getName()), c); - closeProgress.update(1); - if (closedChange != null) { - byKey.remove(closedChange); + final Set refs = byCommit.get(c.copy()); + for (Ref ref : refs) { + if (ref != null) { + rw.parseBody(c); + Change.Key closedChange = + closeChange(cmd, PatchSet.Id.fromRef(ref.getName()), c); + closeProgress.update(1); + if (closedChange != null) { + byKey.remove(closedChange); + } } } @@ -2104,9 +2108,9 @@ public class ReceiveCommits { return change.getKey(); } - private Map changeRefsById() throws IOException { + private SetMultimap changeRefsById() throws IOException { if (refsById == null) { - refsById = new HashMap(); + refsById = HashMultimap.create(); for (Ref r : repo.getRefDatabase().getRefs("refs/changes/").values()) { if (PatchSet.isRef(r.getName())) { refsById.put(r.getObjectId(), r);