| // Copyright (C) 2021 The Android Open Source Project |
| // |
| // Licensed under the Apache License, Version 2.0 (the "License"); |
| // you may not use this file except in compliance with the License. |
| // You may obtain a copy of the License at |
| // |
| // http://www.apache.org/licenses/LICENSE-2.0 |
| // |
| // Unless required by applicable law or agreed to in writing, software |
| // distributed under the License is distributed on an "AS IS" BASIS, |
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. |
| // See the License for the specific language governing permissions and |
| // limitations under the License. |
| |
| package com.google.gerrit.server.notedb; |
| |
| import static com.google.common.collect.ImmutableList.toImmutableList; |
| import static com.google.common.truth.Truth.assertThat; |
| import static com.google.gerrit.entities.LabelId.CODE_REVIEW; |
| import static com.google.gerrit.entities.LabelId.VERIFIED; |
| import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC; |
| import static com.google.gerrit.server.notedb.ReviewerStateInternal.REMOVED; |
| import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER; |
| import static java.util.Objects.requireNonNull; |
| |
| import com.google.common.collect.ImmutableList; |
| import com.google.gerrit.entities.Account; |
| import com.google.gerrit.entities.AttentionSetUpdate; |
| import com.google.gerrit.entities.AttentionSetUpdate.Operation; |
| import com.google.gerrit.entities.Change; |
| import com.google.gerrit.entities.ChangeMessage; |
| import com.google.gerrit.entities.LabelId; |
| import com.google.gerrit.entities.PatchSetApproval; |
| import com.google.gerrit.entities.RefNames; |
| import com.google.gerrit.entities.SubmitRecord; |
| import com.google.gerrit.json.OutputFormat; |
| import com.google.gerrit.server.ChangeMessagesUtil; |
| import com.google.gerrit.server.IdentifiedUser; |
| import com.google.gerrit.server.ReviewerStatusUpdate; |
| import com.google.gerrit.server.notedb.ChangeNoteUtil.AttentionStatusInNoteDb; |
| import com.google.gerrit.server.notedb.CommitRewriter.BackfillResult; |
| import com.google.gerrit.server.notedb.CommitRewriter.RunOptions; |
| import com.google.gerrit.server.util.AccountTemplateUtil; |
| import com.google.gerrit.server.util.time.TimeUtil; |
| import com.google.gson.Gson; |
| import com.google.inject.Inject; |
| import java.sql.Timestamp; |
| import java.util.Arrays; |
| import java.util.List; |
| import java.util.stream.IntStream; |
| import org.eclipse.jgit.lib.ObjectId; |
| import org.eclipse.jgit.lib.PersonIdent; |
| import org.eclipse.jgit.lib.Ref; |
| import org.eclipse.jgit.lib.Repository; |
| import org.eclipse.jgit.revwalk.RevCommit; |
| import org.eclipse.jgit.revwalk.RevSort; |
| import org.eclipse.jgit.revwalk.RevWalk; |
| import org.junit.Before; |
| import org.junit.Test; |
| |
| /** Tests for {@link CommitRewriter} */ |
| public class CommitRewriterTest extends AbstractChangeNotesTest { |
| |
| private @Inject CommitRewriter rewriter; |
| @Inject private ChangeNoteUtil changeNoteUtil; |
| |
| private static final Gson gson = OutputFormat.JSON_COMPACT.newGson(); |
| |
| @Before |
| public void setUp() throws Exception {} |
| |
| @Test |
| public void validHistoryNoOp() throws Exception { |
| String tag = "jenkins"; |
| Change c = newChange(); |
| ChangeUpdate update = newUpdate(c, changeOwner); |
| update.setChangeMessage("verification from jenkins"); |
| update.setTag(tag); |
| update.commit(); |
| |
| ChangeUpdate updateWithSubject = newUpdate(c, changeOwner); |
| updateWithSubject.setSubjectForCommit("Update with subject"); |
| updateWithSubject.commit(); |
| |
| ChangeNotes notesBeforeRewrite = newNotes(c); |
| Ref metaRefBefore = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| RunOptions options = new RunOptions(); |
| options.dryRun = false; |
| BackfillResult backfillResult = rewriter.backfillProject(project, repo, options); |
| ChangeNotes notesAfterRewrite = newNotes(c); |
| Ref metaRefAfter = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| |
| assertThat(notesBeforeRewrite.getMetaId()).isEqualTo(notesAfterRewrite.getMetaId()); |
| assertThat(metaRefBefore.getObjectId()).isEqualTo(metaRefAfter.getObjectId()); |
| assertThat(backfillResult.fixedRefDiff).isEmpty(); |
| } |
| |
| @Test |
| public void failedVerification() throws Exception { |
| String tag = "jenkins"; |
| Change c = newChange(); |
| ChangeUpdate update = newUpdate(c, changeOwner); |
| update.setChangeMessage("Unknown commit " + changeOwner.getName()); |
| update.setTag(tag); |
| update.commit(); |
| |
| ChangeUpdate updateWithSubject = newUpdate(c, changeOwner); |
| updateWithSubject.setSubjectForCommit("Update with subject"); |
| updateWithSubject.commit(); |
| |
| ChangeNotes notesBeforeRewrite = newNotes(c); |
| Ref metaRefBefore = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| RunOptions options = new RunOptions(); |
| options.dryRun = false; |
| BackfillResult backfillResult = rewriter.backfillProject(project, repo, options); |
| assertThat(backfillResult.fixedRefDiff).isEmpty(); |
| assertThat(backfillResult.refsStillInvalidAfterFix) |
| .containsExactly(RefNames.changeMetaRef(c.getId())); |
| ChangeNotes notesAfterRewrite = newNotes(c); |
| Ref metaRefAfter = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| |
| assertThat(notesBeforeRewrite.getMetaId()).isEqualTo(notesAfterRewrite.getMetaId()); |
| assertThat(metaRefBefore.getObjectId()).isEqualTo(metaRefAfter.getObjectId()); |
| } |
| |
| @Test |
| public void fixAuthorIdent() throws Exception { |
| Change c = newChange(); |
| Timestamp when = TimeUtil.nowTs(); |
| PersonIdent invalidAuthorIdent = |
| new PersonIdent( |
| changeOwner.getName(), |
| changeNoteUtil.getAccountIdAsEmailAddress(changeOwner.getAccountId()), |
| when, |
| serverIdent.getTimeZone()); |
| RevCommit invalidUpdateCommit = |
| writeUpdate( |
| RefNames.changeMetaRef(c.getId()), |
| getChangeUpdateBody(c, /*changeMessage=*/ null), |
| invalidAuthorIdent); |
| ChangeUpdate validUpdate = newUpdate(c, changeOwner); |
| validUpdate.setChangeMessage("verification from jenkins"); |
| validUpdate.setTag("jenkins"); |
| validUpdate.commit(); |
| |
| Ref metaRefBeforeRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| |
| ImmutableList<RevCommit> commitsBeforeRewrite = logMetaRef(repo, metaRefBeforeRewrite); |
| ChangeNotes notesBeforeRewrite = newNotes(c); |
| |
| RunOptions options = new RunOptions(); |
| options.dryRun = false; |
| BackfillResult result = rewriter.backfillProject(project, repo, options); |
| assertThat(result.fixedRefDiff.keySet()).containsExactly(RefNames.changeMetaRef(c.getId())); |
| |
| ChangeNotes notesAfterRewrite = newNotes(c); |
| |
| assertThat(notesAfterRewrite.getChange().getOwner()) |
| .isEqualTo(notesBeforeRewrite.getChange().getOwner()); |
| Ref metaRefAfterRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| assertThat(metaRefAfterRewrite.getObjectId()).isNotEqualTo(metaRefBeforeRewrite.getObjectId()); |
| ImmutableList<RevCommit> commitsAfterRewrite = logMetaRef(repo, metaRefAfterRewrite); |
| int invalidCommitIndex = commitsBeforeRewrite.indexOf(invalidUpdateCommit); |
| |
| assertValidCommits( |
| commitsBeforeRewrite, commitsAfterRewrite, ImmutableList.of(invalidCommitIndex)); |
| RevCommit fixedUpdateCommit = commitsAfterRewrite.get(invalidCommitIndex); |
| PersonIdent originalAuthorIdent = invalidUpdateCommit.getAuthorIdent(); |
| PersonIdent fixedAuthorIdent = fixedUpdateCommit.getAuthorIdent(); |
| assertThat(originalAuthorIdent).isNotEqualTo(fixedAuthorIdent); |
| assertThat(fixedUpdateCommit.getAuthorIdent().getName()) |
| .isEqualTo("Gerrit User " + changeOwner.getAccountId()); |
| assertThat(originalAuthorIdent.getEmailAddress()).isEqualTo(fixedAuthorIdent.getEmailAddress()); |
| assertThat(originalAuthorIdent.getWhen()).isEqualTo(fixedAuthorIdent.getWhen()); |
| assertThat(originalAuthorIdent.getTimeZone()).isEqualTo(fixedAuthorIdent.getTimeZone()); |
| assertThat(invalidUpdateCommit.getFullMessage()).isEqualTo(fixedUpdateCommit.getFullMessage()); |
| assertThat(invalidUpdateCommit.getCommitterIdent()) |
| .isEqualTo(fixedUpdateCommit.getCommitterIdent()); |
| assertThat(fixedUpdateCommit.getFullMessage()).doesNotContain(changeOwner.getName()); |
| List<String> commitHistoryDiff = result.fixedRefDiff.get(RefNames.changeMetaRef(c.getId())); |
| assertThat(commitHistoryDiff).hasSize(1); |
| assertThat(commitHistoryDiff.get(0)).contains("-author Change Owner <1@gerrit>"); |
| assertThat(commitHistoryDiff.get(0)).contains("+author Gerrit User 1 <1@gerrit>"); |
| } |
| |
| @Test |
| public void fixRealUserFooterIdent() throws Exception { |
| Change c = newChange(); |
| |
| String realUserIdentToFix = getAccountIdentToFix(otherUser.getAccount()); |
| RevCommit invalidUpdateCommit = |
| writeUpdate( |
| RefNames.changeMetaRef(c.getId()), |
| getChangeUpdateBody(c, "Comment on behalf of user", "Real-user: " + realUserIdentToFix), |
| getAuthorIdent(changeOwner.getAccount())); |
| |
| IdentifiedUser impersonatedChangeOwner = |
| this.userFactory.runAs( |
| null, changeOwner.getAccountId(), requireNonNull(otherUser).getRealUser()); |
| ChangeUpdate impersonatedChangeMessageUpdate = newUpdate(c, impersonatedChangeOwner); |
| impersonatedChangeMessageUpdate.setChangeMessage("Other comment on behalf of"); |
| impersonatedChangeMessageUpdate.commit(); |
| |
| Ref metaRefBeforeRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| |
| ImmutableList<RevCommit> commitsBeforeRewrite = logMetaRef(repo, metaRefBeforeRewrite); |
| |
| int invalidCommitIndex = commitsBeforeRewrite.indexOf(invalidUpdateCommit); |
| ChangeNotes notesBeforeRewrite = newNotes(c); |
| |
| RunOptions options = new RunOptions(); |
| options.dryRun = false; |
| BackfillResult result = rewriter.backfillProject(project, repo, options); |
| assertThat(result.fixedRefDiff.keySet()).containsExactly(RefNames.changeMetaRef(c.getId())); |
| |
| ChangeNotes notesAfterRewrite = newNotes(c); |
| assertThat(changeMessages(notesBeforeRewrite)) |
| .containsExactly("Comment on behalf of user", "Other comment on behalf of"); |
| assertThat(notesBeforeRewrite.getChangeMessages().get(0).getAuthor()) |
| .isEqualTo(changeOwner.getAccountId()); |
| assertThat(notesBeforeRewrite.getChangeMessages().get(0).getRealAuthor()) |
| .isEqualTo(otherUser.getAccountId()); |
| assertThat(changeMessages(notesAfterRewrite)) |
| .containsExactly("Comment on behalf of user", "Other comment on behalf of"); |
| assertThat(notesBeforeRewrite.getChangeMessages().get(0).getAuthor()) |
| .isEqualTo(changeOwner.getAccountId()); |
| assertThat(notesBeforeRewrite.getChangeMessages().get(0).getRealAuthor()) |
| .isEqualTo(otherUser.getAccountId()); |
| |
| Ref metaRefAfterRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| assertThat(metaRefAfterRewrite.getObjectId()).isNotEqualTo(metaRefBeforeRewrite.getObjectId()); |
| |
| ImmutableList<RevCommit> commitsAfterRewrite = logMetaRef(repo, metaRefAfterRewrite); |
| assertValidCommits( |
| commitsBeforeRewrite, commitsAfterRewrite, ImmutableList.of(invalidCommitIndex)); |
| |
| List<String> commitHistoryDiff = result.fixedRefDiff.get(RefNames.changeMetaRef(c.getId())); |
| assertThat(commitHistoryDiff) |
| .containsExactly( |
| "@@ -9 +9 @@\n" |
| + "-Real-user: Other Account <2@gerrit>\n" |
| + "+Real-user: Gerrit User 2 <2@gerrit>\n"); |
| } |
| |
| @Test |
| public void fixReviewerFooterIdent() throws Exception { |
| Change c = newChange(); |
| String reviewerIdentToFix = getAccountIdentToFix(otherUser.getAccount()); |
| ImmutableList<RevCommit> commitsToFix = |
| new ImmutableList.Builder<RevCommit>() |
| .add( |
| writeUpdate( |
| RefNames.changeMetaRef(c.getId()), |
| getChangeUpdateBody( |
| c, /*changeMessage=*/ null, "Reviewer: " + reviewerIdentToFix), |
| getAuthorIdent(changeOwner.getAccount()))) |
| .add( |
| writeUpdate( |
| RefNames.changeMetaRef(c.getId()), |
| getChangeUpdateBody(c, /*changeMessage=*/ null, "CC: " + reviewerIdentToFix), |
| getAuthorIdent(otherUser.getAccount()))) |
| .add( |
| writeUpdate( |
| RefNames.changeMetaRef(c.getId()), |
| getChangeUpdateBody(c, "Removed cc", "Removed: " + reviewerIdentToFix), |
| getAuthorIdent(changeOwner.getAccount()))) |
| .build(); |
| |
| Ref metaRefBeforeRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| |
| ImmutableList<RevCommit> commitsBeforeRewrite = logMetaRef(repo, metaRefBeforeRewrite); |
| |
| ImmutableList<Integer> invalidCommits = |
| commitsToFix.stream() |
| .map(commit -> commitsBeforeRewrite.indexOf(commit)) |
| .collect(toImmutableList()); |
| ChangeNotes notesBeforeRewrite = newNotes(c); |
| |
| RunOptions options = new RunOptions(); |
| options.dryRun = false; |
| BackfillResult result = rewriter.backfillProject(project, repo, options); |
| assertThat(result.fixedRefDiff.keySet()).containsExactly(RefNames.changeMetaRef(c.getId())); |
| |
| Timestamp updateTimestamp = new Timestamp(serverIdent.getWhen().getTime()); |
| ImmutableList<ReviewerStatusUpdate> expectedReviewerUpdates = |
| ImmutableList.of( |
| ReviewerStatusUpdate.create( |
| updateTimestamp, changeOwner.getAccountId(), otherUserId, REVIEWER), |
| ReviewerStatusUpdate.create(updateTimestamp, otherUserId, otherUserId, CC), |
| ReviewerStatusUpdate.create( |
| updateTimestamp, changeOwner.getAccountId(), otherUserId, REMOVED)); |
| ChangeNotes notesAfterRewrite = newNotes(c); |
| |
| assertThat(notesBeforeRewrite.getReviewerUpdates()).isEqualTo(expectedReviewerUpdates); |
| assertThat(notesAfterRewrite.getReviewerUpdates()).isEqualTo(expectedReviewerUpdates); |
| |
| Ref metaRefAfterRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| assertThat(metaRefAfterRewrite.getObjectId()).isNotEqualTo(metaRefBeforeRewrite.getObjectId()); |
| |
| ImmutableList<RevCommit> commitsAfterRewrite = logMetaRef(repo, metaRefAfterRewrite); |
| assertValidCommits(commitsBeforeRewrite, commitsAfterRewrite, invalidCommits); |
| |
| List<String> commitHistoryDiff = result.fixedRefDiff.get(RefNames.changeMetaRef(c.getId())); |
| assertThat(commitHistoryDiff) |
| .containsExactly( |
| "@@ -7 +7 @@\n" |
| + "-Reviewer: Other Account <2@gerrit>\n" |
| + "+Reviewer: Gerrit User 2 <2@gerrit>\n", |
| "@@ -7 +7 @@\n" + "-CC: Other Account <2@gerrit>\n" + "+CC: Gerrit User 2 <2@gerrit>\n", |
| "@@ -9 +9 @@\n" |
| + "-Removed: Other Account <2@gerrit>\n" |
| + "+Removed: Gerrit User 2 <2@gerrit>\n"); |
| } |
| |
| @Test |
| public void fixReviewerMessage() throws Exception { |
| Change c = newChange(); |
| ImmutableList.Builder<RevCommit> commitsToFix = new ImmutableList.Builder<>(); |
| ChangeUpdate addReviewerUpdate = newUpdate(c, changeOwner); |
| addReviewerUpdate.putReviewer(otherUserId, REVIEWER); |
| addReviewerUpdate.commit(); |
| |
| commitsToFix.add( |
| writeUpdate( |
| RefNames.changeMetaRef(c.getId()), |
| getChangeUpdateBody( |
| c, |
| "Removed reviewer " + otherUser.getAccount().fullName(), |
| "Removed: " + getValidIdentAsString(otherUser.getAccount())), |
| getAuthorIdent(changeOwner.getAccount()))); |
| |
| ChangeUpdate addCcUpdate = newUpdate(c, changeOwner); |
| addCcUpdate.putReviewer(otherUserId, CC); |
| addCcUpdate.commit(); |
| |
| commitsToFix.add( |
| writeUpdate( |
| RefNames.changeMetaRef(c.getId()), |
| getChangeUpdateBody( |
| c, |
| "Removed cc " + otherUser.getAccount().fullName(), |
| "Removed: " + getValidIdentAsString(otherUser.getAccount())), |
| getAuthorIdent(changeOwner.getAccount()))); |
| |
| Ref metaRefBeforeRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| |
| ImmutableList<RevCommit> commitsBeforeRewrite = logMetaRef(repo, metaRefBeforeRewrite); |
| |
| ImmutableList<Integer> invalidCommits = |
| commitsToFix.build().stream() |
| .map(commit -> commitsBeforeRewrite.indexOf(commit)) |
| .collect(toImmutableList()); |
| ChangeNotes notesBeforeRewrite = newNotes(c); |
| |
| RunOptions options = new RunOptions(); |
| options.dryRun = false; |
| BackfillResult result = rewriter.backfillProject(project, repo, options); |
| assertThat(result.fixedRefDiff.keySet()).containsExactly(RefNames.changeMetaRef(c.getId())); |
| |
| Timestamp updateTimestamp = new Timestamp(serverIdent.getWhen().getTime()); |
| ImmutableList<ReviewerStatusUpdate> expectedReviewerUpdates = |
| ImmutableList.of( |
| ReviewerStatusUpdate.create( |
| new Timestamp(addReviewerUpdate.when.getTime()), |
| changeOwner.getAccountId(), |
| otherUserId, |
| REVIEWER), |
| ReviewerStatusUpdate.create( |
| updateTimestamp, changeOwner.getAccountId(), otherUserId, REMOVED), |
| ReviewerStatusUpdate.create( |
| new Timestamp(addCcUpdate.when.getTime()), |
| changeOwner.getAccountId(), |
| otherUserId, |
| CC), |
| ReviewerStatusUpdate.create( |
| updateTimestamp, changeOwner.getAccountId(), otherUserId, REMOVED)); |
| ChangeNotes notesAfterRewrite = newNotes(c); |
| |
| assertThat(notesBeforeRewrite.getReviewerUpdates()).isEqualTo(expectedReviewerUpdates); |
| assertThat(changeMessages(notesBeforeRewrite)) |
| .containsExactly("Removed reviewer Other Account", "Removed cc Other Account"); |
| assertThat(notesAfterRewrite.getReviewerUpdates()).isEqualTo(expectedReviewerUpdates); |
| assertThat(changeMessages(notesAfterRewrite)).containsExactly("Removed reviewer", "Removed cc"); |
| |
| Ref metaRefAfterRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| assertThat(metaRefAfterRewrite.getObjectId()).isNotEqualTo(metaRefBeforeRewrite.getObjectId()); |
| |
| ImmutableList<RevCommit> commitsAfterRewrite = logMetaRef(repo, metaRefAfterRewrite); |
| assertValidCommits(commitsBeforeRewrite, commitsAfterRewrite, invalidCommits); |
| |
| List<String> commitHistoryDiff = result.fixedRefDiff.get(RefNames.changeMetaRef(c.getId())); |
| assertThat(commitHistoryDiff) |
| .containsExactly( |
| "@@ -6 +6 @@\n" + "-Removed reviewer Other Account\n" + "+Removed reviewer\n", |
| "@@ -6 +6 @@\n" + "-Removed cc Other Account\n" + "+Removed cc\n"); |
| } |
| |
| @Test |
| public void fixLabelFooterIdent() throws Exception { |
| Change c = newChange(); |
| String approverIdentToFix = getAccountIdentToFix(otherUser.getAccount()); |
| String changeOwnerIdentToFix = getAccountIdentToFix(changeOwner.getAccount()); |
| ChangeUpdate approvalUpdateByOtherUser = newUpdate(c, otherUser); |
| approvalUpdateByOtherUser.putApproval(VERIFIED, (short) -1); |
| approvalUpdateByOtherUser.commit(); |
| |
| ImmutableList<RevCommit> commitsToFix = |
| new ImmutableList.Builder<RevCommit>() |
| .add( |
| writeUpdate( |
| RefNames.changeMetaRef(c.getId()), |
| getChangeUpdateBody( |
| c, |
| /*changeMessage=*/ null, |
| "Label: -Verified " + approverIdentToFix, |
| "Label: Custom-Label-1=-1 " + approverIdentToFix, |
| "Label: Verified=+1", |
| "Label: Custom-Label-1=+1", |
| "Label: Custom-Label-2=+2 " + approverIdentToFix, |
| "Label: Custom-Label-3=0 " + approverIdentToFix), |
| getAuthorIdent(changeOwner.getAccount()))) |
| .add( |
| writeUpdate( |
| RefNames.changeMetaRef(c.getId()), |
| getChangeUpdateBody( |
| c, |
| /*changeMessage=*/ null, |
| "Label: -Verified " + changeOwnerIdentToFix, |
| "Label: Custom-Label-1=+1"), |
| getAuthorIdent(otherUser.getAccount()))) |
| .build(); |
| |
| Ref metaRefBeforeRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| |
| ImmutableList<RevCommit> commitsBeforeRewrite = logMetaRef(repo, metaRefBeforeRewrite); |
| |
| ImmutableList<Integer> invalidCommits = |
| commitsToFix.stream() |
| .map(commit -> commitsBeforeRewrite.indexOf(commit)) |
| .collect(toImmutableList()); |
| ChangeNotes notesBeforeRewrite = newNotes(c); |
| |
| RunOptions options = new RunOptions(); |
| options.dryRun = false; |
| BackfillResult result = rewriter.backfillProject(project, repo, options); |
| assertThat(result.fixedRefDiff.keySet()).containsExactly(RefNames.changeMetaRef(c.getId())); |
| |
| Timestamp updateTimestamp = new Timestamp(serverIdent.getWhen().getTime()); |
| ImmutableList<PatchSetApproval> expectedApprovals = |
| ImmutableList.of( |
| PatchSetApproval.builder() |
| .key( |
| PatchSetApproval.key( |
| c.currentPatchSetId(), |
| changeOwner.getAccountId(), |
| LabelId.create(VERIFIED))) |
| .value(0) |
| .granted(updateTimestamp) |
| .build(), |
| PatchSetApproval.builder() |
| .key( |
| PatchSetApproval.key( |
| c.currentPatchSetId(), |
| changeOwner.getAccountId(), |
| LabelId.create("Custom-Label-1"))) |
| .value(+1) |
| .granted(updateTimestamp) |
| .build(), |
| PatchSetApproval.builder() |
| .key( |
| PatchSetApproval.key( |
| c.currentPatchSetId(), otherUserId, LabelId.create(VERIFIED))) |
| .value(0) |
| .granted(updateTimestamp) |
| .build(), |
| PatchSetApproval.builder() |
| .key( |
| PatchSetApproval.key( |
| c.currentPatchSetId(), otherUserId, LabelId.create("Custom-Label-1"))) |
| .value(+1) |
| .granted(updateTimestamp) |
| .build(), |
| PatchSetApproval.builder() |
| .key( |
| PatchSetApproval.key( |
| c.currentPatchSetId(), otherUserId, LabelId.create("Custom-Label-2"))) |
| .value(+2) |
| .granted(updateTimestamp) |
| .build(), |
| PatchSetApproval.builder() |
| .key( |
| PatchSetApproval.key( |
| c.currentPatchSetId(), otherUserId, LabelId.create("Custom-Label-3"))) |
| .value(0) |
| .granted(updateTimestamp) |
| .build()); |
| ChangeNotes notesAfterRewrite = newNotes(c); |
| |
| assertThat(notesBeforeRewrite.getApprovals().get(c.currentPatchSetId())) |
| .containsExactlyElementsIn(expectedApprovals); |
| assertThat(notesAfterRewrite.getApprovals().get(c.currentPatchSetId())) |
| .containsExactlyElementsIn(expectedApprovals); |
| |
| Ref metaRefAfterRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| assertThat(metaRefAfterRewrite.getObjectId()).isNotEqualTo(metaRefBeforeRewrite.getObjectId()); |
| |
| ImmutableList<RevCommit> commitsAfterRewrite = logMetaRef(repo, metaRefAfterRewrite); |
| assertValidCommits(commitsBeforeRewrite, commitsAfterRewrite, invalidCommits); |
| |
| List<String> commitHistoryDiff = result.fixedRefDiff.get(RefNames.changeMetaRef(c.getId())); |
| assertThat(commitHistoryDiff) |
| .containsExactly( |
| "@@ -7,2 +7,2 @@\n" |
| + "-Label: -Verified Other Account <2@gerrit>\n" |
| + "-Label: Custom-Label-1=-1 Other Account <2@gerrit>\n" |
| + "+Label: -Verified Gerrit User 2 <2@gerrit>\n" |
| + "+Label: Custom-Label-1=-1 Gerrit User 2 <2@gerrit>\n" |
| + "@@ -11,2 +11,2 @@\n" |
| + "-Label: Custom-Label-2=+2 Other Account <2@gerrit>\n" |
| + "-Label: Custom-Label-3=0 Other Account <2@gerrit>\n" |
| + "+Label: Custom-Label-2=+2 Gerrit User 2 <2@gerrit>\n" |
| + "+Label: Custom-Label-3=0 Gerrit User 2 <2@gerrit>\n", |
| "@@ -7 +7 @@\n" |
| + "-Label: -Verified Change Owner <1@gerrit>\n" |
| + "+Label: -Verified Gerrit User 1 <1@gerrit>\n"); |
| } |
| |
| @Test |
| public void fixRemoveVoteChangeMessage() throws Exception { |
| Change c = newChange(); |
| String approverIdentToFix = getAccountIdentToFix(otherUser.getAccount()); |
| ChangeUpdate approvalUpdateByOtherUser = newUpdate(c, otherUser); |
| approvalUpdateByOtherUser.putApproval(CODE_REVIEW, (short) +2); |
| approvalUpdateByOtherUser.putApproval("Custom-Label", (short) -1); |
| approvalUpdateByOtherUser.putApprovalFor(changeOwner.getAccountId(), VERIFIED, (short) -1); |
| approvalUpdateByOtherUser.commit(); |
| |
| ImmutableList<RevCommit> commitsToFix = |
| new ImmutableList.Builder<RevCommit>() |
| .add( |
| writeUpdate( |
| RefNames.changeMetaRef(c.getId()), |
| getChangeUpdateBody( |
| c, |
| /*changeMessage=*/ "Removed Code-Review+2 by " + otherUser.getNameEmail(), |
| "Label: -Code-Review " + approverIdentToFix), |
| getAuthorIdent(changeOwner.getAccount()))) |
| .add( |
| writeUpdate( |
| RefNames.changeMetaRef(c.getId()), |
| getChangeUpdateBody( |
| c, |
| /*changeMessage=*/ "Removed Custom-Label-1 by " + otherUser.getNameEmail(), |
| "Label: -Custom-Label " + getValidIdentAsString(otherUser.getAccount())), |
| getAuthorIdent(changeOwner.getAccount()))) |
| .add( |
| writeUpdate( |
| RefNames.changeMetaRef(c.getId()), |
| getChangeUpdateBody( |
| c, |
| /*changeMessage=*/ "Removed Verified+2 by " + changeOwner.getNameEmail(), |
| "Label: -Verified"), |
| getAuthorIdent(changeOwner.getAccount()))) |
| .build(); |
| |
| Ref metaRefBeforeRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| |
| ImmutableList<RevCommit> commitsBeforeRewrite = logMetaRef(repo, metaRefBeforeRewrite); |
| |
| ImmutableList<Integer> invalidCommits = |
| commitsToFix.stream() |
| .map(commit -> commitsBeforeRewrite.indexOf(commit)) |
| .collect(toImmutableList()); |
| ChangeNotes notesBeforeRewrite = newNotes(c); |
| |
| RunOptions options = new RunOptions(); |
| options.dryRun = false; |
| BackfillResult result = rewriter.backfillProject(project, repo, options); |
| assertThat(result.fixedRefDiff.keySet()).containsExactly(RefNames.changeMetaRef(c.getId())); |
| |
| Timestamp updateTimestamp = new Timestamp(serverIdent.getWhen().getTime()); |
| ImmutableList<PatchSetApproval> expectedApprovals = |
| ImmutableList.of( |
| PatchSetApproval.builder() |
| .key( |
| PatchSetApproval.key( |
| c.currentPatchSetId(), |
| changeOwner.getAccountId(), |
| LabelId.create(VERIFIED))) |
| .value(0) |
| .granted(updateTimestamp) |
| .build(), |
| PatchSetApproval.builder() |
| .key( |
| PatchSetApproval.key( |
| c.currentPatchSetId(), otherUserId, LabelId.create("Custom-Label"))) |
| .value(0) |
| .granted(updateTimestamp) |
| .build(), |
| PatchSetApproval.builder() |
| .key( |
| PatchSetApproval.key( |
| c.currentPatchSetId(), otherUserId, LabelId.create(CODE_REVIEW))) |
| .value(0) |
| .granted(updateTimestamp) |
| .build()); |
| ChangeNotes notesAfterRewrite = newNotes(c); |
| assertThat(changeMessages(notesBeforeRewrite)) |
| .containsExactly( |
| "Removed Code-Review+2 by Other Account <other@account.com>", |
| "Removed Custom-Label-1 by Other Account <other@account.com>", |
| "Removed Verified+2 by Change Owner <change@owner.com>"); |
| |
| assertThat(notesBeforeRewrite.getApprovals().get(c.currentPatchSetId())) |
| .containsExactlyElementsIn(expectedApprovals); |
| assertThat(changeMessages(notesAfterRewrite)) |
| .containsExactly( |
| "Removed Code-Review+2 by <GERRIT_ACCOUNT_2>", |
| "Removed Custom-Label-1 by <GERRIT_ACCOUNT_2>", |
| "Removed Verified+2 by <GERRIT_ACCOUNT_1>"); |
| assertThat(notesAfterRewrite.getApprovals().get(c.currentPatchSetId())) |
| .containsExactlyElementsIn(expectedApprovals); |
| |
| Ref metaRefAfterRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| assertThat(metaRefAfterRewrite.getObjectId()).isNotEqualTo(metaRefBeforeRewrite.getObjectId()); |
| |
| ImmutableList<RevCommit> commitsAfterRewrite = logMetaRef(repo, metaRefAfterRewrite); |
| assertValidCommits(commitsBeforeRewrite, commitsAfterRewrite, invalidCommits); |
| |
| List<String> commitHistoryDiff = result.fixedRefDiff.get(RefNames.changeMetaRef(c.getId())); |
| assertThat(commitHistoryDiff) |
| .containsExactly( |
| "@@ -6 +6 @@\n" |
| + "-Removed Code-Review+2 by Other Account <other@account.com>\n" |
| + "+Removed Code-Review+2 by <GERRIT_ACCOUNT_2>\n" |
| + "@@ -9 +9 @@\n" |
| + "-Label: -Code-Review Other Account <2@gerrit>\n" |
| + "+Label: -Code-Review Gerrit User 2 <2@gerrit>\n", |
| "@@ -6 +6 @@\n" |
| + "-Removed Custom-Label-1 by Other Account <other@account.com>\n" |
| + "+Removed Custom-Label-1 by <GERRIT_ACCOUNT_2>\n", |
| "@@ -6 +6 @@\n" |
| + "-Removed Verified+2 by Change Owner <change@owner.com>\n" |
| + "+Removed Verified+2 by <GERRIT_ACCOUNT_1>\n"); |
| } |
| |
| @Test |
| public void fixRemoveVoteChangeMessageWithUnparsableAuthorIdent() throws Exception { |
| Change c = newChange(); |
| PersonIdent invalidAuthorIdent = |
| new PersonIdent( |
| changeOwner.getName(), |
| "server@" + serverId, |
| TimeUtil.nowTs(), |
| serverIdent.getTimeZone()); |
| writeUpdate( |
| RefNames.changeMetaRef(c.getId()), |
| getChangeUpdateBody( |
| c, |
| /*changeMessage=*/ "Removed Verified+2 by " + changeOwner.getNameEmail(), |
| "Label: -Verified"), |
| invalidAuthorIdent); |
| |
| RunOptions options = new RunOptions(); |
| options.dryRun = false; |
| BackfillResult result = rewriter.backfillProject(project, repo, options); |
| assertThat(result.fixedRefDiff.keySet()).containsExactly(RefNames.changeMetaRef(c.getId())); |
| |
| List<String> commitHistoryDiff = result.fixedRefDiff.get(RefNames.changeMetaRef(c.getId())); |
| assertThat(commitHistoryDiff) |
| .containsExactly( |
| "@@ -6 +6 @@\n" |
| + "-Removed Verified+2 by Change Owner <change@owner.com>\n" |
| + "+Removed Verified+2 by Gerrit Account\n"); |
| } |
| |
| @Test |
| public void fixAttentionFooter() throws Exception { |
| Change c = newChange(); |
| ImmutableList.Builder<ObjectId> commitsToFix = new ImmutableList.Builder<>(); |
| // Only 'reason' fix is required |
| ChangeUpdate invalidAttentionSetUpdate = newUpdate(c, changeOwner); |
| invalidAttentionSetUpdate.putReviewer(otherUserId, REVIEWER); |
| invalidAttentionSetUpdate.addToPlannedAttentionSetUpdates( |
| AttentionSetUpdate.createForWrite( |
| otherUserId, |
| Operation.ADD, |
| String.format("Added by %s using the hovercard menu", otherUser.getName()))); |
| commitsToFix.add(invalidAttentionSetUpdate.commit()); |
| ChangeUpdate invalidMultipleAttentionSetUpdate = newUpdate(c, changeOwner); |
| invalidMultipleAttentionSetUpdate.addToPlannedAttentionSetUpdates( |
| AttentionSetUpdate.createForWrite( |
| changeOwner.getAccountId(), |
| Operation.ADD, |
| String.format("%s replied on the change", otherUser.getName()))); |
| invalidMultipleAttentionSetUpdate.addToPlannedAttentionSetUpdates( |
| AttentionSetUpdate.createForWrite( |
| otherUserId, |
| Operation.REMOVE, |
| String.format("Removed by %s using the hovercard menu", otherUser.getName()))); |
| commitsToFix.add(invalidMultipleAttentionSetUpdate.commit()); |
| String otherUserIdentToFix = getAccountIdentToFix(otherUser.getAccount()); |
| String changeOwnerIdentToFix = getAccountIdentToFix(changeOwner.getAccount()); |
| commitsToFix.add( |
| writeUpdate( |
| RefNames.changeMetaRef(c.getId()), |
| getChangeUpdateBody( |
| c, |
| /*changeMessage=*/ null, |
| // Only 'person_ident' fix is required |
| "Attention: " |
| + gson.toJson( |
| new AttentionStatusInNoteDb( |
| otherUserIdentToFix, |
| Operation.ADD, |
| "Added by someone using the hovercard menu")), |
| // Both 'reason' and 'person_ident' fix is required |
| "Attention: " |
| + gson.toJson( |
| new AttentionStatusInNoteDb( |
| changeOwnerIdentToFix, |
| Operation.REMOVE, |
| String.format("%s replied on the change", otherUser.getName())))), |
| getAuthorIdent(changeOwner.getAccount()))); |
| |
| ChangeUpdate validAttentionSetUpdate = newUpdate(c, changeOwner); |
| validAttentionSetUpdate.addToPlannedAttentionSetUpdates( |
| AttentionSetUpdate.createForWrite(otherUserId, Operation.REMOVE, "Removed by someone")); |
| validAttentionSetUpdate.commit(); |
| |
| Ref metaRefBeforeRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| |
| ImmutableList<RevCommit> commitsBeforeRewrite = logMetaRef(repo, metaRefBeforeRewrite); |
| |
| ImmutableList<Integer> invalidCommits = |
| commitsToFix.build().stream() |
| .map(commit -> commitsBeforeRewrite.indexOf(commit)) |
| .collect(toImmutableList()); |
| ChangeNotes notesBeforeRewrite = newNotes(c); |
| |
| RunOptions options = new RunOptions(); |
| options.dryRun = false; |
| BackfillResult result = rewriter.backfillProject(project, repo, options); |
| assertThat(result.fixedRefDiff.keySet()).containsExactly(RefNames.changeMetaRef(c.getId())); |
| notesBeforeRewrite.getAttentionSetUpdates(); |
| Timestamp updateTimestamp = new Timestamp(serverIdent.getWhen().getTime()); |
| ImmutableList<AttentionSetUpdate> attentionSetUpdatesBeforeRewrite = |
| ImmutableList.of( |
| AttentionSetUpdate.createFromRead( |
| validAttentionSetUpdate.getWhen().toInstant(), |
| otherUserId, |
| Operation.REMOVE, |
| "Removed by someone"), |
| AttentionSetUpdate.createFromRead( |
| updateTimestamp.toInstant(), |
| changeOwner.getAccountId(), |
| Operation.REMOVE, |
| String.format("%s replied on the change", otherUser.getName())), |
| AttentionSetUpdate.createFromRead( |
| updateTimestamp.toInstant(), |
| otherUserId, |
| Operation.ADD, |
| "Added by someone using the hovercard menu"), |
| AttentionSetUpdate.createFromRead( |
| invalidMultipleAttentionSetUpdate.getWhen().toInstant(), |
| otherUserId, |
| Operation.REMOVE, |
| String.format("Removed by %s using the hovercard menu", otherUser.getName())), |
| AttentionSetUpdate.createFromRead( |
| invalidMultipleAttentionSetUpdate.getWhen().toInstant(), |
| changeOwner.getAccountId(), |
| Operation.ADD, |
| String.format("%s replied on the change", otherUser.getName())), |
| AttentionSetUpdate.createFromRead( |
| invalidAttentionSetUpdate.getWhen().toInstant(), |
| otherUserId, |
| Operation.ADD, |
| String.format("Added by %s using the hovercard menu", otherUser.getName()))); |
| |
| ImmutableList<AttentionSetUpdate> attentionSetUpdatesAfterRewrite = |
| ImmutableList.of( |
| AttentionSetUpdate.createFromRead( |
| validAttentionSetUpdate.getWhen().toInstant(), |
| otherUserId, |
| Operation.REMOVE, |
| "Removed by someone"), |
| AttentionSetUpdate.createFromRead( |
| updateTimestamp.toInstant(), |
| changeOwner.getAccountId(), |
| Operation.REMOVE, |
| "Someone replied on the change"), |
| AttentionSetUpdate.createFromRead( |
| updateTimestamp.toInstant(), |
| otherUserId, |
| Operation.ADD, |
| "Added by someone using the hovercard menu"), |
| AttentionSetUpdate.createFromRead( |
| invalidMultipleAttentionSetUpdate.getWhen().toInstant(), |
| otherUserId, |
| Operation.REMOVE, |
| "Removed by someone using the hovercard menu"), |
| AttentionSetUpdate.createFromRead( |
| invalidMultipleAttentionSetUpdate.getWhen().toInstant(), |
| changeOwner.getAccountId(), |
| Operation.ADD, |
| "Someone replied on the change"), |
| AttentionSetUpdate.createFromRead( |
| invalidAttentionSetUpdate.getWhen().toInstant(), |
| otherUserId, |
| Operation.ADD, |
| "Added by someone using the hovercard menu")); |
| |
| ChangeNotes notesAfterRewrite = newNotes(c); |
| |
| assertThat(notesBeforeRewrite.getAttentionSetUpdates()) |
| .containsExactlyElementsIn(attentionSetUpdatesBeforeRewrite); |
| assertThat(notesAfterRewrite.getAttentionSetUpdates()) |
| .containsExactlyElementsIn(attentionSetUpdatesAfterRewrite); |
| |
| Ref metaRefAfterRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| assertThat(metaRefAfterRewrite.getObjectId()).isNotEqualTo(metaRefBeforeRewrite.getObjectId()); |
| |
| ImmutableList<RevCommit> commitsAfterRewrite = logMetaRef(repo, metaRefAfterRewrite); |
| assertValidCommits(commitsBeforeRewrite, commitsAfterRewrite, invalidCommits); |
| |
| List<String> commitHistoryDiff = result.fixedRefDiff.get(RefNames.changeMetaRef(c.getId())); |
| assertThat(commitHistoryDiff).hasSize(3); |
| assertThat(commitHistoryDiff.get(0)) |
| .isEqualTo( |
| "@@ -8 +8 @@\n" |
| + "-Attention: {\"person_ident\":\"Gerrit User 2 \\u003c2@gerrit\\u003e\",\"operation\":\"ADD\",\"reason\":\"Added by Other Account using the hovercard menu\"}\n" |
| + "+Attention: {\"person_ident\":\"Gerrit User 2 \\u003c2@gerrit\\u003e\",\"operation\":\"ADD\",\"reason\":\"Added by someone using the hovercard menu\"}\n"); |
| assertThat(Arrays.asList(commitHistoryDiff.get(1).split("\n"))) |
| .containsExactly( |
| "@@ -7,2 +7,2 @@", |
| "-Attention: {\"person_ident\":\"Gerrit User 1 \\u003c1@gerrit\\u003e\",\"operation\":\"ADD\",\"reason\":\"Other Account replied on the change\"}", |
| "-Attention: {\"person_ident\":\"Gerrit User 2 \\u003c2@gerrit\\u003e\",\"operation\":\"REMOVE\",\"reason\":\"Removed by Other Account using the hovercard menu\"}", |
| "+Attention: {\"person_ident\":\"Gerrit User 1 \\u003c1@gerrit\\u003e\",\"operation\":\"ADD\",\"reason\":\"Someone replied on the change\"}", |
| "+Attention: {\"person_ident\":\"Gerrit User 2 \\u003c2@gerrit\\u003e\",\"operation\":\"REMOVE\",\"reason\":\"Removed by someone using the hovercard menu\"}"); |
| assertThat(Arrays.asList(commitHistoryDiff.get(2).split("\n"))) |
| .containsExactly( |
| "@@ -7,2 +7,2 @@", |
| "-Attention: {\"person_ident\":\"Other Account \\u003c2@gerrit\\u003e\",\"operation\":\"ADD\",\"reason\":\"Added by someone using the hovercard menu\"}", |
| "-Attention: {\"person_ident\":\"Change Owner \\u003c1@gerrit\\u003e\",\"operation\":\"REMOVE\",\"reason\":\"Other Account replied on the change\"}", |
| "+Attention: {\"person_ident\":\"Gerrit User 2 \\u003c2@gerrit\\u003e\",\"operation\":\"ADD\",\"reason\":\"Added by someone using the hovercard menu\"}", |
| "+Attention: {\"person_ident\":\"Gerrit User 1 \\u003c1@gerrit\\u003e\",\"operation\":\"REMOVE\",\"reason\":\"Someone replied on the change\"}"); |
| } |
| |
| @Test |
| public void fixAttentionFooter_okReason_noRewrite() throws Exception { |
| Change c = newChange(); |
| ImmutableList<String> okAccountNames = |
| ImmutableList.of( |
| "Someone", |
| "Someone else", |
| "someone", |
| "someone else", |
| "Anonymous", |
| "anonymous", |
| "<GERRIT_ACCOUNT_1>", |
| "<GERRIT_ACCOUNT_2>"); |
| ImmutableList.Builder<AttentionSetUpdate> attentionSetUpdatesBeforeRewrite = |
| new ImmutableList.Builder<>(); |
| for (String okAccountName : okAccountNames) { |
| ChangeUpdate firstAttentionSetUpdate = newUpdate(c, changeOwner); |
| firstAttentionSetUpdate.putReviewer(otherUserId, REVIEWER); |
| firstAttentionSetUpdate.addToPlannedAttentionSetUpdates( |
| AttentionSetUpdate.createForWrite( |
| otherUserId, |
| Operation.ADD, |
| String.format("Added by %s using the hovercard menu", okAccountName))); |
| firstAttentionSetUpdate.commit(); |
| ChangeUpdate secondAttentionSetUpdate = newUpdate(c, changeOwner); |
| secondAttentionSetUpdate.addToPlannedAttentionSetUpdates( |
| AttentionSetUpdate.createForWrite( |
| changeOwner.getAccountId(), |
| Operation.ADD, |
| String.format("%s replied on the change", okAccountName))); |
| secondAttentionSetUpdate.addToPlannedAttentionSetUpdates( |
| AttentionSetUpdate.createForWrite( |
| otherUserId, |
| Operation.REMOVE, |
| String.format("Removed by %s using the hovercard menu", okAccountName))); |
| secondAttentionSetUpdate.commit(); |
| ChangeUpdate clearAttentionSetUpdate = newUpdate(c, changeOwner); |
| clearAttentionSetUpdate.addToPlannedAttentionSetUpdates( |
| AttentionSetUpdate.createForWrite(changeOwner.getAccountId(), Operation.REMOVE, "Clear")); |
| clearAttentionSetUpdate.commit(); |
| attentionSetUpdatesBeforeRewrite.add( |
| AttentionSetUpdate.createFromRead( |
| clearAttentionSetUpdate.getWhen().toInstant(), |
| changeOwner.getAccountId(), |
| Operation.REMOVE, |
| "Clear"), |
| AttentionSetUpdate.createFromRead( |
| secondAttentionSetUpdate.getWhen().toInstant(), |
| otherUserId, |
| Operation.REMOVE, |
| String.format("Removed by %s using the hovercard menu", okAccountName)), |
| AttentionSetUpdate.createFromRead( |
| secondAttentionSetUpdate.getWhen().toInstant(), |
| changeOwner.getAccountId(), |
| Operation.ADD, |
| String.format("%s replied on the change", okAccountName)), |
| AttentionSetUpdate.createFromRead( |
| firstAttentionSetUpdate.getWhen().toInstant(), |
| otherUserId, |
| Operation.ADD, |
| String.format("Added by %s using the hovercard menu", okAccountName))); |
| clearAttentionSetUpdate.getNotes(); |
| } |
| |
| ChangeNotes notesBeforeRewrite = newNotes(c); |
| assertThat(notesBeforeRewrite.getAttentionSetUpdates()) |
| .containsExactlyElementsIn(attentionSetUpdatesBeforeRewrite.build()); |
| |
| Ref metaRefBefore = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| RunOptions options = new RunOptions(); |
| options.dryRun = false; |
| BackfillResult backfillResult = rewriter.backfillProject(project, repo, options); |
| ChangeNotes notesAfterRewrite = newNotes(c); |
| Ref metaRefAfter = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| |
| assertThat(notesBeforeRewrite.getMetaId()).isEqualTo(notesAfterRewrite.getMetaId()); |
| assertThat(metaRefBefore.getObjectId()).isEqualTo(metaRefAfter.getObjectId()); |
| assertThat(backfillResult.fixedRefDiff).isEmpty(); |
| } |
| |
| @Test |
| public void fixSubmitChangeMessage() throws Exception { |
| Change c = newChange(); |
| ImmutableList.Builder<ObjectId> commitsToFix = new ImmutableList.Builder<>(); |
| ChangeUpdate invalidMergedMessageUpdate = newUpdate(c, changeOwner); |
| invalidMergedMessageUpdate.setChangeMessage( |
| "Change has been successfully merged by " + changeOwner.getName()); |
| invalidMergedMessageUpdate.setTopic(""); |
| invalidMergedMessageUpdate.setTag(ChangeMessagesUtil.TAG_MERGED); |
| commitsToFix.add(invalidMergedMessageUpdate.commit()); |
| ChangeUpdate invalidCherryPickedMessageUpdate = newUpdate(c, changeOwner); |
| invalidCherryPickedMessageUpdate.setChangeMessage( |
| "Change has been successfully cherry-picked as e40dc1a50dc7f457a37579e2755374f3e1a5413b by " |
| + changeOwner.getName()); |
| invalidCherryPickedMessageUpdate.setTag(ChangeMessagesUtil.TAG_MERGED); |
| commitsToFix.add(invalidCherryPickedMessageUpdate.commit()); |
| ChangeUpdate invalidRebasedMessageUpdate = newUpdate(c, changeOwner); |
| invalidRebasedMessageUpdate.setChangeMessage( |
| "Change has been successfully rebased and submitted as e40dc1a50dc7f457a37579e2755374f3e1a5413b by " |
| + changeOwner.getName()); |
| invalidRebasedMessageUpdate.setTag(ChangeMessagesUtil.TAG_MERGED); |
| commitsToFix.add(invalidRebasedMessageUpdate.commit()); |
| ChangeUpdate validSubmitMessageUpdate = newUpdate(c, changeOwner); |
| validSubmitMessageUpdate.setChangeMessage( |
| "Change has been successfully rebased and submitted as e40dc1a50dc7f457a37579e2755374f3e1a5413b"); |
| validSubmitMessageUpdate.commit(); |
| |
| Ref metaRefBeforeRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| |
| ImmutableList<RevCommit> commitsBeforeRewrite = logMetaRef(repo, metaRefBeforeRewrite); |
| |
| ImmutableList<Integer> invalidCommits = |
| commitsToFix.build().stream() |
| .map(commit -> commitsBeforeRewrite.indexOf(commit)) |
| .collect(toImmutableList()); |
| ChangeNotes notesBeforeRewrite = newNotes(c); |
| |
| RunOptions options = new RunOptions(); |
| options.dryRun = false; |
| BackfillResult result = rewriter.backfillProject(project, repo, options); |
| assertThat(result.fixedRefDiff.keySet()).containsExactly(RefNames.changeMetaRef(c.getId())); |
| |
| ChangeNotes notesAfterRewrite = newNotes(c); |
| |
| assertThat(changeMessages(notesBeforeRewrite)) |
| .containsExactly( |
| "Change has been successfully merged by Change Owner", |
| "Change has been successfully cherry-picked as e40dc1a50dc7f457a37579e2755374f3e1a5413b by Change Owner", |
| "Change has been successfully rebased and submitted as e40dc1a50dc7f457a37579e2755374f3e1a5413b by Change Owner", |
| "Change has been successfully rebased and submitted as e40dc1a50dc7f457a37579e2755374f3e1a5413b"); |
| assertThat(changeMessages(notesAfterRewrite)) |
| .containsExactly( |
| "Change has been successfully merged", |
| "Change has been successfully cherry-picked as e40dc1a50dc7f457a37579e2755374f3e1a5413b", |
| "Change has been successfully rebased and submitted as e40dc1a50dc7f457a37579e2755374f3e1a5413b", |
| "Change has been successfully rebased and submitted as e40dc1a50dc7f457a37579e2755374f3e1a5413b"); |
| |
| Ref metaRefAfterRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| assertThat(metaRefAfterRewrite.getObjectId()).isNotEqualTo(metaRefBeforeRewrite.getObjectId()); |
| |
| ImmutableList<RevCommit> commitsAfterRewrite = logMetaRef(repo, metaRefAfterRewrite); |
| assertValidCommits(commitsBeforeRewrite, commitsAfterRewrite, invalidCommits); |
| |
| List<String> commitHistoryDiff = result.fixedRefDiff.get(RefNames.changeMetaRef(c.getId())); |
| assertThat(commitHistoryDiff) |
| .containsExactly( |
| "@@ -6 +6 @@\n" |
| + "-Change has been successfully merged by Change Owner\n" |
| + "+Change has been successfully merged\n", |
| "@@ -6 +6 @@\n" |
| + "-Change has been successfully cherry-picked as e40dc1a50dc7f457a37579e2755374f3e1a5413b by Change Owner\n" |
| + "+Change has been successfully cherry-picked as e40dc1a50dc7f457a37579e2755374f3e1a5413b\n", |
| "@@ -6 +6 @@\n" |
| + "-Change has been successfully rebased and submitted as e40dc1a50dc7f457a37579e2755374f3e1a5413b by Change Owner\n" |
| + "+Change has been successfully rebased and submitted as e40dc1a50dc7f457a37579e2755374f3e1a5413b\n"); |
| } |
| |
| @Test |
| public void fixSubmitChangeMessageAndFooters() throws Exception { |
| Change c = newChange(); |
| PersonIdent invalidAuthorIdent = |
| new PersonIdent( |
| changeOwner.getName(), |
| changeNoteUtil.getAccountIdAsEmailAddress(changeOwner.getAccountId()), |
| TimeUtil.nowTs(), |
| serverIdent.getTimeZone()); |
| String changeOwnerIdentToFix = getAccountIdentToFix(changeOwner.getAccount()); |
| writeUpdate( |
| RefNames.changeMetaRef(c.getId()), |
| getChangeUpdateBody( |
| c, |
| "Change has been successfully merged by " + changeOwner.getName(), |
| "Status: merged", |
| "Tag: autogenerated:gerrit:merged", |
| "Reviewer: " + changeOwnerIdentToFix, |
| "Label: SUBM=+1", |
| "Submission-id: 6310-1521542139810-cfb7e159", |
| "Submitted-with: OK", |
| "Submitted-with: OK: Code-Review: " + changeOwnerIdentToFix), |
| invalidAuthorIdent); |
| |
| RunOptions options = new RunOptions(); |
| options.dryRun = false; |
| BackfillResult result = rewriter.backfillProject(project, repo, options); |
| assertThat(result.fixedRefDiff.keySet()).containsExactly(RefNames.changeMetaRef(c.getId())); |
| |
| List<String> commitHistoryDiff = result.fixedRefDiff.get(RefNames.changeMetaRef(c.getId())); |
| assertThat(commitHistoryDiff) |
| .containsExactly( |
| "@@ -1 +1 @@\n" |
| + "-author Change Owner <1@gerrit> 1254344405 -0700\n" |
| + "+author Gerrit User 1 <1@gerrit> 1254344405 -0700\n" |
| + "@@ -6 +6 @@\n" |
| + "-Change has been successfully merged by Change Owner\n" |
| + "+Change has been successfully merged\n" |
| + "@@ -11 +11 @@\n" |
| + "-Reviewer: Change Owner <1@gerrit>\n" |
| + "+Reviewer: Gerrit User 1 <1@gerrit>\n" |
| + "@@ -15 +15 @@\n" |
| + "-Submitted-with: OK: Code-Review: Change Owner <1@gerrit>\n" |
| + "+Submitted-with: OK: Code-Review: Gerrit User 1 <1@gerrit>\n"); |
| } |
| |
| @Test |
| public void fixSubmittedWithFooterIdent() throws Exception { |
| Change c = newChange(); |
| |
| ChangeUpdate preSubmitUpdate = newUpdate(c, changeOwner); |
| preSubmitUpdate.setChangeMessage("Per-submit update"); |
| preSubmitUpdate.commit(); |
| |
| String otherUserIdentToFix = getAccountIdentToFix(otherUser.getAccount()); |
| String changeOwnerIdentToFix = getAccountIdentToFix(changeOwner.getAccount()); |
| RevCommit invalidUpdateCommit = |
| writeUpdate( |
| RefNames.changeMetaRef(c.getId()), |
| getChangeUpdateBody( |
| c, |
| /*changeMessage=*/ null, |
| "Label: SUBM=+1", |
| "Submission-id: 5271-1496917120975-10a10df9", |
| "Submitted-with: NOT_READY", |
| "Submitted-with: NEED: Code-Review: " + otherUserIdentToFix, |
| "Submitted-with: OK: Code-Style", |
| "Submitted-with: OK: Verified: " + changeOwnerIdentToFix, |
| "Submitted-with: FORCED with error"), |
| getAuthorIdent(changeOwner.getAccount())); |
| |
| ChangeUpdate postSubmitUpdate = newUpdate(c, changeOwner); |
| postSubmitUpdate.setChangeMessage("Per-submit update"); |
| postSubmitUpdate.commit(); |
| Ref metaRefBeforeRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| |
| ImmutableList<RevCommit> commitsBeforeRewrite = logMetaRef(repo, metaRefBeforeRewrite); |
| |
| int invalidCommitIndex = commitsBeforeRewrite.indexOf(invalidUpdateCommit); |
| ChangeNotes notesBeforeRewrite = newNotes(c); |
| |
| RunOptions options = new RunOptions(); |
| options.dryRun = false; |
| BackfillResult result = rewriter.backfillProject(project, repo, options); |
| assertThat(result.fixedRefDiff.keySet()).containsExactly(RefNames.changeMetaRef(c.getId())); |
| |
| ChangeNotes notesAfterRewrite = newNotes(c); |
| ImmutableList<SubmitRecord> expectedRecords = |
| ImmutableList.of( |
| submitRecord( |
| "NOT_READY", |
| null, |
| submitLabel(CODE_REVIEW, "NEED", otherUserId), |
| submitLabel("Code-Style", "OK", null), |
| submitLabel(VERIFIED, "OK", changeOwner.getAccountId())), |
| submitRecord("FORCED", " with error")); |
| assertThat(notesBeforeRewrite.getSubmitRecords()).isEqualTo(expectedRecords); |
| assertThat(notesAfterRewrite.getSubmitRecords()).isEqualTo(expectedRecords); |
| |
| Ref metaRefAfterRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| assertThat(metaRefAfterRewrite.getObjectId()).isNotEqualTo(metaRefBeforeRewrite.getObjectId()); |
| |
| ImmutableList<RevCommit> commitsAfterRewrite = logMetaRef(repo, metaRefAfterRewrite); |
| assertValidCommits( |
| commitsBeforeRewrite, commitsAfterRewrite, ImmutableList.of(invalidCommitIndex)); |
| |
| List<String> commitHistoryDiff = result.fixedRefDiff.get(RefNames.changeMetaRef(c.getId())); |
| assertThat(commitHistoryDiff) |
| .containsExactly( |
| "@@ -10 +10 @@\n" |
| + "-Submitted-with: NEED: Code-Review: Other Account <2@gerrit>\n" |
| + "+Submitted-with: NEED: Code-Review: Gerrit User 2 <2@gerrit>\n" |
| + "@@ -12 +12 @@\n" |
| + "-Submitted-with: OK: Verified: Change Owner <1@gerrit>\n" |
| + "+Submitted-with: OK: Verified: Gerrit User 1 <1@gerrit>\n"); |
| } |
| |
| @Test |
| public void fixDeleteChangeMessageCommitMessage() throws Exception { |
| Change c = newChange(); |
| ImmutableList.Builder<ObjectId> commitsToFix = new ImmutableList.Builder<>(); |
| ChangeUpdate invalidDeleteChangeMessageUpdate = newUpdate(c, changeOwner); |
| invalidDeleteChangeMessageUpdate.setChangeMessage( |
| "Change message removed by: " + changeOwner.getName()); |
| commitsToFix.add(invalidDeleteChangeMessageUpdate.commit()); |
| ChangeUpdate invalidDeleteChangeMessageUpdateWithReason = newUpdate(c, changeOwner); |
| invalidDeleteChangeMessageUpdateWithReason.setChangeMessage( |
| String.format( |
| "Change message removed by: %s\nReason: %s", |
| changeOwner.getName(), "contains confidential information")); |
| commitsToFix.add(invalidDeleteChangeMessageUpdateWithReason.commit()); |
| ChangeUpdate validDeleteChangeMessageUpdate = newUpdate(c, changeOwner); |
| validDeleteChangeMessageUpdate.setChangeMessage( |
| "Change message removed by: <GERRIT_ACCOUNT_1>"); |
| validDeleteChangeMessageUpdate.commit(); |
| ChangeUpdate validDeleteChangeMessageUpdateWithReason = newUpdate(c, changeOwner); |
| validDeleteChangeMessageUpdateWithReason.setChangeMessage( |
| "Change message removed by: <GERRIT_ACCOUNT_1>\nReason: abusive language"); |
| validDeleteChangeMessageUpdateWithReason.commit(); |
| |
| Ref metaRefBeforeRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| |
| ImmutableList<RevCommit> commitsBeforeRewrite = logMetaRef(repo, metaRefBeforeRewrite); |
| |
| ImmutableList<Integer> invalidCommits = |
| commitsToFix.build().stream() |
| .map(commit -> commitsBeforeRewrite.indexOf(commit)) |
| .collect(toImmutableList()); |
| ChangeNotes notesBeforeRewrite = newNotes(c); |
| |
| RunOptions options = new RunOptions(); |
| options.dryRun = false; |
| BackfillResult result = rewriter.backfillProject(project, repo, options); |
| assertThat(result.fixedRefDiff.keySet()).containsExactly(RefNames.changeMetaRef(c.getId())); |
| |
| ChangeNotes notesAfterRewrite = newNotes(c); |
| |
| assertThat(changeMessages(notesBeforeRewrite)) |
| .containsExactly( |
| "Change message removed by: Change Owner", |
| "Change message removed by: Change Owner\n" |
| + "Reason: contains confidential information", |
| "Change message removed by: <GERRIT_ACCOUNT_1>", |
| "Change message removed by: <GERRIT_ACCOUNT_1>\n" + "Reason: abusive language"); |
| assertThat(changeMessages(notesAfterRewrite)) |
| .containsExactly( |
| "Change message removed", |
| "Change message removed\n" + "Reason: contains confidential information", |
| "Change message removed by: <GERRIT_ACCOUNT_1>", |
| "Change message removed by: <GERRIT_ACCOUNT_1>\n" + "Reason: abusive language"); |
| |
| Ref metaRefAfterRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| assertThat(metaRefAfterRewrite.getObjectId()).isNotEqualTo(metaRefBeforeRewrite.getObjectId()); |
| |
| ImmutableList<RevCommit> commitsAfterRewrite = logMetaRef(repo, metaRefAfterRewrite); |
| assertValidCommits(commitsBeforeRewrite, commitsAfterRewrite, invalidCommits); |
| |
| List<String> commitHistoryDiff = result.fixedRefDiff.get(RefNames.changeMetaRef(c.getId())); |
| assertThat(commitHistoryDiff) |
| .containsExactly( |
| "@@ -6 +6 @@\n" |
| + "-Change message removed by: Change Owner\n" |
| + "+Change message removed\n", |
| "@@ -6 +6 @@\n" |
| + "-Change message removed by: Change Owner\n" |
| + "+Change message removed\n"); |
| } |
| |
| @Test |
| public void fixCodeOwnersOnAddReviewerChangeMessage() throws Exception { |
| |
| Account reviewer = |
| Account.builder(Account.id(3), TimeUtil.nowTs()) |
| .setFullName("Reviewer User") |
| .setPreferredEmail("reviewer@account.com") |
| .build(); |
| accountCache.put(reviewer); |
| Account duplicateCodeOwner = |
| Account.builder(Account.id(4), TimeUtil.nowTs()).setFullName(changeOwner.getName()).build(); |
| accountCache.put(duplicateCodeOwner); |
| Account duplicateReviewer = |
| Account.builder(Account.id(5), TimeUtil.nowTs()).setFullName(reviewer.getName()).build(); |
| accountCache.put(duplicateReviewer); |
| Change c = newChange(); |
| ImmutableList.Builder<ObjectId> commitsToFix = new ImmutableList.Builder<>(); |
| ChangeUpdate addReviewerUpdate = newUpdate(c, changeOwner); |
| addReviewerUpdate.putReviewer(reviewer.id(), REVIEWER); |
| addReviewerUpdate.commit(); |
| ChangeUpdate invalidOnAddReviewerUpdate = newUpdate(c, changeOwner); |
| invalidOnAddReviewerUpdate.setChangeMessage( |
| "Reviewer User who was added as reviewer owns the following files:\n" |
| + " * file1.java\n" |
| + " * file2.ts\n"); |
| commitsToFix.add(invalidOnAddReviewerUpdate.commit()); |
| ChangeUpdate addOtherReviewerUpdate = newUpdate(c, changeOwner); |
| addOtherReviewerUpdate.putReviewer(otherUserId, REVIEWER); |
| addOtherReviewerUpdate.commit(); |
| ChangeUpdate invalidOnAddReviewerMultipleReviewerUpdate = newUpdate(c, changeOwner); |
| invalidOnAddReviewerMultipleReviewerUpdate.setChangeMessage( |
| "Reviewer User who was added as reviewer owns the following files:\n" |
| + " * file1.java\n" |
| + "\nOther Account who was added as reviewer owns the following files:\n" |
| + " * file3.js\n" |
| + "\nMissing Reviewer who was added as reviewer owns the following files:\n" |
| + " * file4.java\n"); |
| commitsToFix.add(invalidOnAddReviewerMultipleReviewerUpdate.commit()); |
| ChangeUpdate addDuplicateReviewerUpdate = newUpdate(c, changeOwner); |
| addDuplicateReviewerUpdate.putReviewer(duplicateReviewer.id(), REVIEWER); |
| addDuplicateReviewerUpdate.commit(); |
| // Reviewer name resolves to multiple accounts in the same change |
| ChangeUpdate onAddReviewerUpdateWithDuplicate = newUpdate(c, changeOwner); |
| onAddReviewerUpdateWithDuplicate.setChangeMessage( |
| "Reviewer User who was added as reviewer owns the following files:\n" |
| + " * file6.java\n"); |
| commitsToFix.add(onAddReviewerUpdateWithDuplicate.commit()); |
| |
| ChangeUpdate validOnAddReviewerUpdate = newUpdate(c, changeOwner); |
| validOnAddReviewerUpdate.setChangeMessage( |
| "Gerrit Account who was added as reviewer owns the following files:\n" |
| + " * file1.java\n" |
| + "\n<GERRIT_ACCOUNT_1> who was added as reviewer owns the following files:\n" |
| + " * file3.js\n"); |
| validOnAddReviewerUpdate.commit(); |
| |
| Ref metaRefBeforeRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| |
| ImmutableList<RevCommit> commitsBeforeRewrite = logMetaRef(repo, metaRefBeforeRewrite); |
| |
| ImmutableList<Integer> invalidCommits = |
| commitsToFix.build().stream() |
| .map(commit -> commitsBeforeRewrite.indexOf(commit)) |
| .collect(toImmutableList()); |
| ChangeNotes notesBeforeRewrite = newNotes(c); |
| |
| RunOptions options = new RunOptions(); |
| options.dryRun = false; |
| BackfillResult result = rewriter.backfillProject(project, repo, options); |
| assertThat(result.fixedRefDiff.keySet()).containsExactly(RefNames.changeMetaRef(c.getId())); |
| |
| ChangeNotes notesAfterRewrite = newNotes(c); |
| |
| assertThat(changeMessages(notesBeforeRewrite)).hasSize(4); |
| assertThat(changeMessages(notesAfterRewrite)) |
| .containsExactly( |
| "<GERRIT_ACCOUNT_3>, who was added as reviewer owns the following files:\n" |
| + " * file1.java\n" |
| + " * file2.ts\n", |
| "<GERRIT_ACCOUNT_3>, who was added as reviewer owns the following files:\n" |
| + " * file1.java\n" |
| + "\n<GERRIT_ACCOUNT_2>, who was added as reviewer owns the following files:\n" |
| + " * file3.js\n" |
| + "\nGerrit Account, who was added as reviewer owns the following files:\n" |
| + " * file4.java\n", |
| "Gerrit Account, who was added as reviewer owns the following files:\n" |
| + " * file6.java\n", |
| "Gerrit Account who was added as reviewer owns the following files:\n" |
| + " * file1.java\n" |
| + "\n<GERRIT_ACCOUNT_1> who was added as reviewer owns the following files:\n" |
| + " * file3.js\n"); |
| |
| Ref metaRefAfterRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| assertThat(metaRefAfterRewrite.getObjectId()).isNotEqualTo(metaRefBeforeRewrite.getObjectId()); |
| |
| ImmutableList<RevCommit> commitsAfterRewrite = logMetaRef(repo, metaRefAfterRewrite); |
| assertValidCommits(commitsBeforeRewrite, commitsAfterRewrite, invalidCommits); |
| |
| List<String> commitHistoryDiff = result.fixedRefDiff.get(RefNames.changeMetaRef(c.getId())); |
| assertThat(commitHistoryDiff) |
| .containsExactly( |
| "@@ -6 +6 @@\n" |
| + "-Reviewer User who was added as reviewer owns the following files:\n" |
| + "+<GERRIT_ACCOUNT_3>, who was added as reviewer owns the following files:\n", |
| "@@ -6 +6 @@\n" |
| + "-Reviewer User who was added as reviewer owns the following files:\n" |
| + "+<GERRIT_ACCOUNT_3>, who was added as reviewer owns the following files:\n" |
| + "@@ -9 +9 @@\n" |
| + "-Other Account who was added as reviewer owns the following files:\n" |
| + "+<GERRIT_ACCOUNT_2>, who was added as reviewer owns the following files:\n" |
| + "@@ -12 +12 @@\n" |
| + "-Missing Reviewer who was added as reviewer owns the following files:\n" |
| + "+Gerrit Account, who was added as reviewer owns the following files:\n", |
| "@@ -6 +6 @@\n" |
| + "-Reviewer User who was added as reviewer owns the following files:\n" |
| + "+Gerrit Account, who was added as reviewer owns the following files:\n"); |
| } |
| |
| @Test |
| public void fixCodeOwnersOnReviewChangeMessage() throws Exception { |
| |
| Change c = newChange(); |
| ImmutableList.Builder<ObjectId> commitsToFix = new ImmutableList.Builder<>(); |
| |
| ChangeUpdate invalidOnApprovalUpdate = newUpdate(c, changeOwner); |
| invalidOnApprovalUpdate.setChangeMessage( |
| "Patch Set 1: Code-Review+2\n\n" |
| + "By voting Code-Review+2 the following files are now code-owner approved by Change Owner:\n" |
| + " * file1.java\n" |
| + " * file2.ts\n"); |
| commitsToFix.add(invalidOnApprovalUpdate.commit()); |
| ChangeUpdate invalidOnApprovalMultipleByUpdate = newUpdate(c, otherUser); |
| invalidOnApprovalMultipleByUpdate.setChangeMessage( |
| "Patch Set 1: -Code-Review\n\n" |
| + "By removing the Code-Review+2 vote the following files are no longer explicitly code-owner approved by Other Account:\n" |
| + " * file1.java\n" |
| + " * file2.ts\n" |
| + "\nThe listed files are still implicitly approved by Other Account.\n"); |
| commitsToFix.add(invalidOnApprovalMultipleByUpdate.commit()); |
| |
| ChangeUpdate invalidOnOverrideUpdate = newUpdate(c, changeOwner); |
| invalidOnOverrideUpdate.setChangeMessage( |
| "Patch Set 1: -Owners-Override\n\n" |
| + "(1 comment)\n\n" |
| + "By removing the Owners-Override+1 vote the code-owners submit requirement is no longer overridden by Change Owner:\n" |
| + " * file3.java\n"); |
| commitsToFix.add(invalidOnOverrideUpdate.commit()); |
| |
| ChangeUpdate validOnApprovalUpdate = newUpdate(c, changeOwner); |
| validOnApprovalUpdate.setChangeMessage( |
| "Patch Set 1: Code-Review-2\n\n" |
| + "By voting Code-Review-2 the following files are no longer explicitly code-owner approved by <GERRIT_ACCOUNT_1>:\n" |
| + " * file4.java\n"); |
| validOnApprovalUpdate.commit(); |
| |
| ChangeUpdate validOnOverrideUpdate = newUpdate(c, changeOwner); |
| validOnOverrideUpdate.setChangeMessage( |
| "Patch Set 1: Owners-Override+1\n\n" |
| + "By voting Owners-Override+1 the code-owners submit requirement is still overridden by <GERRIT_ACCOUNT_1>:\n" |
| + " * file1.java\n"); |
| validOnOverrideUpdate.commit(); |
| |
| Ref metaRefBeforeRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| |
| ImmutableList<RevCommit> commitsBeforeRewrite = logMetaRef(repo, metaRefBeforeRewrite); |
| |
| ImmutableList<Integer> invalidCommits = |
| commitsToFix.build().stream() |
| .map(commit -> commitsBeforeRewrite.indexOf(commit)) |
| .collect(toImmutableList()); |
| ChangeNotes notesBeforeRewrite = newNotes(c); |
| |
| RunOptions options = new RunOptions(); |
| options.dryRun = false; |
| BackfillResult result = rewriter.backfillProject(project, repo, options); |
| assertThat(result.fixedRefDiff.keySet()).containsExactly(RefNames.changeMetaRef(c.getId())); |
| |
| ChangeNotes notesAfterRewrite = newNotes(c); |
| |
| assertThat(changeMessages(notesBeforeRewrite)).hasSize(5); |
| assertThat(changeMessages(notesAfterRewrite)) |
| .containsExactly( |
| "Patch Set 1: Code-Review+2\n" |
| + "\n" |
| + "By voting Code-Review+2 the following files are now code-owner approved by <GERRIT_ACCOUNT_1>:\n" |
| + " * file1.java\n" |
| + " * file2.ts\n", |
| "Patch Set 1: -Code-Review\n" |
| + "\n" |
| + "By removing the Code-Review+2 vote the following files are no longer explicitly code-owner approved by <GERRIT_ACCOUNT_2>:\n" |
| + " * file1.java\n" |
| + " * file2.ts\n" |
| + "\nThe listed files are still implicitly approved by <GERRIT_ACCOUNT_2>.\n", |
| "Patch Set 1: -Owners-Override\n" |
| + "\n" |
| + "(1 comment)\n" |
| + "\n" |
| + "By removing the Owners-Override+1 vote the code-owners submit requirement is no longer overridden by <GERRIT_ACCOUNT_1>:\n" |
| + " * file3.java\n", |
| "Patch Set 1: Code-Review-2\n\n" |
| + "By voting Code-Review-2 the following files are no longer explicitly code-owner approved by <GERRIT_ACCOUNT_1>:\n" |
| + " * file4.java\n", |
| "Patch Set 1: Owners-Override+1\n" |
| + "\n" |
| + "By voting Owners-Override+1 the code-owners submit requirement is still overridden by <GERRIT_ACCOUNT_1>:\n" |
| + " * file1.java\n"); |
| |
| Ref metaRefAfterRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| assertThat(metaRefAfterRewrite.getObjectId()).isNotEqualTo(metaRefBeforeRewrite.getObjectId()); |
| |
| ImmutableList<RevCommit> commitsAfterRewrite = logMetaRef(repo, metaRefAfterRewrite); |
| assertValidCommits(commitsBeforeRewrite, commitsAfterRewrite, invalidCommits); |
| |
| List<String> commitHistoryDiff = result.fixedRefDiff.get(RefNames.changeMetaRef(c.getId())); |
| assertThat(commitHistoryDiff) |
| .containsExactly( |
| "@@ -8 +8 @@\n" |
| + "-By voting Code-Review+2 the following files are now code-owner approved by Change Owner:\n" |
| + "+By voting Code-Review+2 the following files are now code-owner approved by <GERRIT_ACCOUNT_1>:\n", |
| "@@ -8 +8 @@\n" |
| + "-By removing the Code-Review+2 vote the following files are no longer explicitly code-owner approved by Other Account:\n" |
| + "+By removing the Code-Review+2 vote the following files are no longer explicitly code-owner approved by <GERRIT_ACCOUNT_2>:\n" |
| + "@@ -12 +12 @@\n" |
| + "-The listed files are still implicitly approved by Other Account.\n" |
| + "+The listed files are still implicitly approved by <GERRIT_ACCOUNT_2>.\n", |
| "@@ -10 +10 @@\n" |
| + "-By removing the Owners-Override+1 vote the code-owners submit requirement is no longer overridden by Change Owner:\n" |
| + "+By removing the Owners-Override+1 vote the code-owners submit requirement is no longer overridden by <GERRIT_ACCOUNT_1>:\n"); |
| } |
| |
| @Test |
| public void fixAssigneeFooterIdent() throws Exception { |
| Change c = newChange(); |
| |
| String assigneeIdentToFix = getAccountIdentToFix(changeOwner.getAccount()); |
| RevCommit invalidUpdateCommit = |
| writeUpdate( |
| RefNames.changeMetaRef(c.getId()), |
| getChangeUpdateBody(c, "Assignee added", "Assignee: " + assigneeIdentToFix), |
| getAuthorIdent(changeOwner.getAccount())); |
| |
| ChangeUpdate changeAssigneeUpdate = newUpdate(c, changeOwner); |
| changeAssigneeUpdate.setAssignee(otherUserId); |
| changeAssigneeUpdate.commit(); |
| |
| ChangeUpdate removeAssigneeUpdate = newUpdate(c, changeOwner); |
| removeAssigneeUpdate.removeAssignee(); |
| removeAssigneeUpdate.commit(); |
| |
| Ref metaRefBeforeRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| |
| ImmutableList<RevCommit> commitsBeforeRewrite = logMetaRef(repo, metaRefBeforeRewrite); |
| |
| int invalidCommitIndex = commitsBeforeRewrite.indexOf(invalidUpdateCommit); |
| ChangeNotes notesBeforeRewrite = newNotes(c); |
| |
| RunOptions options = new RunOptions(); |
| options.dryRun = false; |
| BackfillResult result = rewriter.backfillProject(project, repo, options); |
| assertThat(result.fixedRefDiff.keySet()).containsExactly(RefNames.changeMetaRef(c.getId())); |
| |
| ChangeNotes notesAfterRewrite = newNotes(c); |
| assertThat(notesBeforeRewrite.getPastAssignees()) |
| .containsExactly(changeOwner.getAccountId(), otherUser.getAccountId()); |
| assertThat(notesBeforeRewrite.getChange().getAssignee()).isNull(); |
| assertThat(notesAfterRewrite.getPastAssignees()) |
| .containsExactly(changeOwner.getAccountId(), otherUser.getAccountId()); |
| assertThat(notesAfterRewrite.getChange().getAssignee()).isNull(); |
| |
| Ref metaRefAfterRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| assertThat(metaRefAfterRewrite.getObjectId()).isNotEqualTo(metaRefBeforeRewrite.getObjectId()); |
| |
| ImmutableList<RevCommit> commitsAfterRewrite = logMetaRef(repo, metaRefAfterRewrite); |
| assertValidCommits( |
| commitsBeforeRewrite, commitsAfterRewrite, ImmutableList.of(invalidCommitIndex)); |
| |
| RevCommit fixedUpdateCommit = commitsAfterRewrite.get(invalidCommitIndex); |
| assertThat(invalidUpdateCommit.getAuthorIdent()).isEqualTo(fixedUpdateCommit.getAuthorIdent()); |
| assertThat(invalidUpdateCommit.getCommitterIdent()) |
| .isEqualTo(fixedUpdateCommit.getCommitterIdent()); |
| assertThat(invalidUpdateCommit.getFullMessage()) |
| .isNotEqualTo(fixedUpdateCommit.getFullMessage()); |
| assertThat(fixedUpdateCommit.getFullMessage()).doesNotContain(changeOwner.getName()); |
| assertThat(invalidUpdateCommit.getFullMessage()).contains(assigneeIdentToFix); |
| String expectedFixedIdent = getValidIdentAsString(changeOwner.getAccount()); |
| assertThat(fixedUpdateCommit.getFullMessage()).contains(expectedFixedIdent); |
| |
| List<String> commitHistoryDiff = result.fixedRefDiff.get(RefNames.changeMetaRef(c.getId())); |
| assertThat(commitHistoryDiff) |
| .containsExactly( |
| "@@ -9 +9 @@\n" |
| + "-Assignee: Change Owner <1@gerrit>\n" |
| + "+Assignee: Gerrit User 1 <1@gerrit>\n"); |
| } |
| |
| @Test |
| public void fixAssigneeChangeMessage() throws Exception { |
| Change c = newChange(); |
| |
| ImmutableList<RevCommit> commitsToFix = |
| new ImmutableList.Builder<RevCommit>() |
| .add( |
| writeUpdate( |
| RefNames.changeMetaRef(c.getId()), |
| getChangeUpdateBody( |
| c, |
| "Assignee added: " + changeOwner.getNameEmail(), |
| "Assignee: " + getValidIdentAsString(changeOwner.getAccount())), |
| getAuthorIdent(changeOwner.getAccount()))) |
| .add( |
| writeUpdate( |
| RefNames.changeMetaRef(c.getId()), |
| getChangeUpdateBody( |
| c, |
| String.format( |
| "Assignee changed from: %s to: %s", |
| changeOwner.getNameEmail(), otherUser.getNameEmail()), |
| "Assignee: " + getValidIdentAsString(otherUser.getAccount())), |
| getAuthorIdent(changeOwner.getAccount()))) |
| .add( |
| writeUpdate( |
| RefNames.changeMetaRef(c.getId()), |
| getChangeUpdateBody( |
| c, "Assignee deleted: " + otherUser.getNameEmail(), "Assignee:"), |
| getAuthorIdent(changeOwner.getAccount()))) |
| .build(); |
| |
| Ref metaRefBeforeRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| |
| ImmutableList<RevCommit> commitsBeforeRewrite = logMetaRef(repo, metaRefBeforeRewrite); |
| |
| ImmutableList<Integer> invalidCommits = |
| commitsToFix.stream() |
| .map(commit -> commitsBeforeRewrite.indexOf(commit)) |
| .collect(toImmutableList()); |
| ChangeNotes notesBeforeRewrite = newNotes(c); |
| |
| RunOptions options = new RunOptions(); |
| options.dryRun = false; |
| BackfillResult result = rewriter.backfillProject(project, repo, options); |
| assertThat(result.fixedRefDiff.keySet()).containsExactly(RefNames.changeMetaRef(c.getId())); |
| |
| ChangeNotes notesAfterRewrite = newNotes(c); |
| assertThat(notesBeforeRewrite.getPastAssignees()) |
| .containsExactly(changeOwner.getAccountId(), otherUser.getAccountId()); |
| assertThat(notesBeforeRewrite.getChange().getAssignee()).isNull(); |
| assertThat(changeMessages(notesBeforeRewrite)) |
| .containsExactly( |
| "Assignee added: Change Owner <change@owner.com>", |
| "Assignee changed from: Change Owner <change@owner.com> to: Other Account <other@account.com>", |
| "Assignee deleted: Other Account <other@account.com>"); |
| |
| assertThat(notesAfterRewrite.getPastAssignees()) |
| .containsExactly(changeOwner.getAccountId(), otherUser.getAccountId()); |
| assertThat(notesAfterRewrite.getChange().getAssignee()).isNull(); |
| assertThat(changeMessages(notesAfterRewrite)) |
| .containsExactly( |
| "Assignee added: " + AccountTemplateUtil.getAccountTemplate(changeOwner.getAccountId()), |
| String.format( |
| "Assignee changed from: %s to: %s", |
| AccountTemplateUtil.getAccountTemplate(changeOwner.getAccountId()), |
| AccountTemplateUtil.getAccountTemplate(otherUser.getAccountId())), |
| "Assignee deleted: " |
| + AccountTemplateUtil.getAccountTemplate(otherUser.getAccountId())); |
| |
| Ref metaRefAfterRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| assertThat(metaRefAfterRewrite.getObjectId()).isNotEqualTo(metaRefBeforeRewrite.getObjectId()); |
| |
| ImmutableList<RevCommit> commitsAfterRewrite = logMetaRef(repo, metaRefAfterRewrite); |
| assertValidCommits(commitsBeforeRewrite, commitsAfterRewrite, invalidCommits); |
| List<String> commitHistoryDiff = result.fixedRefDiff.get(RefNames.changeMetaRef(c.getId())); |
| assertThat(commitHistoryDiff) |
| .containsExactly( |
| "@@ -6 +6 @@\n" |
| + "-Assignee added: Change Owner <change@owner.com>\n" |
| + "+Assignee added: <GERRIT_ACCOUNT_1>\n", |
| "@@ -6 +6 @@\n" |
| + "-Assignee changed from: Change Owner <change@owner.com> to: Other Account <other@account.com>\n" |
| + "+Assignee changed from: <GERRIT_ACCOUNT_1> to: <GERRIT_ACCOUNT_2>\n", |
| "@@ -6 +6 @@\n" |
| + "-Assignee deleted: Other Account <other@account.com>\n" |
| + "+Assignee deleted: <GERRIT_ACCOUNT_2>\n" |
| // Both empty value and space are parsed as deleted assignee anyway. |
| + "@@ -9 +9 @@\n" |
| + "-Assignee:\n" |
| + "+Assignee: \n"); |
| } |
| |
| @Test |
| public void singleRunFixesAll() throws Exception { |
| Change c = newChange(); |
| Timestamp when = TimeUtil.nowTs(); |
| String assigneeIdentToFix = getAccountIdentToFix(otherUser.getAccount()); |
| PersonIdent authorIdentToFix = |
| new PersonIdent( |
| changeOwner.getName(), |
| changeNoteUtil.getAccountIdAsEmailAddress(changeOwner.getAccountId()), |
| when, |
| serverIdent.getTimeZone()); |
| |
| RevCommit invalidUpdateCommit = |
| writeUpdate( |
| RefNames.changeMetaRef(c.getId()), |
| getChangeUpdateBody( |
| c, |
| "Assignee added: Other Account <other@account.com>", |
| "Assignee: " + assigneeIdentToFix), |
| authorIdentToFix); |
| Ref metaRefBeforeRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| |
| ImmutableList<RevCommit> commitsBeforeRewrite = logMetaRef(repo, metaRefBeforeRewrite); |
| |
| int invalidCommitIndex = commitsBeforeRewrite.indexOf(invalidUpdateCommit); |
| ChangeNotes notesBeforeRewrite = newNotes(c); |
| |
| RunOptions options = new RunOptions(); |
| options.dryRun = false; |
| BackfillResult result = rewriter.backfillProject(project, repo, options); |
| assertThat(result.fixedRefDiff.keySet()).containsExactly(RefNames.changeMetaRef(c.getId())); |
| |
| ChangeNotes notesAfterRewrite = newNotes(c); |
| assertThat(notesBeforeRewrite.getChange().getAssignee()).isEqualTo(otherUserId); |
| assertThat(notesAfterRewrite.getChange().getAssignee()).isEqualTo(otherUserId); |
| |
| Ref metaRefAfterRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId())); |
| assertThat(metaRefAfterRewrite.getObjectId()).isNotEqualTo(metaRefBeforeRewrite.getObjectId()); |
| |
| ImmutableList<RevCommit> commitsAfterRewrite = logMetaRef(repo, metaRefAfterRewrite); |
| assertValidCommits( |
| commitsBeforeRewrite, commitsAfterRewrite, ImmutableList.of(invalidCommitIndex)); |
| |
| RevCommit fixedUpdateCommit = commitsAfterRewrite.get(invalidCommitIndex); |
| assertThat(invalidUpdateCommit.getAuthorIdent()) |
| .isNotEqualTo(fixedUpdateCommit.getAuthorIdent()); |
| assertThat(invalidUpdateCommit.getFullMessage()).contains(otherUser.getName()); |
| assertThat(fixedUpdateCommit.getFullMessage()).doesNotContain(changeOwner.getName()); |
| assertThat(fixedUpdateCommit.getFullMessage()).doesNotContain(otherUser.getName()); |
| |
| List<String> commitHistoryDiff = result.fixedRefDiff.get(RefNames.changeMetaRef(c.getId())); |
| assertThat(commitHistoryDiff).hasSize(1); |
| assertThat(commitHistoryDiff.get(0)).contains("-author Change Owner <1@gerrit>"); |
| assertThat(commitHistoryDiff.get(0)).contains("+author Gerrit User 1 <1@gerrit>"); |
| assertThat(commitHistoryDiff.get(0)) |
| .contains( |
| "@@ -6 +6 @@\n" |
| + "-Assignee added: Other Account <other@account.com>\n" |
| + "+Assignee added: <GERRIT_ACCOUNT_2>\n" |
| + "@@ -9 +9 @@\n" |
| + "-Assignee: Other Account <2@gerrit>\n" |
| + "+Assignee: Gerrit User 2 <2@gerrit>"); |
| } |
| |
| private RevCommit writeUpdate(String metaRef, String body, PersonIdent author) throws Exception { |
| return tr.branch(metaRef).commit().message(body).author(author).committer(serverIdent).create(); |
| } |
| |
| private String getChangeUpdateBody(Change change, String changeMessage, String... footers) { |
| StringBuilder commitBody = new StringBuilder(); |
| commitBody.append("Update patch set ").append(change.currentPatchSetId().get()); |
| commitBody.append("\n\n"); |
| if (changeMessage != null) { |
| commitBody.append(changeMessage); |
| commitBody.append("\n\n"); |
| } |
| commitBody.append("Patch-set: " + change.currentPatchSetId().get()); |
| commitBody.append("\n"); |
| for (String footer : footers) { |
| commitBody.append(footer); |
| commitBody.append("\n"); |
| } |
| return commitBody.toString(); |
| } |
| |
| private ImmutableList<RevCommit> logMetaRef(Repository repo, Ref metaRef) throws Exception { |
| try (RevWalk rw = new RevWalk(repo)) { |
| rw.sort(RevSort.TOPO); |
| rw.sort(RevSort.REVERSE); |
| if (metaRef == null) { |
| return ImmutableList.of(); |
| } |
| rw.markStart(rw.parseCommit(metaRef.getObjectId())); |
| return ImmutableList.copyOf(rw); |
| } |
| } |
| |
| private void assertValidCommits( |
| ImmutableList<RevCommit> commitsBeforeRewrite, |
| ImmutableList<RevCommit> commitsAfterRewrite, |
| ImmutableList<Integer> invalidCommits) { |
| ImmutableList<RevCommit> validCommitsBeforeRewrite = |
| IntStream.range(0, commitsBeforeRewrite.size()) |
| .filter(i -> !invalidCommits.contains(i)) |
| .mapToObj(commitsBeforeRewrite::get) |
| .collect(toImmutableList()); |
| |
| ImmutableList<RevCommit> validCommitsAfterRewrite = |
| IntStream.range(0, commitsAfterRewrite.size()) |
| .filter(i -> !invalidCommits.contains(i)) |
| .mapToObj(commitsAfterRewrite::get) |
| .collect(toImmutableList()); |
| |
| assertThat(validCommitsBeforeRewrite).hasSize(validCommitsAfterRewrite.size()); |
| for (int i = 0; i < validCommitsAfterRewrite.size(); i++) { |
| RevCommit actual = validCommitsAfterRewrite.get(i); |
| RevCommit expected = validCommitsBeforeRewrite.get(i); |
| assertThat(actual.getAuthorIdent()).isEqualTo(expected.getAuthorIdent()); |
| assertThat(actual.getCommitterIdent()).isEqualTo(expected.getCommitterIdent()); |
| assertThat(actual.getFullMessage()).isEqualTo(expected.getFullMessage()); |
| } |
| } |
| |
| private String getAccountIdentToFix(Account account) { |
| return String.format("%s <%s>", account.getName(), account.id().get() + "@" + serverId); |
| } |
| |
| private String getValidIdentAsString(Account account) { |
| return String.format( |
| "%s <%s>", |
| ChangeNoteUtil.getAccountIdAsUsername(account.id()), account.id().get() + "@" + serverId); |
| } |
| |
| private ImmutableList<String> changeMessages(ChangeNotes changeNotes) { |
| return changeNotes.getChangeMessages().stream() |
| .map(ChangeMessage::getMessage) |
| .collect(toImmutableList()); |
| } |
| |
| private PersonIdent getAuthorIdent(Account account) { |
| Timestamp when = TimeUtil.nowTs(); |
| return changeNoteUtil.newAccountIdIdent(account.id(), when, serverIdent); |
| } |
| } |
