BaseCommitUtil: Fix behavior when cache-automerge refs points to a tree

Versions of Gerrit older than v2.13 may have cache-automerge refs
which point to a tree. These older refs are not considered valid by
AutoMerger, but are not removed or invalidated.

BaseCommitUtil's methods' callers expect to be returned objects that
are persisted in the repository and creating auto-merges in-memory
is incompatible with that expectation. Solve this by no longer doing
in-memory merges and by validating if the cache-automerge ref is
actually usable(i.e exists and points to a commit). If the ref is found
to be invalid, we create the auto-merge commit in git and force update
the ref. Other callers of AutoMerger should be unchanged as the behavior
of methods in there is not modified. Instead a new helper is created for
BaseCommitUtil.

Release-Notes: skip
Change-Id: I15f9c951cdaa3733963e6f8b38066c11be8712f7

2 files changed, 44 insertions, 45 deletions

diff --git a/java/com/google/gerrit/server/patch/AutoMerger.java b/java/com/google/gerrit/server/patch/AutoMerger.java
index 2529c04e52..9a84398a56 100644
--- a/java/com/google/gerrit/server/patch/AutoMerger.java
+++ b/java/com/google/gerrit/server/patch/AutoMerger.java
@@ -174,17 +174,42 @@ public class AutoMerger {
       return Optional.empty();
     }
 
+    return Optional.of(
+        new ReceiveCommand(
+            ObjectId.zeroId(),
+            createAutoMergeCommit(repoView, rw, ins, maybeMergeCommit),
+            RefNames.refsCacheAutomerge(maybeMergeCommit.name())));
+  }
+
+  /**
+   * Creates an auto merge commit for the provided merge commit.
+   *
+   * <p>Callers are expected to ensure that the provided commit indeed has 2 parents.
+   *
+   * @return An auto-merge commit. Headers of the returned RevCommit are parsed.
+   */
+  ObjectId createAutoMergeCommit(
+      RepoView repoView, RevWalk rw, ObjectInserter ins, RevCommit mergeCommit) throws IOException {
     ObjectId autoMerge;
     try (Timer1.Context<OperationType> ignored = latency.start(OperationType.ON_DISK_WRITE)) {
       autoMerge =
           createAutoMergeCommit(
-              repoView.getConfig(), rw, ins, maybeMergeCommit, configuredMergeStrategy);
+              repoView.getConfig(), rw, ins, mergeCommit, configuredMergeStrategy);
     }
     counter.increment(OperationType.ON_DISK_WRITE);
     logger.atFine().log("Added %s AutoMerge ref update for commit", autoMerge.name());
