Merge changes from topic 'rebase-cleanup'

* changes:
  RebaseChange: Minor cleanup
  Rebase: Implement hasOneParent using commit parents
  Rebase: Use Ints.tryParse instead of catching exception
  Rebase: Minor cleanup
  Rebase: Extract a method to find base revision
  Rebase: Check commit ancestry using git

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

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.change;
 
+import com.google.common.primitives.Ints;
 import com.google.gerrit.common.errors.EmailException;
 import com.google.gerrit.extensions.api.changes.RebaseInput;
 import com.google.gerrit.extensions.client.ListChangesOption;
@@ -26,9 +27,10 @@ import com.google.gerrit.extensions.webui.UiAction;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Change.Status;
 import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.PatchSetAncestor;
+import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.InvalidChangeOperationException;
 import com.google.gerrit.server.project.NoSuchChangeException;
@@ -37,26 +39,32 @@ import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
 
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
-import java.util.ArrayList;
 
 @Singleton
 public class Rebase implements RestModifyView<RevisionResource, RebaseInput>,
     UiAction<RevisionResource> {
 
-  private static final Logger log =
+  private static final Logger log = LoggerFactory.getLogger(Rebase.class);
-      LoggerFactory.getLogger(Rebase.class);
 
+  private final GitRepositoryManager repoManager;
   private final Provider<RebaseChange> rebaseChange;
   private final ChangeJson json;
   private final Provider<ReviewDb> dbProvider;
 
   @Inject
-  public Rebase(Provider<RebaseChange> rebaseChange, ChangeJson json,
+  public Rebase(GitRepositoryManager repoManager,
+      Provider<RebaseChange> rebaseChange,
+      ChangeJson json,
       Provider<ReviewDb> dbProvider) {
+    this.repoManager = repoManager;
     this.rebaseChange = rebaseChange;
     this.json = json
         .addOption(ListChangesOption.CURRENT_REVISION)
@@ -70,24 +78,40 @@ public class Rebase implements RestModifyView<RevisionResource, RebaseInput>,
       ResourceConflictException, EmailException, OrmException, IOException {
     ChangeControl control = rsrc.getControl();
     Change change = rsrc.getChange();
+    try (Repository repo = repoManager.openRepository(change.getProject());
+        RevWalk rw = new RevWalk(repo)) {
       if (!control.canRebase()) {
         throw new AuthException("rebase not permitted");
       } else if (!change.getStatus().isOpen()) {
         throw new ResourceConflictException("change is "
             + change.getStatus().name().toLowerCase());
-    } else if (!hasOneParent(rsrc.getPatchSet().getId())) {
+      } else if (!hasOneParent(rw, rsrc.getPatchSet())) {
         throw new ResourceConflictException(
             "cannot rebase merge commits or commit with no ancestor");
       }
+      rebaseChange.get().rebase(repo, rw, change, rsrc.getPatchSet().getId(),
+          rsrc.getUser(), findBaseRev(rw, rsrc, input));
+    } catch (InvalidChangeOperationException e) {
+      throw new ResourceConflictException(e.getMessage());
+    } catch (NoSuchChangeException e) {
+      throw new ResourceNotFoundException(change.getId().toString());
+    }
 
-    String baseRev = null;
+    return json.format(change.getId());
-    if (input != null && input.base != null) {
+  }
+
+  private String findBaseRev(RevWalk rw, RevisionResource rsrc,
+      RebaseInput input) throws AuthException, ResourceConflictException,
+      OrmException, IOException {
+    if (input == null || input.base == null) {
+      return null;
+    }
+
+    Change change = rsrc.getChange();
     String base = input.base.trim();
-      do {
     if (base.equals("")) {
       // remove existing dependency to other patch set
-          baseRev = change.getDest().get();
+      return change.getDest().get();
-          break;
     }
 
     ReviewDb db = dbProvider.get();
@@ -101,122 +125,93 @@ public class Rebase implements RestModifyView<RevisionResource, RebaseInput>,
     }
 
     Change baseChange = db.changes().get(basePatchSet.getId().getParentKey());
-        if (baseChange != null) {
+    if (baseChange == null) {
+      return null;
+    }
     if (!baseChange.getProject().equals(change.getProject())) {
-            throw new ResourceConflictException("base change is in wrong project: "
+      throw new ResourceConflictException(
-                                                + baseChange.getProject());
+          "base change is in wrong project: " + baseChange.getProject());
     } else if (!baseChange.getDest().equals(change.getDest())) {
-            throw new ResourceConflictException("base change is targetting wrong branch: "
+      throw new ResourceConflictException(
-                                                + baseChange.getDest());
+          "base change is targeting wrong branch: " + baseChange.getDest());
     } else if (baseChange.getStatus() == Status.ABANDONED) {
-            throw new ResourceConflictException("base change is abandoned: "
+      throw new ResourceConflictException(
-                                                + baseChange.getKey());
+          "base change is abandoned: " + baseChange.getKey());
-          } else if (isDescendantOf(baseChange.getId(), rsrc.getPatchSet().getRevision())) {
+    } else if (isMergedInto(rw, rsrc.getPatchSet(), basePatchSet)) {
-            throw new ResourceConflictException("base change " + baseChange.getKey()
+      throw new ResourceConflictException(
-                                                + " is a descendant of the current "
+          "base change " + baseChange.getKey()
-                                                + " change - recursion not allowed");
+          + " is a descendant of the current  change - recursion not allowed");
     }
-          baseRev = basePatchSet.getRevision().get();
+    return basePatchSet.getRevision().get();
-          break;
-        }
-      } while (false);  // just wanted to use the break statement
   }
 
-    try {
+  private boolean isMergedInto(RevWalk rw, PatchSet base, PatchSet tip)
-      rebaseChange.get().rebase(change, rsrc.getPatchSet().getId(),
+      throws IOException {
-          rsrc.getUser(), baseRev);
+    ObjectId baseId = ObjectId.fromString(base.getRevision().get());
-    } catch (InvalidChangeOperationException e) {
+    ObjectId tipId = ObjectId.fromString(tip.getRevision().get());
-      throw new ResourceConflictException(e.getMessage());
+    return rw.isMergedInto(rw.parseCommit(baseId), rw.parseCommit(tipId));
-    } catch (NoSuchChangeException e) {
-      throw new ResourceNotFoundException(change.getId().toString());
   }
 
-    return json.format(change.getId());
+  private PatchSet parseBase(String base) throws OrmException {
-  }
-
-  private boolean isDescendantOf(Change.Id child, RevId ancestor)
-      throws OrmException {
-    ReviewDb db = dbProvider.get();
-
-    ArrayList<RevId> parents = new ArrayList<>();
-    parents.add(ancestor);
-    while (!parents.isEmpty()) {
-      RevId parent = parents.remove(0);
-      // get direct descendants of change
-      for (PatchSetAncestor desc : db.patchSetAncestors().descendantsOf(parent)) {
-        PatchSet descPatchSet = db.patchSets().get(desc.getPatchSet());
-        Change.Id descChangeId = descPatchSet.getId().getParentKey();
-        if (child.equals(descChangeId)) {
-          PatchSet.Id descCurrentPatchSetId =
-              db.changes().get(descChangeId).currentPatchSetId();
-          // it's only bad if the descendant patch set is current
-          return descPatchSet.getId().equals(descCurrentPatchSetId);
-        } else {
-          // process indirect descendants as well
-          parents.add(descPatchSet.getRevision());
-        }
-      }
-    }
-
-    return false;
-  }
-
-  private PatchSet parseBase(final String base) throws OrmException {
     ReviewDb db = dbProvider.get();
 
     PatchSet.Id basePatchSetId = PatchSet.Id.fromRef(base);
     if (basePatchSetId != null) {
-      // try parsing the base as a ref string
+      // Try parsing the base as a ref string.
       return db.patchSets().get(basePatchSetId);
     }
 
-    // try parsing base as a change number (assume current patch set)
+    // Try parsing base as a change number (assume current patch set).
     PatchSet basePatchSet = null;
-    try {
+    Integer baseChangeId = Ints.tryParse(base);
-      Change.Id baseChangeId = Change.Id.parse(base);
     if (baseChangeId != null) {
-        for (PatchSet ps : db.patchSets().byChange(baseChangeId)) {
+      for (PatchSet ps : db.patchSets().byChange(new Change.Id(baseChangeId))) {
-          if (basePatchSet == null || basePatchSet.getId().get() < ps.getId().get()){
+        if (basePatchSet == null
+            || basePatchSet.getId().get() < ps.getId().get()) {
           basePatchSet = ps;
         }
       }
+      if (basePatchSet != null) {
+        return basePatchSet;
       }
-    } catch (NumberFormatException e) {  // probably a SHA1
     }
 
-    // try parsing as SHA1
+    // Try parsing as SHA-1.
-    if (basePatchSet == null) {
     for (PatchSet ps : db.patchSets().byRevision(new RevId(base))) {
-        if (basePatchSet == null || basePatchSet.getId().get() < ps.getId().get()) {
+      if (basePatchSet == null
+          || basePatchSet.getId().get() < ps.getId().get()) {
         basePatchSet = ps;
       }
     }
-    }
-
     return basePatchSet;
   }
 
-  private boolean hasOneParent(final PatchSet.Id patchSetId) {
+  private boolean hasOneParent(RevWalk rw, PatchSet ps) throws IOException {
-    try {
+    // Prevent rebase of exotic changes (merge commit, no ancestor).
-      // prevent rebase of exotic changes (merge commit, no ancestor).
+    RevCommit c = rw.parseCommit(ObjectId.fromString(ps.getRevision().get()));
-      return (dbProvider.get().patchSetAncestors()
+    return c.getParentCount() == 1;
-          .ancestorsOf(patchSetId).toList().size() == 1);
-    } catch (OrmException e) {
-      log.error("Failed to get ancestors of patch set "
-          + patchSetId.toRefName(), e);
-      return false;
-    }
   }
 
   @Override
   public UiAction.Description getDescription(RevisionResource resource) {
+    Project.NameKey project = resource.getChange().getProject();
+    boolean visible = resource.getChange().getStatus().isOpen()
+          && resource.isCurrent()
+          && resource.getControl().canRebase();
+    if (visible) {
+      try (Repository repo = repoManager.openRepository(project);
+          RevWalk rw = new RevWalk(repo)) {
+        visible = hasOneParent(rw, resource.getPatchSet());
+      } catch (IOException e) {
+        log.error("Failed to get ancestors of patch set "
+            + resource.getPatchSet().getId(), e);
+        visible = false;
+      }
+    }
     UiAction.Description descr = new UiAction.Description()
       .setLabel("Rebase")
       .setTitle("Rebase onto tip of branch or parent change")
-      .setVisible(resource.getChange().getStatus().isOpen()
+      .setVisible(visible);
-          && resource.isCurrent()
-          && resource.getControl().canRebase()
-          && hasOneParent(resource.getPatchSet().getId()));
     if (descr.isVisible()) {
       // Disable the rebase button in the RebaseDialog if
       // the change cannot be rebased.

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

@@ -67,12 +67,12 @@ public class RebaseChange {
   private final PatchSetInserter.Factory patchSetInserterFactory;
 
   @Inject
-  RebaseChange(final ChangeControl.GenericFactory changeControlFactory,
+  RebaseChange(ChangeControl.GenericFactory changeControlFactory,
-      final Provider<ReviewDb> db,
+      Provider<ReviewDb> db,
-      @GerritPersonIdent final PersonIdent myIdent,
+      @GerritPersonIdent PersonIdent myIdent,
-      final GitRepositoryManager gitManager,
+      GitRepositoryManager gitManager,
-      final MergeUtil.Factory mergeUtilFactory,
+      MergeUtil.Factory mergeUtilFactory,
-      final PatchSetInserter.Factory patchSetInserterFactory) {
+      PatchSetInserter.Factory patchSetInserterFactory) {
     this.changeControlFactory = changeControlFactory;
     this.db = db;
     this.gitManager = gitManager;
@@ -82,64 +82,64 @@ public class RebaseChange {
   }
 
   /**
-   * Rebases the change of the given patch set.
+   * Rebase the change of the given patch set.
-   *
+   * <p>
    * It is verified that the current user is allowed to do the rebase.
-   *
+   * <p>
    * If the patch set has no dependency to an open change, then the change is
    * rebased on the tip of the destination branch.
-   *
+   * <p>
    * If the patch set depends on an open change, it is rebased on the latest
    * patch set of this change.
-   *
+   * <p>
    * The rebased commit is added as new patch set to the change.
-   *
+   * <p>
    * E-mail notification and triggering of hooks happens for the creation of the
    * new patch set.
    *
-   * @param change the change to perform the rebase for
+   * @param git the repository.
-   * @param patchSetId the id of the patch set
+   * @param rw the RevWalk.
-   * @param uploader the user that creates the rebased patch set
+   * @param change the change to rebase.
-   * @param newBaseRev the commit that should be the new base
+   * @param patchSetId the patch set ID to rebase.
-   * @throws NoSuchChangeException thrown if the change to which the patch set
+   * @param uploader the user that creates the rebased patch set.
-   *         belongs does not exist or is not visible to the user
+   * @param newBaseRev the commit that should be the new base.
-   * @throws EmailException thrown if sending the e-mail to notify about the new
+   * @throws NoSuchChangeException if the change to which the patch set belongs
-   *         patch set fails
+   *     does not exist or is not visible to the user.
-   * @throws OrmException thrown in case accessing the database fails
+   * @throws EmailException if sending the e-mail to notify about the new patch
-   * @throws IOException thrown if rebase is not possible or not needed
+   *     set fails.
-   * @throws InvalidChangeOperationException thrown if rebase is not allowed
+   * @throws OrmException if accessing the database fails.
+   * @throws IOException if accessing the repository fails.
+   * @throws InvalidChangeOperationException if rebase is not possible or not
+   *     allowed.
    */
-  public void rebase(Change change, PatchSet.Id patchSetId, final IdentifiedUser uploader,
+  public void rebase(Repository git, RevWalk rw, Change change,
-      final String newBaseRev) throws NoSuchChangeException, EmailException, OrmException,
+      PatchSet.Id patchSetId, IdentifiedUser uploader, String newBaseRev)
-      IOException, InvalidChangeOperationException {
+      throws NoSuchChangeException, EmailException, OrmException, IOException,
-    final Change.Id changeId = patchSetId.getParentKey();
+      InvalidChangeOperationException {
-    final ChangeControl changeControl =
+    Change.Id changeId = patchSetId.getParentKey();
+    ChangeControl changeControl =
         changeControlFactory.validateFor(change, uploader);
     if (!changeControl.canRebase()) {
-      throw new InvalidChangeOperationException(
+      throw new InvalidChangeOperationException("Cannot rebase: New patch sets"
-          "Cannot rebase: New patch sets are not allowed to be added to change: "
+          + " are not allowed to be added to change: " + changeId);
-              + changeId.toString());
     }
-    try (Repository git = gitManager.openRepository(change.getProject());
+    try (ObjectInserter inserter = git.newObjectInserter()) {
-        RevWalk rw = new RevWalk(git);
-        ObjectInserter inserter = git.newObjectInserter()) {
       String baseRev = newBaseRev;
       if (baseRev == null) {
-        baseRev =
+        baseRev = findBaseRevision(
-            findBaseRevision(patchSetId, db.get(), change.getDest(), git, rw);
+            patchSetId, db.get(), change.getDest(), git, rw);
       }
       ObjectId baseObjectId = git.resolve(baseRev);
       if (baseObjectId == null) {
         throw new InvalidChangeOperationException(
           "Cannot rebase: Failed to resolve baseRev: " + baseRev);
       }
-      final RevCommit baseCommit = rw.parseCommit(baseObjectId);
+      RevCommit baseCommit = rw.parseCommit(baseObjectId);
 
       PersonIdent committerIdent =
-          uploader.newCommitterIdent(TimeUtil.nowTs(),
+          uploader.newCommitterIdent(TimeUtil.nowTs(), serverTimeZone);
-              serverTimeZone);
 
-      rebase(git, rw, inserter, patchSetId, change,
+      rebase(git, rw, inserter, change, patchSetId,
           uploader, baseCommit, mergeUtilFactory.create(
               changeControl.getProjectControl().getProjectState(), true),
           committerIdent, true, ValidatePolicy.GERRIT);
@@ -149,24 +149,27 @@ public class RebaseChange {
   }
 
   /**
-     * Finds the revision of commit on which the given patch set should be based.
+   * Find the commit onto which a patch set should be rebased.
+   * <p>
+   * This is defined as the latest patch set of the change corresponding to
+   * this commit's parent, or the destination branch tip in the case where the
+   * parent's change is merged.
    *
-     * @param patchSetId the id of the patch set for which the new base commit
+   * @param patchSetId patch set ID for which the new base commit should be
-     *        should be found
+   *     found.
-     * @param db the ReviewDb
+   * @param db the ReviewDb.
-     * @param destBranch the destination branch
+   * @param destBranch the destination branch.
-     * @param git the repository
+   * @param git the repository.
-     * @param rw the RevWalk
+   * @param rw the RevWalk.
-     * @return the revision of commit on which the given patch set should be based
+   * @return the commit onto which the patch set should be rebased.
    * @throws InvalidChangeOperationException if rebase is not possible or not
-     *     allowed
+   *     allowed.
-     * @throws IOException thrown if accessing the repository fails
+   * @throws IOException if accessing the repository fails.
-     * @throws OrmException thrown if accessing the database fails
+   * @throws OrmException if accessing the database fails.
    */
   private static String findBaseRevision(PatchSet.Id patchSetId,
       ReviewDb db, Branch.NameKey destBranch, Repository git, RevWalk rw)
       throws InvalidChangeOperationException, IOException, OrmException {
-
     String baseRev = null;
 
     PatchSet patchSet = db.patchSets().get(patchSetId);
@@ -189,7 +192,6 @@ public class RebaseChange {
     RevId parentRev = new RevId(commit.getParent(0).name());
 
     for (PatchSet depPatchSet : db.patchSets().byRevision(parentRev)) {
-
       Change.Id depChangeId = depPatchSet.getId().getParentKey();
       Change depChange = db.changes().get(depChangeId);
       if (!depChange.getDest().equals(destBranch)) {
@@ -197,14 +199,16 @@ public class RebaseChange {
       }
 
       if (depChange.getStatus() == Status.ABANDONED) {
-          throw new InvalidChangeOperationException("Cannot rebase a change with an abandoned parent: "
+        throw new InvalidChangeOperationException(
-              + depChange.getKey().toString());
+            "Cannot rebase a change with an abandoned parent: "
+            + depChange.getKey());
       }
 
       if (depChange.getStatus().isOpen()) {
         if (depPatchSet.getId().equals(depChange.currentPatchSetId())) {
           throw new InvalidChangeOperationException(
-                "Change is already based on the latest patch set of the dependent change.");
+              "Change is already based on the latest patch set of the"
+              + " dependent change.");
         }
         PatchSet latestDepPatchSet =
             db.patchSets().get(depChange.currentPatchSetId());
@@ -223,71 +227,72 @@ public class RebaseChange {
       }
       baseRev = destRef.getObjectId().getName();
       if (baseRev.equals(parentRev.get())) {
-          throw new InvalidChangeOperationException("Change is already up to date.");
+        throw new InvalidChangeOperationException(
+            "Change is already up to date.");
       }
     }
     return baseRev;
   }
 
   /**
-   * Rebases the change of the given patch set on the given base commit.
+   * Rebase the change of the given patch set on the given base commit.
-   *
+   * <p>
    * The rebased commit is added as new patch set to the change.
+   * <p>
+   * E-mail notification and triggering of hooks is only done for the creation
+   * of the new patch set if {@code sendEmail} and {@code runHooks} are true,
+   * respectively.
    *
-   * E-mail notification and triggering of hooks is only done for the creation of
+   * @param git the repository.
-   * the new patch set if `sendEmail` and `runHooks` are set to true.
+   * @param inserter the object inserter.
-   *
+   * @param change the change to rebase.
-   * @param git the repository
+   * @param patchSetId the patch set ID to rebase.
-   * @param revWalk the RevWalk
+   * @param uploader the user that creates the rebased patch set.
-   * @param inserter the object inserter
+   * @param baseCommit the commit that should be the new base.
-   * @param patchSetId the id of the patch set
+   * @param mergeUtil merge utilities for the destination project.
-   * @param change the change that should be rebased
+   * @param committerIdent the committer's identity.
-   * @param uploader the user that creates the rebased patch set
+   * @param runHooks if hooks should be run for the new patch set.
-   * @param baseCommit the commit that should be the new base
+   * @param validate if commit validation should be run for the new patch set.
-   * @param mergeUtil merge utilities for the destination project
+   * @param rw the RevWalk.
-   * @param committerIdent the committer's identity
+   * @return the new patch set, which is based on the given base commit.
-   * @param runHooks if hooks should be run for the new patch set
+   * @throws NoSuchChangeException if the change to which the patch set belongs
-   * @param validate if commit validation should be run for the new patch set
+   *     does not exist or is not visible to the user.
-   * @return the new patch set which is based on the given base commit
+   * @throws OrmException if accessing the database fails.
-   * @throws NoSuchChangeException thrown if the change to which the patch set
+   * @throws IOException if rebase is not possible.
-   *         belongs does not exist or is not visible to the user
+   * @throws InvalidChangeOperationException if rebase is not possible or not
-   * @throws OrmException thrown in case accessing the database fails
+   *     allowed.
-   * @throws IOException thrown if rebase is not possible or not needed
-   * @throws InvalidChangeOperationException thrown if rebase is not allowed
    */
-  public PatchSet rebase(final Repository git, final RevWalk revWalk,
+  public PatchSet rebase(Repository git, RevWalk rw,
-      final ObjectInserter inserter, final PatchSet.Id patchSetId,
+      ObjectInserter inserter, Change change, PatchSet.Id patchSetId,
-      final Change change, final IdentifiedUser uploader, final RevCommit baseCommit,
+      IdentifiedUser uploader, RevCommit baseCommit, MergeUtil mergeUtil,
-      final MergeUtil mergeUtil, PersonIdent committerIdent,
+      PersonIdent committerIdent, boolean runHooks, ValidatePolicy validate)
-      boolean runHooks, ValidatePolicy validate)
+      throws NoSuchChangeException, OrmException, IOException,
-          throws NoSuchChangeException,
+      InvalidChangeOperationException, MergeConflictException {
-      OrmException, IOException, InvalidChangeOperationException,
-      MergeConflictException {
     if (!change.currentPatchSetId().equals(patchSetId)) {
       throw new InvalidChangeOperationException("patch set is not current");
     }
-    final PatchSet originalPatchSet = db.get().patchSets().get(patchSetId);
+    PatchSet originalPatchSet = db.get().patchSets().get(patchSetId);
 
-    final RevCommit rebasedCommit;
+    RevCommit rebasedCommit;
     ObjectId oldId = ObjectId.fromString(originalPatchSet.getRevision().get());
-    ObjectId newId = rebaseCommit(git, inserter, revWalk.parseCommit(oldId),
+    ObjectId newId = rebaseCommit(git, inserter, rw.parseCommit(oldId),
         baseCommit, mergeUtil, committerIdent);
 
-    rebasedCommit = revWalk.parseCommit(newId);
+    rebasedCommit = rw.parseCommit(newId);
 
-    final ChangeControl changeControl =
+    ChangeControl changeControl =
         changeControlFactory.validateFor(change, uploader);
 
     PatchSetInserter patchSetInserter = patchSetInserterFactory
-        .create(git, revWalk, changeControl, rebasedCommit)
+        .create(git, rw, changeControl, rebasedCommit)
         .setValidatePolicy(validate)
         .setDraft(originalPatchSet.isDraft())
         .setUploader(uploader.getAccountId())
         .setSendMail(false)
         .setRunHooks(runHooks);
 
-    final PatchSet.Id newPatchSetId = patchSetInserter.getPatchSetId();
+    PatchSet.Id newPatchSetId = patchSetInserter.getPatchSetId();
-    final ChangeMessage cmsg = new ChangeMessage(
+    ChangeMessage cmsg = new ChangeMessage(
         new ChangeMessage.Key(change.getId(),
             ChangeUtil.messageUUID(db.get())), uploader.getAccountId(),
             TimeUtil.nowTs(), patchSetId);
@@ -356,8 +361,8 @@ public class RebaseChange {
         r.getPatchSet().getId(), r.getChange().getDest());
   }
 
-  public boolean canRebase(Project.NameKey project,
+  public boolean canRebase(Project.NameKey project, PatchSet.Id patchSetId,
-      PatchSet.Id patchSetId, Branch.NameKey branch) {
+      Branch.NameKey branch) {
     Repository git;
     try {
       git = gitManager.openRepository(project);
@@ -367,12 +372,7 @@ public class RebaseChange {
       return false;
     }
     try (RevWalk rw = new RevWalk(git)) {
-      findBaseRevision(
+      findBaseRevision(patchSetId, db.get(), branch, git, rw);
-          patchSetId,
-          db.get(),
-          branch,
-          git,
-          rw);
       return true;
     } catch (InvalidChangeOperationException e) {
       return false;

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

@@ -89,7 +89,7 @@ public class RebaseIfNecessary extends SubmitStrategy {
                     .getSubmitter(n).getAccountId());
             PatchSet newPatchSet =
                 rebaseChange.rebase(args.repo, args.rw, args.inserter,
-                    n.getPatchsetId(), n.change(), uploader,
+                    n.change(), n.getPatchsetId(), uploader,
                     mergeTip.getCurrentTip(), args.mergeUtil,
                     args.serverIdent.get(), false, ValidatePolicy.NONE);