UpdateOrderCalculator: Move update order calculation out of submoduleOp

This cryptic code calculates the precedence of updates, so all
subprojects are updated before the superprojects. It is in submoduleOp,
but actually it only depends on the subscription graph. Note that the
subscription graph is immutable for the request and therefore this class
too. All this becomes more visible with the code in its own class.

Change-Id: I66ed2bd8520718a5023d64935db410ee16bf220d

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

@@ -607,7 +607,7 @@ public class MergeOp implements AutoCloseable {
     try {
       SubmoduleOp submoduleOp = subOpFactory.create(branches, orm);
       List<SubmitStrategy> strategies = getSubmitStrategies(toSubmit, submoduleOp, dryrun);
-      this.allProjects = submoduleOp.getProjectsInOrder();
+      this.allProjects = submoduleOp.getUpdateOrderCalculator().getProjectsInOrder();
       try {
         BatchUpdate.execute(
             orm.batchUpdates(allProjects),
@@ -661,7 +661,7 @@ public class MergeOp implements AutoCloseable {
       Map<BranchNameKey, BranchBatch> toSubmit, SubmoduleOp submoduleOp, boolean dryrun)
       throws IntegrationConflictException, NoSuchProjectException, IOException {
     List<SubmitStrategy> strategies = new ArrayList<>();
-    Set<BranchNameKey> allBranches = submoduleOp.getBranchesInOrder();
+    Set<BranchNameKey> allBranches = submoduleOp.getUpdateOrderCalculator().getBranchesInOrder();
     Set<CodeReviewCommit> allCommits =
         toSubmit.values().stream().map(BranchBatch::commits).flatMap(Set::stream).collect(toSet());
 

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

@@ -19,7 +19,6 @@ import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.common.UsedAt;
 import com.google.gerrit.entities.BranchNameKey;
 import com.google.gerrit.entities.Project;
-import com.google.gerrit.entities.SubmoduleSubscription;
 import com.google.gerrit.exceptions.StorageException;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.server.config.GerritServerConfig;
@@ -31,8 +30,6 @@ import com.google.gerrit.server.update.UpdateException;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 import java.io.IOException;
-import java.util.Collection;
-import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.Set;
 import org.eclipse.jgit.lib.Config;
@@ -73,6 +70,7 @@ public class SubmoduleOp {
   private final MergeOpRepoManager orm;
   private final SubscriptionGraph subscriptionGraph;
   private final SubmoduleCommits submoduleCommits;
+  private final UpdateOrderCalculator updateOrderCalculator;
 
   private SubmoduleOp(
       MergeOpRepoManager orm,
@@ -81,6 +79,7 @@ public class SubmoduleOp {
     this.orm = orm;
     this.subscriptionGraph = subscriptionGraph;
     this.submoduleCommits = submoduleCommits;
+    this.updateOrderCalculator = new UpdateOrderCalculator(subscriptionGraph);
   }
 
   // TODO(ifrade): subscription graph should be instantiated somewhere else and passed to
@@ -93,13 +92,17 @@ public class SubmoduleOp {
     return submoduleCommits;
   }
 
+  UpdateOrderCalculator getUpdateOrderCalculator() {
+    return updateOrderCalculator;
+  }
+
   @UsedAt(UsedAt.Project.PLUGIN_DELETE_PROJECT)
   public boolean hasSuperproject(BranchNameKey branch) {
     return subscriptionGraph.hasSuperproject(branch);
   }
 
   public void updateSuperProjects() throws RestApiException {
-    ImmutableSet<Project.NameKey> projects = getProjectsInOrder();
+    ImmutableSet<Project.NameKey> projects = updateOrderCalculator.getProjectsInOrder();
     if (projects == null) {
       return;
     }
@@ -124,55 +127,4 @@ public class SubmoduleOp {
       throw new StorageException("Cannot update gitlinks", e);
     }
   }
-
-  ImmutableSet<Project.NameKey> getProjectsInOrder() throws SubmoduleConflictException {
-    LinkedHashSet<Project.NameKey> projects = new LinkedHashSet<>();
-    for (Project.NameKey project : subscriptionGraph.getAffectedSuperProjects()) {
-      addAllSubmoduleProjects(project, new LinkedHashSet<>(), projects);
-    }
-
-    for (BranchNameKey branch : subscriptionGraph.getUpdatedBranches()) {
-      projects.add(branch.project());
-    }
-    return ImmutableSet.copyOf(projects);
-  }
-
-  private void addAllSubmoduleProjects(
-      Project.NameKey project,
-      LinkedHashSet<Project.NameKey> current,
-      LinkedHashSet<Project.NameKey> projects)
-      throws SubmoduleConflictException {
-    if (current.contains(project)) {
-      throw new SubmoduleConflictException(
-          "Project level circular subscriptions detected:  "
-              + CircularPathFinder.printCircularPath(current, project));
-    }
-
-    if (projects.contains(project)) {
-      return;
-    }
-
-    current.add(project);
-    Set<Project.NameKey> subprojects = new HashSet<>();
-    for (BranchNameKey branch : subscriptionGraph.getAffectedSuperBranches(project)) {
-      Collection<SubmoduleSubscription> subscriptions = subscriptionGraph.getSubscriptions(branch);
-      for (SubmoduleSubscription s : subscriptions) {
-        subprojects.add(s.getSubmodule().project());
-      }
-    }
-
-    for (Project.NameKey p : subprojects) {
-      addAllSubmoduleProjects(p, current, projects);
-    }
-
-    current.remove(project);
-    projects.add(project);
-  }
-
-  ImmutableSet<BranchNameKey> getBranchesInOrder() {
-    LinkedHashSet<BranchNameKey> branches = new LinkedHashSet<>();
-    branches.addAll(subscriptionGraph.getSortedSuperprojectAndSubmoduleBranches());
-    branches.addAll(subscriptionGraph.getUpdatedBranches());
-    return ImmutableSet.copyOf(branches);
-  }
 }

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

@@ -0,0 +1,91 @@
+// 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.common.collect.ImmutableSet;
+import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.SubmoduleSubscription;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.Set;
+
+/**
+ * Sorts the projects or branches affected by the update.
+ *
+ * <p>The subscription graph contains all branches (and projects) affected by the update, but the
+ * updates must be executed in the right order, so no superproject reference is updated before its
+ * target.
+ */
+class UpdateOrderCalculator {
+
+  private final SubscriptionGraph subscriptionGraph;
+
+  UpdateOrderCalculator(SubscriptionGraph subscriptionGraph) {
+    this.subscriptionGraph = subscriptionGraph;
+  }
+
+  ImmutableSet<Project.NameKey> getProjectsInOrder() throws SubmoduleConflictException {
+    LinkedHashSet<Project.NameKey> projects = new LinkedHashSet<>();
+    for (Project.NameKey project : subscriptionGraph.getAffectedSuperProjects()) {
+      addAllSubmoduleProjects(project, new LinkedHashSet<>(), projects);
+    }
+
+    for (BranchNameKey branch : subscriptionGraph.getUpdatedBranches()) {
+      projects.add(branch.project());
+    }
+    return ImmutableSet.copyOf(projects);
+  }
+
+  private void addAllSubmoduleProjects(
+      Project.NameKey project,
+      LinkedHashSet<Project.NameKey> current,
+      LinkedHashSet<Project.NameKey> projects)
+      throws SubmoduleConflictException {
+    if (current.contains(project)) {
+      throw new SubmoduleConflictException(
+          "Project level circular subscriptions detected:  "
+              + CircularPathFinder.printCircularPath(current, project));
+    }
+
+    if (projects.contains(project)) {
+      return;
+    }
+
+    current.add(project);
+    Set<Project.NameKey> subprojects = new HashSet<>();
+    for (BranchNameKey branch : subscriptionGraph.getAffectedSuperBranches(project)) {
+      Collection<SubmoduleSubscription> subscriptions = subscriptionGraph.getSubscriptions(branch);
+      for (SubmoduleSubscription s : subscriptions) {
+        subprojects.add(s.getSubmodule().project());
+      }
+    }
+
+    for (Project.NameKey p : subprojects) {
+      addAllSubmoduleProjects(p, current, projects);
+    }
+
+    current.remove(project);
+    projects.add(project);
+  }
+
+  ImmutableSet<BranchNameKey> getBranchesInOrder() {
+    LinkedHashSet<BranchNameKey> branches = new LinkedHashSet<>();
+    branches.addAll(subscriptionGraph.getSortedSuperprojectAndSubmoduleBranches());
+    branches.addAll(subscriptionGraph.getUpdatedBranches());
+    return ImmutableSet.copyOf(branches);
+  }
+}