diff options
author | Marija Savtchouk <mariasavtchouk@google.com> | 2023-08-21 11:34:09 +0200 |
---|---|---|
committer | Marija Savtchouk <mariasavtchouk@google.com> | 2023-08-22 09:42:46 +0200 |
commit | 43a86e474337085444f7513df685a05ffb8982b0 (patch) | |
tree | b5a50dcf90213dfde11d729d8ba7820dd3909f3e | |
parent | 9b90277bc34de4d7a773e7b375cfe217668b1a64 (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.java | 26 | ||||
-rw-r--r-- | java/com/google/gerrit/server/restapi/change/RevertSubmission.java | 19 |
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 |