BatchUpdate: Remove non-assisted Factory
This class was serving two purposes:
* Delegate to the appropriate implementation's factory depending on
NoteDb migration state.
* Have an extra bulk execute method which delegated to an
implementation's bulk execute method.
With only one implementation, we don't need a separate class for this.
Callers can use the assisted factory directly (now renamed to just
Factory), and call the static method.
Change-Id: I1ce668b63fc5509b329755977378d4708d6bbeb0
diff --git a/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java b/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java
index 16ab812..96b9519 100644
--- a/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java
+++ b/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java
@@ -151,7 +151,7 @@
// Currently there's no way to let some updates succeed even if others fail. Even if there were,
// all updates from this operation only happen in All-Users and thus are fully atomic, so
// allowing partial failure would have little value.
- batchUpdateFactory.execute(updates.values(), BatchUpdateListener.NONE, false);
+ BatchUpdate.execute(updates.values(), BatchUpdateListener.NONE, false);
return ops.stream().map(Op::getResult).filter(Objects::nonNull).collect(toImmutableList());
}
diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java
index 81e2661..13affc3 100644
--- a/java/com/google/gerrit/server/submit/MergeOp.java
+++ b/java/com/google/gerrit/server/submit/MergeOp.java
@@ -607,7 +607,7 @@
SubmoduleOp submoduleOp = subOpFactory.create(branches, orm);
List<SubmitStrategy> strategies = getSubmitStrategies(toSubmit, submoduleOp, dryrun);
this.allProjects = submoduleOp.getProjectsInOrder();
- batchUpdateFactory.execute(
+ BatchUpdate.execute(
orm.batchUpdates(allProjects),
new SubmitStrategyListener(submitInput, strategies, commitStatus),
dryrun);
diff --git a/java/com/google/gerrit/server/submit/SubmoduleOp.java b/java/com/google/gerrit/server/submit/SubmoduleOp.java
index 50be62a..3fefed3 100644
--- a/java/com/google/gerrit/server/submit/SubmoduleOp.java
+++ b/java/com/google/gerrit/server/submit/SubmoduleOp.java
@@ -101,39 +101,29 @@
private final Provider<PersonIdent> serverIdent;
private final Config cfg;
private final ProjectCache projectCache;
- private final BatchUpdate.Factory batchUpdateFactory;
@Inject
Factory(
GitModules.Factory gitmodulesFactory,
@GerritPersonIdent Provider<PersonIdent> serverIdent,
@GerritServerConfig Config cfg,
- ProjectCache projectCache,
- BatchUpdate.Factory batchUpdateFactory) {
+ ProjectCache projectCache) {
this.gitmodulesFactory = gitmodulesFactory;
this.serverIdent = serverIdent;
this.cfg = cfg;
this.projectCache = projectCache;
- this.batchUpdateFactory = batchUpdateFactory;
}
public SubmoduleOp create(Set<Branch.NameKey> updatedBranches, MergeOpRepoManager orm)
throws SubmoduleException {
return new SubmoduleOp(
- gitmodulesFactory,
- serverIdent.get(),
- cfg,
- projectCache,
- batchUpdateFactory,
- updatedBranches,
- orm);
+ gitmodulesFactory, serverIdent.get(), cfg, projectCache, updatedBranches, orm);
}
}
private final GitModules.Factory gitmodulesFactory;
private final PersonIdent myIdent;
private final ProjectCache projectCache;
- private final BatchUpdate.Factory batchUpdateFactory;
private final VerboseSuperprojectUpdate verboseSuperProject;
private final boolean enableSuperProjectSubscriptions;
private final long maxCombinedCommitMessageSize;
@@ -173,14 +163,12 @@
PersonIdent myIdent,
Config cfg,
ProjectCache projectCache,
- BatchUpdate.Factory batchUpdateFactory,
Set<Branch.NameKey> updatedBranches,
MergeOpRepoManager orm)
throws SubmoduleException {
this.gitmodulesFactory = gitmodulesFactory;
this.myIdent = myIdent;
this.projectCache = projectCache;
- this.batchUpdateFactory = batchUpdateFactory;
this.verboseSuperProject =
cfg.getEnum("submodule", null, "verboseSuperprojectUpdate", VerboseSuperprojectUpdate.TRUE);
this.enableSuperProjectSubscriptions =
@@ -420,7 +408,7 @@
}
}
}
- batchUpdateFactory.execute(orm.batchUpdates(superProjects), BatchUpdateListener.NONE, false);
+ BatchUpdate.execute(orm.batchUpdates(superProjects), BatchUpdateListener.NONE, false);
} catch (RestApiException | UpdateException | IOException | NoSuchProjectException e) {
throw new SubmoduleException("Cannot update gitlinks", e);
}
diff --git a/java/com/google/gerrit/server/update/BatchUpdate.java b/java/com/google/gerrit/server/update/BatchUpdate.java
index de47baa..0889f52 100644
--- a/java/com/google/gerrit/server/update/BatchUpdate.java
+++ b/java/com/google/gerrit/server/update/BatchUpdate.java
@@ -56,7 +56,6 @@
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Module;
-import com.google.inject.Singleton;
import com.google.inject.assistedinject.Assisted;
import java.io.IOException;
import java.sql.Timestamp;
@@ -104,56 +103,26 @@
return new FactoryModule() {
@Override
public void configure() {
- factory(BatchUpdate.AssistedFactory.class);
+ factory(BatchUpdate.Factory.class);
}
};
}
- interface AssistedFactory {
+ // TODO(dborowitz): Make this package-private to force all callers to use RetryHelper.
+ public interface Factory {
BatchUpdate create(ReviewDb db, Project.NameKey project, CurrentUser user, Timestamp when);
}
- @Singleton
- public static class Factory {
- private final AssistedFactory assistedFactory;
-
- // TODO(dborowitz): Make this non-injectable to force all callers to use RetryHelper.
- @Inject
- Factory(AssistedFactory assistedFactory) {
- this.assistedFactory = assistedFactory;
- }
-
- public BatchUpdate create(
- ReviewDb db, Project.NameKey project, CurrentUser user, Timestamp when) {
- return assistedFactory.create(db, project, user, when);
- }
-
- @SuppressWarnings({"rawtypes", "unchecked"})
- public void execute(
- Collection<BatchUpdate> updates, BatchUpdateListener listener, boolean dryRun)
- throws UpdateException, RestApiException {
- requireNonNull(listener);
- checkDifferentProject(updates);
- BatchUpdate.execute(ImmutableList.copyOf(updates), listener, dryRun);
- }
-
- private static void checkDifferentProject(Collection<BatchUpdate> updates) {
- Multiset<Project.NameKey> projectCounts =
- updates.stream().map(u -> u.project).collect(toImmutableMultiset());
- checkArgument(
- projectCounts.entrySet().size() == updates.size(),
- "updates must all be for different projects, got: %s",
- projectCounts);
- }
- }
-
- private static void execute(
- ImmutableList<BatchUpdate> updates, BatchUpdateListener listener, boolean dryrun)
+ public static void execute(
+ Collection<BatchUpdate> updates, BatchUpdateListener listener, boolean dryrun)
throws UpdateException, RestApiException {
+ requireNonNull(listener);
if (updates.isEmpty()) {
return;
}
+ checkDifferentProject(updates);
+
try {
@SuppressWarnings("deprecation")
List<com.google.common.util.concurrent.CheckedFuture<?, IOException>> indexFutures =
@@ -235,6 +204,15 @@
}
}
+ private static void checkDifferentProject(Collection<BatchUpdate> updates) {
+ Multiset<Project.NameKey> projectCounts =
+ updates.stream().map(u -> u.project).collect(toImmutableMultiset());
+ checkArgument(
+ projectCounts.entrySet().size() == updates.size(),
+ "updates must all be for different projects, got: %s",
+ projectCounts);
+ }
+
private static Order getOrder(
Collection<? extends BatchUpdate> updates, BatchUpdateListener listener) {
Order o = null;
diff --git a/java/com/google/gerrit/server/update/RetryHelper.java b/java/com/google/gerrit/server/update/RetryHelper.java
index c40bc43..d2bccf1 100644
--- a/java/com/google/gerrit/server/update/RetryHelper.java
+++ b/java/com/google/gerrit/server/update/RetryHelper.java
@@ -147,21 +147,18 @@
@Nullable private final Consumer<RetryerBuilder<?>> overwriteDefaultRetryerStrategySetup;
@Inject
- RetryHelper(
- @GerritServerConfig Config cfg,
- Metrics metrics,
- BatchUpdate.AssistedFactory batchUpdateFactory) {
- this(cfg, metrics, batchUpdateFactory, null);
+ RetryHelper(@GerritServerConfig Config cfg, Metrics metrics, BatchUpdate.Factory updateFactory) {
+ this(cfg, metrics, updateFactory, null);
}
@VisibleForTesting
public RetryHelper(
@GerritServerConfig Config cfg,
Metrics metrics,
- BatchUpdate.AssistedFactory batchUpdateFactory,
+ BatchUpdate.Factory updateFactory,
@Nullable Consumer<RetryerBuilder<?>> overwriteDefaultRetryerStrategySetup) {
this.metrics = metrics;
- this.updateFactory = new BatchUpdate.Factory(batchUpdateFactory);
+ this.updateFactory = updateFactory;
Duration defaultTimeout =
Duration.ofMillis(
