SubscriptionGraph: Inject SubscriptionGraph.Factory
Use Guice to inject SubscriptionGraph.Factory instead of calling
constructor directly, which allows an alternative implementation of
SubmoduleGraph.Factory being injected easily.
Change-Id: I689998da72e3787bf36a2c854c40f206fabacf9e
diff --git a/java/com/google/gerrit/pgm/Daemon.java b/java/com/google/gerrit/pgm/Daemon.java
index 57bec71..63278c1 100644
--- a/java/com/google/gerrit/pgm/Daemon.java
+++ b/java/com/google/gerrit/pgm/Daemon.java
@@ -107,6 +107,7 @@
import com.google.gerrit.server.ssh.NoSshModule;
import com.google.gerrit.server.ssh.SshAddressesModule;
import com.google.gerrit.server.submit.LocalMergeSuperSetComputation;
+import com.google.gerrit.server.submit.SubscriptionGraph;
import com.google.gerrit.sshd.SshHostKeyModule;
import com.google.gerrit.sshd.SshKeyCacheImpl;
import com.google.gerrit.sshd.SshModule;
@@ -411,6 +412,7 @@
// work queue can get stuck waiting on index futures that will never return.
modules.add(createIndexModule());
+ modules.add(new SubscriptionGraph.Module());
modules.add(new WorkQueue.Module());
modules.add(new StreamEventsApiListener.Module());
modules.add(new EventBroker.Module());
diff --git a/java/com/google/gerrit/server/submit/SubmoduleOp.java b/java/com/google/gerrit/server/submit/SubmoduleOp.java
index 5adda2c..5dd6624 100644
--- a/java/com/google/gerrit/server/submit/SubmoduleOp.java
+++ b/java/com/google/gerrit/server/submit/SubmoduleOp.java
@@ -30,7 +30,6 @@
import com.google.gerrit.server.config.VerboseSuperprojectUpdate;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.project.NoSuchProjectException;
-import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.submit.MergeOpRepoManager.OpenRepo;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.BatchUpdateListener;
@@ -89,27 +88,24 @@
@Singleton
public static class Factory {
- private final GitModules.Factory gitmodulesFactory;
+ private final SubscriptionGraph.Factory subscriptionGraphFactory;
private final Provider<PersonIdent> serverIdent;
private final Config cfg;
- private final ProjectCache projectCache;
@Inject
Factory(
- GitModules.Factory gitmodulesFactory,
+ SubscriptionGraph.Factory subscriptionGraphFactory,
@GerritPersonIdent Provider<PersonIdent> serverIdent,
- @GerritServerConfig Config cfg,
- ProjectCache projectCache) {
- this.gitmodulesFactory = gitmodulesFactory;
+ @GerritServerConfig Config cfg) {
+ this.subscriptionGraphFactory = subscriptionGraphFactory;
this.serverIdent = serverIdent;
this.cfg = cfg;
- this.projectCache = projectCache;
}
public SubmoduleOp create(Set<BranchNameKey> updatedBranches, MergeOpRepoManager orm)
throws SubmoduleConflictException {
return new SubmoduleOp(
- gitmodulesFactory, serverIdent.get(), cfg, projectCache, updatedBranches, orm);
+ subscriptionGraphFactory, serverIdent.get(), cfg, updatedBranches, orm);
}
}
@@ -118,16 +114,14 @@
private final long maxCombinedCommitMessageSize;
private final long maxCommitMessages;
private final MergeOpRepoManager orm;
- private final SubscriptionGraph.Factory subscriptionGraphFactory;
private final SubscriptionGraph subscriptionGraph;
private final BranchTips branchTips = new BranchTips();
private SubmoduleOp(
- GitModules.Factory gitmodulesFactory,
+ SubscriptionGraph.Factory subscriptionGraphFactory,
PersonIdent myIdent,
Config cfg,
- ProjectCache projectCache,
Set<BranchNameKey> updatedBranches,
MergeOpRepoManager orm)
throws SubmoduleConflictException {
@@ -138,10 +132,8 @@
cfg.getLong("submodule", "maxCombinedCommitMessageSize", 256 << 10);
this.maxCommitMessages = cfg.getLong("submodule", "maxCommitMessages", 1000);
this.orm = orm;
- this.subscriptionGraphFactory =
- new SubscriptionGraph.DefaultFactory(gitmodulesFactory, projectCache, orm);
if (cfg.getBoolean("submodule", "enableSuperProjectSubscriptions", true)) {
- this.subscriptionGraph = subscriptionGraphFactory.compute(updatedBranches);
+ this.subscriptionGraph = subscriptionGraphFactory.compute(updatedBranches, orm);
} else {
logger.atFine().log("Updating superprojects disabled");
this.subscriptionGraph =
diff --git a/java/com/google/gerrit/server/submit/SubscriptionGraph.java b/java/com/google/gerrit/server/submit/SubscriptionGraph.java
index 9bd1190..f037261 100644
--- a/java/com/google/gerrit/server/submit/SubscriptionGraph.java
+++ b/java/com/google/gerrit/server/submit/SubscriptionGraph.java
@@ -31,6 +31,8 @@
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.submit.MergeOpRepoManager.OpenRepo;
+import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
import java.io.IOException;
import java.util.ArrayDeque;
import java.util.ArrayList;
@@ -150,24 +152,30 @@
}
public interface Factory {
- SubscriptionGraph compute(Set<BranchNameKey> updatedBranches) throws SubmoduleConflictException;
+ SubscriptionGraph compute(Set<BranchNameKey> updatedBranches, MergeOpRepoManager orm)
+ throws SubmoduleConflictException;
+ }
+
+ public static class Module extends AbstractModule {
+ @Override
+ protected void configure() {
+ bind(Factory.class).to(DefaultFactory.class);
+ }
}
static class DefaultFactory implements Factory {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final ProjectCache projectCache;
private final GitModules.Factory gitmodulesFactory;
- private final MergeOpRepoManager orm;
- DefaultFactory(
- GitModules.Factory gitmodulesFactory, ProjectCache projectCache, MergeOpRepoManager orm) {
+ @Inject
+ DefaultFactory(GitModules.Factory gitmodulesFactory, ProjectCache projectCache) {
this.gitmodulesFactory = gitmodulesFactory;
this.projectCache = projectCache;
- this.orm = orm;
}
@Override
- public SubscriptionGraph compute(Set<BranchNameKey> updatedBranches)
+ public SubscriptionGraph compute(Set<BranchNameKey> updatedBranches, MergeOpRepoManager orm)
throws SubmoduleConflictException {
Map<BranchNameKey, GitModules> branchGitModules = new HashMap<>();
// All affected branches, including those in superprojects and submodules.
@@ -191,7 +199,8 @@
targets,
branchesByProject,
subscribedBranches,
- branchGitModules);
+ branchGitModules,
+ orm);
return new SubscriptionGraph(
updatedBranches, targets, branchesByProject, subscribedBranches, sortedBranches);
@@ -217,7 +226,8 @@
SetMultimap<BranchNameKey, SubmoduleSubscription> targets,
SetMultimap<Project.NameKey, BranchNameKey> branchesByProject,
Set<BranchNameKey> subscribedBranches,
- Map<BranchNameKey, GitModules> branchGitModules)
+ Map<BranchNameKey, GitModules> branchGitModules,
+ MergeOpRepoManager orm)
throws SubmoduleConflictException {
logger.atFine().log("Calculating superprojects - submodules map");
LinkedHashSet<BranchNameKey> allVisited = new LinkedHashSet<>();
@@ -234,7 +244,8 @@
targets,
branchesByProject,
subscribedBranches,
- branchGitModules);
+ branchGitModules,
+ orm);
}
// Since the searchForSuperprojects will add all branches (related or
@@ -254,7 +265,8 @@
SetMultimap<BranchNameKey, SubmoduleSubscription> targets,
SetMultimap<Project.NameKey, BranchNameKey> branchesByProject,
Set<BranchNameKey> subscribedBranches,
- Map<BranchNameKey, GitModules> branchGitModules)
+ Map<BranchNameKey, GitModules> branchGitModules,
+ MergeOpRepoManager orm)
throws SubmoduleConflictException {
logger.atFine().log("Now processing %s", current);
@@ -271,7 +283,7 @@
currentVisited.add(current);
try {
Collection<SubmoduleSubscription> subscriptions =
- superProjectSubscriptionsForSubmoduleBranch(current, branchGitModules);
+ superProjectSubscriptionsForSubmoduleBranch(current, branchGitModules, orm);
for (SubmoduleSubscription sub : subscriptions) {
BranchNameKey superBranch = sub.getSuperProject();
searchForSuperprojects(
@@ -282,7 +294,8 @@
targets,
branchesByProject,
subscribedBranches,
- branchGitModules);
+ branchGitModules,
+ orm);
targets.put(superBranch, sub);
branchesByProject.put(superBranch.project(), superBranch);
affectedBranches.add(superBranch);
@@ -296,8 +309,8 @@
allVisited.add(current);
}
- private Collection<BranchNameKey> getDestinationBranches(BranchNameKey src, SubscribeSection s)
- throws IOException {
+ private Collection<BranchNameKey> getDestinationBranches(
+ BranchNameKey src, SubscribeSection s, MergeOpRepoManager orm) throws IOException {
OpenRepo or;
try {
or = orm.getRepo(s.project());
@@ -313,7 +326,9 @@
}
private Collection<SubmoduleSubscription> superProjectSubscriptionsForSubmoduleBranch(
- BranchNameKey srcBranch, Map<BranchNameKey, GitModules> branchGitModules)
+ BranchNameKey srcBranch,
+ Map<BranchNameKey, GitModules> branchGitModules,
+ MergeOpRepoManager orm)
throws IOException {
logger.atFine().log("Calculating possible superprojects for %s", srcBranch);
Collection<SubmoduleSubscription> ret = new ArrayList<>();
@@ -324,7 +339,7 @@
.orElseThrow(illegalState(srcProject))
.getSubscribeSections(srcBranch)) {
logger.atFine().log("Checking subscribe section %s", s);
- Collection<BranchNameKey> branches = getDestinationBranches(srcBranch, s);
+ Collection<BranchNameKey> branches = getDestinationBranches(srcBranch, s, orm);
for (BranchNameKey targetBranch : branches) {
Project.NameKey targetProject = targetBranch.project();
try {
diff --git a/java/com/google/gerrit/testing/InMemoryModule.java b/java/com/google/gerrit/testing/InMemoryModule.java
index a682d33..6c9fbed 100644
--- a/java/com/google/gerrit/testing/InMemoryModule.java
+++ b/java/com/google/gerrit/testing/InMemoryModule.java
@@ -87,6 +87,7 @@
import com.google.gerrit.server.securestore.SecureStore;
import com.google.gerrit.server.ssh.NoSshKeyCache;
import com.google.gerrit.server.submit.LocalMergeSuperSetComputation;
+import com.google.gerrit.server.submit.SubscriptionGraph;
import com.google.gerrit.server.util.ReplicaUtil;
import com.google.inject.AbstractModule;
import com.google.inject.Guice;
@@ -175,6 +176,7 @@
install(new SearchingChangeCacheImpl.Module());
factory(GarbageCollection.Factory.class);
install(new AuditModule());
+ install(new SubscriptionGraph.Module());
bindScope(RequestScoped.class, PerThreadRequestScope.REQUEST);
diff --git a/javatests/com/google/gerrit/server/submit/SubscriptionGraphTest.java b/javatests/com/google/gerrit/server/submit/SubscriptionGraphTest.java
index 489e1b6..5f71544 100644
--- a/javatests/com/google/gerrit/server/submit/SubscriptionGraphTest.java
+++ b/javatests/com/google/gerrit/server/submit/SubscriptionGraphTest.java
@@ -81,9 +81,9 @@
@Test
public void oneSuperprojectOneSubmodule() throws Exception {
- SubscriptionGraph.Factory factory =
- new DefaultFactory(mockGitModulesFactory, mockProjectCache, mergeOpRepoManager);
- SubscriptionGraph subscriptionGraph = factory.compute(ImmutableSet.of(SUB_BRANCH));
+ SubscriptionGraph.Factory factory = new DefaultFactory(mockGitModulesFactory, mockProjectCache);
+ SubscriptionGraph subscriptionGraph =
+ factory.compute(ImmutableSet.of(SUB_BRANCH), mergeOpRepoManager);
assertThat(subscriptionGraph.getAffectedSuperProjects()).containsExactly(SUPER_PROJECT);
assertThat(subscriptionGraph.getAffectedSuperBranches(SUPER_PROJECT))
@@ -98,12 +98,12 @@
@Test
public void circularSubscription() throws Exception {
- SubscriptionGraph.Factory factory =
- new DefaultFactory(mockGitModulesFactory, mockProjectCache, mergeOpRepoManager);
+ SubscriptionGraph.Factory factory = new DefaultFactory(mockGitModulesFactory, mockProjectCache);
setSubscription(SUPER_BRANCH, ImmutableList.of(SUB_BRANCH));
SubmoduleConflictException e =
assertThrows(
- SubmoduleConflictException.class, () -> factory.compute(ImmutableSet.of(SUB_BRANCH)));
+ SubmoduleConflictException.class,
+ () -> factory.compute(ImmutableSet.of(SUB_BRANCH), mergeOpRepoManager));
String expectedErrorMessage =
"Subproject,refs/heads/one->Superproject,refs/heads/one->Subproject,refs/heads/one";
@@ -154,10 +154,9 @@
setSubscription(submoduleBranch2, ImmutableList.of(superBranch1));
setSubscription(submoduleBranch3, ImmutableList.of(superBranch1, superBranch2));
- SubscriptionGraph.Factory factory =
- new DefaultFactory(mockGitModulesFactory, mockProjectCache, mergeOpRepoManager);
+ SubscriptionGraph.Factory factory = new DefaultFactory(mockGitModulesFactory, mockProjectCache);
SubscriptionGraph subscriptionGraph =
- factory.compute(ImmutableSet.of(submoduleBranch1, submoduleBranch2));
+ factory.compute(ImmutableSet.of(submoduleBranch1, submoduleBranch2), mergeOpRepoManager);
assertThat(subscriptionGraph.getAffectedSuperProjects())
.containsExactly(superProject1, superProject2);
