diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java index 80a349bd1e..f32235219c 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java @@ -17,6 +17,9 @@ package com.google.gerrit.acceptance.server.notedb; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.TruthJUnit.assume; import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef; +import static com.google.gerrit.reviewdb.client.RefNames.refsDraftComments; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.eclipse.jgit.lib.Constants.OBJ_BLOB; import com.google.common.collect.ImmutableList; import com.google.gerrit.acceptance.AbstractDaemonTest; @@ -35,7 +38,6 @@ import com.google.gerrit.reviewdb.client.Patch; import com.google.gerrit.reviewdb.client.PatchLineComment; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.Project; -import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RevId; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ChangeUtil; @@ -56,8 +58,11 @@ import com.google.gerrit.testutil.TestTimeUtil; import com.google.inject.Inject; import com.google.inject.Provider; +import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.Repository; import org.junit.After; import org.junit.Before; @@ -298,14 +303,14 @@ public class ChangeRebuilderIT extends AbstractDaemonTest { putDraft(user, id, 1, "comment by user"); ObjectId userDraftsId = getMetaRef( - allUsers, RefNames.refsDraftComments(id, user.getId())); + allUsers, refsDraftComments(id, user.getId())); assertThat(unwrapDb().changes().get(id).getNoteDbState()).isEqualTo( changeMetaId.name() + "," + user.getId() + "=" + userDraftsId.name()); putDraft(admin, id, 2, "comment by admin"); ObjectId adminDraftsId = getMetaRef( - allUsers, RefNames.refsDraftComments(id, admin.getId())); + allUsers, refsDraftComments(id, admin.getId())); assertThat(admin.getId().get()).isLessThan(user.getId().get()); assertThat(unwrapDb().changes().get(id).getNoteDbState()).isEqualTo( changeMetaId.name() @@ -314,7 +319,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest { putDraft(admin, id, 2, "revised comment by admin"); adminDraftsId = getMetaRef( - allUsers, RefNames.refsDraftComments(id, admin.getId())); + allUsers, refsDraftComments(id, admin.getId())); assertThat(unwrapDb().changes().get(id).getNoteDbState()).isEqualTo( changeMetaId.name() + "," + admin.getId() + "=" + adminDraftsId.name() @@ -603,6 +608,32 @@ public class ChangeRebuilderIT extends AbstractDaemonTest { .isEqualTo(oldNotes.getChange().getTopic()); } + @Test + public void rebuildDeletesOldDraftRefs() throws Exception { + PushOneCommit.Result r = createChange(); + Change.Id id = r.getPatchSetId().getParentKey(); + putDraft(user, id, 1, "comment"); + + Account.Id otherAccountId = new Account.Id(user.getId().get() + 1234); + String otherDraftRef = refsDraftComments(id, otherAccountId); + + try (Repository repo = repoManager.openRepository(allUsers); + ObjectInserter ins = repo.newObjectInserter()) { + ObjectId sha = ins.insert(OBJ_BLOB, "garbage data".getBytes(UTF_8)); + ins.flush(); + RefUpdate ru = repo.updateRef(otherDraftRef); + ru.setExpectedOldObjectId(ObjectId.zeroId()); + ru.setNewObjectId(sha); + assertThat(ru.update()).isEqualTo(RefUpdate.Result.NEW); + } + + checker.rebuildAndCheckChanges(id); + + try (Repository repo = repoManager.openRepository(allUsers)) { + assertThat(repo.exactRef(otherDraftRef)).isNull(); + } + } + private void setInvalidNoteDbState(Change.Id id) throws Exception { ReviewDb db = unwrapDb(); Change c = db.changes().get(id); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilderImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilderImpl.java index 6daf457fb2..9aa69bf62f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilderImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeRebuilderImpl.java @@ -47,6 +47,7 @@ import com.google.gerrit.reviewdb.client.PatchLineComment.Status; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.Project; +import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDbUtil; import com.google.gerrit.server.GerritPersonIdent; @@ -54,6 +55,7 @@ import com.google.gerrit.server.PatchLineCommentsUtil; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.config.AnonymousCowardName; import com.google.gerrit.server.git.ChainedReceiveCommands; +import com.google.gerrit.server.notedb.NoteDbUpdateManager.OpenRepo; import com.google.gerrit.server.patch.PatchListCache; import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.ProjectCache; @@ -69,6 +71,7 @@ import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.ProgressMonitor; +import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.TextProgressMonitor; import org.eclipse.jgit.revwalk.RevCommit; @@ -240,7 +243,8 @@ public class ChangeRebuilderImpl extends ChangeRebuilder { events.addAll(getHashtagsEvents(change, manager)); // Delete ref only after hashtags have been read - deleteRef(change, manager.getChangeRepo().cmds); + deleteChangeMetaRef(change, manager.getChangeRepo().cmds); + deleteDraftRefs(change, manager.getAllUsersRepo()); Integer minPsNum = getMinPatchSetNum(bundle); Set psIds = @@ -473,7 +477,7 @@ public class ChangeRebuilderImpl extends ChangeRebuilder { return new PatchSet.Id(change.getId(), psId); } - private void deleteRef(Change change, ChainedReceiveCommands cmds) + private void deleteChangeMetaRef(Change change, ChainedReceiveCommands cmds) throws IOException { String refName = changeMetaRef(change.getId()); Optional old = cmds.get(refName); @@ -482,6 +486,15 @@ public class ChangeRebuilderImpl extends ChangeRebuilder { } } + private void deleteDraftRefs(Change change, OpenRepo allUsersRepo) + throws IOException { + for (Ref r : allUsersRepo.repo.getRefDatabase() + .getRefs(RefNames.refsDraftCommentsPrefix(change.getId())).values()) { + allUsersRepo.cmds.add( + new ReceiveCommand(r.getObjectId(), ObjectId.zeroId(), r.getName())); + } + } + private static final Ordering EVENT_ORDER = new Ordering() { @Override public int compare(Event a, Event b) {