commit 4ad55592f7ae348059531c10b571fbd1df3db3fa
author Yunjie Li <yunjieli@google.com> Mon Jun 08 23:46:16 2020 -0700
committer Yunjie Li <yunjieli@google.com> Wed Jul 01 11:01:10 2020 -0700
tree eb0c1370360118c093e233cbd8e5da082327effb
parent 3313e5f4b94ca581347613e30117b39c5f2331c2

SubmoduleOp: Make the constructor a pure one

Currently we're doing quite some work in the constructor, which is
discouraged. This commit moves the process of computing
SubscriptionGraph to the Factory's create method, which makes the
constructor a pure constructor.

Change-Id: I1582913841fcbf0e04d2da3c4537949c7987873c

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

diff --git a/java/com/google/gerrit/server/submit/SubmoduleOp.java b/java/com/google/gerrit/server/submit/SubmoduleOp.java
index 5dd6624..a1ed373 100644
--- a/java/com/google/gerrit/server/submit/SubmoduleOp.java
+++ b/java/com/google/gerrit/server/submit/SubmoduleOp.java
@@ -104,8 +104,15 @@
 
     public SubmoduleOp create(Set<BranchNameKey> updatedBranches, MergeOpRepoManager orm)
         throws SubmoduleConflictException {
-      return new SubmoduleOp(
-          subscriptionGraphFactory, serverIdent.get(), cfg, updatedBranches, orm);
+      SubscriptionGraph subscriptionGraph;
+      if (cfg.getBoolean("submodule", "enableSuperProjectSubscriptions", true)) {
+        subscriptionGraph = subscriptionGraphFactory.compute(updatedBranches, orm);
+      } else {
+        logger.atFine().log("Updating superprojects disabled");
+        subscriptionGraph =
+            SubscriptionGraph.createEmptyGraph(ImmutableSet.copyOf(updatedBranches));
+      }
+      return new SubmoduleOp(serverIdent.get(), cfg, orm, subscriptionGraph);
     }
   }
 
@@ -119,12 +126,10 @@
   private final BranchTips branchTips = new BranchTips();
 
   private SubmoduleOp(
-      SubscriptionGraph.Factory subscriptionGraphFactory,
       PersonIdent myIdent,
       Config cfg,
-      Set<BranchNameKey> updatedBranches,
-      MergeOpRepoManager orm)
-      throws SubmoduleConflictException {
+      MergeOpRepoManager orm,
+      SubscriptionGraph subscriptionGraph) {
     this.myIdent = myIdent;
     this.verboseSuperProject =
         cfg.getEnum("submodule", null, "verboseSuperprojectUpdate", VerboseSuperprojectUpdate.TRUE);
@@ -132,13 +137,7 @@
         cfg.getLong("submodule", "maxCombinedCommitMessageSize", 256 << 10);
     this.maxCommitMessages = cfg.getLong("submodule", "maxCommitMessages", 1000);
     this.orm = orm;
-    if (cfg.getBoolean("submodule", "enableSuperProjectSubscriptions", true)) {
-      this.subscriptionGraph = subscriptionGraphFactory.compute(updatedBranches, orm);
-    } else {
-      logger.atFine().log("Updating superprojects disabled");
-      this.subscriptionGraph =
-          SubscriptionGraph.createEmptyGraph(ImmutableSet.copyOf(updatedBranches));
-    }
+    this.subscriptionGraph = subscriptionGraph;
   }
 
   @UsedAt(UsedAt.Project.PLUGIN_DELETE_PROJECT)