blob: d13ccdddfd598965f6f03be0ff6270e495f5fbed [file] [log] [blame]
// 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 com.google.gerrit.testing.TestActionRefUpdateContext.testRefAction;
import static java.util.Objects.requireNonNull;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
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.git.RefUpdateUtil;
import com.google.gerrit.json.OutputFormat;
import com.google.gerrit.server.CurrentUser;
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.CommitDiff;
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.time.Instant;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.IntStream;
import org.eclipse.jgit.lib.BatchRefUpdate;
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.eclipse.jgit.transport.ReceiveCommand;
import org.junit.After;
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 {}
@After
public void cleanUp() throws Exception {
BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
bru.setAllowNonFastForwards(true);
for (Ref ref : repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_CHANGES)) {
Change.Id changeId = Change.Id.fromRef(ref.getName());
if (changeId == null || !ref.getName().equals(RefNames.changeMetaRef(changeId))) {
continue;
}
bru.addCommand(new ReceiveCommand(ref.getObjectId(), ObjectId.zeroId(), ref.getName()));
}
testRefAction(() -> RefUpdateUtil.executeChecked(bru, repo));
}
@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 outputDiffOff_refsReported() throws Exception {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
update.setChangeMessage("Change has been successfully merged by " + changeOwner.getName());
ObjectId commitToFix = update.commit();
ChangeUpdate updateWithSubject = newUpdate(c, changeOwner);
updateWithSubject.setSubjectForCommit("Update with subject");
updateWithSubject.commit();
Ref metaRefBefore = repo.exactRef(RefNames.changeMetaRef(c.getId()));
RunOptions options = new RunOptions();
options.dryRun = false;
options.outputDiff = false;
options.verifyCommits = false;
BackfillResult backfillResult = rewriter.backfillProject(project, repo, options);
assertThat(backfillResult.fixedRefDiff.keySet())
.containsExactly(RefNames.changeMetaRef(c.getId()));
Ref metaRefAfter = repo.exactRef(RefNames.changeMetaRef(c.getId()));
assertThat(metaRefBefore.getObjectId()).isNotEqualTo(metaRefAfter.getObjectId());
assertFixedCommits(ImmutableList.of(commitToFix), backfillResult, c.getId());
List<String> commitHistoryDiff = commitHistoryDiff(backfillResult, c.getId());
assertThat(commitHistoryDiff).containsExactly("");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@Test
public void numRefs_greater_maxRefsToUpdate_allFixed() throws Exception {
int numberOfChanges = 12;
ImmutableMap.Builder<String, Ref> refsToOldMetaBuilder = new ImmutableMap.Builder<>();
for (int i = 0; i < numberOfChanges; i++) {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
update.setChangeMessage("Change has been successfully merged by " + changeOwner.getName());
update.commit();
ChangeUpdate updateWithSubject = newUpdate(c, changeOwner);
updateWithSubject.setSubjectForCommit("Update with subject");
updateWithSubject.commit();
String refName = RefNames.changeMetaRef(c.getId());
Ref metaRefBeforeRewrite = repo.exactRef(refName);
refsToOldMetaBuilder.put(refName, metaRefBeforeRewrite);
}
ImmutableMap<String, Ref> refsToOldMeta = refsToOldMetaBuilder.build();
RunOptions options = new RunOptions();
options.dryRun = false;
options.outputDiff = false;
options.verifyCommits = false;
options.maxRefsInBatch = 10;
options.maxRefsToUpdate = 12;
BackfillResult backfillResult = rewriter.backfillProject(project, repo, options);
assertThat(backfillResult.fixedRefDiff.keySet()).isEqualTo(refsToOldMeta.keySet());
for (Map.Entry<String, Ref> refEntry : refsToOldMeta.entrySet()) {
Ref metaRefAfterRewrite = repo.exactRef(refEntry.getKey());
assertThat(refEntry.getValue()).isNotEqualTo(metaRefAfterRewrite);
}
}
@Test
public void maxRefsToUpdate_coversAllInvalid_inMultipleBatches() throws Exception {
testMaxRefsToUpdate(
/* numberOfInvalidChanges= */ 11,
/* numberOfValidChanges= */ 9,
/* maxRefsToUpdate= */ 12,
/* maxRefsInBatch= */ 2);
}
@Test
public void maxRefsToUpdate_coversAllInvalid_inSingleBatch() throws Exception {
testMaxRefsToUpdate(
/* numberOfInvalidChanges= */ 11,
/* numberOfValidChanges= */ 9,
/* maxRefsToUpdate= */ 12,
/* maxRefsInBatch= */ 12);
}
@Test
public void moreInvalidRefs_thenMaxRefsToUpdate_inMultipleBatches() throws Exception {
testMaxRefsToUpdate(
/* numberOfInvalidChanges= */ 11,
/* numberOfValidChanges= */ 9,
/* maxRefsToUpdate= */ 10,
/* maxRefsInBatch= */ 2);
}
@Test
public void moreInvalidRefs_thenMaxRefsToUpdate_inSingleBatch() throws Exception {
testMaxRefsToUpdate(
/* numberOfInvalidChanges= */ 11,
/* numberOfValidChanges= */ 9,
/* maxRefsToUpdate= */ 10,
/* maxRefsInBatch= */ 10);
}
private void testMaxRefsToUpdate(
int numberOfInvalidChanges, int numberOfValidChanges, int maxRefsToUpdate, int maxRefsInBatch)
throws Exception {
ImmutableMap.Builder<String, ObjectId> expectedFixedRefsToOldMetaBuilder =
new ImmutableMap.Builder<>();
ImmutableMap.Builder<String, ObjectId> expectedSkippedRefsToOldMetaBuilder =
new ImmutableMap.Builder<>();
for (int i = 0; i < numberOfValidChanges; i++) {
Change c = newChange();
ChangeUpdate updateWithSubject = newUpdate(c, changeOwner);
updateWithSubject.setSubjectForCommit("Update with subject");
updateWithSubject.commit();
String refName = RefNames.changeMetaRef(c.getId());
Ref metaRefBeforeRewrite = repo.exactRef(refName);
expectedSkippedRefsToOldMetaBuilder.put(refName, metaRefBeforeRewrite.getObjectId());
}
Set<String> invalidRefs = new HashSet<>();
for (int i = 0; i < numberOfInvalidChanges; i++) {
Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner);
update.setChangeMessage("Change has been successfully merged by " + changeOwner.getName());
update.commit();
ChangeUpdate updateWithSubject = newUpdate(c, changeOwner);
updateWithSubject.setSubjectForCommit("Update with subject");
updateWithSubject.commit();
String refName = RefNames.changeMetaRef(c.getId());
invalidRefs.add(refName);
}
int i = 0;
for (Ref ref : repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_CHANGES)) {
Ref metaRefBeforeRewrite = repo.exactRef(ref.getName());
if (!invalidRefs.contains(ref.getName())) {
continue;
}
if (i < maxRefsToUpdate) {
expectedFixedRefsToOldMetaBuilder.put(ref.getName(), metaRefBeforeRewrite.getObjectId());
} else {
expectedSkippedRefsToOldMetaBuilder.put(ref.getName(), metaRefBeforeRewrite.getObjectId());
}
i++;
}
ImmutableMap<String, ObjectId> expectedFixedRefsToOldMeta =
expectedFixedRefsToOldMetaBuilder.build();
ImmutableMap<String, ObjectId> expectedSkippedRefsToOldMeta =
expectedSkippedRefsToOldMetaBuilder.build();
RunOptions options = new RunOptions();
options.dryRun = false;
options.outputDiff = false;
options.verifyCommits = false;
options.maxRefsInBatch = maxRefsInBatch;
options.maxRefsToUpdate = maxRefsToUpdate;
BackfillResult backfillResult = rewriter.backfillProject(project, repo, options);
assertThat(backfillResult.fixedRefDiff.keySet()).isEqualTo(expectedFixedRefsToOldMeta.keySet());
for (Map.Entry<String, ObjectId> refEntry : expectedFixedRefsToOldMeta.entrySet()) {
Ref metaRefAfterRewrite = repo.exactRef(refEntry.getKey());
assertThat(refEntry.getValue()).isNotEqualTo(metaRefAfterRewrite.getObjectId());
}
for (Map.Entry<String, ObjectId> refEntry : expectedSkippedRefsToOldMeta.entrySet()) {
Ref metaRefAfterRewrite = repo.exactRef(refEntry.getKey());
assertThat(refEntry.getValue()).isEqualTo(metaRefAfterRewrite.getObjectId());
}
RunOptions secondRunOptions = new RunOptions();
secondRunOptions.dryRun = false;
secondRunOptions.outputDiff = false;
secondRunOptions.verifyCommits = false;
secondRunOptions.maxRefsInBatch = maxRefsInBatch;
secondRunOptions.maxRefsToUpdate = numberOfInvalidChanges + numberOfValidChanges;
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
int expectedSecondRunResult =
numberOfInvalidChanges > maxRefsToUpdate ? numberOfInvalidChanges - maxRefsToUpdate : 0;
assertThat(secondRunResult.fixedRefDiff.keySet().size()).isEqualTo(expectedSecondRunResult);
}
@Test
public void fixAuthorIdent() throws Exception {
Change c = newChange();
Instant when = TimeUtil.now();
PersonIdent invalidAuthorIdent =
new PersonIdent(
changeOwner.getName(),
changeNoteUtil.getAccountIdAsEmailAddress(changeOwner.getAccountId()),
when,
serverIdent.getZoneId());
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));
assertFixedCommits(ImmutableList.of(invalidUpdateCommit.getId()), result, c.getId());
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.getWhenAsInstant())
.isEqualTo(fixedAuthorIdent.getWhenAsInstant());
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 = commitHistoryDiff(result, 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>");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@Test
public void fixRealUserFooterIdent() throws Exception {
Change c = newChange();
String realUserIdentToFix = getAccountIdentToFix(otherUser.getAccount());
ObjectId invalidUpdateCommit =
writeUpdate(
RefNames.changeMetaRef(c.getId()),
getChangeUpdateBody(c, "Comment on behalf of user", "Real-user: " + realUserIdentToFix),
getAuthorIdent(changeOwner.getAccount()));
IdentifiedUser impersonatedChangeOwner =
this.userFactory.runAs(
/* remotePeer= */ 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));
assertFixedCommits(ImmutableList.of(invalidUpdateCommit), result, c.getId());
List<String> commitHistoryDiff = commitHistoryDiff(result, c.getId());
assertThat(commitHistoryDiff)
.containsExactly(
"@@ -9 +9 @@\n"
+ "-Real-user: Other Account <2@gerrit>\n"
+ "+Real-user: Gerrit User 2 <2@gerrit>\n");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@Test
public void fixReviewerFooterIdent() throws Exception {
Change c = newChange();
String reviewerIdentToFix = getAccountIdentToFix(otherUser.getAccount());
ImmutableList<ObjectId> commitsToFix =
new ImmutableList.Builder<ObjectId>()
.add(
writeUpdate(
RefNames.changeMetaRef(c.getId()),
// valid change message that should not be overwritten
getChangeUpdateBody(
c,
"Removed reviewer <GERRIT_ACCOUNT_1>.",
"Reviewer: " + reviewerIdentToFix),
getAuthorIdent(changeOwner.getAccount())))
.add(
writeUpdate(
RefNames.changeMetaRef(c.getId()),
// valid change message that should not be overwritten
getChangeUpdateBody(
c,
"Removed cc <GERRIT_ACCOUNT_2> with the following votes:\n\n"
+ " * Code-Review+2 by <GERRIT_ACCOUNT_2>",
"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()));
Instant updateTimestamp = serverIdent.getWhenAsInstant();
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);
assertFixedCommits(commitsToFix, result, c.getId());
List<String> commitHistoryDiff = commitHistoryDiff(result, c.getId());
assertThat(commitHistoryDiff)
.containsExactly(
"@@ -9 +9 @@\n"
+ "-Reviewer: Other Account <2@gerrit>\n"
+ "+Reviewer: Gerrit User 2 <2@gerrit>\n",
"@@ -11 +11 @@\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");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@Test
public void fixReviewerMessage() throws Exception {
Change c = newChange();
ImmutableList.Builder<ObjectId> commitsToFix = new ImmutableList.Builder<>();
ChangeUpdate addReviewerUpdate = newUpdate(c, changeOwner);
addReviewerUpdate.putReviewer(otherUserId, REVIEWER);
addReviewerUpdate.commit();
commitsToFix.add(
writeUpdate(
RefNames.changeMetaRef(c.getId()),
getChangeUpdateBody(
c,
String.format("Removed reviewer %s.", 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,
String.format(
"Removed cc %s with the following votes:\n\n * Code-Review+2",
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()));
Instant updateTimestamp = serverIdent.getWhenAsInstant();
ImmutableList<ReviewerStatusUpdate> expectedReviewerUpdates =
ImmutableList.of(
ReviewerStatusUpdate.create(
addReviewerUpdate.when, changeOwner.getAccountId(), otherUserId, REVIEWER),
ReviewerStatusUpdate.create(
updateTimestamp, changeOwner.getAccountId(), otherUserId, REMOVED),
ReviewerStatusUpdate.create(
addCcUpdate.when, 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 with the following votes:\n\n * Code-Review+2");
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);
assertFixedCommits(commitsToFix.build(), result, c.getId());
List<String> commitHistoryDiff = commitHistoryDiff(result, c.getId());
assertThat(commitHistoryDiff)
.containsExactly(
"@@ -6 +6 @@\n" + "-Removed reviewer Other Account.\n" + "+Removed reviewer\n",
"@@ -6,3 +6 @@\n"
+ "-Removed cc Other Account with the following votes:\n"
+ "-\n"
+ "- * Code-Review+2\n"
+ "+Removed cc\n");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@Test
public void fixReviewerMessageNoReviewerFooter() throws Exception {
Change c = newChange();
writeUpdate(
RefNames.changeMetaRef(c.getId()),
getChangeUpdateBody(
c, String.format("Removed reviewer %s.", otherUser.getAccount().fullName())),
getAuthorIdent(changeOwner.getAccount()));
writeUpdate(
RefNames.changeMetaRef(c.getId()),
getChangeUpdateBody(
c,
String.format(
"Removed cc %s with the following votes:\n\n * Code-Review+2",
otherUser.getAccount().fullName())),
getAuthorIdent(changeOwner.getAccount()));
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 = commitHistoryDiff(result, c.getId());
assertThat(commitHistoryDiff)
.containsExactly(
"@@ -6 +6 @@\n" + "-Removed reviewer Other Account.\n" + "+Removed reviewer\n",
"@@ -6,3 +6 @@\n"
+ "-Removed cc Other Account with the following votes:\n"
+ "-\n"
+ "- * Code-Review+2\n"
+ "+Removed cc\n");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@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<ObjectId> commitsToFix =
new ImmutableList.Builder<ObjectId>()
.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()));
Instant updateTimestamp = serverIdent.getWhenAsInstant();
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().all().get(c.currentPatchSetId()))
.containsExactlyElementsIn(expectedApprovals);
assertThat(notesAfterRewrite.getApprovals().all().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);
assertFixedCommits(commitsToFix, result, c.getId());
List<String> commitHistoryDiff = commitHistoryDiff(result, 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");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@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<ObjectId> commitsToFix =
new ImmutableList.Builder<ObjectId>()
.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()));
Instant updateTimestamp = serverIdent.getWhenAsInstant();
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().all().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().all().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);
assertFixedCommits(commitsToFix, result, c.getId());
List<String> commitHistoryDiff = commitHistoryDiff(result, 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");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@Test
public void fixRemoveVoteChangeMessageWithUnparsableAuthorIdent() throws Exception {
Change c = newChange();
PersonIdent invalidAuthorIdent =
new PersonIdent(
changeOwner.getName(), "server@" + serverId, TimeUtil.now(), serverIdent.getZoneId());
writeUpdate(
RefNames.changeMetaRef(c.getId()),
getChangeUpdateBody(
c,
/* changeMessage= */ "Removed Verified+2 by " + otherUser.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 = commitHistoryDiff(result, c.getId());
// Other Account does not applier in any change updates, replaced with default
assertThat(commitHistoryDiff)
.containsExactly(
"@@ -6 +6 @@\n"
+ "-Removed Verified+2 by Other Account <other@account.com>\n"
+ "+Removed Verified+2\n");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@Test
public void fixRemoveVoteChangeMessageWithNoFooterLabel() throws Exception {
Change c = newChange();
ChangeUpdate approvalUpdate = newUpdate(c, changeOwner);
approvalUpdate.putApproval(VERIFIED, (short) +2);
approvalUpdate.putApprovalFor(otherUserId, VERIFIED, (short) -1);
approvalUpdate.commit();
writeUpdate(
RefNames.changeMetaRef(c.getId()),
// Even though footer is missing, accounts are matched among the account in change updates.
getChangeUpdateBody(c, /* changeMessage= */ "Removed Verified-1 by Other Account (0002)"),
getAuthorIdent(changeOwner.getAccount()));
writeUpdate(
RefNames.changeMetaRef(c.getId()),
getChangeUpdateBody(
c, /* changeMessage= */ "Removed Verified+2 by " + changeOwner.getNameEmail()),
getAuthorIdent(changeOwner.getAccount()));
// No rewrite for default
writeUpdate(
RefNames.changeMetaRef(c.getId()),
getChangeUpdateBody(c, /* changeMessage= */ "Removed Verified+2 by Gerrit Account"),
getAuthorIdent(changeOwner.getAccount()));
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 = commitHistoryDiff(result, c.getId());
assertThat(commitHistoryDiff)
.containsExactly(
"@@ -6 +6 @@\n"
+ "-Removed Verified-1 by Other Account (0002)\n"
+ "+Removed Verified-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");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@Test
public void fixRemoveVoteChangeMessageWithNoFooterLabel_matchByEmail() throws Exception {
Change c = newChange();
ChangeUpdate approvalUpdate = newUpdate(c, changeOwner);
approvalUpdate.putApproval(VERIFIED, (short) +2);
approvalUpdate.putApprovalFor(otherUserId, VERIFIED, (short) -1);
approvalUpdate.commit();
writeUpdate(
RefNames.changeMetaRef(c.getId()),
getChangeUpdateBody(
c,
/* changeMessage= */ "Removed Verified+2 by Renamed Change Owner <change@owner.com>"),
getAuthorIdent(changeOwner.getAccount()));
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 = commitHistoryDiff(result, c.getId());
assertThat(commitHistoryDiff)
.containsExactly(
"@@ -6 +6 @@\n"
+ "-Removed Verified+2 by Renamed Change Owner <change@owner.com>\n"
+ "+Removed Verified+2 by <GERRIT_ACCOUNT_1>\n");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@Test
public void fixRemoveVoteChangeMessageWithNoFooterLabel_matchByName() throws Exception {
Change c = newChange();
ChangeUpdate approvalUpdate = newUpdate(c, changeOwner);
approvalUpdate.putApproval(VERIFIED, (short) +2);
approvalUpdate.putApprovalFor(otherUserId, VERIFIED, (short) -1);
approvalUpdate.commit();
writeUpdate(
RefNames.changeMetaRef(c.getId()),
getChangeUpdateBody(c, /* changeMessage= */ "Removed Verified+2 by Change Owner"),
getAuthorIdent(changeOwner.getAccount()));
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 = commitHistoryDiff(result, c.getId());
assertThat(commitHistoryDiff)
.containsExactly(
"@@ -6 +6 @@\n"
+ "-Removed Verified+2 by Change Owner\n"
+ "+Removed Verified+2 by <GERRIT_ACCOUNT_1>\n");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@Test
public void fixRemoveVoteChangeMessageWithNoFooterLabel_matchDuplicateAccounts()
throws Exception {
Account duplicateCodeOwner =
Account.builder(Account.id(4), TimeUtil.now())
.setFullName(changeOwner.getName())
.setPreferredEmail("other@test.com")
.build();
accountCache.put(duplicateCodeOwner);
Change c = newChange();
ChangeUpdate approvalUpdate = newUpdate(c, changeOwner);
approvalUpdate.putApproval(VERIFIED, (short) +2);
approvalUpdate.putApprovalFor(duplicateCodeOwner.id(), VERIFIED, (short) -1);
approvalUpdate.commit();
writeUpdate(
RefNames.changeMetaRef(c.getId()),
getChangeUpdateBody(
c, /* changeMessage= */ "Removed Verified+2 by Change Owner <other@test.com>"),
getAuthorIdent(changeOwner.getAccount()));
writeUpdate(
RefNames.changeMetaRef(c.getId()),
getChangeUpdateBody(
c, /* changeMessage= */ "Removed Verified+2 by Change Owner <change@owner.com>"),
getAuthorIdent(changeOwner.getAccount()));
writeUpdate(
RefNames.changeMetaRef(c.getId()),
getChangeUpdateBody(
c, /* changeMessage= */ "Removed Verified-1 by Change Owner <other@test.com>"),
getAuthorIdent(changeOwner.getAccount()));
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 = commitHistoryDiff(result, c.getId());
assertThat(commitHistoryDiff)
.containsExactly(
"@@ -6 +6 @@\n"
+ "-Removed Verified+2 by Change Owner <other@test.com>\n"
+ "+Removed Verified+2 by <GERRIT_ACCOUNT_4>\n",
"@@ -6 +6 @@\n"
+ "-Removed Verified+2 by Change Owner <change@owner.com>\n"
+ "+Removed Verified+2 by <GERRIT_ACCOUNT_1>\n",
"@@ -6 +6 @@\n"
+ "-Removed Verified-1 by Change Owner <other@test.com>\n"
+ "+Removed Verified-1 by <GERRIT_ACCOUNT_4>\n");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@Test
public void fixRemoveVotesChangeMessage() throws Exception {
Change c = newChange();
ChangeUpdate approvalUpdate = newUpdate(c, changeOwner);
approvalUpdate.putApproval(VERIFIED, (short) +2);
approvalUpdate.putApprovalFor(otherUserId, VERIFIED, (short) -1);
approvalUpdate.commit();
writeUpdate(
RefNames.changeMetaRef(c.getId()),
// Even though footer is missing, accounts are matched among the account in change updates.
getChangeUpdateBody(
c,
/* changeMessage= */ "Removed the following votes:\n"
+ String.format("* Verified-1 by %s\n", otherUser.getNameEmail())),
getAuthorIdent(changeOwner.getAccount()));
writeUpdate(
RefNames.changeMetaRef(c.getId()),
getChangeUpdateBody(
c,
/* changeMessage= */ "Removed the following votes:\n"
+ String.format("* Verified+2 by %s\n", changeOwner.getNameEmail())
+ String.format("* Verified-1 by %s\n", changeOwner.getNameEmail())
+ String.format("* Code-Review by %s\n", otherUser.getNameEmail())),
getAuthorIdent(changeOwner.getAccount()));
// No rewrite for default
writeUpdate(
RefNames.changeMetaRef(c.getId()),
getChangeUpdateBody(
c,
/* changeMessage= */ "Removed the following votes:\n"
+ "* Verified+2 by Gerrit Account\n"
+ "* Verified-1 by <GERRIT_ACCOUNT_2>\n"),
getAuthorIdent(changeOwner.getAccount()));
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 = commitHistoryDiff(result, c.getId());
assertThat(commitHistoryDiff)
.containsExactly(
"@@ -7 +7 @@\n"
+ "-* Verified-1 by Other Account <other@account.com>\n"
+ "+* Verified-1 by <GERRIT_ACCOUNT_2>\n",
"@@ -7,3 +7,3 @@\n"
+ "-* Verified+2 by Change Owner <change@owner.com>\n"
+ "-* Verified-1 by Change Owner <change@owner.com>\n"
+ "-* Code-Review by Other Account <other@account.com>\n"
+ "+* Verified+2 by <GERRIT_ACCOUNT_1>\n"
+ "+* Verified-1 by <GERRIT_ACCOUNT_1>\n"
+ "+* Code-Review by <GERRIT_ACCOUNT_2>\n");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@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.addToPlannedAttentionSetUpdates(
AttentionSetUpdate.createForWrite(
changeOwner.getAccountId(), Operation.ADD, "Added by someone"));
validAttentionSetUpdate.commit();
ChangeUpdate invalidRemovedByClickUpdate = newUpdate(c, changeOwner);
invalidRemovedByClickUpdate.addToPlannedAttentionSetUpdates(
AttentionSetUpdate.createForWrite(
changeOwner.getAccountId(),
Operation.REMOVE,
String.format("Removed by %s by clicking the attention icon", otherUser.getName())));
commitsToFix.add(invalidRemovedByClickUpdate.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();
Instant updateTimestamp = serverIdent.getWhenAsInstant();
ImmutableList<AttentionSetUpdate> attentionSetUpdatesBeforeRewrite =
ImmutableList.of(
AttentionSetUpdate.createFromRead(
invalidRemovedByClickUpdate.getWhen(),
changeOwner.getAccountId(),
Operation.REMOVE,
String.format("Removed by %s by clicking the attention icon", otherUser.getName())),
AttentionSetUpdate.createFromRead(
validAttentionSetUpdate.getWhen(),
changeOwner.getAccountId(),
Operation.ADD,
"Added by someone"),
AttentionSetUpdate.createFromRead(
validAttentionSetUpdate.getWhen(),
otherUserId,
Operation.REMOVE,
"Removed by someone"),
AttentionSetUpdate.createFromRead(
updateTimestamp,
changeOwner.getAccountId(),
Operation.REMOVE,
String.format("%s replied on the change", otherUser.getName())),
AttentionSetUpdate.createFromRead(
updateTimestamp,
otherUserId,
Operation.ADD,
"Added by someone using the hovercard menu"),
AttentionSetUpdate.createFromRead(
invalidMultipleAttentionSetUpdate.getWhen(),
otherUserId,
Operation.REMOVE,
String.format("Removed by %s using the hovercard menu", otherUser.getName())),
AttentionSetUpdate.createFromRead(
invalidMultipleAttentionSetUpdate.getWhen(),
changeOwner.getAccountId(),
Operation.ADD,
String.format("%s replied on the change", otherUser.getName())),
AttentionSetUpdate.createFromRead(
invalidAttentionSetUpdate.getWhen(),
otherUserId,
Operation.ADD,
String.format("Added by %s using the hovercard menu", otherUser.getName())));
ImmutableList<AttentionSetUpdate> attentionSetUpdatesAfterRewrite =
ImmutableList.of(
AttentionSetUpdate.createFromRead(
invalidRemovedByClickUpdate.getWhen(),
changeOwner.getAccountId(),
Operation.REMOVE,
"Removed by someone by clicking the attention icon"),
AttentionSetUpdate.createFromRead(
validAttentionSetUpdate.getWhen(),
changeOwner.getAccountId(),
Operation.ADD,
"Added by someone"),
AttentionSetUpdate.createFromRead(
validAttentionSetUpdate.getWhen(),
otherUserId,
Operation.REMOVE,
"Removed by someone"),
AttentionSetUpdate.createFromRead(
updateTimestamp,
changeOwner.getAccountId(),
Operation.REMOVE,
"Someone replied on the change"),
AttentionSetUpdate.createFromRead(
updateTimestamp,
otherUserId,
Operation.ADD,
"Added by someone using the hovercard menu"),
AttentionSetUpdate.createFromRead(
invalidMultipleAttentionSetUpdate.getWhen(),
otherUserId,
Operation.REMOVE,
"Removed by someone using the hovercard menu"),
AttentionSetUpdate.createFromRead(
invalidMultipleAttentionSetUpdate.getWhen(),
changeOwner.getAccountId(),
Operation.ADD,
"Someone replied on the change"),
AttentionSetUpdate.createFromRead(
invalidAttentionSetUpdate.getWhen(),
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);
assertFixedCommits(commitsToFix.build(), result, c.getId());
List<String> commitHistoryDiff = commitHistoryDiff(result, c.getId());
assertThat(commitHistoryDiff).hasSize(4);
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\"}");
assertThat(commitHistoryDiff.get(3))
.isEqualTo(
"@@ -7 +7 @@\n"
+ "-Attention: {\"person_ident\":\"Gerrit User 1"
+ " \\u003c1@gerrit\\u003e\",\"operation\":\"REMOVE\",\"reason\":\"Removed by Other"
+ " Account by clicking the attention icon\"}\n"
+ "+Attention: {\"person_ident\":\"Gerrit User 1"
+ " \\u003c1@gerrit\\u003e\",\"operation\":\"REMOVE\",\"reason\":\"Removed by"
+ " someone by clicking the attention icon\"}\n");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@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 thirdAttentionSetUpdate = newUpdate(c, changeOwner);
thirdAttentionSetUpdate.addToPlannedAttentionSetUpdates(
AttentionSetUpdate.createForWrite(
changeOwner.getAccountId(),
Operation.REMOVE,
String.format("Removed by %s by clicking the attention icon", okAccountName)));
thirdAttentionSetUpdate.commit();
attentionSetUpdatesBeforeRewrite.add(
AttentionSetUpdate.createFromRead(
thirdAttentionSetUpdate.getWhen(),
changeOwner.getAccountId(),
Operation.REMOVE,
String.format("Removed by %s by clicking the attention icon", okAccountName)),
AttentionSetUpdate.createFromRead(
secondAttentionSetUpdate.getWhen(),
otherUserId,
Operation.REMOVE,
String.format("Removed by %s using the hovercard menu", okAccountName)),
AttentionSetUpdate.createFromRead(
secondAttentionSetUpdate.getWhen(),
changeOwner.getAccountId(),
Operation.ADD,
String.format("%s replied on the change", okAccountName)),
AttentionSetUpdate.createFromRead(
firstAttentionSetUpdate.getWhen(),
otherUserId,
Operation.ADD,
String.format("Added by %s using the hovercard menu", okAccountName)));
}
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();
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).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("");
commitsToFix.add(invalidMergedMessageUpdate.commit());
ChangeUpdate invalidCherryPickedMessageUpdate = newUpdate(c, changeOwner);
invalidCherryPickedMessageUpdate.setChangeMessage(
"Change has been successfully cherry-picked as e40dc1a50dc7f457a37579e2755374f3e1a5413b by "
+ changeOwner.getName());
commitsToFix.add(invalidCherryPickedMessageUpdate.commit());
ChangeUpdate invalidRebasedMessageUpdate = newUpdate(c, changeOwner);
invalidRebasedMessageUpdate.setChangeMessage(
"Change has been successfully rebased and submitted as"
+ " e40dc1a50dc7f457a37579e2755374f3e1a5413b by "
+ changeOwner.getName());
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);
assertFixedCommits(commitsToFix.build(), result, c.getId());
List<String> commitHistoryDiff = commitHistoryDiff(result, 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");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@Test
public void fixSubmitChangeMessageAndFooters() throws Exception {
Change c = newChange();
PersonIdent invalidAuthorIdent =
new PersonIdent(
changeOwner.getName(),
changeNoteUtil.getAccountIdAsEmailAddress(changeOwner.getAccountId()),
TimeUtil.now(),
serverIdent.getZoneId());
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 = commitHistoryDiff(result, 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");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@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));
assertFixedCommits(ImmutableList.of(invalidUpdateCommit.getId()), result, c.getId());
List<String> commitHistoryDiff = commitHistoryDiff(result, 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");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@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);
assertFixedCommits(commitsToFix.build(), result, c.getId());
List<String> commitHistoryDiff = commitHistoryDiff(result, 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");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@Test
public void fixCodeOwnersOnAddReviewerChangeMessage() throws Exception {
Account reviewer =
Account.builder(Account.id(3), TimeUtil.now())
.setFullName("Reviewer User")
.setPreferredEmail("reviewer@account.com")
.build();
accountCache.put(reviewer);
Account duplicateCodeOwner =
Account.builder(Account.id(4), TimeUtil.now()).setFullName(changeOwner.getName()).build();
accountCache.put(duplicateCodeOwner);
Account duplicateReviewer =
Account.builder(Account.id(5), TimeUtil.now()).setFullName(reviewer.getName()).build();
accountCache.put(duplicateReviewer);
Change c = newChange();
ImmutableList.Builder<ObjectId> commitsToFix = new ImmutableList.Builder<>();
ChangeUpdate addReviewerUpdate = newCodeOwnerAddReviewerUpdate(c, changeOwner);
addReviewerUpdate.putReviewer(reviewer.id(), REVIEWER);
addReviewerUpdate.commit();
ChangeUpdate invalidOnAddReviewerUpdate = newCodeOwnerAddReviewerUpdate(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 = newCodeOwnerAddReviewerUpdate(c, changeOwner);
addOtherReviewerUpdate.putReviewer(otherUserId, REVIEWER);
addOtherReviewerUpdate.commit();
ChangeUpdate invalidOnAddReviewerMultipleReviewerUpdate =
newCodeOwnerAddReviewerUpdate(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 = newCodeOwnerAddReviewerUpdate(c, changeOwner);
addDuplicateReviewerUpdate.putReviewer(duplicateReviewer.id(), REVIEWER);
addDuplicateReviewerUpdate.commit();
// Reviewer name resolves to multiple accounts in the same change
ChangeUpdate onAddReviewerUpdateWithDuplicate = newCodeOwnerAddReviewerUpdate(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 = newCodeOwnerAddReviewerUpdate(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"
+ "\nAdded reviewer owns the following files:\n"
+ " * file4.java\n",
"Added 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);
assertFixedCommits(commitsToFix.build(), result, c.getId());
List<String> commitHistoryDiff = commitHistoryDiff(result, 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"
+ "+Added reviewer owns the following files:\n",
"@@ -6 +6 @@\n"
+ "-Reviewer User who was added as reviewer owns the following files:\n"
+ "+Added reviewer owns the following files:\n");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@Test
public void fixCodeOwnersOnReviewChangeMessage() throws Exception {
Change c = newChange();
ImmutableList.Builder<ObjectId> commitsToFix = new ImmutableList.Builder<>();
ChangeUpdate invalidOnReviewUpdate = newUpdate(c, changeOwner);
invalidOnReviewUpdate.setChangeMessage(
"Patch Set 1: Any-Label+2 Other-Label+2 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"
+ "By voting Any-Label+2 the code-owners submit requirement is overridden by Change"
+ " Owner\n"
+ "By voting Other-Label+2 the code-owners submit requirement is still overridden by"
+ " Change Owner\n");
commitsToFix.add(invalidOnReviewUpdate.commit());
ChangeUpdate invalidOnReviewUpdateAnyOrder = newUpdate(c, changeOwner);
invalidOnReviewUpdateAnyOrder.setChangeMessage(
"Patch Set 1: Any-Label+2 Other-Label+2 Code-Review+2\n\n"
+ "By voting Any-Label+2 the code-owners submit requirement is overridden by Change"
+ " Owner\n"
+ "By voting Other-Label+2 the code-owners submit requirement is still overridden by"
+ " Change Owner\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(invalidOnReviewUpdateAnyOrder.commit());
ChangeUpdate invalidOnApprovalUpdate = newUpdate(c, otherUser);
invalidOnApprovalUpdate.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"
+ "\n"
+ "The listed files are still implicitly approved by Other Account.\n");
commitsToFix.add(invalidOnApprovalUpdate.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");
commitsToFix.add(invalidOnOverrideUpdate.commit());
ChangeUpdate partiallyValidOnReviewUpdate = newUpdate(c, changeOwner);
partiallyValidOnReviewUpdate.setChangeMessage(
"Patch Set 1: Any-Label+2 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"
+ "By voting Any-Label+2 the code-owners submit requirement is overridden by Change"
+ " Owner\n");
commitsToFix.add(partiallyValidOnReviewUpdate.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");
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(7);
assertThat(changeMessages(notesAfterRewrite))
.containsExactly(
"Patch Set 1: Any-Label+2 Other-Label+2 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"
+ "By voting Any-Label+2 the code-owners submit requirement is overridden by"
+ " <GERRIT_ACCOUNT_1>\n"
+ "By voting Other-Label+2 the code-owners submit requirement is still overridden"
+ " by <GERRIT_ACCOUNT_1>\n",
"Patch Set 1: Any-Label+2 Other-Label+2 Code-Review+2\n\n"
+ "By voting Any-Label+2 the code-owners submit requirement is overridden by"
+ " <GERRIT_ACCOUNT_1>\n"
+ "By voting Other-Label+2 the code-owners submit requirement is still overridden"
+ " by <GERRIT_ACCOUNT_1>\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"
+ "\n"
+ "The 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",
"Patch Set 1: Any-Label+2 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"
+ "By voting Any-Label+2 the code-owners submit requirement is overridden by"
+ " <GERRIT_ACCOUNT_1>\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");
Ref metaRefAfterRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId()));
assertThat(metaRefAfterRewrite.getObjectId()).isNotEqualTo(metaRefBeforeRewrite.getObjectId());
ImmutableList<RevCommit> commitsAfterRewrite = logMetaRef(repo, metaRefAfterRewrite);
assertValidCommits(commitsBeforeRewrite, commitsAfterRewrite, invalidCommits);
assertFixedCommits(commitsToFix.build(), result, c.getId());
List<String> commitHistoryDiff = commitHistoryDiff(result, 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"
+ "@@ -11,2 +11,2 @@\n"
+ "-By voting Any-Label+2 the code-owners submit requirement is overridden by"
+ " Change Owner\n"
+ "-By voting Other-Label+2 the code-owners submit requirement is still overridden"
+ " by Change Owner\n"
+ "+By voting Any-Label+2 the code-owners submit requirement is overridden by"
+ " <GERRIT_ACCOUNT_1>\n"
+ "+By voting Other-Label+2 the code-owners submit requirement is still overridden"
+ " by <GERRIT_ACCOUNT_1>\n",
"@@ -8,3 +8,3 @@\n"
+ "-By voting Any-Label+2 the code-owners submit requirement is overridden by"
+ " Change Owner\n"
+ "-By voting Other-Label+2 the code-owners submit requirement is still overridden"
+ " by Change Owner\n"
+ "-By voting Code-Review+2 the following files are now code-owner approved by"
+ " Change Owner:\n"
+ "+By voting Any-Label+2 the code-owners submit requirement is overridden by"
+ " <GERRIT_ACCOUNT_1>\n"
+ "+By voting Other-Label+2 the code-owners submit requirement is still overridden"
+ " by <GERRIT_ACCOUNT_1>\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",
"@@ -11 +11 @@\n"
+ "-By voting Any-Label+2 the code-owners submit requirement is overridden by"
+ " Change Owner\n"
+ "+By voting Any-Label+2 the code-owners submit requirement is overridden by"
+ " <GERRIT_ACCOUNT_1>\n");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@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()));
// Valid commits
writeUpdate(
RefNames.changeMetaRef(c.getId()),
getChangeUpdateBody(
c,
"Assignee added: <GERRIT_ACCOUNT_2>",
"Assignee: " + getValidIdentAsString(otherUser.getAccount())),
getAuthorIdent(changeOwner.getAccount()));
writeUpdate(
RefNames.changeMetaRef(c.getId()),
getChangeUpdateBody(c, "Assignee deleted: <GERRIT_ACCOUNT_2>", "Assignee:"),
getAuthorIdent(changeOwner.getAccount()));
Ref metaRefBeforeRewrite = repo.exactRef(RefNames.changeMetaRef(c.getId()));
ImmutableList<RevCommit> commitsBeforeRewrite = logMetaRef(repo, metaRefBeforeRewrite);
int invalidCommitIndex = commitsBeforeRewrite.indexOf(invalidUpdateCommit);
RunOptions options = new RunOptions();
options.dryRun = false;
BackfillResult result = rewriter.backfillProject(project, repo, options);
assertThat(result.fixedRefDiff.keySet()).containsExactly(RefNames.changeMetaRef(c.getId()));
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));
assertFixedCommits(ImmutableList.of(invalidUpdateCommit.getId()), result, c.getId());
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 = commitHistoryDiff(result, c.getId());
assertThat(commitHistoryDiff)
.containsExactly(
"@@ -9 +9 @@\n"
+ "-Assignee: Change Owner <1@gerrit>\n"
+ "+Assignee: Gerrit User 1 <1@gerrit>\n");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@Test
public void fixAssigneeChangeMessage() throws Exception {
Change c = newChange();
ImmutableList<ObjectId> commitsToFix =
new ImmutableList.Builder<ObjectId>()
.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(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(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);
assertFixedCommits(commitsToFix, result, c.getId());
List<String> commitHistoryDiff = commitHistoryDiff(result, 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");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@Test
public void fixAssigneeChangeMessageNoAssigneeFooter() throws Exception {
Change c = newChange();
writeUpdate(
RefNames.changeMetaRef(c.getId()),
getChangeUpdateBody(c, "Assignee added: " + changeOwner.getName()),
getAuthorIdent(changeOwner.getAccount()));
writeUpdate(
RefNames.changeMetaRef(c.getId()),
getChangeUpdateBody(
c,
String.format(
"Assignee changed from: %s to: %s",
changeOwner.getNameEmail(), otherUser.getNameEmail())),
getAuthorIdent(otherUser.getAccount()));
writeUpdate(
RefNames.changeMetaRef(c.getId()),
getChangeUpdateBody(c, "Assignee deleted: " + otherUser.getName()),
getAuthorIdent(changeOwner.getAccount()));
Account reviewer =
Account.builder(Account.id(3), TimeUtil.now())
.setFullName("Reviewer User")
.setPreferredEmail("reviewer@account.com")
.build();
accountCache.put(reviewer);
// Even though account is present in the cache, it won't be used because it does not appear in
// the history of this change.
writeUpdate(
RefNames.changeMetaRef(c.getId()),
getChangeUpdateBody(c, "Assignee added: " + reviewer.getName()),
getAuthorIdent(changeOwner.getAccount()));
writeUpdate(
RefNames.changeMetaRef(c.getId()),
getChangeUpdateBody(c, "Assignee deleted: Gerrit Account"),
getAuthorIdent(changeOwner.getAccount()));
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 = commitHistoryDiff(result, c.getId());
assertThat(commitHistoryDiff)
.containsExactly(
"@@ -6 +6 @@\n"
+ "-Assignee added: Change Owner\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\n"
+ "+Assignee deleted: <GERRIT_ACCOUNT_2>\n",
"@@ -6 +6 @@\n" + "-Assignee added: Reviewer User\n" + "+Assignee was added.\n");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
@Test
public void singleRunFixesAll() throws Exception {
Change c = newChange();
Instant when = TimeUtil.now();
String assigneeIdentToFix = getAccountIdentToFix(otherUser.getAccount());
PersonIdent authorIdentToFix =
new PersonIdent(
changeOwner.getName(),
changeNoteUtil.getAccountIdAsEmailAddress(changeOwner.getAccountId()),
when,
serverIdent.getZoneId());
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);
RunOptions options = new RunOptions();
options.dryRun = false;
BackfillResult result = rewriter.backfillProject(project, repo, options);
assertThat(result.fixedRefDiff.keySet()).containsExactly(RefNames.changeMetaRef(c.getId()));
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));
assertFixedCommits(ImmutableList.of(invalidUpdateCommit.getId()), result, c.getId());
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 = commitHistoryDiff(result, 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>");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
}
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 void assertFixedCommits(
ImmutableList<ObjectId> expectedFixedCommits, BackfillResult result, Change.Id changeId) {
assertThat(
result.fixedRefDiff.get(RefNames.changeMetaRef(changeId)).stream()
.map(CommitDiff::oldSha1)
.collect(toImmutableList()))
.containsExactlyElementsIn(expectedFixedCommits);
}
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());
}
protected ChangeUpdate newCodeOwnerAddReviewerUpdate(Change c, CurrentUser user)
throws Exception {
ChangeUpdate update = newUpdate(c, user, true);
update.setTag("autogenerated:gerrit:code-owners:addReviewer");
return update;
}
private ImmutableList<String> commitHistoryDiff(BackfillResult result, Change.Id changeId) {
return result.fixedRefDiff.get(RefNames.changeMetaRef(changeId)).stream()
.map(CommitDiff::diff)
.collect(toImmutableList());
}
private PersonIdent getAuthorIdent(Account account) {
return changeNoteUtil.newAccountIdIdent(account.id(), TimeUtil.now(), serverIdent);
}
}