SubmoduleOp: Make branchTips its own object

Make branchTips its own object. This makes clearer its role in the code
and reduces the dependency of GitlinkOp to SubmoduleOp.

Change-Id: I9c6b7bafcf6f89a9cfd2f5387a61c93013a4545b

java/com/google/gerrit/server/submit/BranchTips.java

@@ -0,0 +1,62 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.submit;
+
+import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.server.git.CodeReviewCommit;
+import com.google.gerrit.server.submit.MergeOpRepoManager.OpenRepo;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import org.eclipse.jgit.lib.Ref;
+
+/**
+ * Current branch tips, taking into account commits created during the submit process as well as
+ * submodule updates produced by this class.
+ */
+class BranchTips {
+
+  private final Map<BranchNameKey, CodeReviewCommit> branchTips = new HashMap<>();
+
+  /**
+   * Returns current tip of the branch, taking into account commits created during the submit
+   * process or submodule updates.
+   *
+   * @param branch branch
+   * @param repo repository to look for the branch if not cached
+   * @return the current tip. Empty if the branch doesn't exist in the repository
+   * @throws IOException Cannot access the underlying storage
+   */
+  Optional<CodeReviewCommit> getTip(BranchNameKey branch, OpenRepo repo) throws IOException {
+    CodeReviewCommit currentCommit;
+    if (branchTips.containsKey(branch)) {
+      currentCommit = branchTips.get(branch);
+    } else {
+      Ref r = repo.repo.exactRef(branch.branch());
+      if (r == null) {
+        return Optional.empty();
+      }
+      currentCommit = repo.rw.parseCommit(r.getObjectId());
+      branchTips.put(branch, currentCommit);
+    }
+
+    return Optional.of(currentCommit);
+  }
+
+  void put(BranchNameKey branch, CodeReviewCommit c) {
+    branchTips.put(branch, c);
+  }
+}

java/com/google/gerrit/server/submit/SubmoduleOp.java

@@ -58,6 +58,7 @@ import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import org.apache.commons.lang.StringUtils;
 import org.eclipse.jgit.dircache.DirCache;
@@ -82,9 +83,11 @@ public class SubmoduleOp {
   /** Only used for branches without code review changes */
   public class GitlinkOp implements RepoOnlyOp {
     private final BranchNameKey branch;
+    private final BranchTips currentBranchTips;
 
-    GitlinkOp(BranchNameKey branch) {
+    GitlinkOp(BranchNameKey branch, BranchTips branchTips) {
       this.branch = branch;
+      this.currentBranchTips = branchTips;
     }
 
     @Override
@@ -92,7 +95,7 @@ public class SubmoduleOp {
       CodeReviewCommit c = composeGitlinksCommit(branch);
       if (c != null) {
         ctx.addRefUpdate(c.getParent(0), c, branch.branch());
-        addBranchTip(branch, c);
+        currentBranchTips.put(branch, c);
       }
     }
   }
@@ -136,12 +139,6 @@ public class SubmoduleOp {
   /** Branches updated as part of the enclosing submit or push batch. */
   private final ImmutableSet<BranchNameKey> updatedBranches;
 
-  /**
-   * Current branch tips, taking into account commits created during the submit process as well as
-   * submodule updates produced by this class.
-   */
-  private final Map<BranchNameKey, CodeReviewCommit> branchTips;
-
   /**
    * Branches in a superproject that contain submodule subscriptions, plus branches in submodules
    * which are subscribed to by some superproject.
@@ -154,6 +151,7 @@ public class SubmoduleOp {
   /** Multimap of superproject branch to submodule subscriptions contained in that branch. */
   private final SetMultimap<BranchNameKey, SubmoduleSubscription> targets;
 
+  private final BranchTips branchTips = new BranchTips();
   /**
    * Multimap of superproject name to all branch names within that superproject which have submodule
    * subscriptions.
@@ -182,7 +180,6 @@ public class SubmoduleOp {
     this.updatedBranches = ImmutableSet.copyOf(updatedBranches);
     this.targets = MultimapBuilder.hashKeys().hashSetValues().build();
     this.affectedBranches = new HashSet<>();
-    this.branchTips = new HashMap<>();
     this.branchGitModules = new HashMap<>();
     this.branchesByProject = MultimapBuilder.hashKeys().hashSetValues().build();
     this.sortedBranches = calculateSubscriptionMaps();
@@ -432,18 +429,13 @@ public class SubmoduleOp {
       throw new StorageException("Cannot access superproject", e);
     }
 
-    CodeReviewCommit currentCommit;
-    if (branchTips.containsKey(subscriber)) {
-      currentCommit = branchTips.get(subscriber);
-    } else {
-      Ref r = or.repo.exactRef(subscriber.branch());
-      if (r == null) {
-        throw new SubmoduleConflictException(
-            "The branch was probably deleted from the subscriber repository");
-      }
-      currentCommit = or.rw.parseCommit(r.getObjectId());
-      addBranchTip(subscriber, currentCommit);
-    }
+    CodeReviewCommit currentCommit =
+        branchTips
+            .getTip(subscriber, or)
+            .orElseThrow(
+                () ->
+                    new SubmoduleConflictException(
+                        "The branch was probably deleted from the subscriber repository"));
 
     StringBuilder msgbuf = new StringBuilder();
     PersonIdent author = null;
@@ -574,25 +566,14 @@ public class SubmoduleOp {
       }
     }
 
-    final CodeReviewCommit newCommit;
-    if (branchTips.containsKey(s.getSubmodule())) {
-      // This submodule's branch was updated as part of this specific submit batch: update the
-      // gitlink to point to the new commit from the batch.
-      newCommit = branchTips.get(s.getSubmodule());
-    } else {
-      // For whatever reason, this submodule was not updated as part of this submit batch, but the
-      // superproject is still subscribed to this branch. Re-read the ref to see if anything has
-      // changed since the last time the gitlink was updated, and roll that update into the same
-      // commit as all other submodule updates.
-      Ref ref = subOr.repo.getRefDatabase().exactRef(s.getSubmodule().branch());
-      if (ref == null) {
-        ed.add(new DeletePath(s.getPath()));
-        return null;
-      }
-      newCommit = subOr.rw.parseCommit(ref.getObjectId());
-      addBranchTip(s.getSubmodule(), newCommit);
+    Optional<CodeReviewCommit> maybeNewCommit = branchTips.getTip(s.getSubmodule(), subOr);
+    if (!maybeNewCommit.isPresent()) {
+      // This submodule branch is neither in the submit set nor in the repository itself
+      ed.add(new DeletePath(s.getPath()));
+      return null;
     }
 
+    CodeReviewCommit newCommit = maybeNewCommit.get();
     if (Objects.equals(newCommit, oldCommit)) {
       // gitlink have already been updated for this submodule
       return null;
@@ -737,6 +718,6 @@ public class SubmoduleOp {
   }
 
   void addOp(BatchUpdate bu, BranchNameKey branch) {
-    bu.addRepoOnlyOp(new GitlinkOp(branch));
+    bu.addRepoOnlyOp(new GitlinkOp(branch, branchTips));
   }
 }