Let cherrypick API optionally preserve reviewers/ccs

I don't plan to change the default, or the action performed
by the cherry-pick button in the UI, but adding this option
to the API will allow API customers (like Chromium's "Reland"
button) to carry forward some metadata from the original
change to the newly-created one.

Change-Id: I1d149d2758b825135460564dbf8038f4d7d52f0f

Documentation/rest-api-changes.txt

@@ -5808,6 +5808,8 @@ If not set, the default is `NONE`.
 |`notify_details`   |optional|
 Additional information about whom to notify about the update as a map
 of recipient type to link:#notify-info[NotifyInfo] entity.
+|`keep_reviewers`   |optional, defaults to false|
+If true, carries reviewers and ccs over from original change to newly created one.
 |===========================
 
 [[comment-info]]

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java

@@ -22,11 +22,11 @@ import static com.google.gerrit.acceptance.PushOneCommit.PATCH;
 import static com.google.gerrit.acceptance.PushOneCommit.PATCH_FILE_ONLY;
 import static com.google.gerrit.acceptance.PushOneCommit.SUBJECT;
 import static com.google.gerrit.extensions.client.ListChangesOption.DETAILED_LABELS;
-import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
 import static com.google.gerrit.reviewdb.client.Patch.COMMIT_MSG;
 import static com.google.gerrit.reviewdb.client.Patch.MERGE_LIST;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.stream.Collectors.toList;
 import static org.eclipse.jgit.lib.Constants.HEAD;
 import static org.junit.Assert.fail;
 
@@ -54,6 +54,7 @@ import com.google.gerrit.extensions.api.changes.RevisionApi;
 import com.google.gerrit.extensions.api.projects.BranchInput;
 import com.google.gerrit.extensions.client.ChangeStatus;
 import com.google.gerrit.extensions.client.ListChangesOption;
+import com.google.gerrit.extensions.client.ReviewerState;
 import com.google.gerrit.extensions.client.SubmitType;
 import com.google.gerrit.extensions.common.AccountInfo;
 import com.google.gerrit.extensions.common.ApprovalInfo;
@@ -673,6 +674,50 @@ public class RevisionIT extends AbstractDaemonTest {
     assertNotifyTo(userToNotify);
   }
 
+  @Test
+  public void cherryPickKeepReviewers() throws Exception {
+    createBranch(new Branch.NameKey(project, "stable"));
+
+    // Change is created by 'admin'.
+    PushOneCommit.Result r = createChange();
+    // Change is approved by 'admin2'. Change is CC'd to 'user'.
+    setApiUser(accountCreator.admin2());
+    ReviewInput in = ReviewInput.approve();
+    in.reviewer(user.email, ReviewerState.CC, true);
+    gApi.changes().id(r.getChangeId()).current().review(in);
+
+    // Change is cherrypicked by 'user2'.
+    setApiUser(accountCreator.user2());
+    CherryPickInput cin = new CherryPickInput();
+    cin.message = "this need to go to stable";
+    cin.destination = "stable";
+    cin.keepReviewers = true;
+    Map<ReviewerState, Collection<AccountInfo>> result =
+        gApi.changes()
+            .id(r.getChangeId())
+            .current()
+            .cherryPick(cin)
+            .get()
+            .reviewers;
+
+    // 'admin' should be a reviewer as the old owner.
+    // 'admin2' should be a reviewer as the old reviewer.
+    // 'user' should be on CC.
+    assertThat(result).containsKey(ReviewerState.REVIEWER);
+    List<Integer> reviewers =
+        result.get(ReviewerState.REVIEWER).stream().map(a -> a._accountId).collect(toList());
+    if (notesMigration.readChanges()) {
+      assertThat(result).containsKey(ReviewerState.CC);
+      List<Integer> ccs =
+          result.get(ReviewerState.CC).stream().map(a -> a._accountId).collect(toList());
+      assertThat(ccs).containsExactly(user.id.get());
+      assertThat(reviewers).containsExactly(admin.id.get(), accountCreator.admin2().id.get());
+    } else {
+      assertThat(reviewers)
+          .containsExactly(user.id.get(), admin.id.get(), accountCreator.admin2().id.get());
+    }
+  }
+
   @Test
   public void cherryPickToMergedChangeRevision() throws Exception {
     createBranch(new Branch.NameKey(project, "foo"));
@@ -1246,7 +1291,7 @@ public class RevisionIT extends AbstractDaemonTest {
     ChangeMessageInfo message = Iterables.getLast(c.messages);
     assertThat(message.author._accountId).isEqualTo(admin.getId().get());
     assertThat(message.message).isEqualTo("Removed Code-Review+1 by User <user@example.com>\n");
-    assertThat(getReviewers(c.reviewers.get(REVIEWER)))
+    assertThat(getReviewers(c.reviewers.get(ReviewerState.REVIEWER)))
         .containsExactlyElementsIn(ImmutableSet.of(admin.getId(), user.getId()));
   }
 

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/CherryPickInput.java

@@ -26,4 +26,6 @@ public class CherryPickInput {
 
   public NotifyHandling notify = NotifyHandling.NONE;
   public Map<RecipientType, NotifyInfo> notifyDetails;
+
+  public boolean keepReviewers;
 }

gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java

@@ -24,6 +24,7 @@ import com.google.gerrit.extensions.restapi.MergeConflictException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
+import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Change.Status;
@@ -32,11 +33,13 @@ import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.ApprovalsUtil;
 import com.google.gerrit.server.ChangeMessagesUtil;
 import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.ReviewerSet;
 import com.google.gerrit.server.Sequences;
 import com.google.gerrit.server.git.CodeReviewCommit;
 import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
@@ -44,6 +47,8 @@ import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.IntegrationException;
 import com.google.gerrit.server.git.MergeIdenticalTreeException;
 import com.google.gerrit.server.git.MergeUtil;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.ReviewerStateInternal;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.InvalidChangeOperationException;
 import com.google.gerrit.server.project.ProjectState;
@@ -60,7 +65,9 @@ import com.google.inject.Provider;
 import com.google.inject.Singleton;
 import java.io.IOException;
 import java.sql.Timestamp;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.TimeZone;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.errors.InvalidObjectIdException;
@@ -86,6 +93,8 @@ public class CherryPickChange {
   private final ChangeInserter.Factory changeInserterFactory;
   private final PatchSetInserter.Factory patchSetInserterFactory;
   private final MergeUtil.Factory mergeUtilFactory;
+  private final ChangeNotes.Factory changeNotesFactory;
+  private final ApprovalsUtil approvalsUtil;
   private final ChangeMessagesUtil changeMessagesUtil;
   private final PatchSetUtil psUtil;
   private final NotifyUtil notifyUtil;
@@ -101,6 +110,8 @@ public class CherryPickChange {
       ChangeInserter.Factory changeInserterFactory,
       PatchSetInserter.Factory patchSetInserterFactory,
       MergeUtil.Factory mergeUtilFactory,
+      ChangeNotes.Factory changeNotesFactory,
+      ApprovalsUtil approvalsUtil,
       ChangeMessagesUtil changeMessagesUtil,
       PatchSetUtil psUtil,
       NotifyUtil notifyUtil) {
@@ -113,6 +124,8 @@ public class CherryPickChange {
     this.changeInserterFactory = changeInserterFactory;
     this.patchSetInserterFactory = patchSetInserterFactory;
     this.mergeUtilFactory = mergeUtilFactory;
+    this.changeNotesFactory = changeNotesFactory;
+    this.approvalsUtil = approvalsUtil;
     this.changeMessagesUtil = changeMessagesUtil;
     this.psUtil = psUtil;
     this.notifyUtil = notifyUtil;
@@ -128,10 +141,8 @@ public class CherryPickChange {
           UpdateException, RestApiException, ConfigInvalidException {
     return cherryPick(
         batchUpdateFactory,
-        change.getId(),
+        change,
         patch.getId(),
-        change.getDest(),
-        change.getTopic(),
         change.getProject(),
         ObjectId.fromString(patch.getRevision().get()),
         input,
@@ -140,10 +151,8 @@ public class CherryPickChange {
 
   public Change.Id cherryPick(
       BatchUpdate.Factory batchUpdateFactory,
-      @Nullable Change.Id sourceChangeId,
+      @Nullable Change sourceChange,
       @Nullable PatchSet.Id sourcePatchId,
-      @Nullable Branch.NameKey sourceBranch,
-      @Nullable String sourceChangeTopic,
       Project.NameKey project,
       ObjectId sourceCommit,
       CherryPickInput input,
@@ -240,22 +249,16 @@ public class CherryPickChange {
             // Change key not found on destination branch. We can create a new
             // change.
             String newTopic = null;
-            if (!Strings.isNullOrEmpty(sourceChangeTopic)) {
+            if (sourceChange != null && !Strings.isNullOrEmpty(sourceChange.getTopic())) {
-              newTopic = sourceChangeTopic + "-" + newDest.getShortName();
+              newTopic = sourceChange.getTopic() + "-" + newDest.getShortName();
             }
             result =
                 createNewChange(
-                    bu,
+                    bu, cherryPickCommit, destRefName, newTopic, sourceChange, sourceCommit, input);
-                    cherryPickCommit,
-                    destRefControl.getRefName(),
-                    newTopic,
-                    sourceBranch,
-                    sourceCommit,
-                    input);
 
-            if (sourceChangeId != null && sourcePatchId != null) {
+            if (sourceChange != null && sourcePatchId != null) {
               bu.addOp(
-                  sourceChangeId,
+                  sourceChange.getId(),
                   new AddMessageToSourceChangeOp(
                       changeMessagesUtil,
                       sourcePatchId,
@@ -341,16 +344,29 @@ public class CherryPickChange {
       CodeReviewCommit cherryPickCommit,
       String refName,
       String topic,
-      Branch.NameKey sourceBranch,
+      @Nullable Change sourceChange,
       ObjectId sourceCommit,
       CherryPickInput input)
       throws OrmException, IOException, BadRequestException, ConfigInvalidException {
     Change.Id changeId = new Change.Id(seq.nextChangeId());
-    ChangeInserter ins =
+    ChangeInserter ins = changeInserterFactory.create(changeId, cherryPickCommit, refName);
-        changeInserterFactory.create(changeId, cherryPickCommit, refName).setTopic(topic);
+    Branch.NameKey sourceBranch = sourceChange == null ? null : sourceChange.getDest();
     ins.setMessage(messageForDestinationChange(ins.getPatchSetId(), sourceBranch, sourceCommit))
+        .setTopic(topic)
         .setNotify(input.notify)
         .setAccountsToNotify(notifyUtil.resolveAccounts(input.notifyDetails));
+    if (input.keepReviewers && sourceChange != null) {
+      ReviewerSet reviewerSet =
+          approvalsUtil.getReviewers(
+              dbProvider.get(), changeNotesFactory.createChecked(dbProvider.get(), sourceChange));
+      Set<Account.Id> reviewers =
+          new HashSet<>(reviewerSet.byState(ReviewerStateInternal.REVIEWER));
+      reviewers.add(sourceChange.getOwner());
+      reviewers.remove(user.get().getAccountId());
+      Set<Account.Id> ccs = new HashSet<>(reviewerSet.byState(ReviewerStateInternal.CC));
+      ccs.remove(user.get().getAccountId());
+      ins.setReviewers(reviewers).setExtraCC(ccs);
+    }
     bu.insertChange(ins);
     return changeId;
   }

gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickCommit.java

@@ -94,8 +94,6 @@ public class CherryPickCommit
               updateFactory,
               null,
               null,
-              null,
-              null,
               projectName,
               commit,
               input,