Rewrite RebaseChange as a BatchUpdate.Op

This operation does the actual rebase before delegating to a
PatchSetInserter. The actual PatchSetInserter object can't be created
until after we create the rebased commit, which happens in
RebaseChangeOp's updateRepo method. Thus the PatchSetInserter cannot
be added to the enclosing BatchUpdate by the caller, and instead we
hide that instance entirely in RebaseChange, passing through the
subset of setters that is needed.

We need to leave a non-BatchUpdate-related RebaseUtil class that
contains canRebase, which is still used in several places. Refactor
this slightly to avoid double-opening the repository in some cases.

Change-Id: I586a85fe9e6f878fe6a7ba1ab4d641b98b1649ab

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

@@ -15,11 +15,13 @@
 package com.google.gerrit.server.git.strategy;
 
 import com.google.common.collect.Lists;
+import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
-import com.google.gerrit.server.change.RebaseChange;
+import com.google.gerrit.server.change.RebaseChangeOp;
+import com.google.gerrit.server.git.BatchUpdate;
 import com.google.gerrit.server.git.CodeReviewCommit;
 import com.google.gerrit.server.git.CommitMergeStatus;
 import com.google.gerrit.server.git.MergeConflictException;
@@ -29,9 +31,7 @@ import com.google.gerrit.server.git.RebaseSorter;
 import com.google.gerrit.server.git.UpdateException;
 import com.google.gerrit.server.git.validators.CommitValidators;
 import com.google.gerrit.server.patch.PatchSetInfoFactory;
-import com.google.gerrit.server.project.InvalidChangeOperationException;
 import com.google.gerrit.server.project.NoSuchChangeException;
-import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gwtorm.server.OrmException;
 
 import org.eclipse.jgit.lib.ObjectId;