-    return Optional.of(
-        new ReceiveCommand(
-            ObjectId.zeroId(), autoMerge, RefNames.refsCacheAutomerge(maybeMergeCommit.name())));
+    return autoMerge;
+  }
+
+  Optional<RevCommit> lookupCommit(Repository repo, RevWalk rw, String refName) throws IOException {
+    Ref ref = repo.getRefDatabase().exactRef(refName);
+    if (ref != null && ref.getObjectId() != null) {
+      RevObject obj = rw.parseAny(ref.getObjectId());
+      if (obj instanceof RevCommit) {
+        return Optional.of((RevCommit) obj);
+      }
+    }
+    return Optional.empty();
   }
 
   /**
@@ -255,18 +280,6 @@ public class AutoMerger {
     return rw.parseCommit(ins.insert(cb));
   }
 
-  private Optional<RevCommit> lookupCommit(Repository repo, RevWalk rw, String refName)
-      throws IOException {
-    Ref ref = repo.getRefDatabase().exactRef(refName);
-    if (ref != null && ref.getObjectId() != null) {
-      RevObject obj = rw.parseAny(ref.getObjectId());
-      if (obj instanceof RevCommit) {
-        return Optional.of((RevCommit) obj);
-      }
-    }
-    return Optional.empty();
-  }
-
   private static class NonFlushingWrapper extends ObjectInserter.Filter {
     private final ObjectInserter ins;
 
diff --git a/java/com/google/gerrit/server/patch/BaseCommitUtil.java b/java/com/google/gerrit/server/patch/BaseCommitUtil.java
index ed68dfde41..dcd3e85a28 100644
--- a/java/com/google/gerrit/server/patch/BaseCommitUtil.java
+++ b/java/com/google/gerrit/server/patch/BaseCommitUtil.java
@@ -16,11 +16,11 @@ package com.google.gerrit.server.patch;
 
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.RefNames;
 import com.google.gerrit.git.LockFailureException;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.InMemoryInserter;
-import com.google.gerrit.server.git.MergeUtil;
 import com.google.gerrit.server.update.RepoView;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
@@ -33,17 +33,14 @@ import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.merge.ThreeWayMergeStrategy;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevObject;
 import org.eclipse.jgit.revwalk.RevWalk;
-import org.eclipse.jgit.transport.ReceiveCommand;
 
 /** A utility class for computing the base commit / parent for a specific patchset commit. */
 @Singleton
 class BaseCommitUtil {
   private final AutoMerger autoMerger;
-  private final ThreeWayMergeStrategy mergeStrategy;
   private final GitRepositoryManager repoManager;
 
   /** If true, auto-merge results are stored in the repository. */
@@ -52,7 +49,6 @@ class BaseCommitUtil {
   @Inject
   BaseCommitUtil(AutoMerger am, @GerritServerConfig Config cfg, GitRepositoryManager repoManager) {
     this.autoMerger = am;
-    this.mergeStrategy = MergeUtil.getMergeStrategy(cfg);
     this.saveAutomerge = AutoMerger.cacheAutomerge(cfg);
     this.repoManager = repoManager;
   }
@@ -123,39 +119,29 @@ class BaseCommitUtil {
                 "diff against auto-merge commits is only supported if 'change.cacheAutomerge' config is set to true.");
           }
           // TODO(ghareeb): Avoid persisting auto-merge commits.
-          RevCommit autoMerge = createAutoMergeInGitIfNecessary(repo, ins, rw, current);
-          return autoMerge == null ? getAutoMergeFromGit(repo, current) : autoMerge;
+          return getAutoMergeFromGitOrCreate(repo, ins, rw, current);
         }
         return null;
     }
   }
 
   /**
-   * Creates the auto-merge commit in git. If the auto-merge already exists, this does nothing.
-   * Otherwise, the auto-merge is created, persisted in git and the cache-automerge ref is updated
-   * for the merge commit.
+   * Gets the auto-merge commit from git if it already exists. If not, the auto-merge is created,
+   * persisted in git and the cache-automerge ref is updated for the merge commit.
    *
-   * @return null if the auto-merge already exists in git, or the auto-merge {@link RevCommit}
-   *     object otherwise.
+   * @return the auto-merge {@link RevCommit}
    */
-  private RevCommit createAutoMergeInGitIfNecessary(
+  private RevCommit getAutoMergeFromGitOrCreate(
       Repository repo, ObjectInserter ins, RevWalk rw, RevCommit mergeCommit) throws IOException {
-    Optional<ReceiveCommand> receive =
-        autoMerger.createAutoMergeCommitIfNecessary(
-            new RepoView(repo, rw, ins), rw, ins, mergeCommit);
-    if (receive.isPresent()) {
-      ins.flush();
-      return updateRef(repo, rw, receive.get().getRefName(), receive.get().getNewId(), mergeCommit);
-    }
-    return null;
-  }
-
-  private RevCommit getAutoMergeFromGit(Repository repo, RevCommit mergeCommit) throws IOException {
-    try (InMemoryInserter inMemoryIns = new InMemoryInserter(repo);
-        RevWalk inMemoryRw = new RevWalk(inMemoryIns.newReader())) {
-      return autoMerger.lookupFromGitOrMergeInMemory(
-          repo, inMemoryRw, inMemoryIns, mergeCommit, mergeStrategy);
+    String refName = RefNames.refsCacheAutomerge(mergeCommit.name());
+    Optional<RevCommit> autoMergeCommit = autoMerger.lookupCommit(repo, rw, refName);
+    if (autoMergeCommit.isPresent()) {
+      return autoMergeCommit.get();
     }
+    ObjectId autoMergeId =
+        autoMerger.createAutoMergeCommit(new RepoView(repo, rw, ins), rw, ins, mergeCommit);
+    ins.flush();
+    return updateRef(repo, rw, refName, autoMergeId, mergeCommit);
   }
 
   private static RevCommit updateRef(
@@ -164,7 +150,7 @@ class BaseCommitUtil {
     RefUpdate ru = repo.updateRef(refName);
     ru.setNewObjectId(autoMergeId);
     ru.disableRefLog();
-    switch (ru.update()) {
+    switch (ru.forceUpdate()) {
       case FAST_FORWARD:
       case FORCED:
       case NEW: