RebaseIfNecessary: Don't copy approvals with ApprovalCopier

This wastes effort and introduces the constraint violation that we had
to work around and comment. Plus, there is an unfortunate restriction
in the gerrit-review gwtorm code currently where we can't modify the
same entity twice in one transaction.

Selectively bypass approval copying entirely in RebaseChangeOp, and do
it manually in RebaseIfNecessary.

Change-Id: Idd16eaef43916d0004c9622a395bd6be16fbfdd2

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

@@ -101,6 +101,7 @@ public class PatchSetInserter extends BatchUpdate.Op {
   private boolean sendMail = true;
   private Account.Id uploader;
   private boolean allowClosed;
+  private boolean copyApprovals = true;
 
   // Fields set during some phase of BatchUpdate.Op.
   private Change change;
@@ -179,6 +180,11 @@ public class PatchSetInserter extends BatchUpdate.Op {
     return this;
   }
 
+  public PatchSetInserter setCopyApprovals(boolean copyApprovals) {
+    this.copyApprovals = copyApprovals;
+    return this;
+  }
+
   public PatchSetInserter setUploader(Account.Id uploader) {
     this.uploader = uploader;
     return this;
@@ -247,7 +253,9 @@ public class PatchSetInserter extends BatchUpdate.Op {
     change.setCurrentPatchSet(patchSetInfo);
     ChangeUtil.updated(change);
     ctx.saveChange();
-    approvalCopier.copy(db, ctl, patchSet);
+    if (copyApprovals) {
+      approvalCopier.copy(db, ctl, patchSet);
+    }
     if (changeMessage != null) {
       cmUtil.addChangeMessage(db, update, changeMessage);
     }

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

@@ -62,6 +62,7 @@ public class RebaseChangeOp extends BatchUpdate.Op {
   private boolean runHooks = true;
   private CommitValidators.Policy validate;
   private boolean forceContentMerge;
+  private boolean copyApprovals = true;
 
   private RevCommit rebasedCommit;
   private PatchSet.Id rebasedPatchSetId;
@@ -102,6 +103,11 @@ public class RebaseChangeOp extends BatchUpdate.Op {
     return this;
   }
 
+  public RebaseChangeOp setCopyApprovals(boolean copyApprovals) {
+    this.copyApprovals = copyApprovals;
+    return this;
+  }
+
   @Override
   public void updateRepo(RepoContext ctx) throws MergeConflictException,
        InvalidChangeOperationException, RestApiException, IOException,
@@ -133,6 +139,7 @@ public class RebaseChangeOp extends BatchUpdate.Op {
         .setUploader(ctx.getUser().getAccountId())
         .setSendMail(false)
         .setRunHooks(runHooks)
+        .setCopyApprovals(copyApprovals)
         .setMessage(
           "Patch Set " + rebasedPatchSetId.get()
           + ": Patch Set " + originalPatchSet.getId().get() + " was rebased");

gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java

@@ -150,6 +150,9 @@ public class RebaseIfNecessary extends SubmitStrategy {
             args.db.patchSets().get(toMerge.getPatchsetId()),
             mergeTip.getCurrentTip().name())
           .setRunHooks(false)
+          // Bypass approval copier since we're going to copy all approvals
+          // later anyway.
+          .setCopyApprovals(false)
           .setValidatePolicy(CommitValidators.Policy.NONE);
       try {
         rebaseOp.updateRepo(ctx);
@@ -175,9 +178,7 @@ public class RebaseIfNecessary extends SubmitStrategy {
           toMerge.getControl(), toMerge.getPatchsetId())) {
         approvals.add(new PatchSetApproval(newPatchSet.getId(), a));
       }
-      // rebaseOp may already have copied some approvals; use upsert, not
+      args.db.patchSetApprovals().insert(approvals);
-      // insert, to avoid constraint violation on database.
-      args.db.patchSetApprovals().upsert(approvals);
 
       // TODO(dborowitz): Make RevWalk available via BatchUpdate.
       CodeReviewCommit newTip = args.rw.parseCommit(