opendev/gerrit
Code Issues Proposed changes

Make AccountState an AutoValue

Browse Source 
AccountState is a cached entity that is shared between different
threads. We therefore want it to be immutable. AutoValue is the
preferred way of generating immutable entities that have equals and
toString implementations and is used for almost all other caches. Hence,
we use it for AccountState too.

Change-Id: I3b19ef34791c8800667fdcf9ad8beade8539af13
This commit is contained in:
Patrick Hiesel
2019-08-07 15:25:07 +02:00
parent a5dd85e5d2
commit 3e64fd492b
82 changed files with 253 additions and 278 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
View File

@@ -433,7 +433,7 @@ public class AccountIT extends AbstractDaemonTest {
"Create Account Atomically",
accountId,
u -> u.setFullName(fullName).addExternalId(extId));
assertThat(accountState.getAccount().fullName()).isEqualTo(fullName);
assertThat(accountState.account().fullName()).isEqualTo(fullName);
AccountInfo info = gApi.accounts().id(accountId.get()).get();
assertThat(info.name).isEqualTo(fullName);
@@ -473,7 +473,7 @@ public class AccountIT extends AbstractDaemonTest {
.get()
.update("Set status", anonymousCoward.id(), u -> u.setStatus(status));
assertThat(accountState).isPresent();
Account account = accountState.get().getAccount();
Account account = accountState.get().account();
assertThat(account.fullName()).isNull();
assertThat(account.status()).isEqualTo(status);
assertUserBranch(anonymousCoward.id(), null, status);
@@ -596,7 +596,7 @@ public class AccountIT extends AbstractDaemonTest {
new AccountActivationValidationListener() {
@Override
public void validateActivation(AccountState account) throws ValidationException {
String preferredEmail = account.getAccount().preferredEmail();
String preferredEmail = account.account().preferredEmail();
if (preferredEmail == null || !preferredEmail.endsWith("@activatable.com")) {
throw new ValidationException("not allowed to active account");
}
@@ -604,7 +604,7 @@ public class AccountIT extends AbstractDaemonTest {
@Override
public void validateDeactivation(AccountState account) throws ValidationException {
String preferredEmail = account.getAccount().preferredEmail();
String preferredEmail = account.account().preferredEmail();
if (preferredEmail == null || !preferredEmail.endsWith("@deactivatable.com")) {
throw new ValidationException("not allowed to deactive account");
}
@@ -2461,21 +2461,20 @@ public class AccountIT extends AbstractDaemonTest {
@Test
public void checkMetaId() throws Exception {
// metaId is set when account is loaded
assertThat(accounts.get(admin.id()).get().getAccount().metaId())
.isEqualTo(getMetaId(admin.id()));
assertThat(accounts.get(admin.id()).get().account().metaId()).isEqualTo(getMetaId(admin.id()));
// metaId is set when account is created
AccountsUpdate au = accountsUpdateProvider.get();
Account.Id accountId = Account.id(seq.nextAccountId());
AccountState accountState = au.insert("Create Test Account", accountId, u -> {});
assertThat(accountState.getAccount().metaId()).isEqualTo(getMetaId(accountId));
assertThat(accountState.account().metaId()).isEqualTo(getMetaId(accountId));
// metaId is set when account is updated
Optional<AccountState> updatedAccountState =
au.update("Set Full Name", accountId, u -> u.setFullName("foo"));
assertThat(updatedAccountState).isPresent();
Account updatedAccount = updatedAccountState.get().getAccount();
assertThat(accountState.getAccount().metaId()).isNotEqualTo(updatedAccount.metaId());
Account updatedAccount = updatedAccountState.get().account();
assertThat(accountState.account().metaId()).isNotEqualTo(updatedAccount.metaId());
assertThat(updatedAccount.metaId()).isEqualTo(getMetaId(accountId));
}
@@ -2621,7 +2620,7 @@ public class AccountIT extends AbstractDaemonTest {
assertThat(doneBgUpdate.get()).isTrue();
assertThat(updatedAccountState).isPresent();
Account updatedAccount = updatedAccountState.get().getAccount();
Account updatedAccount = updatedAccountState.get().account();
assertThat(updatedAccount.status()).isEqualTo(status);
assertThat(updatedAccount.fullName()).isEqualTo(fullName);
@@ -2678,7 +2677,7 @@ public class AccountIT extends AbstractDaemonTest {
() -> update.update("Set Full Name", admin.id(), u -> u.setFullName(fullName)));
assertThat(bgCounter.get()).isEqualTo(status.size());
Account updatedAccount = accounts.get(admin.id()).get().getAccount();
Account updatedAccount = accounts.get(admin.id()).get().account();
assertThat(updatedAccount.status()).isEqualTo(Iterables.getLast(status));
assertThat(updatedAccount.fullName()).isEqualTo(admin.fullName());
@@ -2730,12 +2729,12 @@ public class AccountIT extends AbstractDaemonTest {
"Set Status",
admin.id(),
(a, u) -> {
if ("A-1".equals(a.getAccount().status())) {
if ("A-1".equals(a.account().status())) {
bgCounterA1.getAndIncrement();
u.setStatus("B-1");
}
if ("A-2".equals(a.getAccount().status())) {
if ("A-2".equals(a.account().status())) {
bgCounterA2.getAndIncrement();
u.setStatus("B-2");
}
@@ -2745,8 +2744,8 @@ public class AccountIT extends AbstractDaemonTest {
assertThat(bgCounterA2.get()).isEqualTo(1);
assertThat(updatedAccountState).isPresent();
assertThat(updatedAccountState.get().getAccount().status()).isEqualTo("B-2");
assertThat(accounts.get(admin.id()).get().getAccount().status()).isEqualTo("B-2");
assertThat(updatedAccountState.get().account().status()).isEqualTo("B-2");
assertThat(accounts.get(admin.id()).get().account().status()).isEqualTo("B-2");
assertThat(gApi.accounts().id(admin.id().get()).get().status).isEqualTo("B-2");
}
@@ -2812,12 +2811,12 @@ public class AccountIT extends AbstractDaemonTest {
"Update External ID",
accountId,
(a, u) -> {
if (a.getExternalIds().contains(extIdA1)) {
if (a.externalIds().contains(extIdA1)) {
bgCounterA1.getAndIncrement();
u.replaceExternalId(extIdA1, extIdB1);
}
if (a.getExternalIds().contains(extIdA2)) {
if (a.externalIds().contains(extIdA2)) {
bgCounterA2.getAndIncrement();
u.replaceExternalId(extIdA2, extIdB2);
}
@@ -2827,8 +2826,8 @@ public class AccountIT extends AbstractDaemonTest {
assertThat(bgCounterA2.get()).isEqualTo(1);
assertThat(updatedAccount).isPresent();
assertThat(updatedAccount.get().getExternalIds()).containsExactly(extIdB2);
assertThat(accounts.get(accountId).get().getExternalIds()).containsExactly(extIdB2);
assertThat(updatedAccount.get().externalIds()).containsExactly(extIdB2);
assertThat(accounts.get(accountId).get().externalIds()).containsExactly(extIdB2);
assertThat(
gApi.accounts().id(accountId.get()).getExternalIds().stream()
.map(i -> i.identity)

10
javatests/com/google/gerrit/acceptance/api/accounts/AccountIndexerIT.java
View File

@@ -66,7 +66,7 @@ public class AccountIndexerIT {
List<AccountState> matchedAccountStates =
accountQueryProvider.get().byPreferredEmail(preferredEmail);
assertThat(matchedAccountStates).hasSize(1);
assertThat(matchedAccountStates.get(0).getAccount().id()).isEqualTo(accountId);
assertThat(matchedAccountStates.get(0).account().id()).isEqualTo(accountId);
}
@Test
@@ -82,7 +82,7 @@ public class AccountIndexerIT {
List<AccountState> matchedAccountStates =
accountQueryProvider.get().byPreferredEmail(preferredEmail);
assertThat(matchedAccountStates).hasSize(1);
assertThat(matchedAccountStates.get(0).getAccount().id()).isEqualTo(accountId);
assertThat(matchedAccountStates.get(0).account().id()).isEqualTo(accountId);
}
@Test
@@ -91,10 +91,10 @@ public class AccountIndexerIT {
loadAccountToCache(accountId);
String status = "ooo";
updateAccountWithoutCacheOrIndex(accountId, newAccountUpdate().setStatus(status).build());
assertThat(accountCache.get(accountId).get().getAccount().status()).isNull();
assertThat(accountCache.get(accountId).get().account().status()).isNull();
accountIndexer.index(accountId);
assertThat(accountCache.get(accountId).get().getAccount().status()).isEqualTo(status);
assertThat(accountCache.get(accountId).get().account().status()).isEqualTo(status);
}
@Test
@@ -109,7 +109,7 @@ public class AccountIndexerIT {
List<AccountState> matchedAccountStates =
accountQueryProvider.get().byPreferredEmail(preferredEmail);
assertThat(matchedAccountStates).hasSize(1);
assertThat(matchedAccountStates.get(0).getAccount().id()).isEqualTo(accountId);
assertThat(matchedAccountStates.get(0).account().id()).isEqualTo(accountId);
}
@Test

12
javatests/com/google/gerrit/acceptance/api/accounts/AccountManagerIT.java
View File

@@ -192,7 +192,7 @@ public class AccountManagerIT extends AbstractDaemonTest {
Optional<AccountState> accountState = accounts.get(accountId);
assertThat(accountState).isPresent();
assertThat(accountState.get().getAccount().preferredEmail()).isEqualTo(newEmail);
assertThat(accountState.get().account().preferredEmail()).isEqualTo(newEmail);
}
@Test
@@ -217,7 +217,7 @@ public class AccountManagerIT extends AbstractDaemonTest {
Optional<AccountState> accountState = accounts.get(accountId);
assertThat(accountState).isPresent();
assertThat(accountState.get().getAccount().fullName()).isEqualTo(newName);
assertThat(accountState.get().account().fullName()).isEqualTo(newName);
}
@Test
@@ -296,7 +296,7 @@ public class AccountManagerIT extends AbstractDaemonTest {
assertAuthResultForExistingAccount(authResult, accountId, gerritExtIdKey);
Optional<AccountState> accountState = accounts.get(accountId);
assertThat(accountState).isPresent();
assertThat(accountState.get().getAccount().isActive()).isTrue();
assertThat(accountState.get().account().isActive()).isTrue();
}
@Test
@@ -317,7 +317,7 @@ public class AccountManagerIT extends AbstractDaemonTest {
assertAuthResultForExistingAccount(authResult, accountId, gerritExtIdKey);
Optional<AccountState> accountState = accounts.get(accountId);
assertThat(accountState).isPresent();
assertThat(accountState.get().getAccount().isActive()).isTrue();
assertThat(accountState.get().account().isActive()).isTrue();
}
@Test
@@ -341,7 +341,7 @@ public class AccountManagerIT extends AbstractDaemonTest {
Optional<AccountState> accountState = accounts.get(accountId);
assertThat(accountState).isPresent();
assertThat(accountState.get().getAccount().isActive()).isFalse();
assertThat(accountState.get().account().isActive()).isFalse();
}
@Test
@@ -433,7 +433,7 @@ public class AccountManagerIT extends AbstractDaemonTest {
// Verify that the preferred email was not updated.
Optional<AccountState> accountState = accounts.get(accountId);
assertThat(accountState).isPresent();
assertThat(accountState.get().getAccount().preferredEmail()).isEqualTo(email);
assertThat(accountState.get().account().preferredEmail()).isEqualTo(email);
}
@Test

4
javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
View File

@@ -112,7 +112,7 @@ public class ExternalIdIT extends AbstractDaemonTest {
@Test
public void getExternalIds() throws Exception {
Collection<ExternalId> expectedIds = getAccountState(user.id()).getExternalIds();
Collection<ExternalId> expectedIds = getAccountState(user.id()).externalIds();
List<AccountExternalIdInfo> expectedIdInfos = toExternalIdInfos(expectedIds);
RestResponse response = userRestSession.get("/accounts/self/external.ids");
@@ -142,7 +142,7 @@ public class ExternalIdIT extends AbstractDaemonTest {
.add(allowCapability(GlobalCapability.ACCESS_DATABASE).group(REGISTERED_USERS))
.update();
Collection<ExternalId> expectedIds = getAccountState(admin.id()).getExternalIds();
Collection<ExternalId> expectedIds = getAccountState(admin.id()).externalIds();
List<AccountExternalIdInfo> expectedIdInfos = toExternalIdInfos(expectedIds);
RestResponse response = userRestSession.get("/accounts/" + admin.id() + "/external.ids");

2
javatests/com/google/gerrit/acceptance/server/account/AccountResolverIT.java
View File

@@ -340,6 +340,6 @@ public class AccountResolverIT extends AbstractDaemonTest {
accountsUpdateProvider
.get()
.update("Force set preferred email", id, (s, u) -> u.setPreferredEmail(email));
assertThat(result.map(a -> a.getAccount().preferredEmail())).hasValue(email);
assertThat(result.map(a -> a.account().preferredEmail())).hasValue(email);
}
}

2
javatests/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java
View File

@@ -198,7 +198,7 @@ public class GerritPublicKeyCheckerTest {
.update(
"Delete External IDs",
user.getAccountId(),
(a, u) -> u.deleteExternalIds(a.getExternalIds()));
(a, u) -> u.deleteExternalIds(a.externalIds()));
reloadUser();
TestKey key = validKeyWithSecondUserId();

18
javatests/com/google/gerrit/server/account/AccountResolverTest.java
View File

@@ -84,7 +84,7 @@ public class AccountResolverTest {
@Override
public String toString() {
return accounts.stream()
.map(a -> a.getAccount().id().toString())
.map(a -> a.account().id().toString())
.collect(joining(",", pattern + "(", ")"));
}
}
@@ -156,7 +156,7 @@ public class AccountResolverTest {
// Searchers always short-circuit when finding a non-empty result list, and this one didn't
// filter out inactive results, so the second searcher never ran.
assertThat(result.asIdSet()).containsExactlyElementsIn(ids(1));
assertThat(getOnlyElement(result.asList()).getAccount().isActive()).isFalse();
assertThat(getOnlyElement(result.asList()).account().isActive()).isFalse();
assertThat(filteredInactiveIds(result)).isEmpty();
}
@@ -173,7 +173,7 @@ public class AccountResolverTest {
// and this one didn't filter out inactive results,
// so the second searcher never ran.
assertThat(result.asIdSet()).containsExactlyElementsIn(ids(1));
assertThat(getOnlyElement(result.asList()).getAccount().isActive()).isFalse();
assertThat(getOnlyElement(result.asList()).account().isActive()).isFalse();
assertThat(filteredInactiveIds(result)).isEmpty();
}
@@ -255,8 +255,8 @@ public class AccountResolverTest {
AccountState account = newAccount(1);
ImmutableList<Searcher<?>> searchers =
ImmutableList.of(new TestSearcher("foo", false, account));
assertThat(search("foo", searchers, allVisible()).asUnique().getAccount().id())
.isEqualTo(account.getAccount().id());
assertThat(search("foo", searchers, allVisible()).asUnique().account().id())
.isEqualTo(account.account().id());
}
@Test
@@ -375,18 +375,16 @@ public class AccountResolverTest {
}
private Predicate<AccountState> activityPrediate() {
return (AccountState accountState) -> accountState.getAccount().isActive();
return (AccountState accountState) -> accountState.account().isActive();
}
private static Supplier<Predicate<AccountState>> only(int... ids) {
ImmutableSet<Account.Id> idSet =
Arrays.stream(ids).mapToObj(Account::id).collect(toImmutableSet());
return () -> a -> idSet.contains(a.getAccount().id());
return () -> a -> idSet.contains(a.account().id());
}
private static ImmutableSet<Account.Id> filteredInactiveIds(Result result) {
return result.filteredInactive().stream()
.map(a -> a.getAccount().id())
.collect(toImmutableSet());
return result.filteredInactive().stream().map(a -> a.account().id()).collect(toImmutableSet());
}
}

6
javatests/com/google/gerrit/server/mail/send/FromAddressGeneratorProviderTest.java
View File

@@ -347,8 +347,8 @@ public class FromAddressGeneratorProviderTest {
private Account.Id user(String name, String email) {
final AccountState s = makeUser(name, email);
when(accountCache.get(eq(s.getAccount().id()))).thenReturn(Optional.of(s));
return s.getAccount().id();
when(accountCache.get(eq(s.account().id()))).thenReturn(Optional.of(s));
return s.account().id();
}
private void verifyAccountCacheGet(Account.Id id) {
@@ -357,7 +357,7 @@ public class FromAddressGeneratorProviderTest {
private Account.Id userNoLookup(String name, String email) {
final AccountState s = makeUser(name, email);
return s.getAccount().id();
return s.account().id();
}
private AccountState makeUser(String name, String email) {

2
javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
View File

@@ -816,7 +816,7 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests {
}
protected void assertAccounts(List<AccountState> accounts, AccountInfo... expectedAccounts) {
assertThat(accounts.stream().map(a -> a.getAccount().id().get()).collect(toList()))
assertThat(accounts.stream().map(a -> a.account().id().get()).collect(toList()))
.containsExactlyElementsIn(
Arrays.asList(expectedAccounts).stream().map(a -> a._accountId).collect(toList()));
}