Require account index and remove fallbacks

The account data is moved from ReviewDb into git.

Change-Id: I643827179b24601b138f394cfff5890f919b9da9
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin 2016-07-07 10:40:15 +02:00
parent 9b52b4502e
commit 10aa4e2bbf
16 changed files with 77 additions and 468 deletions

View File

@ -30,7 +30,6 @@ import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestAccount;
@ -957,14 +956,6 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
pushWithReviewerInFooter("Notauser", null);
}
@Test
// TODO(dborowitz): This is to exercise a specific case in the database search
// path. Once the account index becomes obligatory this method can be removed.
@GerritConfig(name = "index.testDisable", value = "accounts")
public void pushWithNameInFooterNotFoundWithDbSearch() throws Exception {
pushWithReviewerInFooter("Notauser", null);
}
@Test
public void pushNewPatchsetOverridingStickyLabel() throws Exception {
ProjectConfig cfg = projectCache.checkedGet(project).getConfig();

View File

@ -23,12 +23,10 @@ import com.google.common.collect.Maps;
import com.google.common.io.BaseEncoding;
import com.google.gerrit.common.PageLinks;
import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.index.account.AccountIndexCollection;
import com.google.gerrit.server.query.account.InternalAccountQuery;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@ -63,8 +61,6 @@ public class GerritPublicKeyChecker extends PublicKeyChecker {
@Singleton
public static class Factory {
private final Provider<ReviewDb> db;
private final AccountIndexCollection accountIndexes;
private final Provider<InternalAccountQuery> accountQueryProvider;
private final String webUrl;
private final IdentifiedUser.GenericFactory userFactory;
@ -73,13 +69,9 @@ public class GerritPublicKeyChecker extends PublicKeyChecker {
@Inject
Factory(@GerritServerConfig Config cfg,
Provider<ReviewDb> db,
AccountIndexCollection accountIndexes,
Provider<InternalAccountQuery> accountQueryProvider,
IdentifiedUser.GenericFactory userFactory,
@CanonicalWebUrl String webUrl) {
this.db = db;
this.accountIndexes = accountIndexes;
this.accountQueryProvider = accountQueryProvider;
this.webUrl = webUrl;
this.userFactory = userFactory;
@ -113,8 +105,6 @@ public class GerritPublicKeyChecker extends PublicKeyChecker {
}
}
private final Provider<ReviewDb> db;
private final AccountIndexCollection accountIndexes;
private final Provider<InternalAccountQuery> accountQueryProvider;
private final String webUrl;
private final IdentifiedUser.GenericFactory userFactory;
@ -122,8 +112,6 @@ public class GerritPublicKeyChecker extends PublicKeyChecker {
private IdentifiedUser expectedUser;
private GerritPublicKeyChecker(Factory factory) {
this.db = factory.db;
this.accountIndexes = factory.accountIndexes;
this.accountQueryProvider = factory.accountQueryProvider;
this.webUrl = factory.webUrl;
this.userFactory = factory.userFactory;
@ -174,25 +162,15 @@ public class GerritPublicKeyChecker extends PublicKeyChecker {
private CheckResult checkIdsForArbitraryUser(PGPPublicKey key)
throws PGPException, OrmException {
IdentifiedUser user;
if (accountIndexes.getSearchIndex() != null) {
List<AccountState> accountStates =
accountQueryProvider.get().byExternalId(toExtIdKey(key).get());
if (accountStates.isEmpty()) {
return CheckResult.bad("Key is not associated with any users");
}
if (accountStates.size() > 1) {
return CheckResult.bad("Key is associated with multiple users");
}
user = userFactory.create(accountStates.get(0));
} else {
AccountExternalId extId = db.get().accountExternalIds().get(
toExtIdKey(key));
if (extId == null) {
return CheckResult.bad("Key is not associated with any users");
}
user = userFactory.create(extId.getAccountId());
List<AccountState> accountStates =
accountQueryProvider.get().byExternalId(toExtIdKey(key).get());
if (accountStates.isEmpty()) {
return CheckResult.bad("Key is not associated with any users");
}
if (accountStates.size() > 1) {
return CheckResult.bad("Key is associated with multiple users");
}
IdentifiedUser user = userFactory.create(accountStates.get(0));
Set<String> allowedUserIds = getAllowedUserIds(user);
if (allowedUserIds.isEmpty()) {

View File

@ -47,7 +47,6 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountResource;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.index.account.AccountIndexCollection;
import com.google.gerrit.server.mail.send.AddKeySender;
import com.google.gerrit.server.query.account.InternalAccountQuery;
import com.google.gwtorm.server.OrmException;
@ -90,7 +89,6 @@ public class PostGpgKeys implements RestModifyView<AccountResource, Input> {
private final GerritPublicKeyChecker.Factory checkerFactory;
private final AddKeySender.Factory addKeyFactory;
private final AccountCache accountCache;
private final AccountIndexCollection accountIndexes;
private final Provider<InternalAccountQuery> accountQueryProvider;
@Inject
@ -101,7 +99,6 @@ public class PostGpgKeys implements RestModifyView<AccountResource, Input> {
GerritPublicKeyChecker.Factory checkerFactory,
AddKeySender.Factory addKeyFactory,
AccountCache accountCache,
AccountIndexCollection accountIndexes,
Provider<InternalAccountQuery> accountQueryProvider) {
this.serverIdent = serverIdent;
this.db = db;
@ -110,7 +107,6 @@ public class PostGpgKeys implements RestModifyView<AccountResource, Input> {
this.checkerFactory = checkerFactory;
this.addKeyFactory = addKeyFactory;
this.accountCache = accountCache;
this.accountIndexes = accountIndexes;
this.accountQueryProvider = accountQueryProvider;
}
@ -131,28 +127,15 @@ public class PostGpgKeys implements RestModifyView<AccountResource, Input> {
for (PGPPublicKeyRing keyRing : newKeys) {
PGPPublicKey key = keyRing.getPublicKey();
AccountExternalId.Key extIdKey = toExtIdKey(key.getFingerprint());
if (accountIndexes.getSearchIndex() != null) {
Account account = getAccountByExternalId(extIdKey.get());
if (account != null) {
if (!account.getId().equals(rsrc.getUser().getAccountId())) {
throw new ResourceConflictException(
"GPG key already associated with another account");
}
} else {
newExtIds.add(
new AccountExternalId(rsrc.getUser().getAccountId(), extIdKey));
Account account = getAccountByExternalId(extIdKey.get());
if (account != null) {
if (!account.getId().equals(rsrc.getUser().getAccountId())) {
throw new ResourceConflictException(
"GPG key already associated with another account");
}
} else {
AccountExternalId existing = db.get().accountExternalIds().get(extIdKey);
if (existing != null) {
if (!existing.getAccountId().equals(rsrc.getUser().getAccountId())) {
throw new ResourceConflictException(
"GPG key already associated with another account");
}
} else {
newExtIds.add(
new AccountExternalId(rsrc.getUser().getAccountId(), extIdKey));
}
newExtIds.add(
new AccountExternalId(rsrc.getUser().getAccountId(), extIdKey));
}
}

View File

@ -28,20 +28,9 @@ public interface AccountExternalIdAccess extends
@PrimaryKey("key")
AccountExternalId get(AccountExternalId.Key key) throws OrmException;
@Query("WHERE key >= ? AND key <= ? ORDER BY key LIMIT ?")
ResultSet<AccountExternalId> suggestByKey(AccountExternalId.Key keyA,
AccountExternalId.Key keyB, int limit) throws OrmException;
@Query("WHERE accountId = ?")
ResultSet<AccountExternalId> byAccount(Account.Id id) throws OrmException;
@Query("WHERE emailAddress = ?")
ResultSet<AccountExternalId> byEmailAddress(String email) throws OrmException;
@Query("WHERE emailAddress >= ? AND emailAddress <= ? ORDER BY emailAddress LIMIT ?")
ResultSet<AccountExternalId> suggestByEmailAddress(String emailA,
String emailB, int limit) throws OrmException;
@Query
ResultSet<AccountExternalId> all() throws OrmException;
}

View File

@ -21,10 +21,6 @@ ON accounts (full_name);
CREATE INDEX account_external_ids_byAccount
ON account_external_ids (account_id);
-- covers: byEmailAddress, suggestByEmailAddress
CREATE INDEX account_external_ids_byEmail
ON account_external_ids (email_address);
-- *********************************************************************
-- AccountGroupMemberAccess

View File

@ -25,11 +25,6 @@ CREATE INDEX account_external_ids_byAccount
ON account_external_ids (account_id)
#
-- covers: byEmailAddress, suggestByEmailAddress
CREATE INDEX account_external_ids_byEmail
ON account_external_ids (email_address)
#
-- *********************************************************************
-- AccountGroupMemberAccess

View File

@ -68,10 +68,6 @@ ON accounts (full_name);
CREATE INDEX account_external_ids_byAccount
ON account_external_ids (account_id);
-- covers: byEmailAddress, suggestByEmailAddress
CREATE INDEX account_external_ids_byEmail
ON account_external_ids (email_address);
-- *********************************************************************
-- AccountGroupMemberAccess

View File

@ -30,11 +30,7 @@ import com.google.gerrit.metrics.Description.Units;
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.metrics.Timer0;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountControl;
import com.google.gerrit.server.account.AccountDirectory.FillOptions;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.account.AccountState;
@ -42,8 +38,6 @@ import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.GroupMembers;
import com.google.gerrit.server.change.PostReviewers;
import com.google.gerrit.server.change.SuggestReviewers;
import com.google.gerrit.server.index.account.AccountIndex;
import com.google.gerrit.server.index.account.AccountIndexCollection;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectControl;
@ -102,47 +96,34 @@ public class ReviewersUtil {
}
}
private static final String MAX_SUFFIX = "\u9fa5";
// Generate a candidate list at 3x the size of what the user wants to see to
// give the ranking algorithm a good set of candidates it can work with
private static final int CANDIDATE_LIST_MULTIPLIER = 3;
private final AccountCache accountCache;
private final AccountControl accountControl;
private final AccountIndexCollection accountIndexes;
private final AccountLoader accountLoader;
private final AccountQueryBuilder accountQueryBuilder;
private final AccountQueryProcessor accountQueryProcessor;
private final GroupBackend groupBackend;
private final GroupMembers.Factory groupMembersFactory;
private final Provider<CurrentUser> currentUser;
private final Provider<ReviewDb> dbProvider;
private final ReviewerRecommender reviewerRecommender;
private final Metrics metrics;
@Inject
ReviewersUtil(AccountCache accountCache,
AccountControl.Factory accountControlFactory,
AccountIndexCollection accountIndexes,
AccountLoader.Factory accountLoaderFactory,
ReviewersUtil(AccountLoader.Factory accountLoaderFactory,
AccountQueryBuilder accountQueryBuilder,
AccountQueryProcessor accountQueryProcessor,
GroupBackend groupBackend,
GroupMembers.Factory groupMembersFactory,
Provider<CurrentUser> currentUser,
Provider<ReviewDb> dbProvider,
ReviewerRecommender reviewerRecommender,
Metrics metrics) {
Set<FillOptions> fillOptions = EnumSet.of(FillOptions.SECONDARY_EMAILS);
fillOptions.addAll(AccountLoader.DETAILED_OPTIONS);
this.accountCache = accountCache;
this.accountControl = accountControlFactory.get();
this.accountIndexes = accountIndexes;
this.accountLoader = accountLoaderFactory.create(fillOptions);
this.accountQueryBuilder = accountQueryBuilder;
this.accountQueryProcessor = accountQueryProcessor;
this.currentUser = currentUser;
this.dbProvider = dbProvider;
this.groupBackend = groupBackend;
this.groupMembersFactory = groupMembersFactory;
this.reviewerRecommender = reviewerRecommender;
@ -189,90 +170,24 @@ public class ReviewersUtil {
}
private List<Account.Id> suggestAccounts(SuggestReviewers suggestReviewers,
VisibilityControl visibilityControl)
throws OrmException {
VisibilityControl visibilityControl) throws OrmException {
try (Timer0.Context ctx = metrics.queryAccountsLatency.start()) {
AccountIndex searchIndex = accountIndexes.getSearchIndex();
if (searchIndex != null) {
return suggestAccountsFromIndex(suggestReviewers, visibilityControl);
}
return suggestAccountsFromDb(suggestReviewers, visibilityControl);
}
}
private List<Account.Id> suggestAccountsFromIndex(
SuggestReviewers suggestReviewers, VisibilityControl visibilityControl)
throws OrmException {
try {
Set<Account.Id> matches = new HashSet<>();
QueryResult<AccountState> result = accountQueryProcessor
.setLimit(suggestReviewers.getLimit() * CANDIDATE_LIST_MULTIPLIER)
.query(accountQueryBuilder.defaultQuery(suggestReviewers.getQuery()));
for (AccountState accountState : result.entities()) {
Account.Id id = accountState.getAccount().getId();
if (visibilityControl.isVisibleTo(id)) {
matches.add(id);
}
}
return new ArrayList<>(matches);
} catch (QueryParseException e) {
return ImmutableList.of();
}
}
private List<Account.Id> suggestAccountsFromDb(
SuggestReviewers suggestReviewers, VisibilityControl visibilityControl)
throws OrmException {
String query = suggestReviewers.getQuery();
int limit = suggestReviewers.getLimit() * CANDIDATE_LIST_MULTIPLIER;
String a = query;
String b = a + MAX_SUFFIX;
Set<Account.Id> r = new HashSet<>();
for (Account p : dbProvider.get().accounts()
.suggestByFullName(a, b, limit)) {
if (p.isActive()) {
addSuggestion(r, p.getId(), visibilityControl);
}
}
if (r.size() < limit) {
for (Account p : dbProvider.get().accounts()
.suggestByPreferredEmail(a, b, limit - r.size())) {
if (p.isActive()) {
addSuggestion(r, p.getId(), visibilityControl);
}
}
}
if (r.size() < limit) {
for (AccountExternalId e : dbProvider.get().accountExternalIds()
.suggestByEmailAddress(a, b, limit - r.size())) {
if (!r.contains(e.getAccountId())) {
Account p = accountCache.get(e.getAccountId()).getAccount();
if (p.isActive()) {
addSuggestion(r, p.getId(), visibilityControl);
try {
Set<Account.Id> matches = new HashSet<>();
QueryResult<AccountState> result = accountQueryProcessor
.setLimit(suggestReviewers.getLimit() * CANDIDATE_LIST_MULTIPLIER)
.query(accountQueryBuilder.defaultQuery(suggestReviewers.getQuery()));
for (AccountState accountState : result.entities()) {
Account.Id id = accountState.getAccount().getId();
if (visibilityControl.isVisibleTo(id)) {
matches.add(id);
}
}
return new ArrayList<>(matches);
} catch (QueryParseException e) {
return ImmutableList.of();
}
}
return new ArrayList<>(r);
}
private boolean addSuggestion(Set<Account.Id> map,
Account.Id account, VisibilityControl visibilityControl)
throws OrmException {
if (!map.contains(account)
// Can the suggestion see the change?
&& visibilityControl.isVisibleTo(account)
// Can the current user see the account?
&& accountControl.canSee(account)) {
map.add(account);
return true;
}
return false;
}
private List<Account.Id> recommendAccounts(ChangeNotes changeNotes,

View File

@ -21,7 +21,6 @@ import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.index.account.AccountIndexCollection;
import com.google.gerrit.server.query.account.InternalAccountQuery;
import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
@ -87,15 +86,12 @@ public class AccountByEmailCacheImpl implements AccountByEmailCache {
static class Loader extends CacheLoader<String, Set<Account.Id>> {
private final SchemaFactory<ReviewDb> schema;
private final AccountIndexCollection accountIndexes;
private final Provider<InternalAccountQuery> accountQueryProvider;
@Inject
Loader(SchemaFactory<ReviewDb> schema,
AccountIndexCollection accountIndexes,
Provider<InternalAccountQuery> accountQueryProvider) {
this.schema = schema;
this.accountIndexes = accountIndexes;
this.accountQueryProvider = accountQueryProvider;
}
@ -106,18 +102,11 @@ public class AccountByEmailCacheImpl implements AccountByEmailCache {
for (Account a : db.accounts().byPreferredEmail(email)) {
r.add(a.getId());
}
if (accountIndexes.getSearchIndex() != null) {
for (AccountState accountState : accountQueryProvider.get()
.byExternalId(
(new AccountExternalId.Key(AccountExternalId.SCHEME_MAILTO,
email)).get())) {
r.add(accountState.getAccount().getId());
}
} else {
for (AccountExternalId a : db.accountExternalIds()
.byEmailAddress(email)) {
r.add(a.getAccountId());
}
for (AccountState accountState : accountQueryProvider.get()
.byExternalId(
(new AccountExternalId.Key(AccountExternalId.SCHEME_MAILTO,
email)).get())) {
r.add(accountState.getAccount().getId());
}
return ImmutableSet.copyOf(r);
}

View File

@ -28,7 +28,6 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.WatchConfig.ProjectWatchKey;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.index.account.AccountIndexCollection;
import com.google.gerrit.server.index.account.AccountIndexer;
import com.google.gerrit.server.query.account.InternalAccountQuery;
import com.google.gwtorm.server.OrmException;
@ -231,35 +230,22 @@ public class AccountCacheImpl implements AccountCache {
}
static class ByNameLoader extends CacheLoader<String, Optional<Account.Id>> {
private final SchemaFactory<ReviewDb> schema;
private final AccountIndexCollection accountIndexes;
private final Provider<InternalAccountQuery> accountQueryProvider;
@Inject
ByNameLoader(SchemaFactory<ReviewDb> sf,
AccountIndexCollection accountIndexes,
Provider<InternalAccountQuery> accountQueryProvider) {
this.schema = sf;
this.accountIndexes = accountIndexes;
ByNameLoader(Provider<InternalAccountQuery> accountQueryProvider) {
this.accountQueryProvider = accountQueryProvider;
}
@Override
public Optional<Account.Id> load(String username) throws Exception {
AccountExternalId.Key key = new AccountExternalId.Key( //
AccountExternalId.SCHEME_USERNAME, //
username);
if (accountIndexes.getSearchIndex() != null) {
AccountState accountState =
accountQueryProvider.get().oneByExternalId(key.get());
return Optional.ofNullable(accountState)
.map(s -> s.getAccount().getId());
}
try (ReviewDb db = schema.open()) {
return Optional.ofNullable(db.accountExternalIds().get(key))
.map(AccountExternalId::getAccountId);
}
AccountExternalId.Key key = new AccountExternalId.Key( //
AccountExternalId.SCHEME_USERNAME, //
username);
AccountState accountState =
accountQueryProvider.get().oneByExternalId(key.get());
return Optional.ofNullable(accountState)
.map(s -> s.getAccount().getId());
}
}
}

View File

@ -28,7 +28,6 @@ import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountGroupMember;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.index.account.AccountIndexCollection;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.query.account.InternalAccountQuery;
import com.google.gwtorm.server.OrmException;
@ -63,7 +62,6 @@ public class AccountManager {
private final ProjectCache projectCache;
private final AtomicBoolean awaitsFirstAccountCheck;
private final AuditService auditService;
private final AccountIndexCollection accountIndexes;
private final Provider<InternalAccountQuery> accountQueryProvider;
@Inject
@ -75,7 +73,6 @@ public class AccountManager {
ChangeUserName.Factory changeUserNameFactory,
ProjectCache projectCache,
AuditService auditService,
AccountIndexCollection accountIndexes,
Provider<InternalAccountQuery> accountQueryProvider) {
this.schema = schema;
this.byIdCache = byIdCache;
@ -86,7 +83,6 @@ public class AccountManager {
this.projectCache = projectCache;
this.awaitsFirstAccountCheck = new AtomicBoolean(true);
this.auditService = auditService;
this.accountIndexes = accountIndexes;
this.accountQueryProvider = accountQueryProvider;
}
@ -96,21 +92,11 @@ public class AccountManager {
public Optional<Account.Id> lookup(String externalId)
throws AccountException {
try {
if (accountIndexes.getSearchIndex() != null) {
AccountState accountState =
accountQueryProvider.get().oneByExternalId(externalId);
return accountState != null
? Optional.of(accountState.getAccount().getId())
: Optional.empty();
}
try (ReviewDb db = schema.open()) {
AccountExternalId ext =
db.accountExternalIds().get(new AccountExternalId.Key(externalId));
return ext != null
? Optional.of(ext.getAccountId())
: Optional.empty();
}
AccountState accountState =
accountQueryProvider.get().oneByExternalId(externalId);
return accountState != null
? Optional.of(accountState.getAccount().getId())
: Optional.empty();
} catch (OrmException e) {
throw new AccountException("Cannot lookup account " + externalId, e);
}
@ -130,7 +116,7 @@ public class AccountManager {
try {
try (ReviewDb db = schema.open()) {
AccountExternalId.Key key = id(who);
AccountExternalId id = getAccountExternalId(db, key);
AccountExternalId id = getAccountExternalId(key);
if (id == null) {
// New account, automatically create and return.
//
@ -152,37 +138,18 @@ public class AccountManager {
}
}
private AccountExternalId getAccountExternalId(ReviewDb db,
AccountExternalId.Key key) throws OrmException {
if (accountIndexes.getSearchIndex() != null) {
AccountState accountState =
accountQueryProvider.get().oneByExternalId(key.get());
if (accountState != null) {
for (AccountExternalId extId : accountState.getExternalIds()) {
if (extId.getKey().equals(key)) {
return extId;
}
}
}
return null;
}
// We don't have at the moment an account_by_external_id cache
// but by using the accounts cache we get the list of external_ids
// without having to query the DB every time
if (key.getScheme().equals(AccountExternalId.SCHEME_GERRIT)
|| key.getScheme().equals(AccountExternalId.SCHEME_USERNAME)) {
AccountState state = byIdCache.getByUsername(
key.get().substring(key.getScheme().length()));
if (state != null) {
for (AccountExternalId accountExternalId : state.getExternalIds()) {
if (accountExternalId.getKey().equals(key)) {
return accountExternalId;
}
private AccountExternalId getAccountExternalId(AccountExternalId.Key key)
throws OrmException {
AccountState accountState =
accountQueryProvider.get().oneByExternalId(key.get());
if (accountState != null) {
for (AccountExternalId extId : accountState.getExternalIds()) {
if (extId.getKey().equals(key)) {
return extId;
}
}
}
return db.accountExternalIds().get(key);
return null;
}
private void update(ReviewDb db, AuthRequest who, AccountExternalId extId)
@ -390,7 +357,7 @@ public class AccountManager {
throws AccountException, OrmException, IOException {
try (ReviewDb db = schema.open()) {
AccountExternalId.Key key = id(who);
AccountExternalId extId = getAccountExternalId(db, key);
AccountExternalId extId = getAccountExternalId(key);
if (extId != null) {
if (!extId.getAccountId().equals(to)) {
throw new AccountException("Identity in use by another account");
@ -474,7 +441,7 @@ public class AccountManager {
throws AccountException, OrmException, IOException {
try (ReviewDb db = schema.open()) {
AccountExternalId.Key key = id(who);
AccountExternalId extId = getAccountExternalId(db, key);
AccountExternalId extId = getAccountExternalId(key);
if (extId != null) {
if (!extId.getAccountId().equals(from)) {
throw new AccountException(

View File

@ -17,9 +17,7 @@ package com.google.gerrit.server.account;
import static java.util.stream.Collectors.toSet;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.index.account.AccountIndexCollection;
import com.google.gerrit.server.query.account.InternalAccountQuery;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@ -38,19 +36,16 @@ public class AccountResolver {
private final Realm realm;
private final AccountByEmailCache byEmail;
private final AccountCache byId;
private final AccountIndexCollection accountIndexes;
private final Provider<InternalAccountQuery> accountQueryProvider;
@Inject
AccountResolver(Realm realm,
AccountByEmailCache byEmail,
AccountCache byId,
AccountIndexCollection accountIndexes,
Provider<InternalAccountQuery> accountQueryProvider) {
this.realm = realm;
this.byEmail = byEmail;
this.byId = byId;
this.accountIndexes = accountIndexes;
this.accountQueryProvider = accountQueryProvider;
}
@ -183,42 +178,15 @@ public class AccountResolver {
return Collections.singleton(id);
}
if (accountIndexes.getSearchIndex() != null) {
List<AccountState> m = accountQueryProvider.get().byFullName(nameOrEmail);
if (m.size() == 1) {
return Collections.singleton(m.get(0).getAccount().getId());
}
// At this point we have no clue. Just perform a whole bunch of suggestions
// and pray we come up with a reasonable result list.
return accountQueryProvider.get().byDefault(nameOrEmail).stream()
.map(a -> a.getAccount().getId())
.collect(toSet());
}
List<Account> m = db.accounts().byFullName(nameOrEmail).toList();
List<AccountState> m = accountQueryProvider.get().byFullName(nameOrEmail);
if (m.size() == 1) {
return Collections.singleton(m.get(0).getId());
return Collections.singleton(m.get(0).getAccount().getId());
}
// At this point we have no clue. Just perform a whole bunch of suggestions
// and pray we come up with a reasonable result list.
Set<Account.Id> result = new HashSet<>();
String a = nameOrEmail;
String b = nameOrEmail + "\u9fa5";
for (Account act : db.accounts().suggestByFullName(a, b, 10)) {
result.add(act.getId());
}
for (AccountExternalId extId : db.accountExternalIds()
.suggestByKey(
new AccountExternalId.Key(AccountExternalId.SCHEME_USERNAME, a),
new AccountExternalId.Key(AccountExternalId.SCHEME_USERNAME, b), 10)) {
result.add(extId.getAccountId());
}
for (AccountExternalId extId : db.accountExternalIds()
.suggestByEmailAddress(a, b, 10)) {
result.add(extId.getAccountId());
}
return result;
return accountQueryProvider.get().byDefault(nameOrEmail).stream()
.map(a -> a.getAccount().getId())
.collect(toSet());
}
}

View File

@ -23,13 +23,9 @@ import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.AccountDirectory.FillOptions;
import com.google.gerrit.server.api.accounts.AccountInfoComparator;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.index.account.AccountIndex;
import com.google.gerrit.server.index.account.AccountIndexCollection;
import com.google.gerrit.server.query.Predicate;
import com.google.gerrit.server.query.QueryParseException;
import com.google.gerrit.server.query.QueryResult;
@ -43,7 +39,6 @@ import org.kohsuke.args4j.Option;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
@ -51,15 +46,10 @@ import java.util.Set;
public class QueryAccounts implements RestReadView<TopLevelResource> {
private static final int MAX_SUGGEST_RESULTS = 100;
private static final String MAX_SUFFIX = "\u9fa5";
private final AccountControl accountControl;
private final AccountLoader.Factory accountLoaderFactory;
private final AccountCache accountCache;
private final AccountIndexCollection indexes;
private final AccountQueryBuilder queryBuilder;
private final AccountQueryProcessor queryProcessor;
private final ReviewDb db;
private final boolean suggestConfig;
private final int suggestFrom;
@ -110,21 +100,13 @@ public class QueryAccounts implements RestReadView<TopLevelResource> {
}
@Inject
QueryAccounts(AccountControl.Factory accountControlFactory,
AccountLoader.Factory accountLoaderFactory,
AccountCache accountCache,
AccountIndexCollection indexes,
QueryAccounts(AccountLoader.Factory accountLoaderFactory,
AccountQueryBuilder queryBuilder,
AccountQueryProcessor queryProcessor,
ReviewDb db,
@GerritServerConfig Config cfg) {
this.accountControl = accountControlFactory.get();
this.accountLoaderFactory = accountLoaderFactory;
this.accountCache = accountCache;
this.indexes = indexes;
this.queryBuilder = queryBuilder;
this.queryProcessor = queryProcessor;
this.db = db;
this.suggestFrom = cfg.getInt("suggest", null, "from", 0);
this.options = EnumSet.noneOf(ListAccountsOption.class);
@ -169,22 +151,6 @@ public class QueryAccounts implements RestReadView<TopLevelResource> {
}
accountLoader = accountLoaderFactory.create(fillOptions);
AccountIndex searchIndex = indexes.getSearchIndex();
if (searchIndex != null) {
return queryFromIndex();
}
if (!suggest) {
throw new MethodNotAllowedException();
}
if (start != null) {
throw new MethodNotAllowedException("option start not allowed");
}
return queryFromDb();
}
public List<AccountInfo> queryFromIndex()
throws BadRequestException, MethodNotAllowedException, OrmException {
if (queryProcessor.isDisabled()) {
throw new MethodNotAllowedException("query disabled");
}
@ -223,57 +189,4 @@ public class QueryAccounts implements RestReadView<TopLevelResource> {
throw new BadRequestException(e.getMessage());
}
}
public List<AccountInfo> queryFromDb() throws OrmException {
String a = query;
String b = a + MAX_SUFFIX;
Map<Account.Id, AccountInfo> matches = new LinkedHashMap<>();
Map<Account.Id, String> queryEmail = new HashMap<>();
for (Account p : db.accounts().suggestByFullName(a, b, suggestLimit)) {
addSuggestion(matches, p);
}
if (matches.size() < suggestLimit) {
for (Account p : db.accounts()
.suggestByPreferredEmail(a, b, suggestLimit - matches.size())) {
addSuggestion(matches, p);
}
}
if (matches.size() < suggestLimit) {
for (AccountExternalId e : db.accountExternalIds()
.suggestByEmailAddress(a, b, suggestLimit - matches.size())) {
if (addSuggestion(matches, e.getAccountId())) {
queryEmail.put(e.getAccountId(), e.getEmailAddress());
}
}
}
accountLoader.fill();
for (Map.Entry<Account.Id, String> p : queryEmail.entrySet()) {
AccountInfo info = matches.get(p.getKey());
if (info != null) {
info.email = p.getValue();
}
}
return AccountInfoComparator.ORDER_NULLS_LAST.sortedCopy(matches.values());
}
private boolean addSuggestion(Map<Account.Id, AccountInfo> map, Account a) {
if (!a.isActive()) {
return false;
}
Account.Id id = a.getId();
if (!map.containsKey(id) && accountControl.canSee(id)) {
map.put(id, accountLoader.get(id));
return true;
}
return false;
}
private boolean addSuggestion(Map<Account.Id, AccountInfo> map, Account.Id id) {
Account a = accountCache.get(id).getAccount();
return addSuggestion(map, a);
}
}

View File

@ -32,7 +32,6 @@ import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.account.AccountIndexCollection;
import com.google.gerrit.server.mail.EmailSettings;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.PatchListCache;
@ -82,7 +81,6 @@ public class EmailArguments {
final EmailSettings settings;
final DynamicSet<OutgoingEmailValidationListener> outgoingEmailValidationListeners;
final StarredChangesUtil starredChangesUtil;
final AccountIndexCollection accountIndexes;
final Provider<InternalAccountQuery> accountQueryProvider;
@Inject
@ -111,7 +109,6 @@ public class EmailArguments {
SitePaths site,
DynamicSet<OutgoingEmailValidationListener> outgoingEmailValidationListeners,
StarredChangesUtil starredChangesUtil,
AccountIndexCollection accountIndexes,
Provider<InternalAccountQuery> accountQueryProvider) {
this.server = server;
this.projectCache = projectCache;
@ -141,7 +138,6 @@ public class EmailArguments {
this.site = site;
this.outgoingEmailValidationListeners = outgoingEmailValidationListeners;
this.starredChangesUtil = starredChangesUtil;
this.accountIndexes = accountIndexes;
this.accountQueryProvider = accountQueryProvider;
}
}

View File

@ -21,7 +21,6 @@ import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountGroupMember;
import com.google.gerrit.reviewdb.client.AccountProjectWatch;
import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
@ -66,32 +65,6 @@ public class ProjectWatch {
/** Returns all watchers that are relevant */
public final Watchers getWatchers(NotifyType type) throws OrmException {
Watchers matching;
if (args.accountIndexes.getSearchIndex() != null) {
matching = getWatchersFromIndex(type);
} else {
matching = getWatchersFromDb(type);
}
for (ProjectState state : projectState.tree()) {
for (NotifyConfig nc : state.getConfig().getNotifyConfigs()) {
if (nc.isNotify(type)) {
try {
add(matching, nc);
} catch (QueryParseException e) {
log.warn("Project {} has invalid notify {} filter \"{}\": {}",
state.getProject().getName(), nc.getName(),
nc.getFilter(), e.getMessage());
}
}
}
}
return matching;
}
private Watchers getWatchersFromIndex(NotifyType type)
throws OrmException {
Watchers matching = new Watchers();
Set<Account.Id> projectWatchers = new HashSet<>();
@ -120,28 +93,21 @@ public class ProjectWatch {
}
}
}
return matching;
}
private Watchers getWatchersFromDb(NotifyType type)
throws OrmException {
Watchers matching = new Watchers();
Set<Account.Id> projectWatchers = new HashSet<>();
for (AccountProjectWatch w : args.db.get().accountProjectWatches()
.byProject(project)) {
if (add(matching, w, type)) {
// We only want to prevent matching All-Projects if this filter hits
projectWatchers.add(w.getAccountId());
for (ProjectState state : projectState.tree()) {
for (NotifyConfig nc : state.getConfig().getNotifyConfigs()) {
if (nc.isNotify(type)) {
try {
add(matching, nc);
} catch (QueryParseException e) {
log.warn("Project {} has invalid notify {} filter \"{}\": {}",
state.getProject().getName(), nc.getName(),
nc.getFilter(), e.getMessage());
}
}
}
}
for (AccountProjectWatch w : args.db.get().accountProjectWatches()
.byProject(args.allProjectsName)) {
if (!projectWatchers.contains(w.getAccountId())) {
add(matching, w, type);
}
}
return matching;
}
@ -240,25 +206,6 @@ public class ProjectWatch {
return false;
}
private boolean add(Watchers matching, AccountProjectWatch w, NotifyType type)
throws OrmException {
IdentifiedUser user = args.identifiedUserFactory.create(w.getAccountId());
try {
if (filterMatch(user, w.getFilter())) {
// If we are set to notify on this type, add the user.
// Otherwise, still return true to stop notifications for this user.
if (w.isNotify(type)) {
matching.bcc.accounts.add(w.getAccountId());
}
return true;
}
} catch (QueryParseException e) {
// Ignore broken filter expressions.
}
return false;
}
private boolean filterMatch(CurrentUser user, String filter)
throws OrmException, QueryParseException {
ChangeQueryBuilder qb;

@ -1 +1 @@
Subproject commit 7a9b8781cf0850815969f861f5f57178957f55c8
Subproject commit d0d51ea46e9e3220c1d22fda241a50ac341df5de