opendev/gerrit
Code Issues Proposed changes

Add new API to lookup accounts by email

Browse Source 
Looking up accounts by email is currently done via the
AccountByEmailCache. However this cache is unneeded and we can just get
external IDs by email from the ExternalIdCache and then extract the
account IDs from the external IDs. This is exactly what
AccountByEmailCacheImpl.Loader is doing. Follow-up changes adapt the
Gerrit code to use the new API and then the AccountByEmailCache is
removed.

Using the new API is implemented in a separate change because this
change can only be submitted after I repaired our inconsistent internal
accounts. In the meantime plugins (e.g. the find-owners plugin) want to
implement new functionality based on the new API. Hence we want to merge
this change which adds the new API sooner.

Looking up accounts by email via the ExternalIdCache means that the SHA1
of the refs/meta/external-ids branch is read on each lookup (to verify
that the cache is up to date). This is a disadvantage if we need to
lookup many accounts by email in a loop, because then the SHA1 of the
refs/meta/external-ids branch is read once per email. This is why the
new API includes a method to lookup accounts for several emails at once
where the SHA1 of the refs/meta/external-ids branch is read only once.
Within Gerrit core we don't have a need for this method (yet), but
plugins may want to use it.

Change-Id: I757a4d065b4697977dc77cc269cd04e44f980ad0
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2017-07-19 14:00:22 +02:00
parent 8e2036ee1a
commit 5c7864b9a1
7 changed files with 132 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -14,13 +14,18 @@
package com.google.gerrit.server.account;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap;
import static java.util.Comparator.comparing;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toSet;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
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.account.externalids.ExternalIds;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -32,6 +37,7 @@ import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Stream;
@@ -50,17 +56,20 @@ public class Accounts {
private final GitRepositoryManager repoManager;
private final AllUsersName allUsersName;
private final OutgoingEmailValidator emailValidator;
private final ExternalIds externalIds;
@Inject
Accounts(
@GerritServerConfig Config cfg,
GitRepositoryManager repoManager,
AllUsersName allUsersName,
OutgoingEmailValidator emailValidator) {
OutgoingEmailValidator emailValidator,
ExternalIds externalIds) {
this.readFromGit = cfg.getBoolean("user", null, "readAccountsFromGit", false);
this.repoManager = repoManager;
this.allUsersName = allUsersName;
this.emailValidator = emailValidator;
this.externalIds = externalIds;
}
public Account get(ReviewDb db, Account.Id accountId)
@@ -89,6 +98,44 @@ public class Accounts {
return db.accounts().get(accountIds).toList();
}
/**
* Returns the accounts with the given email.
*
* <p>Each email should belong to a single account only. This means if more than one account is
* returned there is an inconsistency in the external IDs.
*
* <p>The accounts are retrieved via the external ID cache. Each access to the external ID cache
* requires reading the SHA1 of the refs/meta/external-ids branch. If accounts for multiple emails
* are needed it is more efficient to use {@link #byEmails(String...)} as this method reads the
* SHA1 of the refs/meta/external-ids branch only once (and not once per email).
*
* @see #byEmails(String...)
*/
public ImmutableSet<Account.Id> byEmail(String email) throws IOException {
return externalIds.byEmail(email).stream().map(e -> e.accountId()).collect(toImmutableSet());
}
/**
* Returns the accounts for the given emails.
*
* <p>Each email should belong to a single account only. This means if more than one account for
* an email is returned there is an inconsistency in the external IDs.
*
* <p>The accounts are retrieved via the external ID cache. Each access to the external ID cache
* requires reading the SHA1 of the refs/meta/external-ids branch. If accounts for multiple emails
* are needed it is more efficient to use this method instead of {@link #byEmail(String)} as this
* method reads the SHA1 of the refs/meta/external-ids branch only once (and not once per email).
*
* @see #byEmail(String)
*/
public ImmutableSetMultimap<String, Account.Id> byEmails(String... emails) throws IOException {
return externalIds
.byEmails(emails)
.entries()
.stream()
.collect(toImmutableSetMultimap(Map.Entry::getKey, e -> e.getValue().accountId()));
}
/**
* Returns all accounts.
*

3
gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/DisabledExternalIdCache.java
View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.account.externalids;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.gerrit.reviewdb.client.Account;
import com.google.inject.AbstractModule;
import com.google.inject.Module;
@@ -89,7 +90,7 @@ public class DisabledExternalIdCache implements ExternalIdCache {
}
@Override
public Set<ExternalId> byEmail(String email) throws IOException {
public ImmutableSetMultimap<String, ExternalId> byEmails(String... emails) throws IOException {
throw new UnsupportedOperationException();
}
}

15
gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdCache.java
View File

@@ -14,6 +14,8 @@
package com.google.gerrit.server.account.externalids;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.gerrit.reviewdb.client.Account;
import java.io.IOException;
import java.util.Collection;
@@ -21,7 +23,12 @@ import java.util.Collections;
import java.util.Set;
import org.eclipse.jgit.lib.ObjectId;
/** Caches external IDs of all accounts */
/**
* Caches external IDs of all accounts.
*
* <p>On each cache access the SHA1 of the refs/meta/external-ids branch is read to verify that the
* cache is up to date.
*/
interface ExternalIdCache {
void onCreate(ObjectId oldNotesRev, ObjectId newNotesRev, Collection<ExternalId> extId)
throws IOException;
@@ -75,7 +82,11 @@ interface ExternalIdCache {
Set<ExternalId> byAccount(Account.Id accountId) throws IOException;
Set<ExternalId> byEmail(String email) throws IOException;
ImmutableSetMultimap<String, ExternalId> byEmails(String... emails) throws IOException;
default ImmutableSet<ExternalId> byEmail(String email) throws IOException {
return byEmails(email).get(email);
}
default void onCreate(ObjectId oldNotesRev, ObjectId newNotesRev, ExternalId extId)
throws IOException {

9
gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdCacheImpl.java
View File

@@ -226,9 +226,14 @@ class ExternalIdCacheImpl implements ExternalIdCache {
}
@Override
public Set<ExternalId> byEmail(String email) throws IOException {
public ImmutableSetMultimap<String, ExternalId> byEmails(String... emails) throws IOException {
try {
return extIdsByAccount.get(externalIdReader.readRevision()).byEmail().get(email);
AllExternalIds allExternalIds = extIdsByAccount.get(externalIdReader.readRevision());
ImmutableSetMultimap.Builder<String, ExternalId> byEmails = ImmutableSetMultimap.builder();
for (String email : emails) {
byEmails.putAll(email, allExternalIds.byEmail().get(email));
}
return byEmails.build();
} catch (ExecutionException e) {
throw new IOException("Cannot list external ids by email", e);
}

32
gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIds.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.server.account.externalids;
import static java.util.stream.Collectors.toSet;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Account;
import com.google.inject.Inject;
@@ -74,7 +75,38 @@ public class ExternalIds {
return byAccount(accountId).stream().filter(e -> e.key().isScheme(scheme)).collect(toSet());
}
/**
* Returns the external ID with the given email.
*
* <p>Each email should belong to a single external ID only. This means if more than one external
* ID is returned there is an inconsistency in the external IDs.
*
* <p>The external IDs are retrieved from the external ID cache. Each access to the external ID
* cache requires reading the SHA1 of the refs/meta/external-ids branch. If external IDs for
* multiple emails are needed it is more efficient to use {@link #byEmails(String...)} as this
* method reads the SHA1 of the refs/meta/external-ids branch only once (and not once per email).
*
* @see #byEmails(String...)
*/
public Set<ExternalId> byEmail(String email) throws IOException {
return externalIdCache.byEmail(email);
}
/**
* Returns the external IDs for the given emails.
*
* <p>Each email should belong to a single external ID only. This means if more than one external
* ID for an email is returned there is an inconsistency in the external IDs.
*
* <p>The external IDs are retrieved from the external ID cache. Each access to the external ID
* cache requires reading the SHA1 of the refs/meta/external-ids branch. If external IDs for
* multiple emails are needed it is more efficient to use this method instead of {@link
* #byEmail(String)} as this method reads the SHA1 of the refs/meta/external-ids branch only once
* (and not once per email).
*
* @see #byEmail(String)
*/
public ImmutableSetMultimap<String, ExternalId> byEmails(String... emails) throws IOException {
return externalIdCache.byEmails(emails);
}
}