diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java index 0d68f4abed..570621f8da 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java @@ -45,7 +45,6 @@ import com.google.common.io.BaseEncoding; import com.google.common.util.concurrent.AtomicLongMap; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.AccountCreator; -import com.google.gerrit.acceptance.GerritConfig; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.Sandboxed; import com.google.gerrit.acceptance.TestAccount; @@ -1024,7 +1023,6 @@ public class AccountIT extends AbstractDaemonTest { @Test @Sandboxed - @GerritConfig(name = "user.readAccountsFromGit", value = "true") public void deleteUserBranchWithAccessDatabaseCapability() throws Exception { allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE); grant( @@ -1282,10 +1280,9 @@ public class AccountIT extends AbstractDaemonTest { } @Test - @GerritConfig(name = "user.readAccountsFromGit", value = "true") public void checkMetaId() throws Exception { // metaId is set when account is loaded - assertThat(accounts.get(db, admin.getId()).getMetaId()).isEqualTo(getMetaId(admin.getId())); + assertThat(accounts.get(admin.getId()).getMetaId()).isEqualTo(getMetaId(admin.getId())); // metaId is set when account is created AccountsUpdate au = accountsUpdate.create(); diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java index 01aec6e010..1f095e0540 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java @@ -239,9 +239,9 @@ class BecomeAnyAccountLoginServlet extends HttpServlet { } catch (NumberFormatException nfe) { return null; } - try (ReviewDb db = schema.open()) { - return auth(accounts.get(db, id)); - } catch (OrmException | IOException | ConfigInvalidException e) { + try { + return auth(accounts.get(id)); + } catch (IOException | ConfigInvalidException e) { getServletContext().log("cannot query database", e); return null; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java index 16901ed213..5ac7ac4cb5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java @@ -208,7 +208,7 @@ public class AccountCacheImpl implements AccountCache { private Optional load(ReviewDb db, Account.Id who) throws OrmException, IOException, ConfigInvalidException { - Account account = accounts.get(db, who); + Account account = accounts.get(who); if (account == null) { return Optional.empty(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResolver.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResolver.java index 894f7a18e4..94f63a7231 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResolver.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResolver.java @@ -98,7 +98,7 @@ public class AccountResolver { Matcher m = Pattern.compile("^.* \\(([1-9][0-9]*)\\)$").matcher(nameOrEmail); if (m.matches()) { Account.Id id = Account.Id.parse(m.group(1)); - if (exists(db, id)) { + if (accounts.get(id) != null) { return Collections.singleton(id); } return Collections.emptySet(); @@ -106,7 +106,7 @@ public class AccountResolver { if (nameOrEmail.matches("^[1-9][0-9]*$")) { Account.Id id = Account.Id.parse(nameOrEmail); - if (exists(db, id)) { + if (accounts.get(id) != null) { return Collections.singleton(id); } return Collections.emptySet(); @@ -122,11 +122,6 @@ public class AccountResolver { return findAllByNameOrEmail(db, nameOrEmail); } - private boolean exists(ReviewDb db, Account.Id id) - throws OrmException, IOException, ConfigInvalidException { - return accounts.get(db, id) != null; - } - /** * Locate exactly one account matching the name or name/email string. * diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/Accounts.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/Accounts.java index cb8287926d..cc35438541 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/Accounts.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/Accounts.java @@ -20,12 +20,9 @@ import static java.util.stream.Collectors.toSet; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.RefNames; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.config.AllUsersName; -import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.mail.send.OutgoingEmailValidator; -import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Singleton; import java.io.IOException; @@ -36,7 +33,6 @@ import java.util.Objects; import java.util.Set; import java.util.stream.Stream; import org.eclipse.jgit.errors.ConfigInvalidException; -import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Repository; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -46,47 +42,35 @@ import org.slf4j.LoggerFactory; public class Accounts { private static final Logger log = LoggerFactory.getLogger(Accounts.class); - private final boolean readFromGit; private final GitRepositoryManager repoManager; private final AllUsersName allUsersName; private final OutgoingEmailValidator emailValidator; @Inject Accounts( - @GerritServerConfig Config cfg, GitRepositoryManager repoManager, AllUsersName allUsersName, OutgoingEmailValidator emailValidator) { - this.readFromGit = cfg.getBoolean("user", null, "readAccountsFromGit", true); this.repoManager = repoManager; this.allUsersName = allUsersName; this.emailValidator = emailValidator; } - public Account get(ReviewDb db, Account.Id accountId) - throws OrmException, IOException, ConfigInvalidException { - if (readFromGit) { - try (Repository repo = repoManager.openRepository(allUsersName)) { - return read(repo, accountId); - } + public Account get(Account.Id accountId) throws IOException, ConfigInvalidException { + try (Repository repo = repoManager.openRepository(allUsersName)) { + return read(repo, accountId); } - - return db.accounts().get(accountId); } - public List get(ReviewDb db, Collection accountIds) - throws OrmException, IOException, ConfigInvalidException { - if (readFromGit) { - List accounts = new ArrayList<>(accountIds.size()); - try (Repository repo = repoManager.openRepository(allUsersName)) { - for (Account.Id accountId : accountIds) { - accounts.add(read(repo, accountId)); - } + public List get(Collection accountIds) + throws IOException, ConfigInvalidException { + List accounts = new ArrayList<>(accountIds.size()); + try (Repository repo = repoManager.openRepository(allUsersName)) { + for (Account.Id accountId : accountIds) { + accounts.add(read(repo, accountId)); } - return accounts; } - - return db.accounts().get(accountIds).toList(); + return accounts; } /** @@ -94,23 +78,19 @@ public class Accounts { * * @return all accounts */ - public List all(ReviewDb db) throws OrmException, IOException { - if (readFromGit) { - Set accountIds = allIds(); - List accounts = new ArrayList<>(accountIds.size()); - try (Repository repo = repoManager.openRepository(allUsersName)) { - for (Account.Id accountId : accountIds) { - try { - accounts.add(read(repo, accountId)); - } catch (Exception e) { - log.error(String.format("Ignoring invalid account %s", accountId.get()), e); - } + public List all() throws IOException { + Set accountIds = allIds(); + List accounts = new ArrayList<>(accountIds.size()); + try (Repository repo = repoManager.openRepository(allUsersName)) { + for (Account.Id accountId : accountIds) { + try { + accounts.add(read(repo, accountId)); + } catch (Exception e) { + log.error(String.format("Ignoring invalid account %s", accountId.get()), e); } } - return accounts; } - - return db.accounts().all().toList(); + return accounts; } /** diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsConsistencyChecker.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsConsistencyChecker.java index 2f3f657fc5..0085303f4e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsConsistencyChecker.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsConsistencyChecker.java @@ -16,11 +16,8 @@ package com.google.gerrit.server.account; import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo; import com.google.gerrit.reviewdb.client.Account; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.account.externalids.ExternalIds; -import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; -import com.google.inject.Provider; import com.google.inject.Singleton; import java.io.IOException; import java.util.ArrayList; @@ -28,22 +25,19 @@ import java.util.List; @Singleton public class AccountsConsistencyChecker { - private final Provider dbProvider; private final Accounts accounts; private final ExternalIds externalIds; @Inject - AccountsConsistencyChecker( - Provider dbProvider, Accounts accounts, ExternalIds externalIds) { - this.dbProvider = dbProvider; + AccountsConsistencyChecker(Accounts accounts, ExternalIds externalIds) { this.accounts = accounts; this.externalIds = externalIds; } - public List check() throws OrmException, IOException { + public List check() throws IOException { List problems = new ArrayList<>(); - for (Account account : accounts.all(dbProvider.get())) { + for (Account account : accounts.all()) { if (account.getPreferredEmail() != null) { if (!externalIds .byAccount(account.getId()) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java index 56f637e9bd..46d80630bb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java @@ -224,10 +224,10 @@ public class ConsistencyChecker { private void checkOwner() { try { - if (accounts.get(db.get(), change().getOwner()) == null) { + if (accounts.get(change().getOwner()) == null) { problem("Missing change owner: " + change().getOwner()); } - } catch (OrmException | IOException | ConfigInvalidException e) { + } catch (IOException | ConfigInvalidException e) { error("Failed to look up owner", e); } } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java index c5eea0f29a..8fec96a0e8 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java @@ -400,7 +400,7 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests { // update account in ReviewDb without reindex so that account index is stale String newName = "Test User"; Account.Id accountId = new Account.Id(user1._accountId); - Account account = accounts.get(db, accountId); + Account account = accounts.get(accountId); account.setFullName(newName); db.accounts().update(ImmutableSet.of(account));