diff --git a/java/com/google/gerrit/pgm/init/InitAdminUser.java b/java/com/google/gerrit/pgm/init/InitAdminUser.java index 674f9c1e4b..45c5ea603a 100644 --- a/java/com/google/gerrit/pgm/init/InitAdminUser.java +++ b/java/com/google/gerrit/pgm/init/InitAdminUser.java @@ -20,7 +20,6 @@ import com.google.common.base.Strings; import com.google.gerrit.common.data.GroupReference; import com.google.gerrit.exceptions.NoSuchGroupException; import com.google.gerrit.extensions.client.AuthType; -import com.google.gerrit.pgm.init.api.AllUsersNameOnInitProvider; import com.google.gerrit.pgm.init.api.ConsoleUI; import com.google.gerrit.pgm.init.api.InitFlags; import com.google.gerrit.pgm.init.api.InitStep; @@ -29,7 +28,6 @@ import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.server.account.AccountSshKey; import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.account.externalids.ExternalId; -import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.group.InternalGroup; import com.google.gerrit.server.index.account.AccountIndex; import com.google.gerrit.server.index.account.AccountIndexCollection; @@ -49,7 +47,6 @@ import org.apache.commons.validator.routines.EmailValidator; public class InitAdminUser implements InitStep { private final InitFlags flags; private final ConsoleUI ui; - private final AllUsersNameOnInitProvider allUsers; private final AccountsOnInit accounts; private final VersionedAuthorizedKeysOnInit.Factory authorizedKeysFactory; private final ExternalIdsOnInit externalIds; @@ -62,7 +59,6 @@ public class InitAdminUser implements InitStep { InitAdminUser( InitFlags flags, ConsoleUI ui, - AllUsersNameOnInitProvider allUsers, AccountsOnInit accounts, VersionedAuthorizedKeysOnInit.Factory authorizedKeysFactory, ExternalIdsOnInit externalIds, @@ -70,7 +66,6 @@ public class InitAdminUser implements InitStep { GroupsOnInit groupsOnInit) { this.flags = flags; this.ui = ui; - this.allUsers = allUsers; this.accounts = accounts; this.authorizedKeysFactory = authorizedKeysFactory; this.externalIds = externalIds; @@ -140,7 +135,7 @@ public class InitAdminUser implements InitStep { authorizedKeys.save("Add SSH key for initial admin user\n"); } - AccountState as = AccountState.forAccount(new AllUsersName(allUsers.get()), a, extIds); + AccountState as = AccountState.forAccount(a, extIds); for (AccountIndex accountIndex : accountIndexCollection.getWriteIndexes()) { accountIndex.replace(as); } diff --git a/java/com/google/gerrit/server/account/AccountCacheImpl.java b/java/com/google/gerrit/server/account/AccountCacheImpl.java index 173d81d82b..3d81052a93 100644 --- a/java/com/google/gerrit/server/account/AccountCacheImpl.java +++ b/java/com/google/gerrit/server/account/AccountCacheImpl.java @@ -26,7 +26,6 @@ import com.google.gerrit.server.FanOutExecutor; import com.google.gerrit.server.account.externalids.ExternalId; import com.google.gerrit.server.account.externalids.ExternalIds; import com.google.gerrit.server.cache.CacheModule; -import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.logging.Metadata; import com.google.gerrit.server.logging.TraceContext; import com.google.gerrit.server.logging.TraceContext.TraceTimer; @@ -69,18 +68,15 @@ public class AccountCacheImpl implements AccountCache { }; } - private final AllUsersName allUsersName; private final ExternalIds externalIds; private final LoadingCache> byId; private final ExecutorService executor; @Inject AccountCacheImpl( - AllUsersName allUsersName, ExternalIds externalIds, @Named(BYID_NAME) LoadingCache> byId, @FanOutExecutor ExecutorService executor) { - this.allUsersName = allUsersName; this.externalIds = externalIds; this.byId = byId; this.executor = executor; @@ -171,7 +167,7 @@ public class AccountCacheImpl implements AccountCache { private AccountState missing(Account.Id accountId) { Account account = new Account(accountId, TimeUtil.nowTs()); account.setActive(false); - return AccountState.forAccount(allUsersName, account); + return AccountState.forAccount(account); } static class ByIdLoader extends CacheLoader> { diff --git a/java/com/google/gerrit/server/account/AccountState.java b/java/com/google/gerrit/server/account/AccountState.java index 46fde8c170..556185eb20 100644 --- a/java/com/google/gerrit/server/account/AccountState.java +++ b/java/com/google/gerrit/server/account/AccountState.java @@ -36,7 +36,6 @@ import com.google.gerrit.server.account.ProjectWatches.ProjectWatchKey; import com.google.gerrit.server.account.externalids.ExternalId; import com.google.gerrit.server.account.externalids.ExternalIdNotes; import com.google.gerrit.server.account.externalids.ExternalIds; -import com.google.gerrit.server.config.AllUsersName; import java.io.IOException; import java.util.Collection; import java.util.Optional; @@ -56,16 +55,14 @@ public class AccountState { /** * Creates an AccountState from the given account config. * - * @param allUsersName the name of the All-Users repository * @param externalIds class to access external IDs * @param accountConfig the account config, must already be loaded * @return the account state, {@link Optional#empty()} if the account doesn't exist * @throws IOException if accessing the external IDs fails */ public static Optional fromAccountConfig( - AllUsersName allUsersName, ExternalIds externalIds, AccountConfig accountConfig) - throws IOException { - return fromAccountConfig(allUsersName, externalIds, accountConfig, null); + ExternalIds externalIds, AccountConfig accountConfig) throws IOException { + return fromAccountConfig(externalIds, accountConfig, null); } /** @@ -78,7 +75,6 @@ public class AccountState { * updated the revision of the external IDs branch in account config is outdated. Hence after * updating external IDs the external ID notes must be provided. * - * @param allUsersName the name of the All-Users repository * @param externalIds class to access external IDs * @param accountConfig the account config, must already be loaded * @param extIdNotes external ID notes, must already be loaded, may be {@code null} @@ -86,10 +82,7 @@ public class AccountState { * @throws IOException if accessing the external IDs fails */ public static Optional fromAccountConfig( - AllUsersName allUsersName, - ExternalIds externalIds, - AccountConfig accountConfig, - @Nullable ExternalIdNotes extIdNotes) + ExternalIds externalIds, AccountConfig accountConfig, @Nullable ExternalIdNotes extIdNotes) throws IOException { if (!accountConfig.getLoadedAccount().isPresent()) { return Optional.empty(); @@ -115,39 +108,29 @@ public class AccountState { return Optional.of( new AccountState( - allUsersName, - account, - extIds, - projectWatches, - generalPreferences, - diffPreferences, - editPreferences)); + account, extIds, projectWatches, generalPreferences, diffPreferences, editPreferences)); } /** * Creates an AccountState for a given account with no external IDs, no project watches and * default preferences. * - * @param allUsersName the name of the All-Users repository * @param account the account * @return the account state */ - public static AccountState forAccount(AllUsersName allUsersName, Account account) { - return forAccount(allUsersName, account, ImmutableSet.of()); + public static AccountState forAccount(Account account) { + return forAccount(account, ImmutableSet.of()); } /** * Creates an AccountState for a given account with no project watches and default preferences. * - * @param allUsersName the name of the All-Users repository * @param account the account * @param extIds the external IDs * @return the account state */ - public static AccountState forAccount( - AllUsersName allUsersName, Account account, Collection extIds) { + public static AccountState forAccount(Account account, Collection extIds) { return new AccountState( - allUsersName, account, ImmutableSet.copyOf(extIds), ImmutableMap.of(), @@ -156,7 +139,6 @@ public class AccountState { EditPreferencesInfo.defaults()); } - private final AllUsersName allUsersName; private final Account account; private final ImmutableSet externalIds; private final Optional userName; @@ -167,14 +149,12 @@ public class AccountState { private Cache, Object> properties; private AccountState( - AllUsersName allUsersName, Account account, ImmutableSet externalIds, ImmutableMap> projectWatches, GeneralPreferencesInfo generalPreferences, DiffPreferencesInfo diffPreferences, EditPreferencesInfo editPreferences) { - this.allUsersName = allUsersName; this.account = account; this.externalIds = externalIds; this.userName = ExternalId.getUserName(externalIds); @@ -184,10 +164,6 @@ public class AccountState { this.editPreferences = editPreferences; } - public AllUsersName getAllUsersNameForIndexing() { - return allUsersName; - } - /** Get the cached account metadata. */ public Account getAccount() { return account; diff --git a/java/com/google/gerrit/server/account/Accounts.java b/java/com/google/gerrit/server/account/Accounts.java index f3758bf2c4..dbe7ba7b4c 100644 --- a/java/com/google/gerrit/server/account/Accounts.java +++ b/java/com/google/gerrit/server/account/Accounts.java @@ -134,9 +134,7 @@ public class Accounts { private Optional read(Repository allUsersRepository, Account.Id accountId) throws IOException, ConfigInvalidException { return AccountState.fromAccountConfig( - allUsersName, - externalIds, - new AccountConfig(accountId, allUsersName, allUsersRepository).load()); + externalIds, new AccountConfig(accountId, allUsersName, allUsersRepository).load()); } public static Stream readUserRefs(Repository repo) throws IOException { diff --git a/java/com/google/gerrit/server/account/AccountsUpdate.java b/java/com/google/gerrit/server/account/AccountsUpdate.java index 20a1c978f1..cd47945998 100644 --- a/java/com/google/gerrit/server/account/AccountsUpdate.java +++ b/java/com/google/gerrit/server/account/AccountsUpdate.java @@ -321,7 +321,7 @@ public class AccountsUpdate { AccountConfig accountConfig = read(r, accountId); Account account = accountConfig.getNewAccount(new Timestamp(committerIdent.getWhen().getTime())); - AccountState accountState = AccountState.forAccount(allUsersName, account); + AccountState accountState = AccountState.forAccount(account); InternalAccountUpdate.Builder updateBuilder = InternalAccountUpdate.builder(); updater.update(accountState, updateBuilder); @@ -377,7 +377,7 @@ public class AccountsUpdate { r -> { AccountConfig accountConfig = read(r, accountId); Optional account = - AccountState.fromAccountConfig(allUsersName, externalIds, accountConfig); + AccountState.fromAccountConfig(externalIds, accountConfig); if (!account.isPresent()) { return null; } @@ -592,8 +592,7 @@ public class AccountsUpdate { } public AccountState getAccount() throws IOException { - return AccountState.fromAccountConfig(allUsersName, externalIds, accountConfig, extIdNotes) - .get(); + return AccountState.fromAccountConfig(externalIds, accountConfig, extIdNotes).get(); } public ExternalIdNotes getExternalIdNotes() { diff --git a/java/com/google/gerrit/server/index/account/AccountField.java b/java/com/google/gerrit/server/index/account/AccountField.java index ab3a96d3e8..b881a35b9d 100644 --- a/java/com/google/gerrit/server/index/account/AccountField.java +++ b/java/com/google/gerrit/server/index/account/AccountField.java @@ -31,6 +31,8 @@ import com.google.gerrit.index.SchemaUtil; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.account.externalids.ExternalId; +import com.google.gerrit.server.config.AllUsersName; +import com.google.gerrit.server.config.AllUsersNameProvider; import java.sql.Timestamp; import java.util.Arrays; import java.util.Collections; @@ -144,7 +146,11 @@ public class AccountField { RefState.create( RefNames.refsUsers(a.getAccount().getId()), ObjectId.fromString(a.getAccount().getMetaId())) - .toByteArray(a.getAllUsersNameForIndexing())); + // We use the default AllUsers name to avoid having to pass around that + // variable just for indexing. + // This field is only used for staleness detection which will discover the + // default name and replace it with the actually configured name. + .toByteArray(new AllUsersName(AllUsersNameProvider.DEFAULT))); }); /** diff --git a/java/com/google/gerrit/server/index/account/StalenessChecker.java b/java/com/google/gerrit/server/index/account/StalenessChecker.java index 46647002f3..0423bb915d 100644 --- a/java/com/google/gerrit/server/index/account/StalenessChecker.java +++ b/java/com/google/gerrit/server/index/account/StalenessChecker.java @@ -32,6 +32,7 @@ import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.server.account.externalids.ExternalId; import com.google.gerrit.server.account.externalids.ExternalIds; import com.google.gerrit.server.config.AllUsersName; +import com.google.gerrit.server.config.AllUsersNameProvider; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.index.IndexUtils; import com.google.inject.Inject; @@ -106,7 +107,11 @@ public class StalenessChecker { for (Map.Entry e : RefState.parseStates(result.get().getValue(AccountField.REF_STATE)).entries()) { - try (Repository repo = repoManager.openRepository(e.getKey())) { + // Custom All-Users repository names are not indexed. Instead, the default name is used. + // Therefore, defer to the currently configured All-Users name. + Project.NameKey repoName = + e.getKey().get().equals(AllUsersNameProvider.DEFAULT) ? allUsersName : e.getKey(); + try (Repository repo = repoManager.openRepository(repoName)) { if (!e.getValue().match(repo)) { // Ref was modified since the account was indexed. return true; diff --git a/java/com/google/gerrit/testing/FakeAccountCache.java b/java/com/google/gerrit/testing/FakeAccountCache.java index b99a32d7df..c86bcf4e30 100644 --- a/java/com/google/gerrit/testing/FakeAccountCache.java +++ b/java/com/google/gerrit/testing/FakeAccountCache.java @@ -20,8 +20,6 @@ import com.google.gerrit.common.Nullable; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountState; -import com.google.gerrit.server.config.AllUsersName; -import com.google.gerrit.server.config.AllUsersNameProvider; import com.google.gerrit.server.util.time.TimeUtil; import java.util.HashMap; import java.util.Map; @@ -78,6 +76,6 @@ public class FakeAccountCache implements AccountCache { } private static AccountState newState(Account account) { - return AccountState.forAccount(new AllUsersName(AllUsersNameProvider.DEFAULT), account); + return AccountState.forAccount(account); } } diff --git a/javatests/com/google/gerrit/server/account/AccountResolverTest.java b/javatests/com/google/gerrit/server/account/AccountResolverTest.java index d4ad7d7751..5470e3c9ca 100644 --- a/javatests/com/google/gerrit/server/account/AccountResolverTest.java +++ b/javatests/com/google/gerrit/server/account/AccountResolverTest.java @@ -27,7 +27,6 @@ import com.google.gerrit.server.account.AccountResolver.Result; import com.google.gerrit.server.account.AccountResolver.Searcher; import com.google.gerrit.server.account.AccountResolver.StringSearcher; import com.google.gerrit.server.account.AccountResolver.UnresolvableAccountException; -import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.util.time.TimeUtil; import java.util.Arrays; import java.util.function.Predicate; @@ -329,14 +328,13 @@ public class AccountResolverTest { } private AccountState newAccount(int id) { - return AccountState.forAccount( - new AllUsersName("All-Users"), new Account(Account.id(id), TimeUtil.nowTs())); + return AccountState.forAccount(new Account(Account.id(id), TimeUtil.nowTs())); } private AccountState newInactiveAccount(int id) { Account a = new Account(Account.id(id), TimeUtil.nowTs()); a.setActive(false); - return AccountState.forAccount(new AllUsersName("All-Users"), a); + return AccountState.forAccount(a); } private static ImmutableSet ids(int... ids) { diff --git a/javatests/com/google/gerrit/server/index/account/AccountFieldTest.java b/javatests/com/google/gerrit/server/index/account/AccountFieldTest.java index 5573be7120..0f73cc5378 100644 --- a/javatests/com/google/gerrit/server/index/account/AccountFieldTest.java +++ b/javatests/com/google/gerrit/server/index/account/AccountFieldTest.java @@ -39,8 +39,7 @@ public class AccountFieldTest { Account account = new Account(Account.id(1), TimeUtil.nowTs()); String metaId = "0e39795bb25dc914118224995c53c5c36923a461"; account.setMetaId(metaId); - List values = - toStrings(AccountField.REF_STATE.get(AccountState.forAccount(allUsersName, account))); + List values = toStrings(AccountField.REF_STATE.get(AccountState.forAccount(account))); assertThat(values).hasSize(1); String expectedValue = allUsersName.get() + ":" + RefNames.refsUsers(account.getId()) + ":" + metaId; @@ -68,7 +67,7 @@ public class AccountFieldTest { List values = toStrings( AccountField.EXTERNAL_ID_STATE.get( - AccountState.forAccount(null, account, ImmutableSet.of(extId1, extId2)))); + AccountState.forAccount(account, ImmutableSet.of(extId1, extId2)))); String expectedValue1 = extId1.key().sha1().name() + ":" + extId1.blobId().name(); String expectedValue2 = extId2.key().sha1().name() + ":" + extId2.blobId().name(); assertThat(values).containsExactly(expectedValue1, expectedValue2); diff --git a/javatests/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java b/javatests/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java index 128279f6bb..0e04739a24 100644 --- a/javatests/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java +++ b/javatests/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java @@ -25,8 +25,6 @@ import com.google.gerrit.mail.Address; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountState; -import com.google.gerrit.server.config.AllUsersName; -import com.google.gerrit.server.config.AllUsersNameProvider; import com.google.gerrit.server.util.time.TimeUtil; import java.util.Arrays; import java.util.List; @@ -383,6 +381,6 @@ public class FromAddressGeneratorProviderTest { final Account account = new Account(userId, TimeUtil.nowTs()); account.setFullName(name); account.setPreferredEmail(email); - return AccountState.forAccount(new AllUsersName(AllUsersNameProvider.DEFAULT), account); + return AccountState.forAccount(account); } }