opendev/gerrit
Code Issues Proposed changes

Add an InternalAccountUpdate class to prepare updates to accounts

Browse Source 
Instead of doing account updates directly on the Account instance have
an InternalAccountUpdate class with a builder for preparing account
updates.

This is done for the following reasons:

1. More consistency with how group updates are done (group updates are
   prepared with an InternalGroupUpdate class).

2. It's a preparation to make updates of different account data types
   atomic (e.g. update of account properties and external IDs should
   become atomic).

3. This change will allow us to replace the Account entity class from
   ReviewDb times with an AutoValue type (similar to how groups are
   represented by the InternalGroup AutoValue type).

To achieve 2. the new InternalAccountUpdate class can be extended to
host updates for further account data like external IDs, preferences
etc. Since all account data is stored in the All-Users repository the
updates in NoteDb can be atomic across different account data types.
In particular this will simplify the account creation which first
creates the external ID and then inserts the account, but then has to
rollback the external ID creation if the account insertion fails.

In contrast to GroupsUpdate AccountsUpdate continues to require a
consumer for updates (instead of an InternalAccountUpdate instance).
This is because some callers of the AccountsUpdate class need to have
access to the loaded account to decide which updates should be done
(e.g. ReceiveCommits wants to set a full name only if a full name wasn't
set yet). This is why the consumer is invoked with a new AccountUpdate
that provides both, access to the current account (for reading) and
access to the InternalAccountUpdate builder (for updates).

Change-Id: I057361b9ca4d3a61475c77a6ff06f40ebedaec0f
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2017-12-08 13:57:26 +01:00
parent 350e3e263f
commit d741a4afb7
18 changed files with 334 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -324,7 +324,7 @@ public class AccountIT extends AbstractDaemonTest {
String status = "OOO";
Account account =
accountsUpdate.create().update(anonymousCoward.getId(), a -> a.setStatus(status));
accountsUpdate.create().update(anonymousCoward.getId(), u -> u.setStatus(status));
assertThat(account).isNotNull();
assertThat(account.getFullName()).isNull();
assertThat(account.getStatus()).isEqualTo(status);
@@ -854,7 +854,7 @@ public class AccountIT extends AbstractDaemonTest {
String prefix = "foo.preferred";
String prefEmail = prefix + "@example.com";
TestAccount foo = accountCreator.create(name("foo"));
accountsUpdate.create().update(foo.id, a -> a.setPreferredEmail(prefEmail));
accountsUpdate.create().update(foo.id, u -> u.setPreferredEmail(prefEmail));
// verify that the account is still found when using the preferred email to lookup the account
ImmutableSet<Account.Id> accountsByPrefEmail = emails.getAccountFor(prefEmail);
@@ -1330,7 +1330,7 @@ public class AccountIT extends AbstractDaemonTest {
String userRef = RefNames.refsUsers(foo.id);
String noEmail = "no.email";
accountsUpdate.create().update(foo.id, a -> a.setPreferredEmail(noEmail));
accountsUpdate.create().update(foo.id, u -> u.setPreferredEmail(noEmail));
accountIndexedCounter.clear();
grant(allUsers, userRef, Permission.PUSH, false, REGISTERED_USERS);
@@ -1812,11 +1812,11 @@ public class AccountIT extends AbstractDaemonTest {
// metaId is set when account is created
AccountsUpdate au = accountsUpdate.create();
Account.Id accountId = new Account.Id(seq.nextAccountId());
Account account = au.insert(accountId, a -> {});
Account account = au.insert(accountId, u -> {});
assertThat(account.getMetaId()).isEqualTo(getMetaId(accountId));
// metaId is set when account is updated
Account updatedAccount = au.update(accountId, a -> a.setFullName("foo"));
Account updatedAccount = au.update(accountId, u -> u.setFullName("foo"));
assertThat(account.getMetaId()).isNotEqualTo(updatedAccount.getMetaId());
assertThat(updatedAccount.getMetaId()).isEqualTo(getMetaId(accountId));
}