Cache the service user db This both improves performance and reduces code repetition. Change-Id: I12c502670bf30e94eebde3fed976b7ff89ca5ca6
diff --git a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/CreateServiceUser.java b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/CreateServiceUser.java index 13ff20c..0bb2cbf 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/CreateServiceUser.java +++ b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/CreateServiceUser.java
@@ -87,6 +87,7 @@ private final DateFormat rfc2822DateFormatter; private final Provider<GetConfig> getConfig; private final AccountLoader.Factory accountLoader; + private final StorageCache storageCache; @Inject CreateServiceUser( @@ -99,7 +100,8 @@ MetaDataUpdate.User metaDataUpdateFactory, ProjectCache projectCache, Provider<GetConfig> getConfig, - AccountLoader.Factory accountLoader) { + AccountLoader.Factory accountLoader, + StorageCache storageCache) { this.cfg = cfgFactory.getFromGerritConfig(pluginName); this.configProvider = configProvider; this.createAccount = createAccount; @@ -120,6 +122,7 @@ Calendar.getInstance(gerritIdent.getTimeZone(), Locale.US)); this.getConfig = getConfig; this.accountLoader = accountLoader; + this.storageCache = storageCache; } @Override @@ -192,6 +195,7 @@ md.setMessage("Create service user '" + username + "'\n"); update.commit(md); + storageCache.invalidate(); } ServiceUserInfo info = new ServiceUserInfo(response); AccountLoader al = accountLoader.create(true);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/GetOwner.java b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/GetOwner.java index c0c245c..bf7227b 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/GetOwner.java +++ b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/GetOwner.java
@@ -14,7 +14,6 @@ package com.googlesource.gerrit.plugins.serviceuser; -import static com.google.gerrit.server.api.ApiUtil.asRestApiException; import static com.googlesource.gerrit.plugins.serviceuser.CreateServiceUser.KEY_OWNER; import static com.googlesource.gerrit.plugins.serviceuser.CreateServiceUser.USER; @@ -25,51 +24,31 @@ import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.extensions.restapi.TopLevelResource; -import com.google.gerrit.server.config.AllProjectsName; -import com.google.gerrit.server.git.meta.MetaDataUpdate; import com.google.gerrit.server.permissions.PermissionBackendException; -import com.google.gerrit.server.project.ProjectLevelConfig; import com.google.gerrit.server.restapi.group.GroupJson; import com.google.gerrit.server.restapi.group.GroupsCollection; import com.google.inject.Inject; -import com.google.inject.Provider; import com.google.inject.Singleton; import java.io.IOException; -import org.eclipse.jgit.errors.ConfigInvalidException; @Singleton class GetOwner implements RestReadView<ServiceUserResource> { private final GroupsCollection groups; private final GroupJson json; - private final Provider<ProjectLevelConfig.Bare> configProvider; - private final MetaDataUpdate.User metaDataUpdateFactory; - private final AllProjectsName allProjectsName; + private final StorageCache storageCache; @Inject - GetOwner( - GroupsCollection groups, - Provider<ProjectLevelConfig.Bare> configProvider, - AllProjectsName allProjectsName, - MetaDataUpdate.User metaDataUpdateFactory, - GroupJson json) { + GetOwner(GroupsCollection groups, GroupJson json, StorageCache storageCache) { this.groups = groups; - this.configProvider = configProvider; - this.allProjectsName = allProjectsName; - this.metaDataUpdateFactory = metaDataUpdateFactory; this.json = json; + this.storageCache = storageCache; } @Override public Response<GroupInfo> apply(ServiceUserResource rsrc) throws IOException, RestApiException, PermissionBackendException { - ProjectLevelConfig.Bare storage = configProvider.get(); - try (MetaDataUpdate md = metaDataUpdateFactory.create(allProjectsName)) { - storage.load(md); - } catch (ConfigInvalidException e) { - throw asRestApiException("Invalid configuration", e); - } String owner = - storage.getConfig().getString(USER, rsrc.getUser().getUserName().get(), KEY_OWNER); + storageCache.get().getString(USER, rsrc.getUser().getUserName().get(), KEY_OWNER); if (owner != null) { GroupDescription.Basic group = groups.parse(TopLevelResource.INSTANCE, IdString.fromDecoded(owner)).getGroup();
diff --git a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/GetServiceUser.java b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/GetServiceUser.java index 6b08307..784c6b0 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/GetServiceUser.java +++ b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/GetServiceUser.java
@@ -28,16 +28,12 @@ import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.server.account.AccountLoader; -import com.google.gerrit.server.config.AllProjectsName; -import com.google.gerrit.server.git.meta.MetaDataUpdate; import com.google.gerrit.server.permissions.PermissionBackendException; -import com.google.gerrit.server.project.ProjectLevelConfig; import com.google.gerrit.server.restapi.account.GetAccount; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; import java.io.IOException; -import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.Config; @Singleton @@ -45,37 +41,25 @@ private final Provider<GetAccount> getAccount; private final GetOwner getOwner; private final AccountLoader.Factory accountLoader; - private final Provider<ProjectLevelConfig.Bare> configProvider; - private final MetaDataUpdate.User metaDataUpdateFactory; - private final AllProjectsName allProjectsName; + private final StorageCache storageCache; @Inject GetServiceUser( Provider<GetAccount> getAccount, - Provider<ProjectLevelConfig.Bare> configProvider, - AllProjectsName allProjectsName, - MetaDataUpdate.User metaDataUpdateFactory, GetOwner getOwner, - AccountLoader.Factory accountLoader) { + AccountLoader.Factory accountLoader, + StorageCache storageCache) { this.getAccount = getAccount; - this.configProvider = configProvider; - this.allProjectsName = allProjectsName; - this.metaDataUpdateFactory = metaDataUpdateFactory; this.getOwner = getOwner; this.accountLoader = accountLoader; + this.storageCache = storageCache; } @Override public Response<ServiceUserInfo> apply(ServiceUserResource rsrc) throws IOException, RestApiException, PermissionBackendException { - ProjectLevelConfig.Bare storage = configProvider.get(); - try (MetaDataUpdate md = metaDataUpdateFactory.create(allProjectsName)) { - storage.load(md); - } catch (ConfigInvalidException e) { - throw asRestApiException("Invalid configuration", e); - } String username = rsrc.getUser().getUserName().get(); - Config db = storage.getConfig(); + Config db = storageCache.get(); if (!db.getSubsections(USER).contains(username)) { throw new ResourceNotFoundException(username); }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ListServiceUsers.java b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ListServiceUsers.java index f0f9a64..a40b989 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ListServiceUsers.java +++ b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ListServiceUsers.java
@@ -27,11 +27,8 @@ import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountState; -import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.config.ConfigResource; -import com.google.gerrit.server.git.meta.MetaDataUpdate; import com.google.gerrit.server.permissions.PermissionBackendException; -import com.google.gerrit.server.project.ProjectLevelConfig; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; @@ -48,44 +45,32 @@ private final AccountCache accountCache; private final Provider<ServiceUserCollection> serviceUsers; private final Provider<GetServiceUser> getServiceUser; - private final Provider<ProjectLevelConfig.Bare> configProvider; - private final MetaDataUpdate.User metaDataUpdateFactory; - private final AllProjectsName allProjectsName; + private final StorageCache storageCache; @Inject ListServiceUsers( Provider<CurrentUser> userProvider, - Provider<ProjectLevelConfig.Bare> configProvider, - AllProjectsName allProjectsName, - MetaDataUpdate.User metaDataUpdateFactory, AccountCache accountCache, Provider<ServiceUserCollection> serviceUsers, - Provider<GetServiceUser> getServiceUser) { + Provider<GetServiceUser> getServiceUser, + StorageCache storageCache) { this.userProvider = userProvider; - this.configProvider = configProvider; - this.allProjectsName = allProjectsName; - this.metaDataUpdateFactory = metaDataUpdateFactory; this.accountCache = accountCache; this.serviceUsers = serviceUsers; this.getServiceUser = getServiceUser; + this.storageCache = storageCache; } @Override public Response<Map<String, ServiceUserInfo>> apply(ConfigResource rscr) throws IOException, RestApiException, PermissionBackendException, ConfigInvalidException { - ProjectLevelConfig.Bare storage = configProvider.get(); - try (MetaDataUpdate md = metaDataUpdateFactory.create(allProjectsName)) { - storage.load(md); - } catch (ConfigInvalidException e) { - throw asRestApiException("Invalid configuration", e); - } CurrentUser user = userProvider.get(); if (user == null || !user.isIdentifiedUser()) { throw new AuthException("Authentication required"); } Map<String, ServiceUserInfo> accounts = Maps.newTreeMap(); - Config db = storage.getConfig(); + Config db = storageCache.get(); for (String username : db.getSubsections(USER)) { Optional<AccountState> account = accountCache.getByUsername(username); if (account.isPresent()) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/Module.java b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/Module.java index 9dc32a4..89510bb 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/Module.java +++ b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/Module.java
@@ -74,6 +74,7 @@ } }); install(new HttpModule()); + install(StorageCache.module()); } @Provides
diff --git a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/PutOwner.java b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/PutOwner.java index 120c464..32ee082 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/PutOwner.java +++ b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/PutOwner.java
@@ -63,6 +63,7 @@ private final GroupJson json; private final Provider<CurrentUser> self; private final PermissionBackend permissionBackend; + private final StorageCache storageCache; @Inject PutOwner( @@ -73,7 +74,8 @@ MetaDataUpdate.User metaDataUpdateFactory, GroupJson json, Provider<CurrentUser> self, - PermissionBackend permissionBackend) { + PermissionBackend permissionBackend, + StorageCache storageCache) { this.getConfig = getConfig; this.groups = groups; this.configProvider = configProvider; @@ -82,6 +84,7 @@ this.json = json; this.self = self; this.permissionBackend = permissionBackend; + this.storageCache = storageCache; } @Override @@ -122,6 +125,7 @@ } md.setMessage("Set owner for service user '" + rsrc.getUser().getUserName() + "'\n"); update.commit(md); + storageCache.invalidate(); } catch (ConfigInvalidException e) { throw asRestApiException("Invalid configuration", e); }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ServiceUserCollection.java b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ServiceUserCollection.java index 5a83d41..f6f50ac 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ServiceUserCollection.java +++ b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/ServiceUserCollection.java
@@ -14,7 +14,6 @@ package com.googlesource.gerrit.plugins.serviceuser; -import static com.google.gerrit.server.api.ApiUtil.asRestApiException; import static com.google.gerrit.server.permissions.GlobalPermission.ADMINISTRATE_SERVER; import static com.googlesource.gerrit.plugins.serviceuser.CreateServiceUser.KEY_CREATOR_ID; import static com.googlesource.gerrit.plugins.serviceuser.CreateServiceUser.KEY_OWNER; @@ -32,12 +31,9 @@ import com.google.gerrit.extensions.restapi.TopLevelResource; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; -import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.config.ConfigResource; -import com.google.gerrit.server.git.meta.MetaDataUpdate; import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackendException; -import com.google.gerrit.server.project.ProjectLevelConfig; import com.google.gerrit.server.restapi.account.AccountsCollection; import com.google.gerrit.server.restapi.group.GroupsCollection; import com.google.inject.Inject; @@ -45,6 +41,7 @@ import com.google.inject.Singleton; import java.io.IOException; import org.eclipse.jgit.errors.ConfigInvalidException; +import org.eclipse.jgit.lib.Config; @Singleton class ServiceUserCollection implements ChildCollection<ConfigResource, ServiceUserResource> { @@ -55,45 +52,33 @@ private final Provider<CurrentUser> userProvider; private final GroupsCollection groups; private final PermissionBackend permissionBackend; - private final Provider<ProjectLevelConfig.Bare> configProvider; - private final MetaDataUpdate.User metaDataUpdateFactory; - private final AllProjectsName allProjectsName; + private final StorageCache storageCache; @Inject ServiceUserCollection( DynamicMap<RestView<ServiceUserResource>> views, Provider<ListServiceUsers> list, Provider<AccountsCollection> accounts, - Provider<ProjectLevelConfig.Bare> configProvider, - AllProjectsName allProjectsName, - MetaDataUpdate.User metaDataUpdateFactory, Provider<CurrentUser> userProvider, GroupsCollection groups, - PermissionBackend permissionBackend) { + PermissionBackend permissionBackend, + StorageCache storageCache) { this.views = views; this.list = list; this.accounts = accounts; - this.configProvider = configProvider; - this.allProjectsName = allProjectsName; - this.metaDataUpdateFactory = metaDataUpdateFactory; this.userProvider = userProvider; this.groups = groups; this.permissionBackend = permissionBackend; + this.storageCache = storageCache; } @Override public ServiceUserResource parse(ConfigResource parent, IdString id) throws ResourceNotFoundException, AuthException, IOException, PermissionBackendException, ConfigInvalidException, RestApiException { - ProjectLevelConfig.Bare storage = configProvider.get(); - try (MetaDataUpdate md = metaDataUpdateFactory.create(allProjectsName)) { - storage.load(md); - } catch (ConfigInvalidException e) { - throw asRestApiException("Invalid configuration", e); - } IdentifiedUser serviceUser = accounts.get().parse(TopLevelResource.INSTANCE, id).getUser(); - if (serviceUser == null - || !storage.getConfig().getSubsections(USER).contains(serviceUser.getUserName().get())) { + Config db = storageCache.get(); + if (serviceUser == null || !db.getSubsections(USER).contains(serviceUser.getUserName().get())) { throw new ResourceNotFoundException(id); } CurrentUser user = userProvider.get(); @@ -102,7 +87,7 @@ } if (!permissionBackend.user(user).testOrFalse(ADMINISTRATE_SERVER)) { String username = serviceUser.getUserName().get(); - String owner = storage.getConfig().getString(USER, username, KEY_OWNER); + String owner = db.getString(USER, username, KEY_OWNER); if (owner != null) { GroupDescription.Basic group = groups.parse(TopLevelResource.INSTANCE, IdString.fromDecoded(owner)).getGroup(); @@ -111,7 +96,7 @@ } } else if (!((IdentifiedUser) user) .getAccountId() - .equals(Account.id(storage.getConfig().getInt(USER, username, KEY_CREATOR_ID, -1)))) { + .equals(Account.id(db.getInt(USER, username, KEY_CREATOR_ID, -1)))) { throw new ResourceNotFoundException(id); } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/serviceuser/StorageCache.java b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/StorageCache.java new file mode 100644 index 0000000..81f543a --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/serviceuser/StorageCache.java
@@ -0,0 +1,93 @@ +// Copyright (C) 2021 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.googlesource.gerrit.plugins.serviceuser; + +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; +import com.google.common.flogger.FluentLogger; +import com.google.gerrit.server.cache.CacheModule; +import com.google.gerrit.server.config.AllProjectsName; +import com.google.gerrit.server.git.meta.MetaDataUpdate; +import com.google.gerrit.server.project.ProjectLevelConfig; +import com.google.gerrit.server.project.ProjectLevelConfig.Bare; +import com.google.inject.Inject; +import com.google.inject.Provider; +import com.google.inject.Singleton; +import com.google.inject.name.Named; +import java.util.concurrent.ExecutionException; +import org.eclipse.jgit.lib.Config; + +@Singleton +public class StorageCache { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + + private static final String CACHE_NAME = "storage"; + private static final Object ALL = new Object(); + + static CacheModule module() { + return new CacheModule() { + @Override + protected void configure() { + cache(CACHE_NAME, Object.class, Config.class).loader(Loader.class); + bind(StorageCache.class); + } + }; + } + + private final LoadingCache<Object, Config> cache; + + @Inject + StorageCache(@Named(CACHE_NAME) LoadingCache<Object, Config> cache) { + this.cache = cache; + } + + public Config get() { + try { + return cache.get(ALL); + } catch (ExecutionException e) { + logger.atSevere().withCause(e).log("Cannot load service users"); + return new Config(); + } + } + + public void invalidate() { + cache.invalidate(ALL); + } + + static class Loader extends CacheLoader<Object, Config> { + private final Provider<Bare> configProvider; + private final MetaDataUpdate.User metaDataUpdateFactory; + private final AllProjectsName allProjects; + + @Inject + Loader( + Provider<ProjectLevelConfig.Bare> configProvider, + MetaDataUpdate.User metaDataUpdateFactory, + AllProjectsName allProjects) { + this.configProvider = configProvider; + this.metaDataUpdateFactory = metaDataUpdateFactory; + this.allProjects = allProjects; + } + + @Override + public Config load(Object key) throws Exception { + ProjectLevelConfig.Bare storage = configProvider.get(); + try (MetaDataUpdate md = metaDataUpdateFactory.create(allProjects)) { + storage.load(md); + } + return storage.getConfig(); + } + } +}