Merge "Add fallback change lookup in existing refs"

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

@@ -652,6 +652,58 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
     assertTwoChangesWithSameRevision(r);
   }
 
+  @Test
+  public void pushSameCommitTwice() 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,
+                "a.txt", "content");
+    PushOneCommit.Result r = push.to("refs/for/master");
+    r.assertOkStatus();
+
+    push =
+        pushFactory
+            .create(db, admin.getIdent(), testRepo, PushOneCommit.SUBJECT,
+                "b.txt", "anotherContent");
+    r = push.to("refs/for/master");
+    r.assertOkStatus();
+
+    assertPushRejected(pushHead(testRepo, "refs/for/master", false),
+        "refs/for/master", "commit(s) already exists (as current patchset)");
+  }
+
+  @Test
+  public void pushSameCommitTwiceWhenIndexFailed() 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,
+                "a.txt", "content");
+    PushOneCommit.Result r = push.to("refs/for/master");
+    r.assertOkStatus();
+
+    push =
+        pushFactory
+            .create(db, admin.getIdent(), testRepo, PushOneCommit.SUBJECT,
+                "b.txt", "anotherContent");
+    r = push.to("refs/for/master");
+    r.assertOkStatus();
+
+    indexer.delete(r.getChange().getId());
+
+    assertPushRejected(pushHead(testRepo, "refs/for/master", false),
+        "refs/for/master", "commit(s) already exists (as current patchset)");
+  }
+
   private void assertTwoChangesWithSameRevision(PushOneCommit.Result result)
       throws Exception {
     List<ChangeInfo> changes = query(result.getCommit().name());

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

@@ -101,6 +101,7 @@ import com.google.gerrit.server.git.validators.CommitValidators;
 import com.google.gerrit.server.git.validators.RefOperationValidationException;
 import com.google.gerrit.server.git.validators.RefOperationValidators;
 import com.google.gerrit.server.git.validators.ValidationMessage;
+import com.google.gerrit.server.index.change.ChangeIndexer;
 import com.google.gerrit.server.mail.MailUtil.MailRecipients;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.NotesMigration;
@@ -333,6 +334,7 @@ public class ReceiveCommits {
   private final DynamicMap<ProjectConfigEntry> pluginConfigEntries;
   private final NotesMigration notesMigration;
   private final ChangeEditUtil editUtil;
+  private final ChangeIndexer indexer;
 
   private final List<ValidationMessage> messages = new ArrayList<>();
   private ListMultimap<Error, String> errors = LinkedListMultimap.create();
@@ -377,6 +379,7 @@ public class ReceiveCommits {
       DynamicMap<ProjectConfigEntry> pluginConfigEntries,
       NotesMigration notesMigration,
       ChangeEditUtil editUtil,
+      ChangeIndexer indexer,
       BatchUpdate.Factory batchUpdateFactory,
       SetHashtagsOp.Factory hashtagsFactory,
       ReplaceOp.Factory replaceOpFactory,
@@ -424,6 +427,7 @@ public class ReceiveCommits {
     this.notesMigration = notesMigration;
 
     this.editUtil = editUtil;
+    this.indexer = indexer;
 
     this.messageSender = new ReceivePackMessageSender();
 
@@ -1826,6 +1830,19 @@ public class ReceiveCommits {
             return;
           }
 
+          // In case the change look up from the index failed,
+          // double check against the existing refs
+          if (foundInExistingRef(existing.get(p.commit))) {
+            if (pending.size() == 1) {
+              reject(magicBranch.cmd,
+                  "commit(s) already exists (as current patchset)");
+              newChanges = Collections.emptyList();
+              return;
+            } else {
+              itr.remove();
+              continue;
+            }
+          }
           newChangeIds.add(p.changeKey);
         }
         newChanges.add(new CreateRequest(p.commit, magicBranch.dest.get()));
@@ -1879,6 +1896,22 @@ public class ReceiveCommits {
     }
   }
 
+  private boolean foundInExistingRef(Collection<Ref> existingRefs)
+      throws OrmException, IOException {
+    for (Ref ref : existingRefs) {
+      ChangeNotes notes = notesFactory.create(db, project.getNameKey(),
+          Change.Id.fromRef(ref.getName()));
+      Change change = notes.getChange();
+      if (change.getDest().equals(magicBranch.dest)) {
+        logDebug("Found change {} from existing refs.", change.getKey());
+        // reindex the change asynchronously
+        indexer.indexAsync(project.getNameKey(), change.getId());
+        return true;
+      }
+    }
+    return false;
+  }
+
   private RevCommit setUpWalkForSelectingChanges() throws IOException {
     RevWalk rw = rp.getRevWalk();
     RevCommit start = rw.parseCommit(magicBranch.cmd.getNewId());