AccountCache: Rename maybeGet to get
maybeGet is the method that most callers should use. Make this more obvious by renaming it to get. Also the return type of Optional already implies that there might not be a result, hence the method name doesn't need to be explicit about this. Change-Id: I571fa9989b2629d96f2238d4b8571d669a9bc89d Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
@@ -814,7 +814,7 @@ public abstract class AbstractDaemonTest {
|
||||
}
|
||||
|
||||
protected AccountState getAccountState(Account.Id accountId) {
|
||||
Optional<AccountState> accountState = accountCache.maybeGet(accountId);
|
||||
Optional<AccountState> accountState = accountCache.get(accountId);
|
||||
assertThat(accountState).named("account %s", accountId.get()).isPresent();
|
||||
return accountState.get();
|
||||
}
|
||||
|
||||
@@ -160,7 +160,7 @@ class BecomeAnyAccountLoginServlet extends HttpServlet {
|
||||
Element userlistElement = HtmlDomUtil.find(doc, "userlist");
|
||||
try (ReviewDb db = schema.open()) {
|
||||
for (Account.Id accountId : accounts.firstNIds(100)) {
|
||||
Optional<AccountState> accountState = accountCache.maybeGet(accountId);
|
||||
Optional<AccountState> accountState = accountCache.get(accountId);
|
||||
if (!accountState.isPresent()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -29,7 +29,7 @@ public interface AccountCache {
|
||||
* @return {@code AccountState} instance for the given account ID, if no account with this ID
|
||||
* exists {@link Optional#empty()} is returned
|
||||
*/
|
||||
Optional<AccountState> maybeGet(Account.Id accountId);
|
||||
Optional<AccountState> get(Account.Id accountId);
|
||||
|
||||
/**
|
||||
* Returns an {@code AccountState} instance for the given account ID. If not cached yet the
|
||||
@@ -39,7 +39,7 @@ public interface AccountCache {
|
||||
* <p>This method should only be used in exceptional cases where it is required to get an account
|
||||
* state even if the account is missing. Callers should leave a comment with the method invocation
|
||||
* explaining why this method is used. Most callers of {@link AccountCache} should use {@link
|
||||
* #maybeGet(Account.Id)} instead and handle the missing account case explicitly.
|
||||
* #get(Account.Id)} instead and handle the missing account case explicitly.
|
||||
*
|
||||
* @param accountId ID of the account that should be retrieved
|
||||
* @return {@code AccountState} instance for the given account ID, if no account with this ID
|
||||
|
||||
@@ -87,7 +87,7 @@ public class AccountCacheImpl implements AccountCache {
|
||||
}
|
||||
|
||||
@Override
|
||||
public Optional<AccountState> maybeGet(Account.Id accountId) {
|
||||
public Optional<AccountState> get(Account.Id accountId) {
|
||||
try {
|
||||
return byId.get(accountId);
|
||||
} catch (ExecutionException e) {
|
||||
@@ -103,7 +103,7 @@ public class AccountCacheImpl implements AccountCache {
|
||||
if (extId == null) {
|
||||
return Optional.empty();
|
||||
}
|
||||
return maybeGet(extId.accountId());
|
||||
return get(extId.accountId());
|
||||
} catch (IOException | ConfigInvalidException e) {
|
||||
log.warn("Cannot load AccountState for username " + username, e);
|
||||
return null;
|
||||
|
||||
@@ -147,7 +147,7 @@ public class AccountManager {
|
||||
return create(db, who);
|
||||
}
|
||||
|
||||
Optional<AccountState> accountState = byIdCache.maybeGet(id.accountId());
|
||||
Optional<AccountState> accountState = byIdCache.get(id.accountId());
|
||||
if (!accountState.isPresent()) {
|
||||
log.error(
|
||||
String.format(
|
||||
@@ -215,7 +215,7 @@ public class AccountManager {
|
||||
throw new AccountException("Unable to deactivate account " + account.getId(), e);
|
||||
}
|
||||
}
|
||||
return byIdCache.maybeGet(account.getId()).map(AccountState::getAccount);
|
||||
return byIdCache.get(account.getId()).map(AccountState::getAccount);
|
||||
}
|
||||
|
||||
private boolean shouldUpdateActiveStatus(AuthRequest authRequest) {
|
||||
|
||||
@@ -69,12 +69,12 @@ public class AccountResolver {
|
||||
public Account find(String nameOrEmail) throws OrmException, IOException, ConfigInvalidException {
|
||||
Set<Account.Id> r = findAll(nameOrEmail);
|
||||
if (r.size() == 1) {
|
||||
return byId.maybeGet(r.iterator().next()).map(AccountState::getAccount).orElse(null);
|
||||
return byId.get(r.iterator().next()).map(AccountState::getAccount).orElse(null);
|
||||
}
|
||||
|
||||
Account match = null;
|
||||
for (Account.Id id : r) {
|
||||
Optional<Account> account = byId.maybeGet(id).map(AccountState::getAccount);
|
||||
Optional<Account> account = byId.get(id).map(AccountState::getAccount);
|
||||
if (!account.map(Account::isActive).orElse(false)) {
|
||||
continue;
|
||||
}
|
||||
@@ -128,7 +128,7 @@ public class AccountResolver {
|
||||
public Account findByNameOrEmail(String nameOrEmail) throws OrmException, IOException {
|
||||
Set<Account.Id> r = findAllByNameOrEmail(nameOrEmail);
|
||||
return r.size() == 1
|
||||
? byId.maybeGet(r.iterator().next()).map(AccountState::getAccount).orElse(null)
|
||||
? byId.get(r.iterator().next()).map(AccountState::getAccount).orElse(null)
|
||||
: null;
|
||||
}
|
||||
|
||||
@@ -152,7 +152,7 @@ public class AccountResolver {
|
||||
String name = nameOrEmail.substring(0, lt - 1);
|
||||
Set<Account.Id> nameMatches = new HashSet<>();
|
||||
for (Account.Id id : ids) {
|
||||
Optional<Account> a = byId.maybeGet(id).map(AccountState::getAccount);
|
||||
Optional<Account> a = byId.get(id).map(AccountState::getAccount);
|
||||
if (a.isPresent() && name.equals(a.get().getFullName())) {
|
||||
nameMatches.add(id);
|
||||
}
|
||||
|
||||
@@ -51,7 +51,7 @@ import org.slf4j.LoggerFactory;
|
||||
* and properties from the account config file. AccountState maps one-to-one to Account.
|
||||
*
|
||||
* <p>Most callers should not construct AccountStates directly but rather lookup accounts via the
|
||||
* account cache (see {@link AccountCache#maybeGet(Account.Id)}).
|
||||
* account cache (see {@link AccountCache#get(Account.Id)}).
|
||||
*/
|
||||
public class AccountState {
|
||||
private static final Logger logger = LoggerFactory.getLogger(AccountState.class);
|
||||
|
||||
@@ -129,7 +129,7 @@ public class GroupMembers {
|
||||
.getMembers()
|
||||
.stream()
|
||||
.filter(groupControl::canSeeMember)
|
||||
.map(accountCache::maybeGet)
|
||||
.map(accountCache::get)
|
||||
.flatMap(Streams::stream)
|
||||
.map(AccountState::getAccount)
|
||||
.collect(toImmutableSet());
|
||||
|
||||
@@ -68,7 +68,7 @@ public class InternalAccountDirectory extends AccountDirectory {
|
||||
}
|
||||
for (AccountInfo info : in) {
|
||||
Account.Id id = new Account.Id(info._accountId);
|
||||
Optional<AccountState> state = accountCache.maybeGet(id);
|
||||
Optional<AccountState> state = accountCache.get(id);
|
||||
if (state.isPresent()) {
|
||||
fill(info, state.get(), options);
|
||||
} else {
|
||||
|
||||
@@ -117,7 +117,7 @@ public class ExternalIdsConsistencyChecker {
|
||||
private List<ConsistencyProblemInfo> validateExternalId(ExternalId extId) {
|
||||
List<ConsistencyProblemInfo> problems = new ArrayList<>();
|
||||
|
||||
if (!accountCache.maybeGet(extId.accountId()).isPresent()) {
|
||||
if (!accountCache.get(extId.accountId()).isPresent()) {
|
||||
addError(
|
||||
String.format(
|
||||
"External ID '%s' belongs to account that doesn't exist: %s",
|
||||
|
||||
@@ -173,7 +173,7 @@ public class ChangeResource implements RestResource, HasETag {
|
||||
}
|
||||
|
||||
for (Account.Id accountId : accounts) {
|
||||
Optional<AccountState> accountState = accountCache.maybeGet(accountId);
|
||||
Optional<AccountState> accountState = accountCache.get(accountId);
|
||||
if (accountState.isPresent()) {
|
||||
hashAccount(h, accountState.get(), buf);
|
||||
} else {
|
||||
|
||||
@@ -572,7 +572,7 @@ public class EventFactory {
|
||||
if (id == null) {
|
||||
return null;
|
||||
}
|
||||
return accountCache.maybeGet(id).map(a -> asAccountAttribute(a)).orElse(null);
|
||||
return accountCache.get(id).map(a -> asAccountAttribute(a)).orElse(null);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -437,7 +437,7 @@ abstract class SubmitStrategyOp implements BatchUpdateOp {
|
||||
private String getByAccountName() {
|
||||
checkNotNull(submitter, "getByAccountName called before submitter populated");
|
||||
Optional<Account> account =
|
||||
args.accountCache.maybeGet(submitter.getAccountId()).map(AccountState::getAccount);
|
||||
args.accountCache.get(submitter.getAccountId()).map(AccountState::getAccount);
|
||||
if (account.isPresent() && account.get().getFullName() != null) {
|
||||
return " by " + account.get().getFullName();
|
||||
}
|
||||
@@ -556,7 +556,7 @@ abstract class SubmitStrategyOp implements BatchUpdateOp {
|
||||
args.changeMerged.fire(
|
||||
updatedChange,
|
||||
mergedPatchSet,
|
||||
args.accountCache.maybeGet(submitter.getAccountId()).orElse(null),
|
||||
args.accountCache.get(submitter.getAccountId()).orElse(null),
|
||||
args.mergeTip.getCurrentTip().name(),
|
||||
ctx.getWhen());
|
||||
}
|
||||
|
||||
@@ -202,6 +202,6 @@ class DbGroupMemberAuditListener implements GroupMemberAuditListener {
|
||||
}
|
||||
|
||||
private Optional<String> getUserName(Account.Id accountId) {
|
||||
return accountCache.maybeGet(accountId).map(AccountState::getUserName).orElse(Optional.empty());
|
||||
return accountCache.get(accountId).map(AccountState::getUserName).orElse(Optional.empty());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -51,7 +51,7 @@ public class AuditLogFormatter {
|
||||
}
|
||||
|
||||
private static Optional<Account> getAccount(AccountCache accountCache, Account.Id accountId) {
|
||||
return accountCache.maybeGet(accountId).map(AccountState::getAccount);
|
||||
return accountCache.get(accountId).map(AccountState::getAccount);
|
||||
}
|
||||
|
||||
private static Optional<GroupDescription.Basic> getGroup(
|
||||
|
||||
@@ -91,7 +91,7 @@ public class AccountIndexerImpl implements AccountIndexer {
|
||||
@Override
|
||||
public void index(Account.Id id) throws IOException {
|
||||
for (Index<Account.Id, AccountState> i : getWriteIndexes()) {
|
||||
Optional<AccountState> accountState = byIdCache.maybeGet(id);
|
||||
Optional<AccountState> accountState = byIdCache.get(id);
|
||||
if (accountState.isPresent()) {
|
||||
i.replace(accountState.get());
|
||||
} else {
|
||||
|
||||
@@ -89,7 +89,7 @@ public class AllAccountsIndexer extends SiteIndexer<Account.Id, AccountState, Ac
|
||||
() -> {
|
||||
try {
|
||||
accountCache.evict(id);
|
||||
Optional<AccountState> a = accountCache.maybeGet(id);
|
||||
Optional<AccountState> a = accountCache.get(id);
|
||||
if (a.isPresent()) {
|
||||
index.replace(a.get());
|
||||
} else {
|
||||
|
||||
@@ -166,7 +166,7 @@ public class MailProcessor {
|
||||
return;
|
||||
}
|
||||
Account.Id accountId = accountIds.iterator().next();
|
||||
Optional<AccountState> accountState = accountCache.maybeGet(accountId);
|
||||
Optional<AccountState> accountState = accountCache.get(accountId);
|
||||
if (!accountState.isPresent()) {
|
||||
log.warn(String.format("Mail: Account %s doesn't exist. Will delete message.", accountId));
|
||||
return;
|
||||
|
||||
@@ -349,7 +349,7 @@ public abstract class ChangeEmail extends NotificationEmail {
|
||||
protected void removeUsersThatIgnoredTheChange() {
|
||||
for (Map.Entry<Account.Id, Collection<String>> e : stars.asMap().entrySet()) {
|
||||
if (e.getValue().contains(StarredChangesUtil.IGNORE_LABEL)) {
|
||||
args.accountCache.maybeGet(e.getKey()).ifPresent(a -> removeUser(a.getAccount()));
|
||||
args.accountCache.get(e.getKey()).ifPresent(a -> removeUser(a.getAccount()));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -123,7 +123,7 @@ public class FromAddressGeneratorProvider implements Provider<FromAddressGenerat
|
||||
public Address from(Account.Id fromId) {
|
||||
String senderName;
|
||||
if (fromId != null) {
|
||||
Optional<Account> a = accountCache.maybeGet(fromId).map(AccountState::getAccount);
|
||||
Optional<Account> a = accountCache.get(fromId).map(AccountState::getAccount);
|
||||
String fullName = a.map(Account::getFullName).orElse(null);
|
||||
String userEmail = a.map(Account::getPreferredEmail).orElse(null);
|
||||
if (canRelay(userEmail)) {
|
||||
@@ -209,7 +209,7 @@ public class FromAddressGeneratorProvider implements Provider<FromAddressGenerat
|
||||
|
||||
if (fromId != null) {
|
||||
String fullName =
|
||||
accountCache.maybeGet(fromId).map(a -> a.getAccount().getFullName()).orElse(null);
|
||||
accountCache.get(fromId).map(a -> a.getAccount().getFullName()).orElse(null);
|
||||
if (fullName == null || "".equals(fullName)) {
|
||||
fullName = anonymousCowardName;
|
||||
}
|
||||
|
||||
@@ -122,7 +122,7 @@ public abstract class OutgoingEmail {
|
||||
Set<Address> smtpRcptToPlaintextOnly = new HashSet<>();
|
||||
if (shouldSendMessage()) {
|
||||
if (fromId != null) {
|
||||
Optional<AccountState> fromUser = args.accountCache.maybeGet(fromId);
|
||||
Optional<AccountState> fromUser = args.accountCache.get(fromId);
|
||||
if (fromUser.isPresent()) {
|
||||
GeneralPreferencesInfo senderPrefs = fromUser.get().getGeneralPreferences();
|
||||
if (senderPrefs != null && senderPrefs.getEmailStrategy() == CC_ON_OWN_COMMENTS) {
|
||||
@@ -143,7 +143,7 @@ public abstract class OutgoingEmail {
|
||||
// his email notifications then drop him from recipients' list.
|
||||
// In addition, check if users only want to receive plaintext email.
|
||||
for (Account.Id id : rcptTo) {
|
||||
Optional<AccountState> thisUser = args.accountCache.maybeGet(id);
|
||||
Optional<AccountState> thisUser = args.accountCache.get(id);
|
||||
if (thisUser.isPresent()) {
|
||||
Account thisUserAccount = thisUser.get().getAccount();
|
||||
GeneralPreferencesInfo prefs = thisUser.get().getGeneralPreferences();
|
||||
@@ -254,7 +254,7 @@ public abstract class OutgoingEmail {
|
||||
|
||||
protected String getFromLine() {
|
||||
StringBuilder f = new StringBuilder();
|
||||
Optional<Account> account = args.accountCache.maybeGet(fromId).map(AccountState::getAccount);
|
||||
Optional<Account> account = args.accountCache.get(fromId).map(AccountState::getAccount);
|
||||
if (account.isPresent()) {
|
||||
String name = account.get().getFullName();
|
||||
String email = account.get().getPreferredEmail();
|
||||
@@ -336,7 +336,7 @@ public abstract class OutgoingEmail {
|
||||
return args.gerritPersonIdent.getName();
|
||||
}
|
||||
|
||||
Optional<Account> account = args.accountCache.maybeGet(accountId).map(AccountState::getAccount);
|
||||
Optional<Account> account = args.accountCache.get(accountId).map(AccountState::getAccount);
|
||||
String name = null;
|
||||
if (account.isPresent()) {
|
||||
name = account.get().getFullName();
|
||||
@@ -358,7 +358,7 @@ public abstract class OutgoingEmail {
|
||||
* @return name/email of account, or Anonymous Coward if unset.
|
||||
*/
|
||||
public String getNameEmailFor(Account.Id accountId) {
|
||||
Optional<Account> account = args.accountCache.maybeGet(accountId).map(AccountState::getAccount);
|
||||
Optional<Account> account = args.accountCache.get(accountId).map(AccountState::getAccount);
|
||||
if (account.isPresent()) {
|
||||
String name = account.get().getFullName();
|
||||
String email = account.get().getPreferredEmail();
|
||||
@@ -381,7 +381,7 @@ public abstract class OutgoingEmail {
|
||||
* @return name/email of account, username, or null if unset.
|
||||
*/
|
||||
public String getUserNameEmailFor(Account.Id accountId) {
|
||||
Optional<AccountState> accountState = args.accountCache.maybeGet(accountId);
|
||||
Optional<AccountState> accountState = args.accountCache.get(accountId);
|
||||
if (!accountState.isPresent()) {
|
||||
return null;
|
||||
}
|
||||
@@ -518,7 +518,7 @@ public abstract class OutgoingEmail {
|
||||
}
|
||||
|
||||
private Address toAddress(Account.Id id) {
|
||||
Optional<Account> accountState = args.accountCache.maybeGet(id).map(AccountState::getAccount);
|
||||
Optional<Account> accountState = args.accountCache.get(id).map(AccountState::getAccount);
|
||||
if (!accountState.isPresent()) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -132,7 +132,7 @@ public class ChangeNoteUtil {
|
||||
}
|
||||
|
||||
public PersonIdent newIdent(Account.Id authorId, Date when, PersonIdent serverIdent) {
|
||||
Optional<Account> author = accountCache.maybeGet(authorId).map(AccountState::getAccount);
|
||||
Optional<Account> author = accountCache.get(authorId).map(AccountState::getAccount);
|
||||
return new PersonIdent(
|
||||
author.map(Account::getName).orElseGet(() -> Account.getName(authorId)),
|
||||
authorId.get() + "@" + serverId,
|
||||
|
||||
@@ -56,7 +56,7 @@ public class GetDiffPreferences implements RestReadView<AccountResource> {
|
||||
|
||||
Account.Id id = rsrc.getUser().getAccountId();
|
||||
return accountCache
|
||||
.maybeGet(id)
|
||||
.get(id)
|
||||
.map(AccountState::getDiffPreferences)
|
||||
.orElseThrow(() -> new ResourceNotFoundException(IdString.fromDecoded(id.toString())));
|
||||
}
|
||||
|
||||
@@ -56,7 +56,7 @@ public class GetEditPreferences implements RestReadView<AccountResource> {
|
||||
|
||||
Account.Id id = rsrc.getUser().getAccountId();
|
||||
return accountCache
|
||||
.maybeGet(id)
|
||||
.get(id)
|
||||
.map(AccountState::getEditPreferences)
|
||||
.orElseThrow(() -> new ResourceNotFoundException(IdString.fromDecoded(id.toString())));
|
||||
}
|
||||
|
||||
@@ -54,7 +54,7 @@ public class GetPreferences implements RestReadView<AccountResource> {
|
||||
|
||||
Account.Id id = rsrc.getUser().getAccountId();
|
||||
return accountCache
|
||||
.maybeGet(id)
|
||||
.get(id)
|
||||
.map(AccountState::getGeneralPreferences)
|
||||
.orElseThrow(() -> new ResourceNotFoundException(IdString.fromDecoded(id.toString())));
|
||||
}
|
||||
|
||||
@@ -207,7 +207,7 @@ public class PostReviewersOp implements BatchUpdateOp {
|
||||
List<AccountState> reviewers =
|
||||
addedReviewers
|
||||
.stream()
|
||||
.map(r -> accountCache.maybeGet(r.getAccountId()))
|
||||
.map(r -> accountCache.get(r.getAccountId()))
|
||||
.flatMap(Streams::stream)
|
||||
.collect(toList());
|
||||
reviewerAdded.fire(rsrc.getChange(), patchSet, reviewers, ctx.getAccount(), ctx.getWhen());
|
||||
|
||||
@@ -197,7 +197,7 @@ public class AddMembers implements RestModifyView<GroupResource, Input> {
|
||||
AuthRequest req = AuthRequest.forUser(user);
|
||||
req.setSkipAuthentication(true);
|
||||
return accountCache
|
||||
.maybeGet(accountManager.authenticate(req).getAccountId())
|
||||
.get(accountManager.authenticate(req).getAccountId())
|
||||
.map(AccountState::getAccount);
|
||||
} catch (AccountException e) {
|
||||
return Optional.empty();
|
||||
|
||||
@@ -142,7 +142,7 @@ public class SetMembersCommand extends SshCommand {
|
||||
.stream()
|
||||
.map(
|
||||
accountId -> {
|
||||
Optional<AccountState> accountState = accountCache.maybeGet(accountId);
|
||||
Optional<AccountState> accountState = accountCache.get(accountId);
|
||||
if (!accountState.isPresent()) {
|
||||
return "n/a";
|
||||
}
|
||||
|
||||
@@ -43,7 +43,7 @@ public class FakeAccountCache implements AccountCache {
|
||||
}
|
||||
|
||||
@Override
|
||||
public synchronized Optional<AccountState> maybeGet(Account.Id accountId) {
|
||||
public synchronized Optional<AccountState> get(Account.Id accountId) {
|
||||
return Optional.ofNullable(byId.get(accountId));
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user