opendev/gerrit
Code Issues Proposed changes

Fix bug: cherry-picks were not "related"

Browse Source 
Currently, when creating a cherry-pick and specifying a base,
if the base is an open change, we should show the created
cherry-pick and the base as related changes. However, this doesn't
happen.

This change ensures that those changes will have the same "groups"
thus the GetRelated endpoint will identify those changes as related.

Also removed legacy workaround for RevertSubmission that allowed
those changes to be related.

Change-Id: Id712918b2fc8e25ea997a668c1137fd59fd687b6
This commit is contained in:
Gal Paikin
2020-04-29 17:16:19 +02:00
parent 96b5fa47c4
commit d39e31c156
3 changed files with 79 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
java/com/google/gerrit/server/restapi/change/CherryPickChange.java
View File

@@ -19,7 +19,6 @@ import static com.google.gerrit.server.project.ProjectCache.noSuchProject;
import com.google.auto.value.AutoValue;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.Nullable;
@@ -47,6 +46,7 @@ import com.google.gerrit.server.change.SetCherryPickOp;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.GroupCollector;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
@@ -173,7 +173,6 @@ public class CherryPickChange {
TimeUtil.nowTs(),
null,
null,
null,
null);
}
@@ -205,7 +204,7 @@ public class CherryPickChange {
throws IOException, InvalidChangeOperationException, UpdateException, RestApiException,
ConfigInvalidException, NoSuchProjectException {
return cherryPick(
sourceChange, project, sourceCommit, input, dest, TimeUtil.nowTs(), null, null, null, null);
sourceChange, project, sourceCommit, input, dest, TimeUtil.nowTs(), null, null, null);
}
/**
@@ -227,8 +226,6 @@ 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 groupName The name of the group for grouping related changes (used by GetRelated
* endpoint).
* @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,8 +245,7 @@ public class CherryPickChange {
Timestamp timestamp,
@Nullable Change.Id revertedChange,
@Nullable ObjectId changeIdForNewChange,
@Nullable Change.Id idForNewChange,
@Nullable String groupName)
@Nullable Change.Id idForNewChange)
throws IOException, InvalidChangeOperationException, UpdateException, RestApiException,
ConfigInvalidException, NoSuchProjectException {
@@ -379,12 +375,12 @@ public class CherryPickChange {
cherryPickCommit,
dest.branch(),
newTopic,
project,
sourceChange,
sourceCommit,
input,
revertedChange,
idForNewChange,
groupName);
idForNewChange);
}
bu.execute();
return Result.create(changeId, cherryPickCommit.getFilesWithGitConflicts());
@@ -473,13 +469,13 @@ public class CherryPickChange {
CodeReviewCommit cherryPickCommit,
String refName,
String topic,
Project.NameKey project,
@Nullable Change sourceChange,
@Nullable ObjectId sourceCommit,
CherryPickInput input,
@Nullable Change.Id revertOf,
@Nullable Change.Id idForNewChange,
@Nullable String groupName)
throws IOException {
@Nullable Change.Id idForNewChange)
throws IOException, InvalidChangeOperationException {
Change.Id changeId = idForNewChange != null ? idForNewChange : Change.id(seq.nextChangeId());
ChangeInserter ins = changeInserterFactory.create(changeId, cherryPickCommit, refName);
ins.setRevertOf(revertOf);
@@ -506,8 +502,23 @@ public class CherryPickChange {
Set<Account.Id> ccs = new HashSet<>(reviewerSet.byState(ReviewerStateInternal.CC));
ccs.remove(user.get().getAccountId());
ins.setReviewersAndCcs(reviewers, ccs);
if (groupName != null) {
ins.setGroups(ImmutableList.of(groupName));
}
// If there is a base, and the base is not merged, the groups will be overridden by the base's
// groups.
ins.setGroups(GroupCollector.getDefaultGroups(cherryPickCommit.getId()));
if (input.base != null) {
List<ChangeData> changes =
queryProvider.get().setLimit(2).byBranchCommitOpen(project.get(), refName, input.base);
if (changes.size() > 1) {
throw new InvalidChangeOperationException(
"Several changes with key "
+ input.base
+ " reside on the same branch. "
+ "Cannot cherry-pick on target branch.");
}
if (changes.size() == 1) {
Change change = changes.get(0).change();
ins.setGroups(changeNotesFactory.createChecked(change).getCurrentPatchSet().groups());
}
}
bu.insertChange(ins);

17
java/com/google/gerrit/server/restapi/change/RevertSubmission.java
View File

@@ -286,7 +286,6 @@ public class RevertSubmission
throws IOException, RestApiException, UpdateException, ConfigInvalidException,
PermissionBackendException {
String groupName = null;
String initialMessage = revertInput.message;
while (sortedChangesInProjectAndBranch.hasNext()) {
ChangeNotes changeNotes = sortedChangesInProjectAndBranch.next().data().notes();
@@ -300,13 +299,8 @@ public class RevertSubmission
// 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.
craeteNormalRevert(revertInput, changeNotes, timestamp);
groupName = cherryPickInput.base;
} else {
// This is the code in case this is the second revert (or more) of this project + branch.
if (groupName == null) {
groupName = cherryPickInput.base;
}
createCherryPickedRevert(revertInput, project, groupName, changeNotes, timestamp);
createCherryPickedRevert(revertInput, project, changeNotes, timestamp);
}
}
}
@@ -314,7 +308,6 @@ public class RevertSubmission
private void createCherryPickedRevert(
RevertInput revertInput,
Project.NameKey project,
String groupName,
ChangeNotes changeNotes,
Timestamp timestamp)
throws IOException, ConfigInvalidException, UpdateException, RestApiException {
@@ -333,7 +326,7 @@ public class RevertSubmission
bu.addOp(
changeNotes.getChange().getId(),
new CreateCherryPickOp(
revCommitId, generatedChangeId, cherryPickRevertChangeId, groupName, timestamp));
revCommitId, generatedChangeId, cherryPickRevertChangeId, timestamp));
bu.addOp(changeNotes.getChange().getId(), new PostRevertedMessageOp(generatedChangeId));
bu.addOp(
cherryPickRevertChangeId,
@@ -547,19 +540,16 @@ public class RevertSubmission
private final ObjectId revCommitId;
private final ObjectId computedChangeId;
private final Change.Id cherryPickRevertChangeId;
private final String groupName;
private final Timestamp timestamp;
CreateCherryPickOp(
ObjectId revCommitId,
ObjectId computedChangeId,
Change.Id cherryPickRevertChangeId,
String groupName,
Timestamp timestamp) {
this.revCommitId = revCommitId;
this.computedChangeId = computedChangeId;
this.cherryPickRevertChangeId = cherryPickRevertChangeId;
this.groupName = groupName;
this.timestamp = timestamp;
}
@@ -577,8 +567,7 @@ public class RevertSubmission
timestamp,
change.getId(),
computedChangeId,
cherryPickRevertChangeId,
groupName);
cherryPickRevertChangeId);
// save the commit as base for next cherryPick of that branch
cherryPickInput.base =
changeNotesFactory