ChangeRebuilderImpl: Delete old draft refs
Like with the change meta ref, we want ChangeRebuilderImpl to start
with a blank slate for draft refs. This avoids grafting the new,
rebuilt history onto an existing draft comment notes ref.
Change-Id: I0db0931654f7de3759fed232c1fad2017c529b73
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 80a349b..f322352 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 @@
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.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.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 @@
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 @@
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 @@
.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 6daf457..9aa69bf 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.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.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.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 @@
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<PatchSet.Id> psIds =
@@ -473,7 +477,7 @@
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<ObjectId> old = cmds.get(refName);
@@ -482,6 +486,15 @@
}
}
+ 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> EVENT_ORDER = new Ordering<Event>() {
@Override
public int compare(Event a, Event b) {
