From 2803609f8e38f21fa4e38f0f8b95f8177c7b8a5c Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Thu, 13 Apr 2017 15:48:54 +0200 Subject: [PATCH] Update accounts through AccountsUpdate class The AccountsUpdate class should become the single class that updates accounts. At the moment it always updates the accounts in ReviewDb, but in future it will update accounts in NoteDb. Atomic account updates are not yet moved to AccountsUpdate. This will be done in a follow-up change. Change-Id: I2d1b2e054cb47269942b094363824485c4760af2 Signed-off-by: Edwin Kempin --- .../google/gerrit/gpg/GerritPublicKeyCheckerTest.java | 5 ++++- .../com/google/gerrit/server/account/AccountManager.java | 6 +++--- .../com/google/gerrit/server/account/AccountsUpdate.java | 5 +++++ .../com/google/gerrit/server/git/ReceiveCommits.java | 6 +++++- .../server/query/account/AbstractQueryAccountsTest.java | 9 +++++---- .../server/query/change/AbstractQueryChangesTest.java | 6 ++++-- .../server/query/group/AbstractQueryGroupsTest.java | 6 ++++-- 7 files changed, 30 insertions(+), 13 deletions(-) diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java index ffb1030b0a..17d01732fc 100644 --- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java +++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java @@ -39,6 +39,7 @@ import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountManager; import com.google.gerrit.server.account.Accounts; +import com.google.gerrit.server.account.AccountsUpdate; import com.google.gerrit.server.account.AuthRequest; import com.google.gerrit.server.account.externalids.ExternalId; import com.google.gerrit.server.account.externalids.ExternalIdsUpdate; @@ -74,6 +75,8 @@ import org.junit.Test; public class GerritPublicKeyCheckerTest { @Inject private Accounts accounts; + @Inject private AccountsUpdate.Server accountsUpdate; + @Inject private AccountCache accountCache; @Inject private AccountManager accountManager; @@ -121,7 +124,7 @@ public class GerritPublicKeyCheckerTest { Account userAccount = accounts.get(db, userId); // Note: does not match any key in TestKeys. userAccount.setPreferredEmail("user@example.com"); - db.accounts().update(ImmutableList.of(userAccount)); + accountsUpdate.create().update(db, userAccount); user = reloadUser(); requestContext.setContext( diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java index e496f9b017..96fa60b949 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java @@ -193,7 +193,7 @@ public class AccountManager { } if (toUpdate != null) { - db.accounts().update(Collections.singleton(toUpdate)); + accountsUpdateFactory.create().update(db, toUpdate); } if (newEmail != null && !newEmail.equals(oldEmail)) { @@ -379,7 +379,7 @@ public class AccountManager { Account a = accounts.get(db, to); if (a.getPreferredEmail() == null) { a.setPreferredEmail(who.getEmailAddress()); - db.accounts().update(Collections.singleton(a)); + accountsUpdateFactory.create().update(db, a); } } @@ -449,7 +449,7 @@ public class AccountManager { if (a.getPreferredEmail() != null && a.getPreferredEmail().equals(who.getEmailAddress())) { a.setPreferredEmail(null); - db.accounts().update(Collections.singleton(a)); + accountsUpdateFactory.create().update(db, a); } byEmailCache.evict(who.getEmailAddress()); byIdCache.evict(from); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsUpdate.java index de87fc1dec..0f109f2bb7 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsUpdate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsUpdate.java @@ -145,6 +145,11 @@ public class AccountsUpdate { createUserBranchIfNeeded(account); } + /** Updates the account. */ + public void update(ReviewDb db, Account account) throws OrmException { + db.accounts().update(ImmutableSet.of(account)); + } + /** Deletes the account. */ public void delete(ReviewDb db, Account account) throws OrmException, IOException { db.accounts().delete(ImmutableSet.of(account)); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 9d6cd925a8..8f795e012c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -85,6 +85,7 @@ import com.google.gerrit.server.Sequences; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountResolver; import com.google.gerrit.server.account.Accounts; +import com.google.gerrit.server.account.AccountsUpdate; import com.google.gerrit.server.change.ChangeInserter; import com.google.gerrit.server.change.SetHashtagsOp; import com.google.gerrit.server.config.AllProjectsName; @@ -301,6 +302,7 @@ public class ReceiveCommits { private final Provider queryProvider; private final ChangeNotes.Factory notesFactory; private final Accounts accounts; + private final AccountsUpdate.Server accountsUpdate; private final AccountResolver accountResolver; private final PermissionBackend permissionBackend; private final PermissionBackend.ForProject permissions; @@ -376,6 +378,7 @@ public class ReceiveCommits { Provider queryProvider, ChangeNotes.Factory notesFactory, Accounts accounts, + AccountsUpdate.Server accountsUpdate, AccountResolver accountResolver, PermissionBackend permissionBackend, CmdLineParser.Factory optionParserFactory, @@ -416,6 +419,7 @@ public class ReceiveCommits { this.queryProvider = queryProvider; this.notesFactory = notesFactory; this.accounts = accounts; + this.accountsUpdate = accountsUpdate; this.accountResolver = accountResolver; this.permissionBackend = permissionBackend; this.optionParserFactory = optionParserFactory; @@ -2744,7 +2748,7 @@ public class ReceiveCommits { Account a = accounts.get(db, user.getAccountId()); if (a != null && Strings.isNullOrEmpty(a.getFullName())) { a.setFullName(c.getCommitterIdent().getName()); - db.accounts().update(Collections.singleton(a)); + accountsUpdate.create().update(db, a); user.getAccount().setFullName(a.getFullName()); accountCache.evict(a.getId()); } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java index 7c3ea3307b..703523ce08 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java @@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertThat; import static java.util.stream.Collectors.toList; import static org.junit.Assert.fail; -import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.gerrit.extensions.api.GerritApi; import com.google.gerrit.extensions.api.accounts.Accounts.QueryRequest; @@ -38,6 +37,7 @@ import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountManager; import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.account.Accounts; +import com.google.gerrit.server.account.AccountsUpdate; import com.google.gerrit.server.account.AuthRequest; import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.schema.SchemaCreator; @@ -54,7 +54,6 @@ import com.google.inject.Provider; import com.google.inject.util.Providers; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.Iterator; import java.util.List; import org.eclipse.jgit.lib.Config; @@ -74,6 +73,8 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests { @Inject protected Accounts accounts; + @Inject protected AccountsUpdate.Server accountsUpdate; + @Inject protected AccountCache accountCache; @Inject protected AccountManager accountManager; @@ -385,7 +386,7 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests { String newName = "Test User"; Account account = accounts.get(db, new Account.Id(user1._accountId)); account.setFullName(newName); - db.accounts().update(Collections.singleton(account)); + accountsUpdate.create().update(db, account); assertQuery("name:" + quote(user1.name), user1); assertQuery("name:" + quote(newName)); @@ -471,7 +472,7 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests { a.setFullName(fullName); a.setPreferredEmail(email); a.setActive(active); - db.accounts().update(ImmutableList.of(a)); + accountsUpdate.create().update(db, a); accountCache.evict(id); return id; } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 23849ee652..711a5eca98 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -71,6 +71,7 @@ import com.google.gerrit.server.StarredChangesUtil; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountManager; import com.google.gerrit.server.account.Accounts; +import com.google.gerrit.server.account.AccountsUpdate; import com.google.gerrit.server.account.AuthRequest; import com.google.gerrit.server.account.externalids.ExternalId; import com.google.gerrit.server.account.externalids.ExternalIdsUpdate; @@ -150,6 +151,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { @Inject protected Accounts accounts; @Inject protected AccountCache accountCache; + @Inject protected AccountsUpdate.Server accountsUpdate; @Inject protected AccountManager accountManager; @Inject protected AllUsersName allUsersName; @Inject protected BatchUpdate.Factory updateFactory; @@ -210,7 +212,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { String email = "user@example.com"; externalIdsUpdate.create().insert(ExternalId.createEmail(userId, email)); userAccount.setPreferredEmail(email); - db.accounts().update(ImmutableList.of(userAccount)); + accountsUpdate.create().update(db, userAccount); user = userFactory.create(userId); requestContext.setContext(newRequestContext(userAccount.getId())); } @@ -2310,7 +2312,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { a.setFullName(fullName); a.setPreferredEmail(email); a.setActive(active); - db.accounts().update(ImmutableList.of(a)); + accountsUpdate.create().update(db, a); accountCache.evict(id); return id; } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java index aa9e06fa8e..7c3479a64c 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java @@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertThat; import static java.util.stream.Collectors.toList; import com.google.common.base.CharMatcher; -import com.google.common.collect.ImmutableList; import com.google.gerrit.extensions.api.GerritApi; import com.google.gerrit.extensions.api.groups.GroupInput; import com.google.gerrit.extensions.api.groups.Groups.QueryRequest; @@ -35,6 +34,7 @@ import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountManager; import com.google.gerrit.server.account.Accounts; +import com.google.gerrit.server.account.AccountsUpdate; import com.google.gerrit.server.account.AuthRequest; import com.google.gerrit.server.account.GroupCache; import com.google.gerrit.server.config.AllProjectsName; @@ -73,6 +73,8 @@ public abstract class AbstractQueryGroupsTest extends GerritServerTests { @Inject protected Accounts accounts; + @Inject protected AccountsUpdate.Server accountsUpdate; + @Inject protected AccountCache accountCache; @Inject protected AccountManager accountManager; @@ -321,7 +323,7 @@ public abstract class AbstractQueryGroupsTest extends GerritServerTests { a.setFullName(fullName); a.setPreferredEmail(email); a.setActive(active); - db.accounts().update(ImmutableList.of(a)); + accountsUpdate.create().update(db, a); accountCache.evict(id); return id; }