opendev/gerrit
Code Issues Proposed changes

Merge "Modify draft ref updates commits to point to an empty parent" into stable-2.16

Browse Source
This commit is contained in:
Youssef Elghareeb
2020-02-14 11:53:14 +00:00
committed by Gerrit Code Review
parent 03e0ff2a3e 4f77b29cb9
commit e9ab0e6564
3 changed files with 54 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
View File

@@ -197,6 +197,14 @@ public abstract class AbstractChangeUpdate {
protected abstract String getRefName();
protected void setParentCommit(CommitBuilder cb, ObjectId parentCommitId) {
if (!parentCommitId.equals(ObjectId.zeroId())) {
cb.setParentId(parentCommitId);
} else {
cb.setParentIds(); // Ref is currently nonexistent, commit has no parents.
}
}
/**
* Apply this update to the given inserter.
*
@@ -236,11 +244,7 @@ public abstract class AbstractChangeUpdate {
}
cb.setAuthor(authorIdent);
cb.setCommitter(new PersonIdent(serverIdent, when));
if (!curr.equals(z)) {
cb.setParentId(curr);
} else {
cb.setParentIds(); // Ref is currently nonexistent, commit has no parents.
}
setParentCommit(cb, curr);
if (cb.getTreeId() == null) {
if (curr.equals(z)) {
cb.setTreeId(emptyTree(ins)); // No parent, assume empty tree.

5
java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
View File

@@ -263,6 +263,11 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
return RefNames.refsDraftComments(getId(), accountId);
}
@Override
protected void setParentCommit(CommitBuilder cb, ObjectId parentCommitId) {
cb.setParentIds(); // Draft updates should not keep history of parent commits
}
@Override
public boolean isEmpty() {
return delete.isEmpty() && put.isEmpty();

40
javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
View File

@@ -360,6 +360,40 @@ public class CommentsIT extends AbstractDaemonTest {
.containsExactlyElementsIn(expectedComments);
}
/**
* This test makes sure that the commits in the refs/draft-comments ref in NoteDb have no parent
* commits. This is important so that each new draft update (add, modify, delete) does not keep
* track of previous history. Run the test with this flag: --test_env=GERRIT_NOTEDB=ON
*/
@Test
public void commitsInDraftCommentsRefHaveNoParent() throws Exception {
assume().that(notesMigration.disableChangeReviewDb()).isTrue();
PushOneCommit.Result r = createChange();
String changeId = r.getChangeId();
String revId = r.getCommit().getName();
String draftRefName = RefNames.refsDraftComments(r.getChange().getId(), user.getId());
DraftInput comment1 = newDraft("file_1", Side.REVISION, 1, "comment 1");
CommentInfo commentInfo1 = addDraft(changeId, revId, comment1);
assertThat(getHeadOfDraftCommentsRef(draftRefName).getParentCount()).isEqualTo(0);
DraftInput comment2 = newDraft("file_2", Side.REVISION, 2, "comment 2");
CommentInfo commentInfo2 = addDraft(changeId, revId, comment2);
assertThat(getHeadOfDraftCommentsRef(draftRefName).getParentCount()).isEqualTo(0);
deleteDraft(changeId, revId, commentInfo1.id);
assertThat(getHeadOfDraftCommentsRef(draftRefName).getParentCount()).isEqualTo(0);
assertThat(
getDraftComments(changeId, revId).values().stream()
.flatMap(List::stream)
.map(commentInfo -> commentInfo.message))
.containsExactly("comment 2");
deleteDraft(changeId, revId, commentInfo2.id);
assertThat(getHeadOfDraftCommentsRef(draftRefName)).isNull();
assertThat(getDraftComments(changeId, revId).values().stream().flatMap(List::stream)).isEmpty();
}
@Test
public void putDraft() throws Exception {
for (Integer line : lines) {
@@ -1112,6 +1146,12 @@ public class CommentsIT extends AbstractDaemonTest {
}
}
private RevCommit getHeadOfDraftCommentsRef(String refName) throws Exception {
try (Repository repo = repoManager.openRepository(allUsers)) {
return getHead(repo, refName);
}
}
private static String extractComments(String msg) {
// Extract lines between start "....." and end "-- ".
Pattern p = Pattern.compile(".*[.]{5}\n+(.*)\\n+-- \n.*", Pattern.DOTALL);