opendev/gerrit
Code Issues Proposed changes

Get accounts through Accounts class

Browse Source 
The Accounts class should become the single class that reads accounts.
At the moment it always reads the accounts from ReviewDb, but in future
it will read accounts from NoteDb.

Change-Id: I7507b734e302ca3953e86fe612aac63ea10c75da
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin 2017-04-13 15:09:47 +02:00
parent bcfae3e37c
commit 2a3b883511
11 changed files with 46 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java
View File

@ -38,6 +38,7 @@ import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.account.externalids.ExternalIdsUpdate;
@ -71,6 +72,8 @@ import org.junit.Test;
/** Unit tests for {@link GerritPublicKeyChecker}. */
public class GerritPublicKeyCheckerTest {
@Inject private Accounts accounts;
@Inject private AccountCache accountCache;
@Inject private AccountManager accountManager;
@ -115,7 +118,7 @@ public class GerritPublicKeyCheckerTest {
db = schemaFactory.open();
schemaCreator.create(db);
userId = accountManager.authenticate(AuthRequest.forUser("user")).getAccountId();
Account userAccount = db.accounts().get(userId);
Account userAccount = accounts.get(db, userId);
// Note: does not match any key in TestKeys.
userAccount.setPreferredEmail("user@example.com");
db.accounts().update(ImmutableList.of(userAccount));

2
gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java
View File

@ -230,7 +230,7 @@ class BecomeAnyAccountLoginServlet extends HttpServlet {
return null;
}
try (ReviewDb db = schema.open()) {
return auth(db.accounts().get(id));
return auth(accounts.get(db, id));
} catch (OrmException e) {
getServletContext().log("cannot query database", e);
return null;

5
gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java
View File

@ -154,6 +154,7 @@ public class AccountCacheImpl implements AccountCache {
static class ByIdLoader extends CacheLoader<Account.Id, Optional<AccountState>> {
private final SchemaFactory<ReviewDb> schema;
private final Accounts accounts;
private final GroupCache groupCache;
private final GeneralPreferencesLoader loader;
private final LoadingCache<String, Optional<Account.Id>> byName;
@ -163,11 +164,13 @@ public class AccountCacheImpl implements AccountCache {
@Inject
ByIdLoader(
SchemaFactory<ReviewDb> sf,
Accounts accounts,
GroupCache groupCache,
GeneralPreferencesLoader loader,
@Named(BYUSER_NAME) LoadingCache<String, Optional<Account.Id>> byUsername,
Provider<WatchConfig.Accessor> watchConfig,
ExternalIds externalIds) {
this.accounts = accounts;
this.schema = sf;
this.groupCache = groupCache;
this.loader = loader;
@ -193,7 +196,7 @@ public class AccountCacheImpl implements AccountCache {
private Optional<AccountState> load(final ReviewDb db, final Account.Id who)
throws OrmException, IOException, ConfigInvalidException {
Account account = db.accounts().get(who);
Account account = accounts.get(db, who);
if (account == null) {
return Optional.empty();
}

6
gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java
View File

@ -207,7 +207,7 @@ public class AccountManager {
private Account load(Account toUpdate, Account.Id accountId, ReviewDb db) throws OrmException {
if (toUpdate == null) {
toUpdate = db.accounts().get(accountId);
toUpdate = accounts.get(db, accountId);
if (toUpdate == null) {
throw new OrmException("Account " + accountId + " has been deleted");
}
@ -376,7 +376,7 @@ public class AccountManager {
.insert(ExternalId.createWithEmail(who.getExternalIdKey(), to, who.getEmailAddress()));
if (who.getEmailAddress() != null) {
Account a = db.accounts().get(to);
Account a = accounts.get(db, to);
if (a.getPreferredEmail() == null) {
a.setPreferredEmail(who.getEmailAddress());
db.accounts().update(Collections.singleton(a));
@ -445,7 +445,7 @@ public class AccountManager {
externalIdsUpdateFactory.create().delete(extId);
if (who.getEmailAddress() != null) {
Account a = db.accounts().get(from);
Account a = accounts.get(db, from);
if (a.getPreferredEmail() != null
&& a.getPreferredEmail().equals(who.getEmailAddress())) {
a.setPreferredEmail(null);

5
gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResolver.java
View File

@ -33,6 +33,7 @@ import java.util.regex.Pattern;
@Singleton
public class AccountResolver {
private final Realm realm;
private final Accounts accounts;
private final AccountByEmailCache byEmail;
private final AccountCache byId;
private final Provider<InternalAccountQuery> accountQueryProvider;
@ -40,10 +41,12 @@ public class AccountResolver {
@Inject
AccountResolver(
Realm realm,
Accounts accounts,
AccountByEmailCache byEmail,
AccountCache byId,
Provider<InternalAccountQuery> accountQueryProvider) {
this.realm = realm;
this.accounts = accounts;
this.byEmail = byEmail;
this.byId = byId;
this.accountQueryProvider = accountQueryProvider;
@ -116,7 +119,7 @@ public class AccountResolver {
}
private boolean exists(ReviewDb db, Account.Id id) throws OrmException {
return db.accounts().get(id) != null;
return accounts.get(db, id) != null;
}
/**

6
gerrit-server/src/main/java/com/google/gerrit/server/account/Accounts.java
View File

@ -20,8 +20,10 @@ 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.git.GitRepositoryManager;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
@ -43,6 +45,10 @@ public class Accounts {
this.allUsersName = allUsersName;
}
public Account get(ReviewDb db, Account.Id accountId) throws OrmException {
return db.accounts().get(accountId);
}
/**
* Returns all account IDs.
*

6
gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java
View File

@ -44,6 +44,7 @@ import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.PatchSetState;
@ -105,6 +106,7 @@ public class ConsistencyChecker {
private final ChangeControl.GenericFactory changeControlFactory;
private final ChangeNotes.Factory notesFactory;
private final Accounts accounts;
private final DynamicItem<AccountPatchReviewStore> accountPatchReviewStore;
private final GitRepositoryManager repoManager;
private final PatchSetInfoFactory patchSetInfoFactory;
@ -134,6 +136,7 @@ public class ConsistencyChecker {
@GerritPersonIdent Provider<PersonIdent> serverIdent,
ChangeControl.GenericFactory changeControlFactory,
ChangeNotes.Factory notesFactory,
Accounts accounts,
DynamicItem<AccountPatchReviewStore> accountPatchReviewStore,
GitRepositoryManager repoManager,
PatchSetInfoFactory patchSetInfoFactory,
@ -142,6 +145,7 @@ public class ConsistencyChecker {
Provider<CurrentUser> user,
Provider<ReviewDb> db,
RetryHelper retryHelper) {
this.accounts = accounts;
this.accountPatchReviewStore = accountPatchReviewStore;
this.changeControlFactory = changeControlFactory;
this.db = db;
@ -219,7 +223,7 @@ public class ConsistencyChecker {
private void checkOwner() {
try {
if (db.get().accounts().get(change().getOwner()) == null) {
if (accounts.get(db.get(), change().getOwner()) == null) {
problem("Missing change owner: " + change().getOwner());
}
} catch (OrmException e) {

8
gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
View File

@ -84,6 +84,7 @@ import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.Sequences;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.SetHashtagsOp;
import com.google.gerrit.server.config.AllProjectsName;
@ -299,6 +300,7 @@ public class ReceiveCommits {
private final Sequences seq;
private final Provider<InternalChangeQuery> queryProvider;
private final ChangeNotes.Factory notesFactory;
private final Accounts accounts;
private final AccountResolver accountResolver;
private final PermissionBackend permissionBackend;
private final PermissionBackend.ForProject permissions;
@ -373,6 +375,7 @@ public class ReceiveCommits {
Sequences seq,
Provider<InternalChangeQuery> queryProvider,
ChangeNotes.Factory notesFactory,
Accounts accounts,
AccountResolver accountResolver,
PermissionBackend permissionBackend,
CmdLineParser.Factory optionParserFactory,
@ -412,6 +415,7 @@ public class ReceiveCommits {
this.seq = seq;
this.queryProvider = queryProvider;
this.notesFactory = notesFactory;
this.accounts = accounts;
this.accountResolver = accountResolver;
this.permissionBackend = permissionBackend;
this.optionParserFactory = optionParserFactory;
@ -1334,7 +1338,7 @@ public class ReceiveCommits {
if (!hashtag.isEmpty()) {
hashtags.add(hashtag);
}
//TODO(dpursehouse): validate hashtags
// TODO(dpursehouse): validate hashtags
}
MagicBranchInput(
@ -2737,7 +2741,7 @@ public class ReceiveCommits {
if (defaultName && user.hasEmailAddress(c.getCommitterIdent().getEmailAddress())) {
try {
Account a = db.accounts().get(user.getAccountId());
Account a = accounts.get(db, user.getAccountId());
if (a != null && Strings.isNullOrEmpty(a.getFullName())) {
a.setFullName(c.getCommitterIdent().getName());
db.accounts().update(Collections.singleton(a));

7
gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
View File

@ -37,6 +37,7 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.schema.SchemaCreator;
@ -71,6 +72,8 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests {
return cfg;
}
@Inject protected Accounts accounts;
@Inject protected AccountCache accountCache;
@Inject protected AccountManager accountManager;
@ -380,7 +383,7 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests {
// update account in the database so that account index is stale
String newName = "Test User";
Account account = db.accounts().get(new Account.Id(user1._accountId));
Account account = accounts.get(db, new Account.Id(user1._accountId));
account.setFullName(newName);
db.accounts().update(Collections.singleton(account));
@ -464,7 +467,7 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests {
if (email != null) {
accountManager.link(id, AuthRequest.forEmail(email));
}
Account a = db.accounts().get(id);
Account a = accounts.get(db, id);
a.setFullName(fullName);
a.setPreferredEmail(email);
a.setActive(active);

6
gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
View File

@ -70,6 +70,7 @@ import com.google.gerrit.server.Sequences;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.account.externalids.ExternalIdsUpdate;
@ -147,6 +148,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
return cfg;
}
@Inject protected Accounts accounts;
@Inject protected AccountCache accountCache;
@Inject protected AccountManager accountManager;
@Inject protected AllUsersName allUsersName;
@ -204,7 +206,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
db = schemaFactory.open();
userId = accountManager.authenticate(AuthRequest.forUser("user")).getAccountId();
Account userAccount = db.accounts().get(userId);
Account userAccount = accounts.get(db, userId);
String email = "user@example.com";
externalIdsUpdate.create().insert(ExternalId.createEmail(userId, email));
userAccount.setPreferredEmail(email);
@ -2304,7 +2306,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
if (email != null) {
accountManager.link(id, AuthRequest.forEmail(email));
}
Account a = db.accounts().get(id);
Account a = accounts.get(db, id);
a.setFullName(fullName);
a.setPreferredEmail(email);
a.setActive(active);

5
gerrit-server/src/test/java/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
View File

@ -34,6 +34,7 @@ import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.config.AllProjectsName;
@ -70,6 +71,8 @@ public abstract class AbstractQueryGroupsTest extends GerritServerTests {
return cfg;
}
@Inject protected Accounts accounts;
@Inject protected AccountCache accountCache;
@Inject protected AccountManager accountManager;
@ -314,7 +317,7 @@ public abstract class AbstractQueryGroupsTest extends GerritServerTests {
if (email != null) {
accountManager.link(id, AuthRequest.forEmail(email));
}
Account a = db.accounts().get(id);
Account a = accounts.get(db, id);
a.setFullName(fullName);
a.setPreferredEmail(email);
a.setActive(active);