Merge "Fix duplicate revert messages in `RevertSubmission`."
diff --git a/java/com/google/gerrit/server/git/CommitUtil.java b/java/com/google/gerrit/server/git/CommitUtil.java
index c9c090d..3c5da5b 100644
--- a/java/com/google/gerrit/server/git/CommitUtil.java
+++ b/java/com/google/gerrit/server/git/CommitUtil.java
@@ -179,6 +179,26 @@
public Change.Id createRevertChange(
ChangeNotes notes, CurrentUser user, RevertInput input, Instant timestamp)
throws RestApiException, UpdateException, ConfigInvalidException, IOException {
+ return createRevertChange(notes, user, input, timestamp, false);
+ }
+
+ /**
+ * Allows creating a revert change with an option to specify if it is a submission revert.
+ *
+ * @param notes ChangeNotes of the change being reverted.
+ * @param user Current User performing the revert.
+ * @param input the RevertInput entity for conducting the revert.
+ * @param timestamp timestamp for the created change.
+ * @param isSubmission whether the revert is being done as part of reverting a submission.
+ * @return ObjectId that represents the newly created commit.
+ */
+ public Change.Id createRevertChange(
+ ChangeNotes notes,
+ CurrentUser user,
+ RevertInput input,
+ Instant timestamp,
+ boolean isSubmission)
+ throws RestApiException, UpdateException, ConfigInvalidException, IOException {
String message = Strings.emptyToNull(input.message);
try (RefUpdateContext ctx = RefUpdateContext.open(CHANGE_MODIFICATION)) {
try (Repository git = repoManager.openRepository(notes.getProjectName());
@@ -187,7 +207,8 @@
CodeReviewRevWalk revWalk = CodeReviewCommit.newRevWalk(reader)) {
ObjectId generatedChangeId = CommitMessageUtil.generateChangeId();
CodeReviewCommit revertCommit =
- createRevertCommit(message, notes, user, timestamp, oi, revWalk, generatedChangeId);
+ createRevertCommit(
+ message, notes, user, timestamp, oi, revWalk, generatedChangeId, isSubmission);
return createRevertChangeFromCommit(
revertCommit, input, notes, user, generatedChangeId, timestamp, oi, revWalk, git);
} catch (RepositoryNotFoundException e) {
@@ -208,12 +229,28 @@
public CodeReviewCommit createRevertCommit(
String message, ChangeNotes notes, CurrentUser user, Instant ts)
throws RestApiException, IOException {
+ return createRevertCommit(message, notes, user, ts, false);
+ }
+
+ /**
+ * Wrapper function for creating a revert Commit with an option to specify if it is a submission.
+ *
+ * @param message Commit message for the revert commit.
+ * @param notes ChangeNotes of the change being reverted.
+ * @param user Current User performing the revert.
+ * @param ts Timestamp of creation for the commit.
+ * @param isSubmission whether the revert is being done as part of reverting a submission.
+ * @return that newly created revert commit.
+ */
+ public CodeReviewCommit createRevertCommit(
+ String message, ChangeNotes notes, CurrentUser user, Instant ts, boolean isSubmission)
+ throws RestApiException, IOException {
try (Repository git = repoManager.openRepository(notes.getProjectName());
ObjectInserter oi = git.newObjectInserter();
ObjectReader reader = oi.newReader();
CodeReviewRevWalk revWalk = CodeReviewCommit.newRevWalk(reader)) {
- return createRevertCommit(message, notes, user, ts, oi, revWalk, null);
+ return createRevertCommit(message, notes, user, ts, oi, revWalk, null, isSubmission);
} catch (RepositoryNotFoundException e) {
throw new ResourceNotFoundException(notes.getProjectName().toString(), e);
}
@@ -274,6 +311,7 @@
* @param revWalk Used for parsing the original commit.
* @param generatedChangeId The changeId for the commit message, can be null since it is not
* needed for commits, only for changes.
+ * @param isSubmission whether the revert is being done as part of reverting a submission.
* @return ObjectId that represents the newly created commit.
* @throws ResourceConflictException Can't revert the initial commit.
* @throws IOException Thrown in case of I/O errors.
@@ -285,7 +323,8 @@
Instant ts,
ObjectInserter oi,
CodeReviewRevWalk revWalk,
- @Nullable ObjectId generatedChangeId)
+ @Nullable ObjectId generatedChangeId,
+ boolean isSubmission)
throws ResourceConflictException, IOException {
PatchSet patch = notes.getCurrentPatchSet();
@@ -304,7 +343,7 @@
Change changeToRevert = notes.getChange();
String subject = changeToRevert.getSubject();
- message = getRevertMessage(message, subject, patch.commitId().name(), false);
+ message = getRevertMessage(message, subject, patch.commitId().name(), isSubmission);
String newFooters = getBugAndIssueFooters(commitToRevert);
if (!newFooters.isEmpty()) {
diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
index 3d37424..3c2ef66 100644
--- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
+++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
@@ -243,7 +243,6 @@
cherryPickInput = createCherryPickInput(revertInput);
Instant timestamp = TimeUtil.now();
- String initialMessage = revertInput.message;
for (BranchNameKey projectAndBranch : changesPerProjectAndBranch.keySet()) {
cherryPickInput.base = null;
Project.NameKey project = projectAndBranch.project();
@@ -260,7 +259,6 @@
.collect(Collectors.toSet());
revertAllChangesInProjectAndBranch(
- initialMessage,
revertInput,
project,
sortedChangesInProjectAndBranch,
@@ -273,9 +271,7 @@
return revertSubmissionInfo;
}
- // Warning: reuses and modifies revertInput.message.
private void revertAllChangesInProjectAndBranch(
- String initialMessage,
RevertInput revertInput,
Project.NameKey project,
Iterator<PatchSetData> sortedChangesInProjectAndBranch,
@@ -293,8 +289,6 @@
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
// revert would be on top of the change being reverted.
@@ -309,7 +303,8 @@
RevertInput revertInput, Project.NameKey project, ChangeNotes changeNotes, Instant timestamp)
throws IOException, ConfigInvalidException, UpdateException, RestApiException {
RevCommit revCommit =
- commitUtil.createRevertCommit(revertInput.message, changeNotes, user.get(), timestamp);
+ commitUtil.createRevertCommit(
+ revertInput.message, changeNotes, user.get(), timestamp, true);
// TODO (paiking): As a future change, the revert should just be done directly on the
// target rather than just creating a commit and then cherry-picking it.
cherryPickInput.message = revCommit.getFullMessage();
@@ -351,7 +346,7 @@
throws IOException, RestApiException, UpdateException, ConfigInvalidException {
Change.Id revertId =
- commitUtil.createRevertChange(changeNotes, user.get(), revertInput, timestamp);
+ commitUtil.createRevertChange(changeNotes, user.get(), revertInput, timestamp, true);
results.add(json.noOptions().format(changeNotes.getProjectName(), revertId));
ChangeNotes revertChange =
changeNotesFactory.createChecked(changeNotes.getProjectName(), revertId);
@@ -376,14 +371,6 @@
return cherryPickInput;
}
- private String getMessage(String initialMessage, ChangeNotes changeNotes) {
- return commitUtil.getRevertMessage(
- initialMessage,
- changeNotes.getChange().getSubject(),
- changeNotes.getCurrentPatchSet().commitId().name(),
- true);
- }
-
/**
* This function finds the base that the first revert in a project + branch should be based on.
*
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
index 6de06b0..ccb9b13 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
@@ -1095,6 +1095,68 @@
}
@Test
+ public void revertSubmissionOfRevertSubmissionWithSetMessage() throws Exception {
+ String firstResult = createChange("first change", "a.txt", "message").getChangeId();
+ String secondResult = createChange("second change", "b.txt", "message").getChangeId();
+ approve(firstResult);
+ approve(secondResult);
+ gApi.changes().id(secondResult).current().submit();
+
+ // First revert
+ List<ChangeInfo> revertChanges =
+ gApi.changes().id(firstResult).revertSubmission().revertChanges;
+ ChangeInfo firstRevert =
+ revertChanges.stream()
+ .filter(c -> c.subject.equals("Revert \"first change\""))
+ .findFirst()
+ .get();
+ ChangeInfo secondRevert =
+ revertChanges.stream()
+ .filter(c -> c.subject.equals("Revert \"second change\""))
+ .findFirst()
+ .get();
+
+ approve(firstRevert.id);
+ approve(secondRevert.id);
+ gApi.changes().id(firstRevert.id).current().submit();
+
+ // Revert of revert with custom message
+ RevertInput revertInput = new RevertInput();
+ String commitMessage = "Custom message for double revert";
+ revertInput.message = commitMessage;
+ List<ChangeInfo> doubleRevertChanges =
+ gApi.changes().id(firstRevert.id).revertSubmission(revertInput).revertChanges;
+
+ ChangeInfo doubleRevertFirst =
+ doubleRevertChanges.stream()
+ .filter(c -> c.subject.equals("Revert^2 \"first change\""))
+ .findFirst()
+ .get();
+ ChangeInfo doubleRevertSecond =
+ doubleRevertChanges.stream()
+ .filter(c -> c.subject.equals("Revert^2 \"second change\""))
+ .findFirst()
+ .get();
+
+ String firstRevertCommit = gApi.changes().id(firstRevert.id).current().commit(false).commit;
+ String commitMessage1 = gApi.changes().id(doubleRevertFirst.id).current().commit(false).message;
+ assertThat(commitMessage1)
+ .isEqualTo(
+ String.format(
+ "Revert^2 \"first change\"\n\nThis reverts commit %s.\n\n%s\n\nChange-Id: %s\n",
+ firstRevertCommit, commitMessage, doubleRevertFirst.changeId));
+
+ String secondRevertCommit = gApi.changes().id(secondRevert.id).current().commit(false).commit;
+ String commitMessage2 =
+ gApi.changes().id(doubleRevertSecond.id).current().commit(false).message;
+ assertThat(commitMessage2)
+ .isEqualTo(
+ String.format(
+ "Revert^2 \"second change\"\n\nThis reverts commit %s.\n\n%s\n\nChange-Id: %s\n",
+ secondRevertCommit, commitMessage, doubleRevertSecond.changeId));
+ }
+
+ @Test
public void revertSubmissionWithSetMessageChangeIdIgnored() throws Exception {
String firstResult = createChange("first change", "a.txt", "message").getChangeId();
String secondResult = createChange("second change", "b.txt", "message").getChangeId();
