diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java index 6e2e8b1281..1e2fed51e2 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java +++ b/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 changes = query(result.getCommit().name()); 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 a23b78c5b0..e5bc2153f9 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 @@ -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 pluginConfigEntries; private final NotesMigration notesMigration; private final ChangeEditUtil editUtil; + private final ChangeIndexer indexer; private final List messages = new ArrayList<>(); private ListMultimap errors = LinkedListMultimap.create(); @@ -377,6 +379,7 @@ public class ReceiveCommits { DynamicMap 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 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());