Merge "Fix bug: cherry-picks were not "related"" into stable-3.2
diff --git a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
index 0ba4905..44dc6e1 100644
--- a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java
@@ -19,7 +19,6 @@
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.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 @@
TimeUtil.nowTs(),
null,
null,
- null,
null);
}
@@ -205,7 +204,7 @@
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 @@
* @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 @@
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 @@
cherryPickCommit,
dest.branch(),
newTopic,
+ project,
sourceChange,
sourceCommit,
input,
revertedChange,
- idForNewChange,
- groupName);
+ idForNewChange);
}
bu.execute();
return Result.create(changeId, cherryPickCommit.getFilesWithGitConflicts());
@@ -473,13 +469,13 @@
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 @@
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);
diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
index 7a04f05..846b80c 100644
--- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
+++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
@@ -286,7 +286,6 @@
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 @@
// 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 @@
private void createCherryPickedRevert(
RevertInput revertInput,
Project.NameKey project,
- String groupName,
ChangeNotes changeNotes,
Timestamp timestamp)
throws IOException, ConfigInvalidException, UpdateException, RestApiException {
@@ -333,7 +326,7 @@
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 @@
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 @@
timestamp,
change.getId(),
computedChangeId,
- cherryPickRevertChangeId,
- groupName);
+ cherryPickRevertChangeId);
// save the commit as base for next cherryPick of that branch
cherryPickInput.base =
changeNotesFactory
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index 17391de..74f9134 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -1097,6 +1097,57 @@
}
@Test
+ public void getRelatedCherryPicks() throws Exception {
+ PushOneCommit.Result r1 = createChange(SUBJECT, "a.txt", "a");
+ PushOneCommit.Result r2 = createChange(SUBJECT, "b.txt", "b");
+
+ String branch = "foo";
+ // Create target branch to cherry-pick to.
+ gApi.projects().name(project.get()).branch(branch).create(new BranchInput());
+
+ CherryPickInput input = new CherryPickInput();
+ input.message = "message";
+ input.destination = branch;
+ ChangeInfo firstCherryPickResult =
+ gApi.changes().id(r1.getChangeId()).current().cherryPickAsInfo(input);
+
+ input.base = gApi.changes().id(firstCherryPickResult.changeId).current().commit(false).commit;
+ ChangeInfo secondCherryPickResult =
+ gApi.changes().id(r2.getChangeId()).current().cherryPickAsInfo(input);
+ assertThat(gApi.changes().id(firstCherryPickResult.changeId).current().related().changes)
+ .hasSize(2);
+ assertThat(gApi.changes().id(secondCherryPickResult.changeId).current().related().changes)
+ .hasSize(2);
+ }
+
+ @Test
+ public void cherryPickOnMergedChangeIsNotRelated() throws Exception {
+ PushOneCommit.Result r1 = createChange(SUBJECT, "a.txt", "a");
+ PushOneCommit.Result r2 = createChange(SUBJECT, "b.txt", "b");
+
+ String branch = "foo";
+ // Create target branch to cherry-pick to.
+ gApi.projects().name(project.get()).branch(branch).create(new BranchInput());
+
+ CherryPickInput input = new CherryPickInput();
+ input.message = "message";
+ input.destination = branch;
+ ChangeInfo firstCherryPickResult =
+ gApi.changes().id(r1.getChangeId()).current().cherryPickAsInfo(input);
+
+ gApi.changes().id(firstCherryPickResult.id).current().review(ReviewInput.approve());
+ gApi.changes().id(firstCherryPickResult.id).current().submit();
+
+ input.base = gApi.changes().id(firstCherryPickResult.changeId).current().commit(false).commit;
+ ChangeInfo secondCherryPickResult =
+ gApi.changes().id(r2.getChangeId()).current().cherryPickAsInfo(input);
+ assertThat(gApi.changes().id(firstCherryPickResult.changeId).current().related().changes)
+ .hasSize(0);
+ assertThat(gApi.changes().id(secondCherryPickResult.changeId).current().related().changes)
+ .hasSize(0);
+ }
+
+ @Test
public void canRebase() throws Exception {
PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
PushOneCommit.Result r1 = push.to("refs/for/master");