@@ -46,15 +46,15 @@ import java.util.Map;
 
 public class RebaseIfNecessary extends SubmitStrategy {
   private final PatchSetInfoFactory patchSetInfoFactory;
-  private final RebaseChange rebaseChange;
+  private final RebaseChangeOp.Factory rebaseFactory;
   private final Map<Change.Id, CodeReviewCommit> newCommits;
 
   RebaseIfNecessary(SubmitStrategy.Arguments args,
       PatchSetInfoFactory patchSetInfoFactory,
-      RebaseChange rebaseChange) {
+      RebaseChangeOp.Factory rebaseFactory) {
     super(args);
     this.patchSetInfoFactory = patchSetInfoFactory;
-    this.rebaseChange = rebaseChange;
+    this.rebaseFactory = rebaseFactory;
     this.newCommits = new HashMap<>();
   }
 
@@ -87,12 +87,7 @@ public class RebaseIfNecessary extends SubmitStrategy {
 
         } else {
           try {
-            PatchSet newPatchSet =
-                rebaseChange.rebase(args.repo, args.rw, args.inserter,
-                    n.change(), n.getPatchsetId(), args.caller,
-                    mergeTip.getCurrentTip(), args.mergeUtil,
-                    args.serverIdent.get(), false,
-                    CommitValidators.Policy.NONE);
+            PatchSet newPatchSet = rebase(n, mergeTip);
             List<PatchSetApproval> approvals = Lists.newArrayList();
             for (PatchSetApproval a : args.approvalsUtil.byPatchSet(args.db,
                 n.getControl(), n.getPatchsetId())) {
@@ -116,11 +111,13 @@ public class RebaseIfNecessary extends SubmitStrategy {
             newCommits.put(newPatchSet.getId().getParentKey(),
                 mergeTip.getCurrentTip());
             setRefLogIdent();
-          } catch (MergeConflictException e) {
-            n.setStatusCode(CommitMergeStatus.REBASE_MERGE_CONFLICT);
-          } catch (NoSuchChangeException | NoSuchProjectException
-              | OrmException | IOException | InvalidChangeOperationException
-              | UpdateException | RestApiException e) {
+          } catch (UpdateException e) {
+            if (e.getCause() instanceof MergeConflictException) {
+              n.setStatusCode(CommitMergeStatus.REBASE_MERGE_CONFLICT);
+            }
+            throw new MergeException("Cannot rebase " + n.name(), e);
+          } catch (NoSuchChangeException | OrmException | IOException
+              | RestApiException e) {
             // TODO(dborowitz): Allow Submit to unwrap ResourceConflictException
             // so it can turn into a 409.
             throw new MergeException("Cannot rebase " + n.name(), e);
@@ -170,6 +167,22 @@ public class RebaseIfNecessary extends SubmitStrategy {
     }
   }
 
+  private PatchSet rebase(CodeReviewCommit n, MergeTip mergeTip)
+      throws RestApiException, UpdateException, OrmException {
+    RebaseChangeOp op = rebaseFactory.create(
+          n.getControl(),
+          args.db.patchSets().get(n.getPatchsetId()),
+          mergeTip.getCurrentTip().name())
+        .setCommitterIdent(args.serverIdent.get())
+        .setRunHooks(false)
+        .setValidatePolicy(CommitValidators.Policy.NONE);
+    try (BatchUpdate bu = args.newBatchUpdate(TimeUtil.nowTs())) {
+      bu.addOp(n.change().getId(), op);
+      bu.execute();
+    }
+    return op.getPatchSet();
+  }
+
   @Override
   public Map<Change.Id, CodeReviewCommit> getNewCommits() {
     return newCommits;

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

@@ -20,7 +20,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ApprovalsUtil;
 import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.change.RebaseChange;
+import com.google.gerrit.server.change.RebaseChangeOp;
 import com.google.gerrit.server.git.BatchUpdate;
 import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
 import com.google.gerrit.server.git.MergeException;
@@ -56,7 +56,7 @@ public class SubmitStrategyFactory {
   private final BatchUpdate.Factory batchUpdateFactory;
   private final ChangeControl.GenericFactory changeControlFactory;
   private final PatchSetInfoFactory patchSetInfoFactory;
-  private final RebaseChange rebaseChange;
+  private final RebaseChangeOp.Factory rebaseFactory;
   private final ProjectCache projectCache;
   private final ApprovalsUtil approvalsUtil;
   private final MergeUtil.Factory mergeUtilFactory;
@@ -69,7 +69,7 @@ public class SubmitStrategyFactory {
       final BatchUpdate.Factory batchUpdateFactory,
       final ChangeControl.GenericFactory changeControlFactory,
       final PatchSetInfoFactory patchSetInfoFactory,
-      final RebaseChange rebaseChange,
+      final RebaseChangeOp.Factory rebaseFactory,
       final ProjectCache projectCache,
       final ApprovalsUtil approvalsUtil,
       final MergeUtil.Factory mergeUtilFactory,
@@ -79,7 +79,7 @@ public class SubmitStrategyFactory {
     this.batchUpdateFactory = batchUpdateFactory;
     this.changeControlFactory = changeControlFactory;
     this.patchSetInfoFactory = patchSetInfoFactory;
-    this.rebaseChange = rebaseChange;
+    this.rebaseFactory = rebaseFactory;
     this.projectCache = projectCache;
     this.approvalsUtil = approvalsUtil;
     this.mergeUtilFactory = mergeUtilFactory;
@@ -107,7 +107,7 @@ public class SubmitStrategyFactory {
       case MERGE_IF_NECESSARY:
         return new MergeIfNecessary(args);
       case REBASE_IF_NECESSARY:
-        return new RebaseIfNecessary(args, patchSetInfoFactory, rebaseChange);
+        return new RebaseIfNecessary(args, patchSetInfoFactory, rebaseFactory);
       default:
         final String errorMsg = "No submit strategy for: " + submitType;
         log.error(errorMsg);