diff --git a/java/com/google/gerrit/pgm/init/InitAdminUser.java b/java/com/google/gerrit/pgm/init/InitAdminUser.java index 3251c010ab..e9f5cd5b71 100644 --- a/java/com/google/gerrit/pgm/init/InitAdminUser.java +++ b/java/com/google/gerrit/pgm/init/InitAdminUser.java @@ -21,6 +21,7 @@ import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.data.GroupReference; import com.google.gerrit.common.errors.NoSuchGroupException; import com.google.gerrit.extensions.client.AuthType; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo; import com.google.gerrit.pgm.init.api.AllUsersNameOnInitProvider; import com.google.gerrit.pgm.init.api.ConsoleUI; import com.google.gerrit.pgm.init.api.InitFlags; @@ -151,7 +152,12 @@ public class InitAdminUser implements InitStep { } AccountState as = - new AccountState(new AllUsersName(allUsers.get()), a, extIds, new HashMap<>()); + new AccountState( + new AllUsersName(allUsers.get()), + a, + extIds, + new HashMap<>(), + GeneralPreferencesInfo.defaults()); for (AccountIndex accountIndex : accountIndexCollection.getWriteIndexes()) { accountIndex.replace(as); } diff --git a/java/com/google/gerrit/reviewdb/client/Account.java b/java/com/google/gerrit/reviewdb/client/Account.java index bce07aade0..1f9ae0e399 100644 --- a/java/com/google/gerrit/reviewdb/client/Account.java +++ b/java/com/google/gerrit/reviewdb/client/Account.java @@ -19,7 +19,6 @@ import static com.google.gerrit.reviewdb.client.RefNames.REFS_STARRED_CHANGES; import static com.google.gerrit.reviewdb.client.RefNames.REFS_USERS; import com.google.gerrit.extensions.client.DiffPreferencesInfo; -import com.google.gerrit.extensions.client.GeneralPreferencesInfo; import com.google.gwtorm.client.Column; import com.google.gwtorm.client.IntKey; import java.sql.Timestamp; @@ -180,9 +179,6 @@ public final class Account { /** computed the username selected from the identities. */ protected String userName; - /** stored in git, used for caching the user's preferences. */ - private GeneralPreferencesInfo generalPreferences; - /** * ID of the user branch from which the account was read, {@code null} if the account was read * from ReviewDb. @@ -286,14 +282,6 @@ public final class Account { return registeredOn; } - public GeneralPreferencesInfo getGeneralPreferencesInfo() { - return generalPreferences; - } - - public void setGeneralPreferences(GeneralPreferencesInfo p) { - generalPreferences = p; - } - public String getMetaId() { return metaId; } diff --git a/java/com/google/gerrit/server/account/AccountCacheImpl.java b/java/com/google/gerrit/server/account/AccountCacheImpl.java index a7a795969c..963da62d14 100644 --- a/java/com/google/gerrit/server/account/AccountCacheImpl.java +++ b/java/com/google/gerrit/server/account/AccountCacheImpl.java @@ -20,6 +20,7 @@ import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.TimeUtil; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.server.account.externalids.ExternalId; import com.google.gerrit.server.account.externalids.ExternalIds; @@ -129,7 +130,12 @@ public class AccountCacheImpl implements AccountCache { private AccountState missing(Account.Id accountId) { Account account = new Account(accountId, TimeUtil.nowTs()); account.setActive(false); - return new AccountState(allUsersName, account, Collections.emptySet(), new HashMap<>()); + return new AccountState( + allUsersName, + account, + Collections.emptySet(), + new HashMap<>(), + GeneralPreferencesInfo.defaults()); } static class ByIdLoader extends CacheLoader> { diff --git a/java/com/google/gerrit/server/account/AccountState.java b/java/com/google/gerrit/server/account/AccountState.java index 01d31c9f67..6601df7cf8 100644 --- a/java/com/google/gerrit/server/account/AccountState.java +++ b/java/com/google/gerrit/server/account/AccountState.java @@ -22,6 +22,7 @@ import com.google.common.base.Strings; import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; import com.google.gerrit.common.Nullable; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.server.CurrentUser.PropertyKey; import com.google.gerrit.server.IdentifiedUser; @@ -51,17 +52,20 @@ public class AccountState { private final Account account; private final Collection externalIds; private final Map> projectWatches; + private final GeneralPreferencesInfo generalPreferences; private Cache, Object> properties; public AccountState( AllUsersName allUsersName, Account account, Collection externalIds, - Map> projectWatches) { + Map> projectWatches, + GeneralPreferencesInfo generalPreferences) { this.allUsersName = allUsersName; this.account = account; this.externalIds = externalIds; this.projectWatches = projectWatches; + this.generalPreferences = generalPreferences; this.account.setUserName(getUserName(externalIds)); } @@ -117,6 +121,11 @@ public class AccountState { return projectWatches; } + /** The general preferences of the account. */ + public GeneralPreferencesInfo getGeneralPreferences() { + return generalPreferences; + } + public static String getUserName(Collection ids) { for (ExternalId extId : ids) { if (extId.isScheme(SCHEME_USERNAME)) { diff --git a/java/com/google/gerrit/server/account/Accounts.java b/java/com/google/gerrit/server/account/Accounts.java index e8b5d88bce..45831ae9c8 100644 --- a/java/com/google/gerrit/server/account/Accounts.java +++ b/java/com/google/gerrit/server/account/Accounts.java @@ -141,7 +141,6 @@ public class Accounts { return Optional.empty(); } Account account = accountConfig.getLoadedAccount().get(); - account.setGeneralPreferences(accountConfig.getGeneralPreferences()); return Optional.of( new AccountState( allUsersName, @@ -149,7 +148,8 @@ public class Accounts { accountConfig.getExternalIdsRev().isPresent() ? externalIds.byAccount(accountId, accountConfig.getExternalIdsRev().get()) : ImmutableSet.of(), - accountConfig.getProjectWatches())); + accountConfig.getProjectWatches(), + accountConfig.getGeneralPreferences())); } public static Stream readUserRefs(Repository repo) throws IOException { diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java index a058aec797..00f25053e1 100644 --- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java +++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java @@ -1333,11 +1333,10 @@ class ReceiveCommits { this.publish = cmd.getRefName().startsWith(MagicBranch.NEW_PUBLISH_CHANGE); this.labelTypes = labelTypes; this.notesMigration = notesMigration; - GeneralPreferencesInfo prefs = user.getAccount().getGeneralPreferencesInfo(); + GeneralPreferencesInfo prefs = user.state().getGeneralPreferences(); this.defaultPublishComments = prefs != null - ? firstNonNull( - user.getAccount().getGeneralPreferencesInfo().publishCommentsOnPush, false) + ? firstNonNull(user.state().getGeneralPreferences().publishCommentsOnPush, false) : false; } diff --git a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java index ec67d9da33..8eb55fa7ed 100644 --- a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java +++ b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java @@ -121,34 +121,38 @@ public abstract class OutgoingEmail { Set
smtpRcptToPlaintextOnly = new HashSet<>(); if (shouldSendMessage()) { if (fromId != null) { - final Account fromUser = args.accountCache.get(fromId).getAccount(); - GeneralPreferencesInfo senderPrefs = fromUser.getGeneralPreferencesInfo(); - - if (senderPrefs != null && senderPrefs.getEmailStrategy() == CC_ON_OWN_COMMENTS) { - // If we are impersonating a user, make sure they receive a CC of - // this message so they can always review and audit what we sent - // on their behalf to others. - // - add(RecipientType.CC, fromId); - } else if (!accountsToNotify.containsValue(fromId) && rcptTo.remove(fromId)) { - // If they don't want a copy, but we queued one up anyway, - // drop them from the recipient lists. - // - removeUser(fromUser); + AccountState fromUser = args.accountCache.get(fromId); + if (fromUser != null) { + GeneralPreferencesInfo senderPrefs = fromUser.getGeneralPreferences(); + if (senderPrefs != null && senderPrefs.getEmailStrategy() == CC_ON_OWN_COMMENTS) { + // If we are impersonating a user, make sure they receive a CC of + // this message so they can always review and audit what we sent + // on their behalf to others. + // + add(RecipientType.CC, fromId); + } else if (!accountsToNotify.containsValue(fromId) && rcptTo.remove(fromId)) { + // If they don't want a copy, but we queued one up anyway, + // drop them from the recipient lists. + // + removeUser(fromUser.getAccount()); + } } } // Check the preferences of all recipients. If any user has disabled // 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) { - Account thisUser = args.accountCache.get(id).getAccount(); - GeneralPreferencesInfo prefs = thisUser.getGeneralPreferencesInfo(); - if (prefs == null || prefs.getEmailStrategy() == DISABLED) { - removeUser(thisUser); - } else if (useHtml() && prefs.getEmailFormat() == EmailFormat.PLAINTEXT) { - removeUser(thisUser); - smtpRcptToPlaintextOnly.add( - new Address(thisUser.getFullName(), thisUser.getPreferredEmail())); + AccountState thisUser = args.accountCache.get(id); + if (thisUser != null) { + Account thisUserAccount = thisUser.getAccount(); + GeneralPreferencesInfo prefs = thisUser.getGeneralPreferences(); + if (prefs == null || prefs.getEmailStrategy() == DISABLED) { + removeUser(thisUserAccount); + } else if (useHtml() && prefs.getEmailFormat() == EmailFormat.PLAINTEXT) { + removeUser(thisUserAccount); + smtpRcptToPlaintextOnly.add( + new Address(thisUserAccount.getFullName(), thisUserAccount.getPreferredEmail())); + } } if (smtpRcptTo.isEmpty() && smtpRcptToPlaintextOnly.isEmpty()) { return; diff --git a/java/com/google/gerrit/server/restapi/account/GetPreferences.java b/java/com/google/gerrit/server/restapi/account/GetPreferences.java index b071ade66d..46bc389cf1 100644 --- a/java/com/google/gerrit/server/restapi/account/GetPreferences.java +++ b/java/com/google/gerrit/server/restapi/account/GetPreferences.java @@ -50,6 +50,6 @@ public class GetPreferences implements RestReadView { } Account.Id id = rsrc.getUser().getAccountId(); - return accountCache.get(id).getAccount().getGeneralPreferencesInfo(); + return accountCache.get(id).getGeneralPreferences(); } } diff --git a/java/com/google/gerrit/server/restapi/account/SetPreferences.java b/java/com/google/gerrit/server/restapi/account/SetPreferences.java index 19c445c30f..8584240abc 100644 --- a/java/com/google/gerrit/server/restapi/account/SetPreferences.java +++ b/java/com/google/gerrit/server/restapi/account/SetPreferences.java @@ -90,7 +90,7 @@ public class SetPreferences implements RestModifyView u.setGeneralPreferences(input)); - return cache.get(id).getAccount().getGeneralPreferencesInfo(); + return cache.get(id).getGeneralPreferences(); } public static void storeMyMenus(VersionedAccountPreferences prefs, List my) diff --git a/java/com/google/gerrit/server/restapi/change/CreateChange.java b/java/com/google/gerrit/server/restapi/change/CreateChange.java index d4e1c405d3..a6a4631aaf 100644 --- a/java/com/google/gerrit/server/restapi/change/CreateChange.java +++ b/java/com/google/gerrit/server/restapi/change/CreateChange.java @@ -231,7 +231,7 @@ public class CreateChange IdentifiedUser me = user.get().asIdentifiedUser(); PersonIdent author = me.newCommitterIdent(now, serverTimeZone); AccountState account = accountCache.get(me.getAccountId()); - GeneralPreferencesInfo info = account.getAccount().getGeneralPreferencesInfo(); + GeneralPreferencesInfo info = account.getGeneralPreferences(); ObjectId treeId = mergeTip == null ? emptyTreeId(oi) : mergeTip.getTree(); ObjectId id = ChangeIdUtil.computeChangeId(treeId, mergeTip, author, author, input.subject); diff --git a/java/com/google/gerrit/testing/FakeAccountCache.java b/java/com/google/gerrit/testing/FakeAccountCache.java index 825676509b..7668912462 100644 --- a/java/com/google/gerrit/testing/FakeAccountCache.java +++ b/java/com/google/gerrit/testing/FakeAccountCache.java @@ -17,6 +17,7 @@ package com.google.gerrit.testing; import com.google.common.collect.ImmutableSet; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.TimeUtil; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountState; @@ -79,6 +80,7 @@ public class FakeAccountCache implements AccountCache { new AllUsersName(AllUsersNameProvider.DEFAULT), account, ImmutableSet.of(), - new HashMap<>()); + new HashMap<>(), + GeneralPreferencesInfo.defaults()); } } diff --git a/javatests/com/google/gerrit/server/index/account/AccountFieldTest.java b/javatests/com/google/gerrit/server/index/account/AccountFieldTest.java index 6bba51b8ae..1542fe5258 100644 --- a/javatests/com/google/gerrit/server/index/account/AccountFieldTest.java +++ b/javatests/com/google/gerrit/server/index/account/AccountFieldTest.java @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Streams; import com.google.gerrit.common.TimeUtil; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.server.account.AccountState; @@ -44,7 +45,12 @@ public class AccountFieldTest extends GerritBaseTests { List values = toStrings( AccountField.REF_STATE.get( - new AccountState(allUsersName, account, ImmutableSet.of(), ImmutableMap.of()))); + new AccountState( + allUsersName, + account, + ImmutableSet.of(), + ImmutableMap.of(), + GeneralPreferencesInfo.defaults()))); assertThat(values).hasSize(1); String expectedValue = allUsersName.get() + ":" + RefNames.refsUsers(account.getId()) + ":" + metaId; @@ -73,7 +79,11 @@ public class AccountFieldTest extends GerritBaseTests { toStrings( AccountField.EXTERNAL_ID_STATE.get( new AccountState( - null, account, ImmutableSet.of(extId1, extId2), ImmutableMap.of()))); + null, + account, + ImmutableSet.of(extId1, extId2), + ImmutableMap.of(), + GeneralPreferencesInfo.defaults()))); String expectedValue1 = extId1.key().sha1().name() + ":" + extId1.blobId().name(); String expectedValue2 = extId2.key().sha1().name() + ":" + extId2.blobId().name(); assertThat(values).containsExactly(expectedValue1, expectedValue2); diff --git a/javatests/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java b/javatests/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java index d65dd47501..01e8225e67 100644 --- a/javatests/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java +++ b/javatests/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java @@ -22,6 +22,7 @@ import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.verify; import com.google.gerrit.common.TimeUtil; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountState; @@ -388,6 +389,7 @@ public class FromAddressGeneratorProviderTest { new AllUsersName(AllUsersNameProvider.DEFAULT), account, Collections.emptySet(), - new HashMap<>()); + new HashMap<>(), + GeneralPreferencesInfo.defaults()); } }