Revert "Use changeRefsById to track existing revisions"

This change reverts the commit that broke the %base option [1] which
allows to push a commit, which is already merged into a branch, for
review to another branch of the same project (same SHA-1).

Since 2.11 it is also new and shiny option on the project config page
"Create a new change for every commit not in the target branch" which
makes use of the %base parameter internally. This option is broken as
well, obviously.

New unit test is added that verifies this functionality and is failing
without this change, so that such regressions can be avoided in future.

This reverts commit 2ffd2cb83f.

[1] https://gerrit-review.googlesource.com/Documentation/user-upload.html#base

Bug: Issue 3426
Change-Id: If4a2375db6a3195c7cceeefe127f5c84b0142e3a

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java

@@ -20,8 +20,11 @@ import static com.google.gerrit.acceptance.GitUtil.cloneProject;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.GitUtil;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.extensions.api.projects.BranchInput;
 import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.extensions.common.EditInfo;
 import com.google.gerrit.extensions.common.LabelInfo;
@@ -36,10 +39,12 @@ import com.jcraft.jsch.JSchException;
 
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.transport.PushResult;
 import org.junit.Before;
 import org.junit.Test;
 
 import java.io.IOException;
+import java.util.List;
 import java.util.Set;
 
 public abstract class AbstractPushForReview extends AbstractDaemonTest {
@@ -305,4 +310,37 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
     PushOneCommit.Result r = pushTo("refs/for/master%hashtag=tag1");
     r.assertErrorStatus("cannot add hashtags; noteDb is disabled");
   }
+
+  @Test
+  public void testPushSameCommitTwiceUsingMagicBranchBaseOption()
+      throws Exception {
+    grant(Permission.PUSH, project, "refs/heads/master");
+    PushOneCommit.Result rBase = pushTo("refs/heads/master");
+    rBase.assertOkStatus();
+
+    gApi.projects()
+        .name(project.get())
+        .branch("foo")
+        .create(new BranchInput());
+
+    PushOneCommit push =
+        pushFactory.create(db, admin.getIdent(), PushOneCommit.SUBJECT,
+            "b.txt", "anotherContent");
+
+    PushOneCommit.Result r = push.to(git, "refs/for/master");
+    r.assertOkStatus();
+
+    PushResult pr = GitUtil.pushHead(
+        git, "refs/for/foo%base=" + rBase.getCommitId().name(), false, false);
+    assertThat(pr.getMessages()).contains("changes: new: 1, refs: 1, done");
+
+    List<ChangeInfo> changes = query(r.getCommitId().name());
+    assertThat(changes).hasSize(2);
+    ChangeInfo c1 = get(changes.get(0).id);
+    ChangeInfo c2 = get(changes.get(1).id);
+    assertThat(c1.project).isEqualTo(c2.project);
+    assertThat(c1.branch).isNotEqualTo(c2.branch);
+    assertThat(c1.changeId).isEqualTo(c2.changeId);
+    assertThat(c1.currentRevision).isEqualTo(c2.currentRevision);
+  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java

@@ -1472,6 +1472,7 @@ public class ReceiveCommits {
     walk.sort(RevSort.TOPO);
     walk.sort(RevSort.REVERSE, true);
     try {
+      Set<ObjectId> existing = Sets.newHashSet();
       walk.markStart(walk.parseCommit(magicBranch.cmd.getNewId()));
       if (magicBranch.baseCommit != null) {
         for (RevCommit c : magicBranch.baseCommit) {
@@ -1484,10 +1485,10 @@ public class ReceiveCommits {
       } else {
         markHeadsAsUninteresting(
             walk,
+            existing,
             magicBranch.ctl != null ? magicBranch.ctl.getRefName() : null);
       }
 
-      Set<ObjectId> existing = changeRefsById().keySet();
       List<ChangeLookup> pending = Lists.newArrayList();
       final Set<Change.Key> newChangeIds = new HashSet<>();
       final int maxBatchChanges =
@@ -1611,15 +1612,23 @@ public class ReceiveCommits {
     }
   }
 
-  private void markHeadsAsUninteresting(RevWalk rw, @Nullable String forRef) {
+  private void markHeadsAsUninteresting(
+      final RevWalk walk,
+      Set<ObjectId> existing,
+      @Nullable String forRef) {
     for (Ref ref : allRefs.values()) {
-      if ((ref.getName().startsWith(R_HEADS) || ref.getName().equals(forRef))
-          && ref.getObjectId() != null) {
+      if (ref.getObjectId() == null) {
+        continue;
+      } else if (ref.getName().startsWith(REFS_CHANGES)) {
+        existing.add(ref.getObjectId());
+      } else if (ref.getName().startsWith(R_HEADS)
+          || (forRef != null && forRef.equals(ref.getName()))) {
         try {
-          rw.markUninteresting(rw.parseCommit(ref.getObjectId()));
+          walk.markUninteresting(walk.parseCommit(ref.getObjectId()));
         } catch (IOException e) {
           log.warn(String.format("Invalid ref %s in %s",
               ref.getName(), project.getName()), e);
+          continue;
         }
       }
     }
@@ -2372,14 +2381,12 @@ public class ReceiveCommits {
     walk.reset();
     walk.sort(RevSort.NONE);
     try {
-      RevObject parsedObject = walk.parseAny(cmd.getNewId());
-      if (!(parsedObject instanceof RevCommit)) {
-        return;
-      }
-      walk.markStart((RevCommit)parsedObject);
-      markHeadsAsUninteresting(walk, cmd.getRefName());
-      Set<ObjectId> existing = changeRefsById().keySet();
-      for (RevCommit c; (c = walk.next()) != null;) {
+      Set<ObjectId> existing = Sets.newHashSet();
+      walk.markStart(walk.parseCommit(cmd.getNewId()));
+      markHeadsAsUninteresting(walk, existing, cmd.getRefName());
+
+      RevCommit c;
+      while ((c = walk.next()) != null) {
         if (existing.contains(c)) {
           continue;
         } else if (!validCommit(ctl, cmd, c)) {