Add fallback change lookup in existing refs
In some cases, the change lookup from the index for an existing
change will fail. Add a backup lookup from the existing ref to
check if the change exist.
Change-Id: Id6358b12288328e8b8ff21e7ef6afc841181fa13
Helped-By: Jonathan Nieder <jrn@google.com>
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 6e2e8b1..1e2fed5 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 @@
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());
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 4139fc5..df69c92 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
@@ -102,6 +102,7 @@
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 @@
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 @@
DynamicMap<ProjectConfigEntry> pluginConfigEntries,
NotesMigration notesMigration,
ChangeEditUtil editUtil,
+ ChangeIndexer indexer,
BatchUpdate.Factory batchUpdateFactory,
SetHashtagsOp.Factory hashtagsFactory,
ReplaceOp.Factory replaceOpFactory,
@@ -424,6 +427,7 @@
this.notesMigration = notesMigration;
this.editUtil = editUtil;
+ this.indexer = indexer;
this.messageSender = new ReceivePackMessageSender();
@@ -1826,6 +1830,19 @@
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 @@
}
}
+ 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());
