Fix the bug in revert submission subject generation.
RevertInput is reused when creating individual changes. However the
`initialMessage` is taken before processing all changes within the same
branch. Instead of before any changes to revertInput are introduced.
Google-Bug-Id: b/204391110
Release-Notes: skip
Change-Id: If823d63c5bcff4878df3aec027da47e0dc356cb9
diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
index 947ca6a..62fdcbb 100644
--- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
+++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
@@ -236,6 +236,7 @@
cherryPickInput = createCherryPickInput(revertInput);
Instant timestamp = TimeUtil.now();
+ String initialMessage = revertInput.message;
for (BranchNameKey projectAndBranch : changesPerProjectAndBranch.keySet()) {
cherryPickInput.base = null;
Project.NameKey project = projectAndBranch.project();
@@ -253,6 +254,7 @@
.collect(Collectors.toSet());
revertAllChangesInProjectAndBranch(
+ initialMessage,
revertInput,
project,
sortedChangesInProjectAndBranch,
@@ -265,7 +267,9 @@
return revertSubmissionInfo;
}
+ // Warning: reuses and modifies revertInput.message.
private void revertAllChangesInProjectAndBranch(
+ String initialMessage,
RevertInput revertInput,
Project.NameKey project,
Iterator<PatchSetData> sortedChangesInProjectAndBranch,
@@ -273,8 +277,6 @@
Instant timestamp)
throws IOException, RestApiException, UpdateException, ConfigInvalidException,
PermissionBackendException {
-
- String initialMessage = revertInput.message;
while (sortedChangesInProjectAndBranch.hasNext()) {
ChangeNotes changeNotes = sortedChangesInProjectAndBranch.next().data().notes();
if (cherryPickInput.base == null) {
@@ -282,6 +284,7 @@
cherryPickInput.base = getBase(changeNotes, commitIdsInProjectAndBranch).name();
}
+ // Set revert message for the current revert change.
revertInput.message = getMessage(initialMessage, changeNotes);
if (cherryPickInput.base.equals(changeNotes.getCurrentPatchSet().commitId().getName())) {
// This is the code in case this is the first revert of this project + branch, and the
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
index 527253c..e0e980e 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
@@ -63,6 +63,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
+import java.util.regex.Pattern;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Repository;
@@ -1278,10 +1279,38 @@
.distinct()
.count())
.isEqualTo(1);
+
+ // Size
List<ChangeApi> revertChanges = getChangeApis(revertSubmissionInfo);
+ assertThat(revertChanges).hasSize(3);
+ assertThat(gApi.changes().id(revertChanges.get(1).id()).current().related().changes).hasSize(2);
+
+ // Contents
assertThat(revertChanges.get(0).current().files().get("c.txt").linesDeleted).isEqualTo(1);
assertThat(revertChanges.get(1).current().files().get("a.txt").linesDeleted).isEqualTo(1);
assertThat(revertChanges.get(2).current().files().get("b.txt").linesDeleted).isEqualTo(1);
+
+ // Commit message
+ assertThat(revertChanges.get(0).current().commit(false).message)
+ .matches(
+ Pattern.compile(
+ "Revert \"first change\"\n\n"
+ + "This reverts commit [a-f0-9]+\\.\n\n"
+ + "Change-Id: I[a-f0-9]+\n"));
+ assertThat(revertChanges.get(1).current().commit(false).message)
+ .matches(
+ Pattern.compile(
+ "Revert \"second change\"\n\n"
+ + "This reverts commit [a-f0-9]+\\.\n\n"
+ + "Change-Id: I[a-f0-9]+\n"));
+ assertThat(revertChanges.get(2).current().commit(false).message)
+ .matches(
+ Pattern.compile(
+ "Revert \"third change\"\n\n"
+ + "This reverts commit [a-f0-9]+\\.\n\n"
+ + "Change-Id: I[a-f0-9]+\n"));
+
+ // Relationships
String sha1FirstChange = resultCommits.get(0).getCommit().getName();
String sha1ThirdChange = resultCommits.get(2).getCommit().getName();
String sha1SecondRevert = revertChanges.get(2).current().commit(false).commit;
@@ -1291,9 +1320,6 @@
.isEqualTo(sha1ThirdChange);
assertThat(revertChanges.get(1).current().commit(false).parents.get(0).commit)
.isEqualTo(sha1SecondRevert);
-
- assertThat(revertChanges).hasSize(3);
- assertThat(gApi.changes().id(revertChanges.get(1).id()).current().related().changes).hasSize(2);
}
@Test
