Revert "Reapply: Revert "Use changeRefsById to track existing revisions""

This commit broke pushing of a series of two commits for review when
the createNewChangeForAllNotInTarget option on the project was set.
A new test case is added to cover this case.

This reverts commit 767fdb668e.

Change-Id: I0d6163ca4c656254a2c3dcdae6f996b1bf61b4a3
Signed-off-by: Edwin Kempin <ekempin@google.com>

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

@@ -32,7 +32,6 @@ import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput.NotifyHandling;
-import com.google.gerrit.extensions.api.projects.BranchInput;
 import com.google.gerrit.extensions.client.InheritableBoolean;
 import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.extensions.common.ChangeMessageInfo;
@@ -47,14 +46,12 @@ import com.google.gerrit.testutil.FakeEmailSender.Message;
 import com.google.gerrit.testutil.TestTimeUtil;
 
 import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.transport.PushResult;
 import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
 import java.util.Collection;
-import java.util.List;
 import java.util.Set;
 
 public abstract class AbstractPushForReview extends AbstractDaemonTest {
@@ -493,35 +490,21 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
   }
 
   @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());
+  public void testCreateNewChangeForAllNotInTarget() throws Exception {
+    ProjectConfig config = projectCache.checkedGet(project).getConfig();
+    config.getProject().setCreateNewChangeForAllNotInTarget(InheritableBoolean.TRUE);
+    saveProjectConfig(project, config);
 
     PushOneCommit push =
         pushFactory.create(db, admin.getIdent(), testRepo, PushOneCommit.SUBJECT,
-            "b.txt", "anotherContent");
-
+            "a.txt", "content");
     PushOneCommit.Result r = push.to("refs/for/master");
     r.assertOkStatus();
 
-    PushResult pr = GitUtil.pushHead(
-        testRepo, "refs/for/foo%base=" + rBase.getCommit().name(), false, false);
-    assertThat(pr.getMessages()).contains("changes: new: 1, refs: 1, done");
-
-    List<ChangeInfo> changes = query(r.getCommit().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);
+    push =
+        pushFactory.create(db, admin.getIdent(), testRepo, PushOneCommit.SUBJECT,
+            "b.txt", "anotherContent");
+    r = push.to("refs/for/master");
+    r.assertOkStatus();
   }
 }

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

@@ -1504,8 +1504,8 @@ public class ReceiveCommits {
   private void selectNewAndReplacedChangesFromMagicBranch() {
     newChanges = Lists.newArrayList();
 
-    SetMultimap<ObjectId, Ref> existing = HashMultimap.create();
-    GroupCollector groupCollector = GroupCollector.create(changeRefsById(), db, psUtil,
+    SetMultimap<ObjectId, Ref> existing = changeRefsById();
+    GroupCollector groupCollector = GroupCollector.create(refsById, db, psUtil,
         notesFactory, project.getNameKey());
 
     rp.getRevWalk().reset();
@@ -1526,7 +1526,6 @@ public class ReceiveCommits {
       } else {
         markHeadsAsUninteresting(
             rp.getRevWalk(),
-            existing,
             magicBranch.ctl != null ? magicBranch.ctl.getRefName() : null);
       }
 
@@ -1683,23 +1682,15 @@ public class ReceiveCommits {
     }
   }
 
-  private void markHeadsAsUninteresting(
-      final RevWalk walk,
-      SetMultimap<ObjectId, Ref> existing,
-      @Nullable String forRef) {
+  private void markHeadsAsUninteresting(RevWalk rw, @Nullable String forRef) {
     for (Ref ref : allRefs.values()) {
-      if (ref.getObjectId() == null) {
-        continue;
-      } else if (ref.getName().startsWith(REFS_CHANGES)) {
-        existing.put(ref.getObjectId(), ref);
-      } else if (ref.getName().startsWith(R_HEADS)
-          || (forRef != null && forRef.equals(ref.getName()))) {
+      if ((ref.getName().startsWith(R_HEADS) || ref.getName().equals(forRef))
+          && ref.getObjectId() != null) {
         try {
-          walk.markUninteresting(walk.parseCommit(ref.getObjectId()));
+          rw.markUninteresting(rw.parseCommit(ref.getObjectId()));
         } catch (IOException e) {
           log.warn(String.format("Invalid ref %s in %s",
               ref.getName(), project.getName()), e);
-          continue;
         }
       }
     }
@@ -2342,11 +2333,11 @@ public class ReceiveCommits {
       if (!(parsedObject instanceof RevCommit)) {
         return;
       }
-      SetMultimap<ObjectId, Ref> existing = HashMultimap.create();
       walk.markStart((RevCommit)parsedObject);
-      markHeadsAsUninteresting(walk, existing, cmd.getRefName());
+      markHeadsAsUninteresting(walk, cmd.getRefName());
+      Set<ObjectId> existing = changeRefsById().keySet();
       for (RevCommit c; (c = walk.next()) != null;) {
-        if (existing.keySet().contains(c)) {
+        if (existing.contains(c)) {
           continue;
         } else if (!validCommit(walk, ctl, cmd, c)) {
           break;