MetaDataUpdate: Make factories singletons

Typically factories are singletons, which means callers are tempted to
inject them directly instead of using providers, which would be the
correct thing to do in this case.

However, the only thing that is non-singleton-ish about them was that
they directly injected the @GerritPersonIdent; there's no need for this,
as they can inject a provider instead.

Change-Id: If7fbe2ccdc26a5cfc258efaf6495eb4ca7260b2d
This commit is contained in:
Dave Borowitz
2018-12-05 12:07:28 -08:00
parent bd2844f3ac
commit 45db4616c2

View File

@@ -22,6 +22,7 @@ import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
import com.google.inject.Singleton;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
import java.io.IOException; import java.io.IOException;
import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.errors.RepositoryNotFoundException;
@@ -33,21 +34,22 @@ import org.eclipse.jgit.lib.Repository;
/** Helps with the updating of a {@link VersionedMetaData}. */ /** Helps with the updating of a {@link VersionedMetaData}. */
public class MetaDataUpdate implements AutoCloseable { public class MetaDataUpdate implements AutoCloseable {
@Singleton
public static class User { public static class User {
private final InternalFactory factory; private final InternalFactory factory;
private final GitRepositoryManager mgr; private final GitRepositoryManager mgr;
private final PersonIdent serverIdent; private final Provider<PersonIdent> serverIdentProvider;
private final Provider<IdentifiedUser> identifiedUser; private final Provider<IdentifiedUser> identifiedUser;
@Inject @Inject
User( User(
InternalFactory factory, InternalFactory factory,
GitRepositoryManager mgr, GitRepositoryManager mgr,
@GerritPersonIdent PersonIdent serverIdent, @GerritPersonIdent Provider<PersonIdent> serverIdentProvider,
Provider<IdentifiedUser> identifiedUser) { Provider<IdentifiedUser> identifiedUser) {
this.factory = factory; this.factory = factory;
this.mgr = mgr; this.mgr = mgr;
this.serverIdent = serverIdent; this.serverIdentProvider = serverIdentProvider;
this.identifiedUser = identifiedUser; this.identifiedUser = identifiedUser;
} }
@@ -126,29 +128,31 @@ public class MetaDataUpdate implements AutoCloseable {
public MetaDataUpdate create( public MetaDataUpdate create(
Project.NameKey name, Repository repository, IdentifiedUser user, BatchRefUpdate batch) { Project.NameKey name, Repository repository, IdentifiedUser user, BatchRefUpdate batch) {
MetaDataUpdate md = factory.create(name, repository, batch); MetaDataUpdate md = factory.create(name, repository, batch);
md.getCommitBuilder().setCommitter(serverIdent); md.getCommitBuilder().setCommitter(serverIdentProvider.get());
md.setAuthor(user); md.setAuthor(user);
return md; return md;
} }
private PersonIdent createPersonIdent(IdentifiedUser user) { private PersonIdent createPersonIdent(IdentifiedUser user) {
PersonIdent serverIdent = serverIdentProvider.get();
return user.newCommitterIdent(serverIdent.getWhen(), serverIdent.getTimeZone()); return user.newCommitterIdent(serverIdent.getWhen(), serverIdent.getTimeZone());
} }
} }
@Singleton
public static class Server { public static class Server {
private final InternalFactory factory; private final InternalFactory factory;
private final GitRepositoryManager mgr; private final GitRepositoryManager mgr;
private final PersonIdent serverIdent; private final Provider<PersonIdent> serverIdentProvider;
@Inject @Inject
Server( Server(
InternalFactory factory, InternalFactory factory,
GitRepositoryManager mgr, GitRepositoryManager mgr,
@GerritPersonIdent PersonIdent serverIdent) { @GerritPersonIdent Provider<PersonIdent> serverIdentProvider) {
this.factory = factory; this.factory = factory;
this.mgr = mgr; this.mgr = mgr;
this.serverIdent = serverIdent; this.serverIdentProvider = serverIdentProvider;
} }
public MetaDataUpdate create(Project.NameKey name) public MetaDataUpdate create(Project.NameKey name)
@@ -162,6 +166,7 @@ public class MetaDataUpdate implements AutoCloseable {
Repository repo = mgr.openRepository(name); Repository repo = mgr.openRepository(name);
MetaDataUpdate md = factory.create(name, repo, batch); MetaDataUpdate md = factory.create(name, repo, batch);
md.setCloseRepository(true); md.setCloseRepository(true);
PersonIdent serverIdent = serverIdentProvider.get();
md.getCommitBuilder().setAuthor(serverIdent); md.getCommitBuilder().setAuthor(serverIdent);
md.getCommitBuilder().setCommitter(serverIdent); md.getCommitBuilder().setCommitter(serverIdent);
return md; return md;