summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMarija Savtchouk <mariasavtchouk@google.com>2023-08-21 11:34:09 +0200
committerMarija Savtchouk <mariasavtchouk@google.com>2023-08-22 09:42:46 +0200
commit43a86e474337085444f7513df685a05ffb8982b0 (patch)
treeb5a50dcf90213dfde11d729d8ba7820dd3909f3e
parent9b90277bc34de4d7a773e7b375cfe217668b1a64 (diff)
Pass base RevCommit from RevertSubmission to CherryPickChange.
When reverting a chain of changes, the last submitted change is created as a 'normal' revert, and reverts of its predecessors are created as cherry-picks on top of each other. To create this cherry-picks, CherryPickChange with the CherryPickInput is reused. Because CherryPickInput normally contains the user-provided parameters, the cherryPickInput.base is additionally verified (i.e. there is exactly one change associated with this commit and this change belongs to the destination branch). This also involves the index lookup, which might suffer from the staleness. When the cherry-pick is created from the RevertSubmission endpoint, this verification is not necessary, since we just created this base on the correct branch. Explicitly pass baseRevCommit from RevertSubmission to CherryPickChange, to avoid the lookup and verification. Change-Id: Ib7262fc0c4be65cddeae7f83ecc869a413597e7a Google-Bug-Id: b/295535767 Release-Notes: skip
-rw-r--r--java/com/google/gerrit/server/restapi/change/CherryPickChange.java26
-rw-r--r--java/com/google/gerrit/server/restapi/change/RevertSubmission.java19
2 files changed, 37 insertions, 8 deletions
diff --git a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
index 8b99e1c4d7..677279e663 100644
--- a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
@@ -175,7 +175,8 @@ public class CherryPickChange {
null,
null,
null,
- null);
+ null,
+ Optional.empty());
}
/**
@@ -206,7 +207,17 @@ public class CherryPickChange {
throws IOException, InvalidChangeOperationException, UpdateException, RestApiException,
ConfigInvalidException, NoSuchProjectException {
return cherryPick(
- sourceChange, project, sourceCommit, input, dest, TimeUtil.now(), null, null, null, null);
+ sourceChange,
+ project,
+ sourceCommit,
+ input,
+ dest,
+ TimeUtil.now(),
+ null,
+ null,
+ null,
+ null,
+ Optional.empty());
}
/**
@@ -228,6 +239,9 @@ public class CherryPickChange {
* @param idForNewChange The ID that the new change of the cherry pick will have. If provided and
* the cherry-pick doesn't result in creating a new change, then
* InvalidChangeOperationException is thrown.
+ * @param verifiedBaseCommit - base commit for the cherry-pick, which is guaranteed to be
+ * associated with exactly one change and belong to a {@code dest} branch. This is currently
+ * only used when this base commit was created in the same API call.
* @return Result object that describes the cherry pick.
* @throws IOException Unable to open repository or read from the database.
* @throws InvalidChangeOperationException Parent or branch don't exist, or two changes with same
@@ -248,7 +262,8 @@ public class CherryPickChange {
@Nullable Change.Id revertedChange,
@Nullable ObjectId changeIdForNewChange,
@Nullable Change.Id idForNewChange,
- @Nullable Boolean workInProgress)
+ @Nullable Boolean workInProgress,
+ Optional<RevCommit> verifiedBaseCommit)
throws IOException, InvalidChangeOperationException, UpdateException, RestApiException,
ConfigInvalidException, NoSuchProjectException {
IdentifiedUser identifiedUser = user.get();
@@ -266,8 +281,9 @@ public class CherryPickChange {
}
RevCommit baseCommit =
- CommitUtil.getBaseCommit(
- project.get(), queryProvider.get(), revWalk, destRef, input.base);
+ verifiedBaseCommit.orElse(
+ CommitUtil.getBaseCommit(
+ project.get(), queryProvider.get(), revWalk, destRef, input.base));
CodeReviewCommit commitToCherryPick = revWalk.parseCommit(sourceCommit);
diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
index 691fc7588a..3851e826da 100644
--- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
+++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
@@ -86,6 +86,7 @@ import java.util.Comparator;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
+import java.util.Optional;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -310,6 +311,13 @@ public class RevertSubmission
cherryPickInput.message = revertInput.message;
ObjectId generatedChangeId = CommitMessageUtil.generateChangeId();
Change.Id cherryPickRevertChangeId = Change.id(seq.nextChangeId());
+ RevCommit baseCommit = null;
+ if (cherryPickInput.base != null) {
+ try (Repository git = repoManager.openRepository(changeNotes.getProjectName());
+ RevWalk revWalk = new RevWalk(git.newObjectReader())) {
+ baseCommit = revWalk.parseCommit(ObjectId.fromString(cherryPickInput.base));
+ }
+ }
try (RefUpdateContext ctx = RefUpdateContext.open(CHANGE_MODIFICATION)) {
try (BatchUpdate bu = updateFactory.create(project, user.get(), TimeUtil.now())) {
bu.setNotify(
@@ -323,7 +331,8 @@ public class RevertSubmission
generatedChangeId,
cherryPickRevertChangeId,
timestamp,
- revertInput.workInProgress));
+ revertInput.workInProgress,
+ baseCommit));
if (!revertInput.workInProgress) {
commitUtil.addChangeRevertedNotificationOps(
bu, changeNotes.getChangeId(), cherryPickRevertChangeId, generatedChangeId.name());
@@ -548,18 +557,21 @@ public class RevertSubmission
private final Change.Id cherryPickRevertChangeId;
private final Instant timestamp;
private final boolean workInProgress;
+ private final RevCommit baseCommit;
CreateCherryPickOp(
ObjectId revCommitId,
ObjectId computedChangeId,
Change.Id cherryPickRevertChangeId,
Instant timestamp,
- Boolean workInProgress) {
+ Boolean workInProgress,
+ RevCommit baseCommit) {
this.revCommitId = revCommitId;
this.computedChangeId = computedChangeId;
this.cherryPickRevertChangeId = cherryPickRevertChangeId;
this.timestamp = timestamp;
this.workInProgress = workInProgress;
+ this.baseCommit = baseCommit;
}
@Override
@@ -577,7 +589,8 @@ public class RevertSubmission
change.getId(),
computedChangeId,
cherryPickRevertChangeId,
- workInProgress);
+ workInProgress,
+ Optional.ofNullable(baseCommit));
// save the commit as base for next cherryPick of that branch
cherryPickInput.base =
changeNotesFactory