Add AccountCacheImpl.AccountCacheBindingModule This change is a followup for Icfcf3ad89b5da37a4d345d291be64e51aea4eb07. For Google-internal reasons, `AccountCacheImpl` can no longer be annotated as a Singleton. Instead, the scope declaration should be done upon binding. To avoid misconfiguration, this change is adding a binding module. This is part of a series of changes intended to add support for different account storage systems. Release-Notes: skip Bug: Google b/289356238 Change-Id: Ib4a49405ee2c989620ea05329c27fe16c7a00c7d
diff --git a/java/com/google/gerrit/httpd/init/WebAppInitializer.java b/java/com/google/gerrit/httpd/init/WebAppInitializer.java index 8d7fe67..ba88617 100644 --- a/java/com/google/gerrit/httpd/init/WebAppInitializer.java +++ b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
@@ -51,7 +51,6 @@ import com.google.gerrit.server.LibModuleType; import com.google.gerrit.server.ModuleOverloader; import com.google.gerrit.server.StartupChecks.StartupChecksModule; -import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountCacheImpl; import com.google.gerrit.server.account.AccountDeactivator.AccountDeactivatorModule; import com.google.gerrit.server.account.InternalAccountDirectory.InternalAccountDirectoryModule; @@ -313,13 +312,7 @@ modules.add(new MimeUtil2Module()); modules.add(cfgInjector.getInstance(AccountCacheImpl.AccountCacheModule.class)); - modules.add( - new AbstractModule() { - @Override - protected void configure() { - bind(AccountCache.class).to(AccountCacheImpl.class); - } - }); + modules.add(cfgInjector.getInstance(AccountCacheImpl.AccountCacheBindingModule.class)); modules.add(cfgInjector.getInstance(GerritGlobalModule.class)); modules.add(new GerritApiModule());
diff --git a/java/com/google/gerrit/pgm/Daemon.java b/java/com/google/gerrit/pgm/Daemon.java index ff00391..d213a60 100644 --- a/java/com/google/gerrit/pgm/Daemon.java +++ b/java/com/google/gerrit/pgm/Daemon.java
@@ -60,7 +60,6 @@ import com.google.gerrit.server.LibModuleType; import com.google.gerrit.server.ModuleOverloader; import com.google.gerrit.server.StartupChecks.StartupChecksModule; -import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountCacheImpl; import com.google.gerrit.server.account.AccountDeactivator.AccountDeactivatorModule; import com.google.gerrit.server.account.InternalAccountDirectory.InternalAccountDirectoryModule; @@ -472,13 +471,7 @@ modules.add(new MimeUtil2Module()); modules.add(cfgInjector.getInstance(AccountCacheImpl.AccountCacheModule.class)); - modules.add( - new AbstractModule() { - @Override - protected void configure() { - bind(AccountCache.class).to(AccountCacheImpl.class); - } - }); + modules.add(cfgInjector.getInstance(AccountCacheImpl.AccountCacheBindingModule.class)); modules.add(new AccountNoteDbWriteStorageModule()); modules.add(new AccountNoteDbReadStorageModule());
diff --git a/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java index 58abe12..f45f1be 100644 --- a/java/com/google/gerrit/pgm/util/BatchProgramModule.java +++ b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -34,7 +34,6 @@ import com.google.gerrit.server.LibModuleLoader; import com.google.gerrit.server.LibModuleType; import com.google.gerrit.server.ModuleOverloader; -import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountCacheImpl; import com.google.gerrit.server.account.AccountVisibilityProvider; import com.google.gerrit.server.account.CapabilityCollection; @@ -186,6 +185,7 @@ modules.add(new GroupModule()); modules.add(new NoteDbModule()); modules.add(AccountCacheImpl.module()); + modules.add(AccountCacheImpl.bindingModule()); modules.add(ConflictsCacheImpl.module()); modules.add(DefaultPreferencesCacheImpl.module()); modules.add(GroupCacheImpl.module()); @@ -230,7 +230,6 @@ bind(WorkInProgressStateChanged.class).toInstance(WorkInProgressStateChanged.DISABLED); bind(AccountVisibility.class).toProvider(AccountVisibilityProvider.class).in(SINGLETON); bind(AttentionSetObserver.class).toInstance(AttentionSetObserver.DISABLED); - bind(AccountCache.class).to(AccountCacheImpl.class); ModuleOverloader.override( modules,
diff --git a/java/com/google/gerrit/server/account/AccountCacheImpl.java b/java/com/google/gerrit/server/account/AccountCacheImpl.java index 8549409..e01b206 100644 --- a/java/com/google/gerrit/server/account/AccountCacheImpl.java +++ b/java/com/google/gerrit/server/account/AccountCacheImpl.java
@@ -16,6 +16,7 @@ import static com.google.gerrit.server.account.AccountCacheImpl.AccountCacheModule.ACCOUNT_CACHE_MODULE; import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME; +import static com.google.inject.Scopes.SINGLETON; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; @@ -37,6 +38,7 @@ import com.google.gerrit.server.logging.TraceContext; import com.google.gerrit.server.logging.TraceContext.TraceTimer; import com.google.gerrit.server.util.time.TimeUtil; +import com.google.inject.AbstractModule; import com.google.inject.Inject; import com.google.inject.Module; import com.google.inject.Singleton; @@ -51,8 +53,17 @@ import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; -/** Caches important (but small) account state to avoid database hits. */ -@Singleton +/** + * Caches important (but small) account state to avoid database hits. + * + * <p>This class should be bounded as a Singleton. However, due to internal limitations in Google, + * it cannot be marked as a singleton. The common installation pattern should therefore be: + * + * <pre>{@code + * install(AccountCacheImpl.module()); + * install(AccountCacheImpl.bindingModule()); + * }</pre> + */ public class AccountCacheImpl implements AccountCache { @ModuleImpl(name = ACCOUNT_CACHE_MODULE) public static class AccountCacheModule extends CacheModule { @@ -65,8 +76,14 @@ .keySerializer(CachedAccountDetails.Key.Serializer.INSTANCE) .valueSerializer(CachedAccountDetails.Serializer.INSTANCE) .loader(Loader.class); + } + } - bind(AccountCacheImpl.class); + public static class AccountCacheBindingModule extends AbstractModule { + @Override + protected void configure() { + bind(AccountCacheImpl.class).in(SINGLETON); + bind(AccountCache.class).to(AccountCacheImpl.class).in(SINGLETON); } } @@ -78,6 +95,10 @@ return new AccountCacheModule(); } + public static Module bindingModule() { + return new AccountCacheBindingModule(); + } + private final ExternalIds externalIds; private final LoadingCache<CachedAccountDetails.Key, CachedAccountDetails> accountDetailsCache; private final GitRepositoryManager repoManager;
diff --git a/java/com/google/gerrit/testing/InMemoryModule.java b/java/com/google/gerrit/testing/InMemoryModule.java index 9f1e4c5..b0045e3 100644 --- a/java/com/google/gerrit/testing/InMemoryModule.java +++ b/java/com/google/gerrit/testing/InMemoryModule.java
@@ -47,7 +47,6 @@ import com.google.gerrit.server.PluginUser; import com.google.gerrit.server.Sequence; import com.google.gerrit.server.Sequence.LightweightGroups; -import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountCacheImpl; import com.google.gerrit.server.account.GroupBackend; import com.google.gerrit.server.account.storage.notedb.AccountNoteDbReadStorageModule; @@ -202,7 +201,7 @@ bind(MetricMaker.class).to(DisabledMetricMaker.class); install(cfgInjector.getInstance(AccountCacheImpl.AccountCacheModule.class)); - bind(AccountCache.class).to(AccountCacheImpl.class); + install(cfgInjector.getInstance(AccountCacheImpl.AccountCacheBindingModule.class)); install(cfgInjector.getInstance(GerritGlobalModule.class)); install(new AccountNoteDbWriteStorageModule());