Merge changes I68e3bd39,I30556192,Ie90281fa,Ic23811c1,I8ffe3686, ...

* changes:
  AbstractQueryChangesTest: Make account update atomic
  GerritPublicKeyCheckerTest: Use AccountsUpdate instead of ExternalIdsUpdate
  AccountIT: Use AccountsUpdate instead of ExternalIdsUpdate
  ExternalIdIT: Use AccountsUpdate instead of ExternalIdsUpdate
  AccountManager#link: Update account and external IDs atomically
  AccountManager#create: Create account and external ID atomically
  AccountManager#update: Update account and external IDs atomically
  CreateAccount: Create account and external IDs atomically
  Add specific exception for insert of duplicate external ID key
  CreateAccount: Rollback creation of all ext IDs if insert of email fails
  ChangeUserName: Use AccountsUpdate instead of ExternalIdsUpdate
  ChangeUserName: Remove unused handling of old usernames
  AccountIT#stalenessChecker(): Use ExternalIdNotes instead of ExternalIdsUpdate
  AccountIT: Remove code to reset external IDs after each test
  Add API to update external IDs as part of an account update
  Allow to update external IDs with BatchRefUpdate
  Make Git history of user branches more useful by specific commit messages
  Use BatchRefUpdate to update accounts
  Retry account updates on LockFailureException
  Add an InternalAccountUpdate class to prepare updates to accounts

java/com/google/gerrit/acceptance/AccountCreator.java

@@ -119,12 +119,7 @@ public class AccountCreator {
 
       accountsUpdate
           .create()
-          .insert(
+          .insert("Create Test Account", id, u -> u.setFullName(fullName).setPreferredEmail(email));
-              id,
-              a -> {
-                a.setFullName(fullName);
-                a.setPreferredEmail(email);
-              });
 
       if (groupNames != null) {
         for (String n : groupNames) {

java/com/google/gerrit/pgm/LocalUsernamesToLowerCase.java

@@ -17,20 +17,28 @@ package com.google.gerrit.pgm;
 import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_GERRIT;
 import static com.google.gerrit.server.schema.DataSourceProvider.Context.MULTI_USER;
 
+import com.google.gerrit.extensions.config.FactoryModule;
 import com.google.gerrit.lifecycle.LifecycleManager;
 import com.google.gerrit.pgm.util.SiteProgram;
 import com.google.gerrit.server.account.externalids.DisabledExternalIdCache;
 import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.account.externalids.ExternalIdNotes;
 import com.google.gerrit.server.account.externalids.ExternalIds;
-import com.google.gerrit.server.account.externalids.ExternalIdsBatchUpdate;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.MetaDataUpdate;
 import com.google.gerrit.server.index.account.AccountSchemaDefinitions;
 import com.google.gerrit.server.schema.SchemaVersionCheck;
-import com.google.inject.AbstractModule;
+import com.google.gwtorm.server.OrmDuplicateKeyException;
 import com.google.inject.Inject;
 import com.google.inject.Injector;
+import com.google.inject.Provider;
+import java.io.IOException;
 import java.util.Collection;
 import java.util.Locale;
 import org.eclipse.jgit.lib.ProgressMonitor;
+import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.lib.TextProgressMonitor;
 
 /** Converts the local username for all accounts to lower case */
@@ -38,10 +46,12 @@ public class LocalUsernamesToLowerCase extends SiteProgram {
   private final LifecycleManager manager = new LifecycleManager();
   private final TextProgressMonitor monitor = new TextProgressMonitor();
 
+  @Inject private GitRepositoryManager repoManager;
+  @Inject private AllUsersName allUsersName;
+  @Inject private Provider<MetaDataUpdate.Server> metaDataUpdateServerFactory;
+  @Inject private ExternalIdNotes.FactoryNoReindex externalIdNotesFactory;
   @Inject private ExternalIds externalIds;
 
-  @Inject private ExternalIdsBatchUpdate externalIdsBatchUpdate;
-
   @Override
   public int run() throws Exception {
     Injector dbInjector = createDbInjector(MULTI_USER);
@@ -49,9 +59,12 @@ public class LocalUsernamesToLowerCase extends SiteProgram {
     manager.start();
     dbInjector
         .createChildInjector(
-            new AbstractModule() {
+            new FactoryModule() {
               @Override
               protected void configure() {
+                bind(GitReferenceUpdated.class).toInstance(GitReferenceUpdated.DISABLED);
+                factory(MetaDataUpdate.InternalFactory.class);
+
                 // The LocalUsernamesToLowerCase program needs to access all external IDs only
                 // once to update them. After the update they are not accessed again. Hence the
                 // LocalUsernamesToLowerCase program doesn't benefit from caching external IDs and
@@ -64,12 +77,18 @@ public class LocalUsernamesToLowerCase extends SiteProgram {
     Collection<ExternalId> todo = externalIds.all();
     monitor.beginTask("Converting local usernames", todo.size());
 
+    try (Repository repo = repoManager.openRepository(allUsersName)) {
+      ExternalIdNotes extIdNotes = externalIdNotesFactory.load(repo);
       for (ExternalId extId : todo) {
-      convertLocalUserToLowerCase(extId);
+        convertLocalUserToLowerCase(extIdNotes, extId);
         monitor.update(1);
       }
+      try (MetaDataUpdate metaDataUpdate = metaDataUpdateServerFactory.get().create(allUsersName)) {
+        metaDataUpdate.setMessage("Convert local usernames to lower case");
+        extIdNotes.commit(metaDataUpdate);
+      }
+    }
 
-    externalIdsBatchUpdate.commit("Convert local usernames to lower case");
     monitor.endTask();
 
     int exitCode = reindexAccounts();
@@ -77,7 +96,8 @@ public class LocalUsernamesToLowerCase extends SiteProgram {
     return exitCode;
   }
 
-  private void convertLocalUserToLowerCase(ExternalId extId) {
+  private void convertLocalUserToLowerCase(ExternalIdNotes extIdNotes, ExternalId extId)
+      throws OrmDuplicateKeyException, IOException {
     if (extId.isScheme(SCHEME_GERRIT)) {
       String localUser = extId.key().id();
       String localUserLowerCase = localUser.toLowerCase(Locale.US);
@@ -89,7 +109,7 @@ public class LocalUsernamesToLowerCase extends SiteProgram {
                 extId.accountId(),
                 extId.email(),
                 extId.password());
-        externalIdsBatchUpdate.replace(extId, extIdLowerCase);
+        extIdNotes.replace(extId, extIdLowerCase);
       }
     }
   }

java/com/google/gerrit/pgm/init/AccountsOnInit.java

@@ -24,6 +24,7 @@ import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.GerritPersonIdentProvider;
 import com.google.gerrit.server.account.AccountConfig;
 import com.google.gerrit.server.account.Accounts;
+import com.google.gerrit.server.account.InternalAccountUpdate;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.inject.Inject;
 import java.io.File;
@@ -69,7 +70,14 @@ public class AccountsOnInit {
                 new GerritPersonIdentProvider(flags.cfg).get(), account.getRegisteredOn());
 
         Config accountConfig = new Config();
-        AccountConfig.writeToConfig(account, accountConfig);
+        AccountConfig.writeToConfig(
+            InternalAccountUpdate.builder()
+                .setActive(account.isActive())
+                .setFullName(account.getFullName())
+                .setPreferredEmail(account.getPreferredEmail())
+                .setStatus(account.getStatus())
+                .build(),
+            accountConfig);
 
         DirCache newTree = DirCache.newInCore();
         DirCacheEditor editor = newTree.editor();

java/com/google/gerrit/pgm/init/ExternalIdsOnInit.java

@@ -19,10 +19,10 @@ import com.google.gerrit.pgm.init.api.InitFlags;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.server.GerritPersonIdentProvider;
 import com.google.gerrit.server.account.externalids.ExternalId;
-import com.google.gerrit.server.account.externalids.ExternalIdReader;
+import com.google.gerrit.server.account.externalids.ExternalIdNotes;
-import com.google.gerrit.server.account.externalids.ExternalIdsUpdate;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.server.git.MetaDataUpdate;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import java.io.File;
@@ -31,13 +31,9 @@ import java.nio.file.Path;
 import java.util.Collection;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.internal.storage.file.FileRepository;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.lib.RepositoryCache.FileKey;
-import org.eclipse.jgit.notes.NoteMap;
-import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.util.FS;
 
 public class ExternalIdsOnInit {
@@ -54,32 +50,20 @@ public class ExternalIdsOnInit {
 
   public synchronized void insert(String commitMessage, Collection<ExternalId> extIds)
       throws OrmException, IOException, ConfigInvalidException {
-
     File path = getPath();
     if (path != null) {
-      try (Repository repo = new FileRepository(path);
+      try (Repository allUsersRepo = new FileRepository(path)) {
-          RevWalk rw = new RevWalk(repo);
+        ExternalIdNotes extIdNotes = ExternalIdNotes.loadNoCacheUpdate(allUsersRepo);
-          ObjectInserter ins = repo.newObjectInserter()) {
+        extIdNotes.insert(extIds);
-        ObjectId rev = ExternalIdReader.readRevision(repo);
+        try (MetaDataUpdate metaDataUpdate =
-
+            new MetaDataUpdate(
-        NoteMap noteMap = ExternalIdReader.readNoteMap(rw, rev);
+                GitReferenceUpdated.DISABLED, new Project.NameKey(allUsers), allUsersRepo)) {
-        for (ExternalId extId : extIds) {
-          ExternalIdsUpdate.insert(rw, ins, noteMap, extId);
-        }
-
           PersonIdent serverIdent = new GerritPersonIdentProvider(flags.cfg).get();
-        ExternalIdsUpdate.commit(
+          metaDataUpdate.getCommitBuilder().setAuthor(serverIdent);
-            new Project.NameKey(allUsers),
+          metaDataUpdate.getCommitBuilder().setCommitter(serverIdent);
-            repo,
+          metaDataUpdate.getCommitBuilder().setMessage(commitMessage);
-            rw,
+          extIdNotes.commit(metaDataUpdate);
-            ins,
+        }
-            rev,
-            noteMap,
-            commitMessage,
-            serverIdent,
-            serverIdent,
-            null,
-            GitReferenceUpdated.DISABLED);
       }
     }
   }

java/com/google/gerrit/server/account/AccountConfig.java

@@ -80,6 +80,7 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
   private final String ref;
 
   private Optional<Account> loadedAccount;
+  private Optional<InternalAccountUpdate> accountUpdate = Optional.empty();
   private Timestamp registeredOn;
   private List<ValidationError> validationErrors;
 
@@ -117,6 +118,14 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
   public void setAccount(Account account) {
     checkLoaded();
     this.loadedAccount = Optional.of(account);
+    this.accountUpdate =
+        Optional.of(
+            InternalAccountUpdate.builder()
+                .setActive(account.isActive())
+                .setFullName(account.getFullName())
+                .setPreferredEmail(account.getPreferredEmail())
+                .setStatus(account.getStatus())
+                .build());
     this.registeredOn = account.getRegisteredOn();
   }
 
@@ -127,15 +136,29 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
    * @throws OrmDuplicateKeyException if the user branch already exists
    */
   public Account getNewAccount() throws OrmDuplicateKeyException {
+    return getNewAccount(TimeUtil.nowTs());
+  }
+
+  /**
+   * Creates a new account.
+   *
+   * @return the new account
+   * @throws OrmDuplicateKeyException if the user branch already exists
+   */
+  Account getNewAccount(Timestamp registeredOn) throws OrmDuplicateKeyException {
     checkLoaded();
     if (revision != null) {
       throw new OrmDuplicateKeyException(String.format("account %s already exists", accountId));
     }
-    this.registeredOn = TimeUtil.nowTs();
+    this.registeredOn = registeredOn;
     this.loadedAccount = Optional.of(new Account(accountId, registeredOn));
     return loadedAccount.get();
   }
 
+  public void setAccountUpdate(InternalAccountUpdate accountUpdate) {
+    this.accountUpdate = Optional.of(accountUpdate);
+  }
+
   @Override
   protected void onLoad() throws IOException, ConfigInvalidException {
     if (revision != null) {
@@ -186,24 +209,39 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
     }
 
     if (revision != null) {
+      if (Strings.isNullOrEmpty(commit.getMessage())) {
         commit.setMessage("Update account\n");
+      }
     } else {
+      if (Strings.isNullOrEmpty(commit.getMessage())) {
         commit.setMessage("Create account\n");
+      }
+
       commit.setAuthor(new PersonIdent(commit.getAuthor(), registeredOn));
       commit.setCommitter(new PersonIdent(commit.getCommitter(), registeredOn));
     }
 
     Config cfg = readConfig(ACCOUNT_CONFIG);
-    writeToConfig(loadedAccount.get(), cfg);
+    if (accountUpdate.isPresent()) {
+      writeToConfig(accountUpdate.get(), cfg);
+    }
     saveConfig(ACCOUNT_CONFIG, cfg);
+
+    // metaId is set in the commit(MetaDataUpdate) method after the commit is created
+    loadedAccount = Optional.of(parse(cfg, null));
+
+    accountUpdate = Optional.empty();
+
     return true;
   }
 
-  public static void writeToConfig(Account account, Config cfg) {
+  public static void writeToConfig(InternalAccountUpdate accountUpdate, Config cfg) {
-    setActive(cfg, account.isActive());
+    accountUpdate.getActive().ifPresent(active -> setActive(cfg, active));
-    set(cfg, KEY_FULL_NAME, account.getFullName());
+    accountUpdate.getFullName().ifPresent(fullName -> set(cfg, KEY_FULL_NAME, fullName));
-    set(cfg, KEY_PREFERRED_EMAIL, account.getPreferredEmail());
+    accountUpdate
-    set(cfg, KEY_STATUS, account.getStatus());
+        .getPreferredEmail()
+        .ifPresent(preferredEmail -> set(cfg, KEY_PREFERRED_EMAIL, preferredEmail));
+    accountUpdate.getStatus().ifPresent(status -> set(cfg, KEY_STATUS, status));
   }
 
   /**

java/com/google/gerrit/server/account/AccountManager.java

@@ -29,6 +29,8 @@ import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.Sequences;
+import com.google.gerrit.server.account.AccountsUpdate.AccountUpdater;
+import com.google.gerrit.server.account.externalids.DuplicateExternalIdKeyException;
 import com.google.gerrit.server.account.externalids.ExternalId;
 import com.google.gerrit.server.account.externalids.ExternalIds;
 import com.google.gerrit.server.account.externalids.ExternalIdsUpdate;
@@ -203,7 +205,7 @@ public class AccountManager {
   private void update(AuthRequest who, ExternalId extId)
       throws OrmException, IOException, ConfigInvalidException {
     IdentifiedUser user = userFactory.create(extId.accountId());
-    List<Consumer<Account>> accountUpdates = new ArrayList<>();
+    List<Consumer<InternalAccountUpdate.Builder>> accountUpdates = new ArrayList<>();
 
     // If the email address was modified by the authentication provider,
     // update our records to match the changed email.
@@ -212,19 +214,20 @@ public class AccountManager {
     String oldEmail = extId.email();
     if (newEmail != null && !newEmail.equals(oldEmail)) {
       if (oldEmail != null && oldEmail.equals(user.getAccount().getPreferredEmail())) {
-        accountUpdates.add(a -> a.setPreferredEmail(newEmail));
+        accountUpdates.add(u -> u.setPreferredEmail(newEmail));
       }
 
-      externalIdsUpdateFactory
+      accountUpdates.add(
-          .create()
+          u ->
-          .replace(
+              u.replaceExternalId(
-              extId, ExternalId.create(extId.key(), extId.accountId(), newEmail, extId.password()));
+                  extId,
+                  ExternalId.create(extId.key(), extId.accountId(), newEmail, extId.password())));
     }
 
     if (!realm.allowsEdit(AccountFieldName.FULL_NAME)
         && !Strings.isNullOrEmpty(who.getDisplayName())
         && !eq(user.getAccount().getFullName(), who.getDisplayName())) {
-      accountUpdates.add(a -> a.setFullName(who.getDisplayName()));
+      accountUpdates.add(u -> u.setFullName(who.getDisplayName()));
     }
 
     if (!realm.allowsEdit(AccountFieldName.USER_NAME)
@@ -236,7 +239,13 @@ public class AccountManager {
     }
 
     if (!accountUpdates.isEmpty()) {
-      Account account = accountsUpdateFactory.create().update(user.getAccountId(), accountUpdates);
+      Account account =
+          accountsUpdateFactory
+              .create()
+              .update(
+                  "Update Account on Login",
+                  user.getAccountId(),
+                  AccountUpdater.joinConsumers(accountUpdates));
       if (account == null) {
         throw new OrmException("Account " + user.getAccountId() + " has been deleted");
       }
@@ -258,27 +267,23 @@ public class AccountManager {
 
     Account account;
     try {
-      AccountsUpdate accountsUpdate = accountsUpdateFactory.create();
       account =
-          accountsUpdate.insert(
+          accountsUpdateFactory
+              .create()
+              .insert(
+                  "Create Account on First Login",
                   newId,
-              a -> {
+                  u ->
-                a.setFullName(who.getDisplayName());
+                      u.setFullName(who.getDisplayName())
-                a.setPreferredEmail(extId.email());
+                          .setPreferredEmail(extId.email())
-              });
+                          .addExternalId(extId));
-
+    } catch (DuplicateExternalIdKeyException e) {
-      ExternalId existingExtId = externalIds.get(extId.key());
-      if (existingExtId != null && !existingExtId.accountId().equals(extId.accountId())) {
-        // external ID is assigned to another account, do not overwrite
-        accountsUpdate.delete(account);
       throw new AccountException(
           "Cannot assign external ID \""
-                + extId.key().get()
+              + e.getDuplicateKey().get()
               + "\" to account "
               + newId
               + "; external ID already in use.");
-      }
-      externalIdsUpdateFactory.create().upsert(extId);
     } finally {
       // If adding the account failed, it may be that it actually was the
       // first account. So we reset the 'check for first account'-guard, as
@@ -308,7 +313,7 @@ public class AccountManager {
       // Only set if the name hasn't been used yet, but was given to us.
       //
       try {
-        changeUserNameFactory.create(user, who.getUserName()).call();
+        changeUserNameFactory.create("Set Username on Login", user, who.getUserName()).call();
       } catch (NameAlreadyUsedException e) {
         String message =
             "Cannot assign user name \""
@@ -407,23 +412,19 @@ public class AccountManager {
       }
       update(who, extId);
     } else {
-      externalIdsUpdateFactory
-          .create()
-          .insert(ExternalId.createWithEmail(who.getExternalIdKey(), to, who.getEmailAddress()));
-
-      if (who.getEmailAddress() != null) {
       accountsUpdateFactory
           .create()
           .update(
+              "Link External ID",
               to,
-                a -> {
+              (a, u) -> {
-                  if (a.getPreferredEmail() == null) {
+                u.addExternalId(
-                    a.setPreferredEmail(who.getEmailAddress());
+                    ExternalId.createWithEmail(who.getExternalIdKey(), to, who.getEmailAddress()));
+                if (who.getEmailAddress() != null && a.getPreferredEmail() == null) {
+                  u.setPreferredEmail(who.getEmailAddress());
                 }
               });
     }
-    }
-
     return new AuthResult(to, who.getExternalIdKey(), false);
   }
 
@@ -503,12 +504,16 @@ public class AccountManager {
       accountsUpdateFactory
           .create()
           .update(
+              "Clear Preferred Email on Unlinking External ID\n"
+                  + "\n"
+                  + "The preferred email is cleared because the corresponding external ID\n"
+                  + "was removed.",
               from,
-              a -> {
+              (a, u) -> {
                 if (a.getPreferredEmail() != null) {
                   for (ExternalId extId : extIds) {
                     if (a.getPreferredEmail().equals(extId.email())) {
-                      a.setPreferredEmail(null);
+                      u.setPreferredEmail(null);
                       break;
                     }
                   }

java/com/google/gerrit/server/account/AccountsUpdate.java

@@ -15,29 +15,40 @@
 package com.google.gerrit.server.account;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
 
-import com.google.common.collect.ImmutableList;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Strings;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import com.google.common.util.concurrent.Runnables;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.account.externalids.ExternalIdNotes;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.MetaDataUpdate;
 import com.google.gerrit.server.index.change.ReindexAfterRefUpdate;
 import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
+import com.google.gerrit.server.update.RefUpdateUtil;
+import com.google.gerrit.server.update.RetryHelper;
 import com.google.gwtorm.server.OrmDuplicateKeyException;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
 import java.io.IOException;
+import java.sql.Timestamp;
 import java.util.List;
 import java.util.Optional;
 import java.util.function.Consumer;
 import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.BatchRefUpdate;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Ref;
@@ -63,6 +74,38 @@ import org.eclipse.jgit.lib.Repository;
  */
 @Singleton
 public class AccountsUpdate {
+  /**
+   * Updater for an account.
+   *
+   * <p>Allows to read the current state of an account and to prepare updates to it.
+   */
+  @FunctionalInterface
+  public static interface AccountUpdater {
+    /**
+     * Prepare updates to an account.
+     *
+     * <p>Use the provided account only to read the current state of the account. Don't do updates
+     * to the account. For updates use the provided account update builder.
+     *
+     * @param account the account that is being updated
+     * @param update account update builder
+     */
+    void update(Account account, InternalAccountUpdate.Builder update);
+
+    public static AccountUpdater join(List<AccountUpdater> updaters) {
+      return (a, u) -> updaters.stream().forEach(updater -> updater.update(a, u));
+    }
+
+    public static AccountUpdater joinConsumers(
+        List<Consumer<InternalAccountUpdate.Builder>> consumers) {
+      return join(Lists.transform(consumers, AccountUpdater::fromConsumer));
+    }
+
+    static AccountUpdater fromConsumer(Consumer<InternalAccountUpdate.Builder> consumer) {
+      return (a, u) -> consumer.accept(u);
+    }
+  }
+
   /**
    * Factory to create an AccountsUpdate instance for updating accounts by the Gerrit server.
    *
@@ -75,8 +118,10 @@ public class AccountsUpdate {
     private final GitReferenceUpdated gitRefUpdated;
     private final AllUsersName allUsersName;
     private final OutgoingEmailValidator emailValidator;
-    private final Provider<PersonIdent> serverIdent;
+    private final Provider<PersonIdent> serverIdentProvider;
-    private final Provider<MetaDataUpdate.Server> metaDataUpdateServerFactory;
+    private final Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory;
+    private final RetryHelper retryHelper;
+    private final ExternalIdNotes.Factory extIdNotesFactory;
 
     @Inject
     public Server(
@@ -84,26 +129,33 @@ public class AccountsUpdate {
         GitReferenceUpdated gitRefUpdated,
         AllUsersName allUsersName,
         OutgoingEmailValidator emailValidator,
-        @GerritPersonIdent Provider<PersonIdent> serverIdent,
+        @GerritPersonIdent Provider<PersonIdent> serverIdentProvider,
-        Provider<MetaDataUpdate.Server> metaDataUpdateServerFactory) {
+        Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory,
+        RetryHelper retryHelper,
+        ExternalIdNotes.Factory extIdNotesFactory) {
       this.repoManager = repoManager;
       this.gitRefUpdated = gitRefUpdated;
       this.allUsersName = allUsersName;
       this.emailValidator = emailValidator;
-      this.serverIdent = serverIdent;
+      this.serverIdentProvider = serverIdentProvider;
-      this.metaDataUpdateServerFactory = metaDataUpdateServerFactory;
+      this.metaDataUpdateInternalFactory = metaDataUpdateInternalFactory;
+      this.retryHelper = retryHelper;
+      this.extIdNotesFactory = extIdNotesFactory;
     }
 
     public AccountsUpdate create() {
-      PersonIdent i = serverIdent.get();
+      PersonIdent serverIdent = serverIdentProvider.get();
       return new AccountsUpdate(
           repoManager,
           gitRefUpdated,
           null,
           allUsersName,
           emailValidator,
-          i,
+          metaDataUpdateInternalFactory,
-          () -> metaDataUpdateServerFactory.get().create(allUsersName));
+          retryHelper,
+          extIdNotesFactory,
+          serverIdent,
+          serverIdent);
     }
   }
 
@@ -119,9 +171,11 @@ public class AccountsUpdate {
     private final GitReferenceUpdated gitRefUpdated;
     private final AllUsersName allUsersName;
     private final OutgoingEmailValidator emailValidator;
-    private final Provider<PersonIdent> serverIdent;
+    private final Provider<PersonIdent> serverIdentProvider;
     private final Provider<IdentifiedUser> identifiedUser;
-    private final Provider<MetaDataUpdate.User> metaDataUpdateUserFactory;
+    private final Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory;
+    private final RetryHelper retryHelper;
+    private final ExternalIdNotes.Factory extIdNotesFactory;
 
     @Inject
     public User(
@@ -129,29 +183,37 @@ public class AccountsUpdate {
         GitReferenceUpdated gitRefUpdated,
         AllUsersName allUsersName,
         OutgoingEmailValidator emailValidator,
-        @GerritPersonIdent Provider<PersonIdent> serverIdent,
+        @GerritPersonIdent Provider<PersonIdent> serverIdentProvider,
         Provider<IdentifiedUser> identifiedUser,
-        Provider<MetaDataUpdate.User> metaDataUpdateUserFactory) {
+        Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory,
+        RetryHelper retryHelper,
+        ExternalIdNotes.Factory extIdNotesFactory) {
       this.repoManager = repoManager;
       this.gitRefUpdated = gitRefUpdated;
       this.allUsersName = allUsersName;
-      this.serverIdent = serverIdent;
+      this.serverIdentProvider = serverIdentProvider;
       this.emailValidator = emailValidator;
       this.identifiedUser = identifiedUser;
-      this.metaDataUpdateUserFactory = metaDataUpdateUserFactory;
+      this.metaDataUpdateInternalFactory = metaDataUpdateInternalFactory;
+      this.retryHelper = retryHelper;
+      this.extIdNotesFactory = extIdNotesFactory;
     }
 
     public AccountsUpdate create() {
       IdentifiedUser user = identifiedUser.get();
-      PersonIdent i = serverIdent.get();
+      PersonIdent serverIdent = serverIdentProvider.get();
+      PersonIdent userIdent = createPersonIdent(serverIdent, user);
       return new AccountsUpdate(
           repoManager,
           gitRefUpdated,
           user,
           allUsersName,
           emailValidator,
-          createPersonIdent(i, user),
+          metaDataUpdateInternalFactory,
-          () -> metaDataUpdateUserFactory.get().create(allUsersName));
+          retryHelper,
+          extIdNotesFactory,
+          serverIdent,
+          userIdent);
     }
 
     private PersonIdent createPersonIdent(PersonIdent ident, IdentifiedUser user) {
@@ -164,8 +226,12 @@ public class AccountsUpdate {
   @Nullable private final IdentifiedUser currentUser;
   private final AllUsersName allUsersName;
   private final OutgoingEmailValidator emailValidator;
+  private final Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory;
+  private final RetryHelper retryHelper;
+  private final ExternalIdNotes.Factory extIdNotesFactory;
   private final PersonIdent committerIdent;
-  private final MetaDataUpdateFactory metaDataUpdateFactory;
+  private final PersonIdent authorIdent;
+  private final Runnable afterReadRevision;
 
   private AccountsUpdate(
       GitRepositoryManager repoManager,
@@ -173,36 +239,99 @@ public class AccountsUpdate {
       @Nullable IdentifiedUser currentUser,
       AllUsersName allUsersName,
       OutgoingEmailValidator emailValidator,
+      Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory,
+      RetryHelper retryHelper,
+      ExternalIdNotes.Factory extIdNotesFactory,
       PersonIdent committerIdent,
-      MetaDataUpdateFactory metaDataUpdateFactory) {
+      PersonIdent authorIdent) {
+    this(
+        repoManager,
+        gitRefUpdated,
+        currentUser,
+        allUsersName,
+        emailValidator,
+        metaDataUpdateInternalFactory,
+        retryHelper,
+        extIdNotesFactory,
+        committerIdent,
+        authorIdent,
+        Runnables.doNothing());
+  }
+
+  @VisibleForTesting
+  public AccountsUpdate(
+      GitRepositoryManager repoManager,
+      GitReferenceUpdated gitRefUpdated,
+      @Nullable IdentifiedUser currentUser,
+      AllUsersName allUsersName,
+      OutgoingEmailValidator emailValidator,
+      Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory,
+      RetryHelper retryHelper,
+      ExternalIdNotes.Factory extIdNotesFactory,
+      PersonIdent committerIdent,
+      PersonIdent authorIdent,
+      Runnable afterReadRevision) {
     this.repoManager = checkNotNull(repoManager, "repoManager");
     this.gitRefUpdated = checkNotNull(gitRefUpdated, "gitRefUpdated");
     this.currentUser = currentUser;
     this.allUsersName = checkNotNull(allUsersName, "allUsersName");
     this.emailValidator = checkNotNull(emailValidator, "emailValidator");
+    this.metaDataUpdateInternalFactory =
+        checkNotNull(metaDataUpdateInternalFactory, "metaDataUpdateInternalFactory");
+    this.retryHelper = checkNotNull(retryHelper, "retryHelper");
+    this.extIdNotesFactory = checkNotNull(extIdNotesFactory, "extIdNotesFactory");
     this.committerIdent = checkNotNull(committerIdent, "committerIdent");
-    this.metaDataUpdateFactory = checkNotNull(metaDataUpdateFactory, "metaDataUpdateFactory");
+    this.authorIdent = checkNotNull(authorIdent, "authorIdent");
+    this.afterReadRevision = afterReadRevision;
   }
 
   /**
    * Inserts a new account.
    *
+   * @param message commit message for the account creation, must not be {@code null or empty}
    * @param accountId ID of the new account
    * @param init consumer to populate the new account
    * @return the newly created account
    * @throws OrmDuplicateKeyException if the account already exists
-   * @throws IOException if updating the user branch fails
+   * @throws IOException if creating the user branch fails due to an IO error
+   * @throws OrmException if creating the user branch fails
    * @throws ConfigInvalidException if any of the account fields has an invalid value
    */
-  public Account insert(Account.Id accountId, Consumer<Account> init)
+  public Account insert(
-      throws OrmDuplicateKeyException, IOException, ConfigInvalidException {
+      String message, Account.Id accountId, Consumer<InternalAccountUpdate.Builder> init)
-    AccountConfig accountConfig = read(accountId);
+      throws OrmException, IOException, ConfigInvalidException {
-    Account account = accountConfig.getNewAccount();
+    return insert(message, accountId, AccountUpdater.fromConsumer(init));
-    init.accept(account);
+  }
 
-    // Create in NoteDb
+  /**
-    commitNew(accountConfig);
+   * Inserts a new account.
-    return account;
+   *
+   * @param message commit message for the account creation, must not be {@code null or empty}
+   * @param accountId ID of the new account
+   * @param updater updater to populate the new account
+   * @return the newly created account
+   * @throws OrmDuplicateKeyException if the account already exists
+   * @throws IOException if creating the user branch fails due to an IO error
+   * @throws OrmException if creating the user branch fails
+   * @throws ConfigInvalidException if any of the account fields has an invalid value
+   */
+  public Account insert(String message, Account.Id accountId, AccountUpdater updater)
+      throws OrmException, IOException, ConfigInvalidException {
+    return updateAccount(
+        r -> {
+          AccountConfig accountConfig = read(r, accountId);
+          Account account =
+              accountConfig.getNewAccount(new Timestamp(committerIdent.getWhen().getTime()));
+          InternalAccountUpdate.Builder updateBuilder = InternalAccountUpdate.builder();
+          updater.update(account, updateBuilder);
+
+          InternalAccountUpdate update = updateBuilder.build();
+          accountConfig.setAccountUpdate(update);
+          ExternalIdNotes extIdNotes = createExternalIdNotes(r, accountId, update);
+          UpdatedAccount updatedAccounts = new UpdatedAccount(message, accountConfig, extIdNotes);
+          updatedAccounts.setCreated(true);
+          return updatedAccounts;
+        });
   }
 
   /**
@@ -210,15 +339,18 @@ public class AccountsUpdate {
    *
    * <p>Changing the registration date of an account is not supported.
    *
+   * @param message commit message for the account update, must not be {@code null or empty}
    * @param accountId ID of the account
-   * @param consumer consumer to update the account, only invoked if the account exists
+   * @param update consumer to update the account, only invoked if the account exists
    * @return the updated account, {@code null} if the account doesn't exist
-   * @throws IOException if updating the user branch fails
+   * @throws IOException if updating the user branch fails due to an IO error
+   * @throws OrmException if updating the user branch fails
    * @throws ConfigInvalidException if any of the account fields has an invalid value
    */
-  public Account update(Account.Id accountId, Consumer<Account> consumer)
+  public Account update(
-      throws IOException, ConfigInvalidException {
+      String message, Account.Id accountId, Consumer<InternalAccountUpdate.Builder> update)
-    return update(accountId, ImmutableList.of(consumer));
+      throws OrmException, IOException, ConfigInvalidException {
+    return update(message, accountId, AccountUpdater.fromConsumer(update));
   }
 
   /**
@@ -226,33 +358,45 @@ public class AccountsUpdate {
    *
    * <p>Changing the registration date of an account is not supported.
    *
+   * @param message commit message for the account update, must not be {@code null or empty}
    * @param accountId ID of the account
-   * @param consumers consumers to update the account, only invoked if the account exists
+   * @param updater updater to update the account, only invoked if the account exists
    * @return the updated account, {@code null} if the account doesn't exist
-   * @throws IOException if updating the user branch fails
+   * @throws IOException if updating the user branch fails due to an IO error
+   * @throws OrmException if updating the user branch fails
    * @throws ConfigInvalidException if any of the account fields has an invalid value
    */
   @Nullable
-  public Account update(Account.Id accountId, List<Consumer<Account>> consumers)
+  public Account update(String message, Account.Id accountId, AccountUpdater updater)
-      throws IOException, ConfigInvalidException {
+      throws OrmException, IOException, ConfigInvalidException {
-    AccountConfig accountConfig = read(accountId);
+    return updateAccount(
+        r -> {
+          AccountConfig accountConfig = read(r, accountId);
           Optional<Account> account = accountConfig.getLoadedAccount();
           if (!account.isPresent()) {
             return null;
           }
 
-    consumers.stream().forEach(c -> c.accept(account.get()));
+          InternalAccountUpdate.Builder updateBuilder = InternalAccountUpdate.builder();
-    commit(accountConfig);
+          updater.update(account.get(), updateBuilder);
-    return account.get();
+
+          InternalAccountUpdate update = updateBuilder.build();
+          accountConfig.setAccountUpdate(update);
+          ExternalIdNotes extIdNotes = createExternalIdNotes(r, accountId, update);
+          UpdatedAccount updatedAccounts = new UpdatedAccount(message, accountConfig, extIdNotes);
+          return updatedAccounts;
+        });
   }
 
   /**
    * Deletes the account.
    *
    * @param account the account that should be deleted
-   * @throws IOException if updating the user branch fails
+   * @throws IOException if deleting the user branch fails due to an IO error
+   * @throws OrmException if deleting the user branch fails
+   * @throws ConfigInvalidException
    */
-  public void delete(Account account) throws IOException {
+  public void delete(Account account) throws IOException, OrmException, ConfigInvalidException {
     deleteByKey(account.getId());
   }
 
@@ -260,15 +404,27 @@ public class AccountsUpdate {
    * Deletes the account.
    *
    * @param accountId the ID of the account that should be deleted
-   * @throws IOException if updating the user branch fails
+   * @throws IOException if deleting the user branch fails due to an IO error
+   * @throws OrmException if deleting the user branch fails
+   * @throws ConfigInvalidException
    */
-  public void deleteByKey(Account.Id accountId) throws IOException {
+  public void deleteByKey(Account.Id accountId)
+      throws IOException, OrmException, ConfigInvalidException {
+    deleteAccount(accountId);
+  }
+
+  private Account deleteAccount(Account.Id accountId)
+      throws IOException, OrmException, ConfigInvalidException {
+    return retryHelper.execute(
+        () -> {
           deleteUserBranch(accountId);
+          return null;
+        });
   }
 
   private void deleteUserBranch(Account.Id accountId) throws IOException {
     try (Repository repo = repoManager.openRepository(allUsersName)) {
-      deleteUserBranch(repo, allUsersName, gitRefUpdated, currentUser, committerIdent, accountId);
+      deleteUserBranch(repo, allUsersName, gitRefUpdated, currentUser, authorIdent, accountId);
     }
   }
 
@@ -299,34 +455,177 @@ public class AccountsUpdate {
     gitRefUpdated.fire(project, ru, user != null ? user.getAccount() : null);
   }
 
-  private AccountConfig read(Account.Id accountId) throws IOException, ConfigInvalidException {
+  private AccountConfig read(Repository allUsersRepo, Account.Id accountId)
-    try (Repository repo = repoManager.openRepository(allUsersName)) {
+      throws IOException, ConfigInvalidException {
     AccountConfig accountConfig = new AccountConfig(emailValidator, accountId);
-      accountConfig.load(repo);
+    accountConfig.load(allUsersRepo);
+
+    afterReadRevision.run();
+
     return accountConfig;
   }
+
+  private Account updateAccount(AccountUpdate accountUpdate)
+      throws IOException, ConfigInvalidException, OrmException {
+    return retryHelper.execute(
+        () -> {
+          try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) {
+            UpdatedAccount updatedAccount = accountUpdate.update(allUsersRepo);
+            if (updatedAccount == null) {
+              return null;
             }
 
-  private void commitNew(AccountConfig accountConfig) throws IOException {
+            commit(allUsersRepo, updatedAccount);
+            return updatedAccount.getAccount();
+          }
+        });
+  }
+
+  private ExternalIdNotes createExternalIdNotes(
+      Repository allUsersRepo, Account.Id accountId, InternalAccountUpdate update)
+      throws IOException, ConfigInvalidException, OrmDuplicateKeyException {
+    ExternalIdNotes.checkSameAccount(
+        Iterables.concat(
+            update.getCreatedExternalIds(),
+            update.getUpdatedExternalIds(),
+            update.getDeletedExternalIds()),
+        accountId);
+
+    ExternalIdNotes extIdNotes = extIdNotesFactory.load(allUsersRepo);
+    extIdNotes.replace(update.getDeletedExternalIds(), update.getCreatedExternalIds());
+    extIdNotes.upsert(update.getUpdatedExternalIds());
+    return extIdNotes;
+  }
+
+  private void commit(Repository allUsersRepo, UpdatedAccount updatedAccount) throws IOException {
+    BatchRefUpdate batchRefUpdate = allUsersRepo.getRefDatabase().newBatchUpdate();
+    if (updatedAccount.isCreated()) {
+      commitNewAccountConfig(
+          updatedAccount.getMessage(),
+          allUsersRepo,
+          batchRefUpdate,
+          updatedAccount.getAccountConfig());
+    } else {
+      commitAccountConfig(
+          updatedAccount.getMessage(),
+          allUsersRepo,
+          batchRefUpdate,
+          updatedAccount.getAccountConfig());
+    }
+
+    commitExternalIdUpdates(
+        updatedAccount.getMessage(),
+        allUsersRepo,
+        batchRefUpdate,
+        updatedAccount.getExternalIdNotes());
+    RefUpdateUtil.executeChecked(batchRefUpdate, allUsersRepo);
+    updatedAccount.getExternalIdNotes().updateCaches();
+    gitRefUpdated.fire(
+        allUsersName, batchRefUpdate, currentUser != null ? currentUser.getAccount() : null);
+  }
+
+  private void commitNewAccountConfig(
+      String message,
+      Repository allUsersRepo,
+      BatchRefUpdate batchRefUpdate,
+      AccountConfig accountConfig)
+      throws IOException {
     // When creating a new account we must allow empty commits so that the user branch gets created
     // with an empty commit when no account properties are set and hence no 'account.config' file
     // will be created.
-    commit(accountConfig, true);
+    commitAccountConfig(message, allUsersRepo, batchRefUpdate, accountConfig, true);
   }
 
-  private void commit(AccountConfig accountConfig) throws IOException {
+  private void commitAccountConfig(
-    commit(accountConfig, false);
+      String message,
+      Repository allUsersRepo,
+      BatchRefUpdate batchRefUpdate,
+      AccountConfig accountConfig)
+      throws IOException {
+    commitAccountConfig(message, allUsersRepo, batchRefUpdate, accountConfig, false);
   }
 
-  private void commit(AccountConfig accountConfig, boolean allowEmptyCommit) throws IOException {
+  private void commitAccountConfig(
-    try (MetaDataUpdate md = metaDataUpdateFactory.create()) {
+      String message,
+      Repository allUsersRepo,
+      BatchRefUpdate batchRefUpdate,
+      AccountConfig accountConfig,
+      boolean allowEmptyCommit)
+      throws IOException {
+    try (MetaDataUpdate md = createMetaDataUpdate(message, allUsersRepo, batchRefUpdate)) {
       md.setAllowEmpty(allowEmptyCommit);
       accountConfig.commit(md);
     }
   }
 
+  private void commitExternalIdUpdates(
+      String message,
+      Repository allUsersRepo,
+      BatchRefUpdate batchRefUpdate,
+      ExternalIdNotes extIdNotes)
+      throws IOException {
+    try (MetaDataUpdate md = createMetaDataUpdate(message, allUsersRepo, batchRefUpdate)) {
+      extIdNotes.commit(md);
+    }
+  }
+
+  private MetaDataUpdate createMetaDataUpdate(
+      String message, Repository allUsersRepo, BatchRefUpdate batchRefUpdate) {
+    MetaDataUpdate metaDataUpdate =
+        metaDataUpdateInternalFactory.get().create(allUsersName, allUsersRepo, batchRefUpdate);
+    if (!message.endsWith("\n")) {
+      message = message + "\n";
+    }
+
+    metaDataUpdate.getCommitBuilder().setMessage(message);
+    metaDataUpdate.getCommitBuilder().setCommitter(committerIdent);
+    metaDataUpdate.getCommitBuilder().setAuthor(authorIdent);
+    return metaDataUpdate;
+  }
+
   @FunctionalInterface
-  private static interface MetaDataUpdateFactory {
+  private static interface AccountUpdate {
-    MetaDataUpdate create() throws IOException;
+    UpdatedAccount update(Repository allUsersRepo)
+        throws IOException, ConfigInvalidException, OrmException;
+  }
+
+  private static class UpdatedAccount {
+    private final String message;
+    private final AccountConfig accountConfig;
+    private final ExternalIdNotes extIdNotes;
+
+    private boolean created;
+
+    private UpdatedAccount(
+        String message, AccountConfig accountConfig, ExternalIdNotes extIdNotes) {
+      checkState(!Strings.isNullOrEmpty(message), "message for account update must be set");
+      this.message = checkNotNull(message);
+      this.accountConfig = checkNotNull(accountConfig);
+      this.extIdNotes = checkNotNull(extIdNotes);
+    }
+
+    public String getMessage() {
+      return message;
+    }
+
+    public AccountConfig getAccountConfig() {
+      return accountConfig;
+    }
+
+    public Account getAccount() {
+      return accountConfig.getLoadedAccount().get();
+    }
+
+    public ExternalIdNotes getExternalIdNotes() {
+      return extIdNotes;
+    }
+
+    public void setCreated(boolean created) {
+      this.created = created;
+    }
+
+    public boolean isCreated() {
+      return created;
+    }
   }
 }

java/com/google/gerrit/server/account/ChangeUserName.java

@@ -22,7 +22,6 @@ import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.externalids.ExternalId;
 import com.google.gerrit.server.account.externalids.ExternalIds;
-import com.google.gerrit.server.account.externalids.ExternalIdsUpdate;
 import com.google.gerrit.server.ssh.SshKeyCache;
 import com.google.gwtjsonrpc.common.VoidResult;
 import com.google.gwtorm.server.OrmDuplicateKeyException;
@@ -30,7 +29,6 @@ import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
 import java.io.IOException;
-import java.util.Collection;
 import java.util.concurrent.Callable;
 import java.util.regex.Pattern;
 import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -43,13 +41,17 @@ public class ChangeUserName implements Callable<VoidResult> {
 
   /** Generic factory to change any user's username. */
   public interface Factory {
-    ChangeUserName create(IdentifiedUser user, String newUsername);
+    ChangeUserName create(
+        @Assisted("message") String message,
+        IdentifiedUser user,
+        @Assisted("newUsername") String newUsername);
   }
 
   private final SshKeyCache sshKeyCache;
   private final ExternalIds externalIds;
-  private final ExternalIdsUpdate.Server externalIdsUpdateFactory;
+  private final AccountsUpdate.Server accountsUpdate;
 
+  private final String message;
   private final IdentifiedUser user;
   private final String newUsername;
 
@@ -57,12 +59,14 @@ public class ChangeUserName implements Callable<VoidResult> {
   ChangeUserName(
       SshKeyCache sshKeyCache,
       ExternalIds externalIds,
-      ExternalIdsUpdate.Server externalIdsUpdateFactory,
+      AccountsUpdate.Server accountsUpdate,
+      @Assisted("message") String message,
       @Assisted IdentifiedUser user,
-      @Nullable @Assisted String newUsername) {
+      @Nullable @Assisted("newUsername") String newUsername) {
     this.sshKeyCache = sshKeyCache;
     this.externalIds = externalIds;
-    this.externalIdsUpdateFactory = externalIdsUpdateFactory;
+    this.accountsUpdate = accountsUpdate;
+    this.message = message;
     this.user = user;
     this.newUsername = newUsername;
   }
@@ -71,12 +75,10 @@ public class ChangeUserName implements Callable<VoidResult> {
   public VoidResult call()
       throws OrmException, NameAlreadyUsedException, InvalidUserNameException, IOException,
           ConfigInvalidException {
-    Collection<ExternalId> old = externalIds.byAccount(user.getAccountId(), SCHEME_USERNAME);
+    if (!externalIds.byAccount(user.getAccountId(), SCHEME_USERNAME).isEmpty()) {
-    if (!old.isEmpty()) {
       throw new IllegalStateException(USERNAME_CANNOT_BE_CHANGED);
     }
 
-    ExternalIdsUpdate externalIdsUpdate = externalIdsUpdateFactory.create();
     if (newUsername != null && !newUsername.isEmpty()) {
       if (!USER_NAME_PATTERN.matcher(newUsername).matches()) {
         throw new InvalidUserNameException();
@@ -84,13 +86,12 @@ public class ChangeUserName implements Callable<VoidResult> {
 
       ExternalId.Key key = ExternalId.Key.create(SCHEME_USERNAME, newUsername);
       try {
-        String password = null;
+        accountsUpdate
-        for (ExternalId i : old) {
+            .create()
-          if (i.password() != null) {
+            .update(
-            password = i.password();
+                message,
-          }
+                user.getAccountId(),
-        }
+                u -> u.addExternalId(ExternalId.create(key, user.getAccountId(), null, null)));
-        externalIdsUpdate.insert(ExternalId.create(key, user.getAccountId(), null, password));
       } catch (OrmDuplicateKeyException dupeErr) {
         // If we are using this identity, don't report the exception.
         //
@@ -105,13 +106,6 @@ public class ChangeUserName implements Callable<VoidResult> {
       }
     }
 
-    // If we have any older user names, remove them.
-    //
-    externalIdsUpdate.delete(old);
-    for (ExternalId extId : old) {
-      sshKeyCache.evict(extId.key().id());
-    }
-
     sshKeyCache.evict(newUsername);
     return VoidResult.INSTANCE;
   }

java/com/google/gerrit/server/account/CreateAccount.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.account;
 
 import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_MAILTO;
+import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
@@ -37,16 +38,14 @@ import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.Sequences;
+import com.google.gerrit.server.account.externalids.DuplicateExternalIdKeyException;
 import com.google.gerrit.server.account.externalids.ExternalId;
-import com.google.gerrit.server.account.externalids.ExternalIds;
-import com.google.gerrit.server.account.externalids.ExternalIdsUpdate;
 import com.google.gerrit.server.group.GroupsCollection;
 import com.google.gerrit.server.group.UserInitiated;
 import com.google.gerrit.server.group.db.GroupsUpdate;
 import com.google.gerrit.server.group.db.InternalGroupUpdate;
 import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
 import com.google.gerrit.server.ssh.SshKeyCache;
-import com.google.gwtorm.server.OrmDuplicateKeyException;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -72,8 +71,6 @@ public class CreateAccount implements RestModifyView<TopLevelResource, AccountIn
   private final AccountsUpdate.User accountsUpdate;
   private final AccountLoader.Factory infoLoader;
   private final DynamicSet<AccountExternalIdCreator> externalIdCreators;
-  private final ExternalIds externalIds;
-  private final ExternalIdsUpdate.User externalIdsUpdateFactory;
   private final Provider<GroupsUpdate> groupsUpdate;
   private final OutgoingEmailValidator validator;
   private final String username;
@@ -88,8 +85,6 @@ public class CreateAccount implements RestModifyView<TopLevelResource, AccountIn
       AccountsUpdate.User accountsUpdate,
       AccountLoader.Factory infoLoader,
       DynamicSet<AccountExternalIdCreator> externalIdCreators,
-      ExternalIds externalIds,
-      ExternalIdsUpdate.User externalIdsUpdateFactory,
       @UserInitiated Provider<GroupsUpdate> groupsUpdate,
       OutgoingEmailValidator validator,
       @Assisted String username) {
@@ -101,8 +96,6 @@ public class CreateAccount implements RestModifyView<TopLevelResource, AccountIn
     this.accountsUpdate = accountsUpdate;
     this.infoLoader = infoLoader;
     this.externalIdCreators = externalIdCreators;
-    this.externalIds = externalIds;
-    this.externalIdsUpdateFactory = externalIdsUpdateFactory;
     this.groupsUpdate = groupsUpdate;
     this.validator = validator;
     this.username = username;
@@ -130,54 +123,39 @@ public class CreateAccount implements RestModifyView<TopLevelResource, AccountIn
     Set<AccountGroup.UUID> groups = parseGroups(input.groups);
 
     Account.Id id = new Account.Id(seq.nextAccountId());
+    List<ExternalId> extIds = new ArrayList<>();
 
-    ExternalId extUser = ExternalId.createUsername(username, id, input.httpPassword);
-    if (externalIds.get(extUser.key()) != null) {
-      throw new ResourceConflictException("username '" + username + "' already exists");
-    }
     if (input.email != null) {
-      if (externalIds.get(ExternalId.Key.create(SCHEME_MAILTO, input.email)) != null) {
-        throw new UnprocessableEntityException("email '" + input.email + "' already exists");
-      }
       if (!validator.isValid(input.email)) {
         throw new BadRequestException("invalid email address");
       }
+      extIds.add(ExternalId.createEmail(id, input.email));
     }
 
-    List<ExternalId> extIds = new ArrayList<>();
+    extIds.add(ExternalId.createUsername(username, id, input.httpPassword));
-    extIds.add(extUser);
     for (AccountExternalIdCreator c : externalIdCreators) {
       extIds.addAll(c.create(id, username, input.email));
     }
 
-    ExternalIdsUpdate externalIdsUpdate = externalIdsUpdateFactory.create();
     try {
-      externalIdsUpdate.insert(extIds);
-    } catch (OrmDuplicateKeyException duplicateKey) {
-      throw new ResourceConflictException("username '" + username + "' already exists");
-    }
-
-    if (input.email != null) {
-      try {
-        externalIdsUpdate.insert(ExternalId.createEmail(id, input.email));
-      } catch (OrmDuplicateKeyException duplicateKey) {
-        try {
-          externalIdsUpdate.delete(extUser);
-        } catch (IOException | ConfigInvalidException cleanupError) {
-          // Ignored
-        }
-        throw new UnprocessableEntityException("email '" + input.email + "' already exists");
-      }
-    }
-
       accountsUpdate
           .create()
           .insert(
+              "Create Account via API",
               id,
-            a -> {
+              u -> u.setFullName(input.name).setPreferredEmail(input.email).addExternalIds(extIds));
-              a.setFullName(input.name);
+    } catch (DuplicateExternalIdKeyException e) {
-              a.setPreferredEmail(input.email);
+      if (e.getDuplicateKey().isScheme(SCHEME_USERNAME)) {
-            });
+        throw new ResourceConflictException(
+            "username '" + e.getDuplicateKey().id() + "' already exists");
+      } else if (e.getDuplicateKey().isScheme(SCHEME_MAILTO)) {
+        throw new UnprocessableEntityException(
+            "email '" + e.getDuplicateKey().id() + "' already exists");
+      } else {
+        // AccountExternalIdCreator returned an external ID that already exists
+        throw e;
+      }
+    }
 
     for (AccountGroup.UUID groupUuid : groups) {
       try {

java/com/google/gerrit/server/account/InternalAccountUpdate.java

@@ -0,0 +1,390 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the Licens
+
+package com.google.gerrit.server.account;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.server.account.externalids.DuplicateExternalIdKeyException;
+import com.google.gerrit.server.account.externalids.ExternalId;
+import java.util.Collection;
+import java.util.Optional;
+
+/**
+ * Class to prepare updates to an account.
+ *
+ * <p>The getters in this class and the setters in the {@link Builder} correspond to fields in
+ * {@link Account}. The account ID and the registration date cannot be updated.
+ */
+@AutoValue
+public abstract class InternalAccountUpdate {
+  public static Builder builder() {
+    return new Builder.WrapperThatConvertsNullStringArgsToEmptyStrings(
+        new AutoValue_InternalAccountUpdate.Builder());
+  }
+
+  /**
+   * Returns the new value for the full name.
+   *
+   * @return the new value for the full name, {@code Optional#empty()} if the full name is not being
+   *     updated, {@code Optional#of("")} if the full name is unset, the wrapped value is never
+   *     {@code null}
+   */
+  public abstract Optional<String> getFullName();
+
+  /**
+   * Returns the new value for the preferred email.
+   *
+   * @return the new value for the preferred email, {@code Optional#empty()} if the preferred email
+   *     is not being updated, {@code Optional#of("")} if the preferred email is unset, the wrapped
+   *     value is never {@code null}
+   */
+  public abstract Optional<String> getPreferredEmail();
+
+  /**
+   * Returns the new value for the active flag.
+   *
+   * @return the new value for the active flag, {@code Optional#empty()} if the active flag is not
+   *     being updated, the wrapped value is never {@code null}
+   */
+  public abstract Optional<Boolean> getActive();
+
+  /**
+   * Returns the new value for the status.
+   *
+   * @return the new value for the status, {@code Optional#empty()} if the status is not being
+   *     updated, {@code Optional#of("")} if the status is unset, the wrapped value is never {@code
+   *     null}
+   */
+  public abstract Optional<String> getStatus();
+
+  /**
+   * Returns external IDs that should be newly created for the account.
+   *
+   * @return external IDs that should be newly created for the account
+   */
+  public abstract ImmutableSet<ExternalId> getCreatedExternalIds();
+
+  /**
+   * Returns external IDs that should be updated for the account.
+   *
+   * @return external IDs that should be updated for the account
+   */
+  public abstract ImmutableSet<ExternalId> getUpdatedExternalIds();
+
+  /**
+   * Returns external IDs that should be deleted for the account.
+   *
+   * @return external IDs that should be deleted for the account
+   */
+  public abstract ImmutableSet<ExternalId> getDeletedExternalIds();
+
+  /**
+   * Class to build an account update.
+   *
+   * <p>Account data is only updated if the corresponding setter is invoked. If a setter is not
+   * invoked the corresponding data stays unchanged. To unset string values the setter can be
+   * invoked with either {@code null} or an empty string ({@code null} is converted to an empty
+   * string by using the {@link WrapperThatConvertsNullStringArgsToEmptyStrings} wrapper, see {@link
+   * InternalAccountUpdate#builder()}).
+   */
+  @AutoValue.Builder
+  public abstract static class Builder {
+    /**
+     * Sets a new full name for the account.
+     *
+     * @param fullName the new full name, if {@code null} or empty string the full name is unset
+     * @return the builder
+     */
+    public abstract Builder setFullName(String fullName);
+
+    /**
+     * Sets a new preferred email for the account.
+     *
+     * @param preferredEmail the new preferred email, if {@code null} or empty string the preferred
+     *     email is unset
+     * @return the builder
+     */
+    public abstract Builder setPreferredEmail(String preferredEmail);
+
+    /**
+     * Sets the active flag for the account.
+     *
+     * @param active {@code true} if the account should be set to active, {@code false} if the
+     *     account should be set to inactive
+     * @return the builder
+     */
+    public abstract Builder setActive(boolean active);
+
+    /**
+     * Sets a new status for the account.
+     *
+     * @param status the new status, if {@code null} or empty string the status is unset
+     * @return the builder
+     */
+    public abstract Builder setStatus(String status);
+
+    /**
+     * Returns a builder for the set of created external IDs.
+     *
+     * @return builder for the set of created external IDs.
+     */
+    abstract ImmutableSet.Builder<ExternalId> createdExternalIdsBuilder();
+
+    /**
+     * Adds a new external ID for the account.
+     *
+     * <p>The account ID of the external ID must match the account ID of the account that is
+     * updated.
+     *
+     * <p>If an external ID with the same ID already exists the account update will fail with {@link
+     * DuplicateExternalIdKeyException}.
+     *
+     * @param extId external ID that should be added
+     * @return the builder
+     */
+    public Builder addExternalId(ExternalId extId) {
+      return addExternalIds(ImmutableSet.of(extId));
+    }
+
+    /**
+     * Adds new external IDs for the account.
+     *
+     * <p>The account IDs of the external IDs must match the account ID of the account that is
+     * updated.
+     *
+     * <p>If any of the external ID keys already exists, the insert fails with {@link
+     * DuplicateExternalIdKeyException}.
+     *
+     * @param extIds external IDs that should be added
+     * @return the builder
+     */
+    public Builder addExternalIds(Collection<ExternalId> extIds) {
+      createdExternalIdsBuilder().addAll(extIds);
+      return this;
+    }
+
+    /**
+     * Returns a builder for the set of updated external IDs.
+     *
+     * @return builder for the set of updated external IDs.
+     */
+    abstract ImmutableSet.Builder<ExternalId> updatedExternalIdsBuilder();
+
+    /**
+     * Updates an external ID for the account.
+     *
+     * <p>The account ID of the external ID must match the account ID of the account that is
+     * updated.
+     *
+     * <p>If no external ID with the ID exists the external ID is created.
+     *
+     * @param extId external ID that should be updated
+     * @return the builder
+     */
+    public Builder updateExternalId(ExternalId extId) {
+      return updateExternalIds(ImmutableSet.of(extId));
+    }
+
+    /**
+     * Updates external IDs for the account.
+     *
+     * <p>The account IDs of the external IDs must match the account ID of the account that is
+     * updated.
+     *
+     * <p>If any of the external IDs already exists, it is overwritten. New external IDs are
+     * inserted.
+     *
+     * @param extIds external IDs that should be updated
+     * @return the builder
+     */
+    public Builder updateExternalIds(Collection<ExternalId> extIds) {
+      updatedExternalIdsBuilder().addAll(extIds);
+      return this;
+    }
+
+    /**
+     * Returns a builder for the set of deleted external IDs.
+     *
+     * @return builder for the set of deleted external IDs.
+     */
+    abstract ImmutableSet.Builder<ExternalId> deletedExternalIdsBuilder();
+
+    /**
+     * Deletes an external ID for the account.
+     *
+     * <p>The account ID of the external ID must match the account ID of the account that is
+     * updated.
+     *
+     * <p>If no external ID with the ID exists this is a no-op.
+     *
+     * @param extId external ID that should be deleted
+     * @return the builder
+     */
+    public Builder deleteExternalId(ExternalId extId) {
+      return deleteExternalIds(ImmutableSet.of(extId));
+    }
+
+    /**
+     * Delete external IDs for the account.
+     *
+     * <p>The account IDs of the external IDs must match the account ID of the account that is
+     * updated.
+     *
+     * <p>For non-existing external IDs this is a no-op.
+     *
+     * @param extIds external IDs that should be deleted
+     * @return the builder
+     */
+    public Builder deleteExternalIds(Collection<ExternalId> extIds) {
+      deletedExternalIdsBuilder().addAll(extIds);
+      return this;
+    }
+
+    /**
+     * Replaces an external ID.
+     *
+     * @param extIdToDelete external ID that should be deleted
+     * @param extIdToAdd external ID that should be added
+     * @return the builder
+     */
+    public Builder replaceExternalId(ExternalId extIdToDelete, ExternalId extIdToAdd) {
+      return replaceExternalIds(ImmutableSet.of(extIdToDelete), ImmutableSet.of(extIdToAdd));
+    }
+
+    /**
+     * Replaces an external IDs.
+     *
+     * @param extIdsToDelete external IDs that should be deleted
+     * @param extIdsToAdd external IDs that should be added
+     * @return the builder
+     */
+    public Builder replaceExternalIds(
+        Collection<ExternalId> extIdsToDelete, Collection<ExternalId> extIdsToAdd) {
+      return deleteExternalIds(extIdsToDelete).addExternalIds(extIdsToAdd);
+    }
+
+    /**
+     * Builds the account update.
+     *
+     * @return the account update
+     */
+    public abstract InternalAccountUpdate build();
+
+    /**
+     * Wrapper for {@link Builder} that converts {@code null} string arguments to empty strings for
+     * all setter methods. This allows us to treat setter invocations with a {@code null} string
+     * argument as signal to unset the corresponding field. E.g. for a builder method {@code
+     * setX(String)} the following semantics apply:
+     *
+     * <ul>
+     *   <li>Method is not invoked: X stays unchanged, X is stored as {@code Optional.empty()}.
+     *   <li>Argument is a non-empty string Y: X is updated to the Y, X is stored as {@code
+     *       Optional.of(Y)}.
+     *   <li>Argument is an empty string: X is unset, X is stored as {@code Optional.of("")}
+     *   <li>Argument is {@code null}: X is unset, X is stored as {@code Optional.of("")} (since the
+     *       wrapper converts {@code null} to an empty string)
+     * </ul>
+     *
+     * Without the wrapper calling {@code setX(null)} would fail with a {@link
+     * NullPointerException}. Hence all callers would need to take care to call {@link
+     * Strings#nullToEmpty(String)} for all string arguments and likely it would be forgotten in
+     * some places.
+     *
+     * <p>This means the stored values are interpreted like this:
+     *
+     * <ul>
+     *   <li>{@code Optional.empty()}: property stays unchanged
+     *   <li>{@code Optional.of(<non-empty-string>)}: property is updated
+     *   <li>{@code Optional.of("")}: property is unset
+     * </ul>
+     *
+     * This wrapper forwards all method invocations to the wrapped {@link Builder} instance that was
+     * created by AutoValue. For methods that return the AutoValue {@link Builder} instance the
+     * return value is replaced with the wrapper instance so that all chained calls go through the
+     * wrapper.
+     */
+    private static class WrapperThatConvertsNullStringArgsToEmptyStrings extends Builder {
+      private final Builder delegate;
+
+      private WrapperThatConvertsNullStringArgsToEmptyStrings(Builder delegate) {
+        this.delegate = delegate;
+      }
+
+      @Override
+      public Builder setFullName(String fullName) {
+        delegate.setFullName(Strings.nullToEmpty(fullName));
+        return this;
+      }
+
+      @Override
+      public Builder setPreferredEmail(String preferredEmail) {
+        delegate.setPreferredEmail(Strings.nullToEmpty(preferredEmail));
+        return this;
+      }
+
+      @Override
+      public Builder setActive(boolean active) {
+        delegate.setActive(active);
+        return this;
+      }
+
+      @Override
+      public Builder setStatus(String status) {
+        delegate.setStatus(Strings.nullToEmpty(status));
+        return this;
+      }
+
+      @Override
+      public InternalAccountUpdate build() {
+        return delegate.build();
+      }
+
+      @Override
+      ImmutableSet.Builder<ExternalId> createdExternalIdsBuilder() {
+        return delegate.createdExternalIdsBuilder();
+      }
+
+      @Override
+      public Builder addExternalIds(Collection<ExternalId> extIds) {
+        delegate.addExternalIds(extIds);
+        return this;
+      }
+
+      @Override
+      ImmutableSet.Builder<ExternalId> updatedExternalIdsBuilder() {
+        return delegate.updatedExternalIdsBuilder();
+      }
+
+      @Override
+      public Builder updateExternalIds(Collection<ExternalId> extIds) {
+        delegate.updateExternalIds(extIds);
+        return this;
+      }
+
+      @Override
+      ImmutableSet.Builder<ExternalId> deletedExternalIdsBuilder() {
+        return delegate.deletedExternalIdsBuilder();
+      }
+
+      @Override
+      public Builder deleteExternalIds(Collection<ExternalId> extIds) {
+        delegate.deleteExternalIds(extIds);
+        return this;
+      }
+    }
+  }
+}

java/com/google/gerrit/server/account/PutName.java

@@ -66,7 +66,7 @@ public class PutName implements RestModifyView<AccountResource, NameInput> {
 
   public Response<String> apply(IdentifiedUser user, NameInput input)
       throws MethodNotAllowedException, ResourceNotFoundException, IOException,
-          ConfigInvalidException {
+          ConfigInvalidException, OrmException {
     if (input == null) {
       input = new NameInput();
     }
@@ -77,7 +77,9 @@ public class PutName implements RestModifyView<AccountResource, NameInput> {
 
     String newName = input.name;
     Account account =
-        accountsUpdate.create().update(user.getAccountId(), a -> a.setFullName(newName));
+        accountsUpdate
+            .create()
+            .update("Set Full Name via API", user.getAccountId(), u -> u.setFullName(newName));
     if (account == null) {
       throw new ResourceNotFoundException("account not found");
     }

java/com/google/gerrit/server/account/PutPreferred.java

@@ -61,18 +61,19 @@ public class PutPreferred implements RestModifyView<AccountResource.Email, Input
   }
 
   public Response<String> apply(IdentifiedUser user, String email)
-      throws ResourceNotFoundException, IOException, ConfigInvalidException {
+      throws ResourceNotFoundException, IOException, ConfigInvalidException, OrmException {
     AtomicBoolean alreadyPreferred = new AtomicBoolean(false);
     Account account =
         accountsUpdate
             .create()
             .update(
+                "Set Preferred Email via API",
                 user.getAccountId(),
-                a -> {
+                (a, u) -> {
                   if (email.equals(a.getPreferredEmail())) {
                     alreadyPreferred.set(true);
                   } else {
-                    a.setPreferredEmail(email);
+                    u.setPreferredEmail(email);
                   }
                 });
     if (account == null) {

java/com/google/gerrit/server/account/PutStatus.java

@@ -60,7 +60,7 @@ public class PutStatus implements RestModifyView<AccountResource, StatusInput> {
   }
 
   public Response<String> apply(IdentifiedUser user, StatusInput input)
-      throws ResourceNotFoundException, IOException, ConfigInvalidException {
+      throws ResourceNotFoundException, IOException, ConfigInvalidException, OrmException {
     if (input == null) {
       input = new StatusInput();
     }
@@ -69,7 +69,7 @@ public class PutStatus implements RestModifyView<AccountResource, StatusInput> {
     Account account =
         accountsUpdate
             .create()
-            .update(user.getAccountId(), a -> a.setStatus(Strings.nullToEmpty(newStatus)));
+            .update("Set Status via API", user.getAccountId(), u -> u.setStatus(newStatus));
     if (account == null) {
       throw new ResourceNotFoundException("account not found");
     }

java/com/google/gerrit/server/account/PutUsername.java

@@ -70,7 +70,7 @@ public class PutUsername implements RestModifyView<AccountResource, UsernameInpu
     }
 
     try {
-      changeUserNameFactory.create(rsrc.getUser(), input.username).call();
+      changeUserNameFactory.create("Set Username via API", rsrc.getUser(), input.username).call();
     } catch (IllegalStateException e) {
       if (ChangeUserName.USERNAME_CANNOT_BE_CHANGED.equals(e.getMessage())) {
         throw new MethodNotAllowedException(e.getMessage());

java/com/google/gerrit/server/account/SetInactiveFlag.java

@@ -19,6 +19,7 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.reviewdb.client.Account;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 import java.io.IOException;
@@ -36,18 +37,19 @@ public class SetInactiveFlag {
   }
 
   public Response<?> deactivate(Account.Id accountId)
-      throws RestApiException, IOException, ConfigInvalidException {
+      throws RestApiException, IOException, ConfigInvalidException, OrmException {
     AtomicBoolean alreadyInactive = new AtomicBoolean(false);
     Account account =
         accountsUpdate
             .create()
             .update(
+                "Deactivate Account via API",
                 accountId,
-                a -> {
+                (a, u) -> {
                   if (!a.isActive()) {
                     alreadyInactive.set(true);
                   } else {
-                    a.setActive(false);
+                    u.setActive(false);
                   }
                 });
     if (account == null) {
@@ -60,18 +62,19 @@ public class SetInactiveFlag {
   }
 
   public Response<String> activate(Account.Id accountId)
-      throws ResourceNotFoundException, IOException, ConfigInvalidException {
+      throws ResourceNotFoundException, IOException, ConfigInvalidException, OrmException {
     AtomicBoolean alreadyActive = new AtomicBoolean(false);
     Account account =
         accountsUpdate
             .create()
             .update(
+                "Activate Account via API",
                 accountId,
-                a -> {
+                (a, u) -> {
                   if (a.isActive()) {
                     alreadyActive.set(true);
                   } else {
-                    a.setActive(true);
+                    u.setActive(true);
                   }
                 });
     if (account == null) {

java/com/google/gerrit/server/account/externalids/DuplicateExternalIdKeyException.java

@@ -0,0 +1,36 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.account.externalids;
+
+import com.google.gwtorm.server.OrmDuplicateKeyException;
+
+/**
+ * Exception that is thrown if an external ID cannot be inserted because an external ID with the
+ * same key already exists.
+ */
+public class DuplicateExternalIdKeyException extends OrmDuplicateKeyException {
+  private static final long serialVersionUID = 1L;
+
+  private final ExternalId.Key duplicateKey;
+
+  public DuplicateExternalIdKeyException(ExternalId.Key duplicateKey) {
+    super("Duplicate external ID key: " + duplicateKey.get());
+    this.duplicateKey = duplicateKey;
+  }
+
+  public ExternalId.Key getDuplicateKey() {
+    return duplicateKey;
+  }
+}

java/com/google/gerrit/server/account/externalids/ExternalId.java

@@ -16,11 +16,13 @@ package com.google.gerrit.server.account.externalids;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.auto.value.AutoValue;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.hash.Hashing;
 import com.google.gerrit.common.Nullable;
@@ -28,6 +30,7 @@ import com.google.gerrit.extensions.client.AuthType;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.server.account.HashedPassword;
 import java.io.Serializable;
+import java.util.Collection;
 import java.util.Objects;
 import java.util.Set;
 import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -123,6 +126,10 @@ public abstract class ExternalId implements Serializable {
     public String toString() {
       return get();
     }
+
+    public static ImmutableSet<ExternalId.Key> from(Collection<ExternalId> extIds) {
+      return extIds.stream().map(ExternalId::key).collect(toImmutableSet());
+    }
   }
 
   public static ExternalId create(String scheme, String id, Account.Id accountId) {

java/com/google/gerrit/server/account/externalids/ExternalIdCacheImpl.java

@@ -127,7 +127,7 @@ class ExternalIdCacheImpl implements ExternalIdCache {
       Collection<ExternalId> toRemove,
       Collection<ExternalId> toAdd)
       throws IOException {
-    ExternalIdsUpdate.checkSameAccount(Iterables.concat(toRemove, toAdd), accountId);
+    ExternalIdNotes.checkSameAccount(Iterables.concat(toRemove, toAdd), accountId);
 
     updateCache(
         oldNotesRev,

java/com/google/gerrit/server/account/externalids/ExternalIdNotes.java

@@ -0,0 +1,754 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.account.externalids;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.stream.Collectors.toSet;
+import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
+
+import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Sets;
+import com.google.common.collect.Streams;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.git.MetaDataUpdate;
+import com.google.gerrit.server.git.VersionedMetaData;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.BlobBasedConfig;
+import org.eclipse.jgit.lib.CommitBuilder;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.notes.Note;
+import org.eclipse.jgit.notes.NoteMap;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevTree;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * {@link VersionedMetaData} subclass to update external IDs.
+ *
+ * <p>On load the note map from {@code refs/meta/external-ids} is read, but the external IDs are not
+ * parsed yet (see {@link #onLoad()}).
+ *
+ * <p>After loading the note map callers can access single or all external IDs. Only now the
+ * requested external IDs are parsed.
+ *
+ * <p>After loading the note map callers can stage various external ID updates (insert, upsert,
+ * delete, replace).
+ *
+ * <p>On save the staged external ID updates are performed (see {@link #onSave(CommitBuilder)}).
+ *
+ * <p>After committing the external IDs a cache update can be requested which also reindexes the
+ * accounts for which external IDs have been updated (see {@link #updateCaches()}).
+ */
+public class ExternalIdNotes extends VersionedMetaData {
+  private static final Logger log = LoggerFactory.getLogger(ExternalIdNotes.class);
+
+  private static final int MAX_NOTE_SZ = 1 << 19;
+
+  @Singleton
+  public static class Factory {
+    private final ExternalIdCache externalIdCache;
+    private final AccountCache accountCache;
+
+    @Inject
+    Factory(ExternalIdCache externalIdCache, AccountCache accountCache) {
+      this.externalIdCache = externalIdCache;
+      this.accountCache = accountCache;
+    }
+
+    public ExternalIdNotes load(Repository allUsersRepo)
+        throws IOException, ConfigInvalidException {
+      return new ExternalIdNotes(externalIdCache, accountCache, allUsersRepo).load();
+    }
+  }
+
+  @Singleton
+  public static class FactoryNoReindex {
+    private final ExternalIdCache externalIdCache;
+
+    @Inject
+    FactoryNoReindex(ExternalIdCache externalIdCache) {
+      this.externalIdCache = externalIdCache;
+    }
+
+    public ExternalIdNotes load(Repository allUsersRepo)
+        throws IOException, ConfigInvalidException {
+      return new ExternalIdNotes(externalIdCache, null, allUsersRepo).load();
+    }
+  }
+
+  public static ExternalIdNotes loadReadOnly(Repository allUsersRepo)
+      throws IOException, ConfigInvalidException {
+    return new ExternalIdNotes(new DisabledExternalIdCache(), null, allUsersRepo)
+        .setReadOnly()
+        .load();
+  }
+
+  public static ExternalIdNotes loadReadOnly(Repository allUsersRepo, ObjectId rev)
+      throws IOException, ConfigInvalidException {
+    return new ExternalIdNotes(new DisabledExternalIdCache(), null, allUsersRepo)
+        .setReadOnly()
+        .load(rev);
+  }
+
+  public static ExternalIdNotes loadNoCacheUpdate(Repository allUsersRepo)
+      throws IOException, ConfigInvalidException {
+    return new ExternalIdNotes(new DisabledExternalIdCache(), null, allUsersRepo).load();
+  }
+
+  private final ExternalIdCache externalIdCache;
+  @Nullable private final AccountCache accountCache;
+  private final Repository repo;
+
+  private NoteMap noteMap;
+  private ObjectId oldRev;
+
+  // Staged note map updates that should be executed on save.
+  private List<NoteMapUpdate> noteMapUpdates = new ArrayList<>();
+
+  // Staged cache updates that should be executed after external ID changes have been committed.
+  private List<CacheUpdate> cacheUpdates = new ArrayList<>();
+
+  private Runnable afterReadRevision;
+  private boolean readOnly = false;
+
+  ExternalIdNotes(
+      ExternalIdCache externalIdCache,
+      @Nullable AccountCache accountCache,
+      Repository allUsersRepo) {
+    this.externalIdCache = checkNotNull(externalIdCache, "externalIdCache");
+    this.accountCache = accountCache;
+    this.repo = checkNotNull(allUsersRepo, "allUsersRepo");
+  }
+
+  public ExternalIdNotes setAfterReadRevision(Runnable afterReadRevision) {
+    this.afterReadRevision = afterReadRevision;
+    return this;
+  }
+
+  private ExternalIdNotes setReadOnly() {
+    this.readOnly = true;
+    return this;
+  }
+
+  public Repository getRepository() {
+    return repo;
+  }
+
+  @Override
+  protected String getRefName() {
+    return RefNames.REFS_EXTERNAL_IDS;
+  }
+
+  ExternalIdNotes load() throws IOException, ConfigInvalidException {
+    load(repo);
+    return this;
+  }
+
+  ExternalIdNotes load(ObjectId rev) throws IOException, ConfigInvalidException {
+    load(repo, rev);
+    return this;
+  }
+
+  /**
+   * Parses and returns the specified external ID.
+   *
+   * @param key the key of the external ID
+   * @return the external ID, {@code Optional.empty()} if it doesn't exist
+   */
+  public Optional<ExternalId> get(ExternalId.Key key) throws IOException, ConfigInvalidException {
+    checkLoaded();
+    ObjectId noteId = key.sha1();
+    if (!noteMap.contains(noteId)) {
+      return Optional.empty();
+    }
+
+    try (RevWalk rw = new RevWalk(repo)) {
+      ObjectId noteDataId = noteMap.get(noteId);
+      byte[] raw = readNoteData(rw, noteDataId);
+      return Optional.of(ExternalId.parse(noteId.name(), raw, noteDataId));
+    }
+  }
+
+  /**
+   * Parses and returns the specified external IDs.
+   *
+   * @param keys the keys of the external IDs
+   * @return the external IDs
+   */
+  public Set<ExternalId> get(Collection<ExternalId.Key> keys)
+      throws IOException, ConfigInvalidException {
+    checkLoaded();
+    HashSet<ExternalId> externalIds = Sets.newHashSetWithExpectedSize(keys.size());
+    for (ExternalId.Key key : keys) {
+      get(key).ifPresent(externalIds::add);
+    }
+    return externalIds;
+  }
+
+  /**
+   * Parses and returns all external IDs.
+   *
+   * <p>Invalid external IDs are ignored.
+   *
+   * @return all external IDs
+   */
+  public Set<ExternalId> all() throws IOException {
+    checkLoaded();
+    try (RevWalk rw = new RevWalk(repo)) {
+      Set<ExternalId> extIds = new HashSet<>();
+      for (Note note : noteMap) {
+        byte[] raw = readNoteData(rw, note.getData());
+        try {
+          extIds.add(ExternalId.parse(note.getName(), raw, note.getData()));
+        } catch (ConfigInvalidException | RuntimeException e) {
+          log.error(String.format("Ignoring invalid external ID note %s", note.getName()), e);
+        }
+      }
+      return extIds;
+    }
+  }
+
+  NoteMap getNoteMap() {
+    checkLoaded();
+    return noteMap;
+  }
+
+  static byte[] readNoteData(RevWalk rw, ObjectId noteDataId) throws IOException {
+    return rw.getObjectReader().open(noteDataId, OBJ_BLOB).getCachedBytes(MAX_NOTE_SZ);
+  }
+
+  /**
+   * Inserts a new external ID.
+   *
+   * @throws IOException on IO error while checking if external ID already exists
+   * @throws DuplicateExternalIdKeyException if the external ID already exists
+   */
+  public void insert(ExternalId extId) throws IOException, DuplicateExternalIdKeyException {
+    insert(Collections.singleton(extId));
+  }
+
+  /**
+   * Inserts new external IDs.
+   *
+   * @throws IOException on IO error while checking if external IDs already exist
+   * @throws DuplicateExternalIdKeyException if any of the external ID already exists
+   */
+  public void insert(Collection<ExternalId> extIds)
+      throws IOException, DuplicateExternalIdKeyException {
+    checkLoaded();
+    checkExternalIdsDontExist(extIds);
+
+    Set<ExternalId> newExtIds = new HashSet<>();
+    noteMapUpdates.add(
+        (rw, n) -> {
+          for (ExternalId extId : extIds) {
+            ExternalId insertedExtId = upsert(rw, inserter, noteMap, extId);
+            newExtIds.add(insertedExtId);
+          }
+        });
+    cacheUpdates.add(cu -> cu.add(newExtIds));
+  }
+
+  /**
+   * Inserts or updates an external ID.
+   *
+   * <p>If the external ID already exists, it is overwritten, otherwise it is inserted.
+   */
+  public void upsert(ExternalId extId) throws IOException, ConfigInvalidException {
+    upsert(Collections.singleton(extId));
+  }
+
+  /**
+   * Inserts or updates external IDs.
+   *
+   * <p>If any of the external IDs already exists, it is overwritten. New external IDs are inserted.
+   */
+  public void upsert(Collection<ExternalId> extIds) throws IOException, ConfigInvalidException {
+    checkLoaded();
+    Set<ExternalId> removedExtIds = get(ExternalId.Key.from(extIds));
+    Set<ExternalId> updatedExtIds = new HashSet<>();
+    noteMapUpdates.add(
+        (rw, n) -> {
+          for (ExternalId extId : extIds) {
+            ExternalId updatedExtId = upsert(rw, inserter, noteMap, extId);
+            updatedExtIds.add(updatedExtId);
+          }
+        });
+    cacheUpdates.add(cu -> cu.remove(removedExtIds).add(updatedExtIds));
+  }
+
+  /**
+   * Deletes an external ID.
+   *
+   * @throws IllegalStateException is thrown if there is an existing external ID that has the same
+   *     key, but otherwise doesn't match the specified external ID.
+   */
+  public void delete(ExternalId extId) {
+    delete(Collections.singleton(extId));
+  }
+
+  /**
+   * Deletes external IDs.
+   *
+   * @throws IllegalStateException is thrown if there is an existing external ID that has the same
+   *     key as any of the external IDs that should be deleted, but otherwise doesn't match the that
+   *     external ID.
+   */
+  public void delete(Collection<ExternalId> extIds) {
+    checkLoaded();
+    Set<ExternalId> removedExtIds = new HashSet<>();
+    noteMapUpdates.add(
+        (rw, n) -> {
+          for (ExternalId extId : extIds) {
+            remove(rw, noteMap, extId);
+            removedExtIds.add(extId);
+          }
+        });
+    cacheUpdates.add(cu -> cu.remove(removedExtIds));
+  }
+
+  /**
+   * Delete an external ID by key.
+   *
+   * @throws IllegalStateException is thrown if the external ID does not belong to the specified
+   *     account.
+   */
+  public void delete(Account.Id accountId, ExternalId.Key extIdKey) {
+    delete(accountId, Collections.singleton(extIdKey));
+  }
+
+  /**
+   * Delete external IDs by external ID key.
+   *
+   * @throws IllegalStateException is thrown if any of the external IDs does not belong to the
+   *     specified account.
+   */
+  public void delete(Account.Id accountId, Collection<ExternalId.Key> extIdKeys) {
+    checkLoaded();
+    Set<ExternalId> removedExtIds = new HashSet<>();
+    noteMapUpdates.add(
+        (rw, n) -> {
+          for (ExternalId.Key extIdKey : extIdKeys) {
+            ExternalId removedExtId = remove(rw, noteMap, extIdKey, accountId);
+            removedExtIds.add(removedExtId);
+          }
+        });
+    cacheUpdates.add(cu -> cu.remove(removedExtIds));
+  }
+
+  /**
+   * Delete external IDs by external ID key.
+   *
+   * <p>The external IDs are deleted regardless of which account they belong to.
+   */
+  public void deleteByKeys(Collection<ExternalId.Key> extIdKeys) {
+    checkLoaded();
+    Set<ExternalId> removedExtIds = new HashSet<>();
+    noteMapUpdates.add(
+        (rw, n) -> {
+          for (ExternalId.Key extIdKey : extIdKeys) {
+            ExternalId extId = remove(rw, noteMap, extIdKey, null);
+            removedExtIds.add(extId);
+          }
+        });
+    cacheUpdates.add(cu -> cu.remove(removedExtIds));
+  }
+
+  /**
+   * Replaces external IDs for an account by external ID keys.
+   *
+   * <p>Deletion of external IDs is done before adding the new external IDs. This means if an
+   * external ID key is specified for deletion and an external ID with the same key is specified to
+   * be added, the old external ID with that key is deleted first and then the new external ID is
+   * added (so the external ID for that key is replaced).
+   *
+   * @throws IllegalStateException is thrown if any of the specified external IDs does not belong to
+   *     the specified account.
+   */
+  public void replace(
+      Account.Id accountId, Collection<ExternalId.Key> toDelete, Collection<ExternalId> toAdd)
+      throws IOException, DuplicateExternalIdKeyException {
+    checkLoaded();
+    checkSameAccount(toAdd, accountId);
+    checkExternalIdKeysDontExist(ExternalId.Key.from(toAdd), toDelete);
+
+    Set<ExternalId> removedExtIds = new HashSet<>();
+    Set<ExternalId> updatedExtIds = new HashSet<>();
+    noteMapUpdates.add(
+        (rw, n) -> {
+          for (ExternalId.Key extIdKey : toDelete) {
+            ExternalId removedExtId = remove(rw, noteMap, extIdKey, accountId);
+            if (removedExtId != null) {
+              removedExtIds.add(removedExtId);
+            }
+          }
+
+          for (ExternalId extId : toAdd) {
+            ExternalId insertedExtId = upsert(rw, inserter, noteMap, extId);
+            updatedExtIds.add(insertedExtId);
+          }
+        });
+    cacheUpdates.add(cu -> cu.add(updatedExtIds).remove(removedExtIds));
+  }
+
+  /**
+   * Replaces external IDs for an account by external ID keys.
+   *
+   * <p>Deletion of external IDs is done before adding the new external IDs. This means if an
+   * external ID key is specified for deletion and an external ID with the same key is specified to
+   * be added, the old external ID with that key is deleted first and then the new external ID is
+   * added (so the external ID for that key is replaced).
+   *
+   * <p>The external IDs are replaced regardless of which account they belong to.
+   */
+  public void replaceByKeys(Collection<ExternalId.Key> toDelete, Collection<ExternalId> toAdd)
+      throws IOException, DuplicateExternalIdKeyException {
+    checkLoaded();
+    checkExternalIdKeysDontExist(ExternalId.Key.from(toAdd), toDelete);
+
+    Set<ExternalId> removedExtIds = new HashSet<>();
+    Set<ExternalId> updatedExtIds = new HashSet<>();
+    noteMapUpdates.add(
+        (rw, n) -> {
+          for (ExternalId.Key extIdKey : toDelete) {
+            ExternalId removedExtId = remove(rw, noteMap, extIdKey, null);
+            removedExtIds.add(removedExtId);
+          }
+
+          for (ExternalId extId : toAdd) {
+            ExternalId insertedExtId = upsert(rw, inserter, noteMap, extId);
+            updatedExtIds.add(insertedExtId);
+          }
+        });
+    cacheUpdates.add(cu -> cu.add(updatedExtIds).remove(removedExtIds));
+  }
+
+  /**
+   * Replaces an external ID.
+   *
+   * @throws IllegalStateException is thrown if the specified external IDs belong to different
+   *     accounts.
+   */
+  public void replace(ExternalId toDelete, ExternalId toAdd)
+      throws IOException, DuplicateExternalIdKeyException {
+    replace(Collections.singleton(toDelete), Collections.singleton(toAdd));
+  }
+
+  /**
+   * Replaces external IDs.
+   *
+   * <p>Deletion of external IDs is done before adding the new external IDs. This means if an
+   * external ID is specified for deletion and an external ID with the same key is specified to be
+   * added, the old external ID with that key is deleted first and then the new external ID is added
+   * (so the external ID for that key is replaced).
+   *
+   * @throws IllegalStateException is thrown if the specified external IDs belong to different
+   *     accounts.
+   */
+  public void replace(Collection<ExternalId> toDelete, Collection<ExternalId> toAdd)
+      throws IOException, DuplicateExternalIdKeyException {
+    Account.Id accountId = checkSameAccount(Iterables.concat(toDelete, toAdd));
+    if (accountId == null) {
+      // toDelete and toAdd are empty -> nothing to do
+      return;
+    }
+
+    replace(accountId, toDelete.stream().map(e -> e.key()).collect(toSet()), toAdd);
+  }
+
+  @Override
+  protected void onLoad() throws IOException, ConfigInvalidException {
+    noteMap = revision != null ? NoteMap.read(reader, revision) : NoteMap.newEmptyMap();
+
+    if (afterReadRevision != null) {
+      afterReadRevision.run();
+    }
+  }
+
+  @Override
+  public RevCommit commit(MetaDataUpdate update) throws IOException {
+    oldRev = revision != null ? revision.copy() : ObjectId.zeroId();
+    return super.commit(update);
+  }
+
+  /**
+   * Updates the caches (external ID cache, account cache) and reindexes the accounts for which
+   * external IDs were modified.
+   *
+   * <p>Must only be called after committing changes.
+   *
+   * <p>No-op if this instance was created by {@link #loadNoCacheUpdate(Repository)}.
+   *
+   * <p>No eviction from account cache if this instance was created by {@link FactoryNoReindex}.
+   */
+  public void updateCaches() throws IOException {
+    checkState(oldRev != null, "no changes committed yet");
+
+    ExternalIdCacheUpdates externalIdCacheUpdates = new ExternalIdCacheUpdates();
+    for (CacheUpdate cacheUpdate : cacheUpdates) {
+      cacheUpdate.execute(externalIdCacheUpdates);
+    }
+
+    externalIdCache.onReplace(
+        oldRev,
+        getRevision(),
+        externalIdCacheUpdates.getRemoved(),
+        externalIdCacheUpdates.getAdded());
+
+    if (accountCache != null) {
+      for (Account.Id id :
+          Streams.concat(
+                  externalIdCacheUpdates.getAdded().stream(),
+                  externalIdCacheUpdates.getRemoved().stream())
+              .map(ExternalId::accountId)
+              .collect(toSet())) {
+        accountCache.evict(id);
+      }
+    }
+
+    cacheUpdates.clear();
+    oldRev = null;
+  }
+
+  @Override
+  protected boolean onSave(CommitBuilder commit) throws IOException, ConfigInvalidException {
+    checkState(!readOnly, "Updating external IDs is disabled");
+
+    if (noteMapUpdates.isEmpty()) {
+      return false;
+    }
+
+    if (Strings.isNullOrEmpty(commit.getMessage())) {
+      commit.setMessage("Update external IDs\n");
+    }
+
+    try (RevWalk rw = new RevWalk(repo)) {
+      for (NoteMapUpdate noteMapUpdate : noteMapUpdates) {
+        try {
+          noteMapUpdate.execute(rw, noteMap);
+        } catch (DuplicateExternalIdKeyException e) {
+          throw new IOException(e);
+        }
+      }
+      noteMapUpdates.clear();
+
+      RevTree oldTree = revision != null ? rw.parseTree(revision) : null;
+      ObjectId newTreeId = noteMap.writeTree(inserter);
+      if (newTreeId.equals(oldTree)) {
+        return false;
+      }
+
+      commit.setTreeId(newTreeId);
+      return true;
+    }
+  }
+
+  /**
+   * Checks that all specified external IDs belong to the same account.
+   *
+   * @return the ID of the account to which all specified external IDs belong.
+   */
+  private static Account.Id checkSameAccount(Iterable<ExternalId> extIds) {
+    return checkSameAccount(extIds, null);
+  }
+
+  /**
+   * Checks that all specified external IDs belong to specified account. If no account is specified
+   * it is checked that all specified external IDs belong to the same account.
+   *
+   * @return the ID of the account to which all specified external IDs belong.
+   */
+  public static Account.Id checkSameAccount(
+      Iterable<ExternalId> extIds, @Nullable Account.Id accountId) {
+    for (ExternalId extId : extIds) {
+      if (accountId == null) {
+        accountId = extId.accountId();
+        continue;
+      }
+      checkState(
+          accountId.equals(extId.accountId()),
+          "external id %s belongs to account %s, expected account %s",
+          extId.key().get(),
+          extId.accountId().get(),
+          accountId.get());
+    }
+    return accountId;
+  }
+
+  /**
+   * Insert or updates an new external ID and sets it in the note map.
+   *
+   * <p>If the external ID already exists it is overwritten.
+   */
+  private static ExternalId upsert(
+      RevWalk rw, ObjectInserter ins, NoteMap noteMap, ExternalId extId)
+      throws IOException, ConfigInvalidException {
+    ObjectId noteId = extId.key().sha1();
+    Config c = new Config();
+    if (noteMap.contains(extId.key().sha1())) {
+      byte[] raw = readNoteData(rw, noteMap.get(noteId));
+      try {
+        c = new BlobBasedConfig(null, raw);
+      } catch (ConfigInvalidException e) {
+        throw new ConfigInvalidException(
+            String.format("Invalid external id config for note %s: %s", noteId, e.getMessage()));
+      }
+    }
+    extId.writeToConfig(c);
+    byte[] raw = c.toText().getBytes(UTF_8);
+    ObjectId noteData = ins.insert(OBJ_BLOB, raw);
+    noteMap.set(noteId, noteData);
+    return ExternalId.create(extId, noteData);
+  }
+
+  /**
+   * Removes an external ID from the note map.
+   *
+   * @throws IllegalStateException is thrown if there is an existing external ID that has the same
+   *     key, but otherwise doesn't match the specified external ID.
+   */
+  private static ExternalId remove(RevWalk rw, NoteMap noteMap, ExternalId extId)
+      throws IOException, ConfigInvalidException {
+    ObjectId noteId = extId.key().sha1();
+    if (!noteMap.contains(noteId)) {
+      return null;
+    }
+
+    ObjectId noteDataId = noteMap.get(noteId);
+    byte[] raw = readNoteData(rw, noteDataId);
+    ExternalId actualExtId = ExternalId.parse(noteId.name(), raw, noteDataId);
+    checkState(
+        extId.equals(actualExtId),
+        "external id %s should be removed, but it's not matching the actual external id %s",
+        extId.toString(),
+        actualExtId.toString());
+    noteMap.remove(noteId);
+    return actualExtId;
+  }
+
+  /**
+   * Removes an external ID from the note map by external ID key.
+   *
+   * @throws IllegalStateException is thrown if an expected account ID is provided and an external
+   *     ID with the specified key exists, but belongs to another account.
+   * @return the external ID that was removed, {@code null} if no external ID with the specified key
+   *     exists
+   */
+  private static ExternalId remove(
+      RevWalk rw, NoteMap noteMap, ExternalId.Key extIdKey, Account.Id expectedAccountId)
+      throws IOException, ConfigInvalidException {
+    ObjectId noteId = extIdKey.sha1();
+    if (!noteMap.contains(noteId)) {
+      return null;
+    }
+
+    ObjectId noteDataId = noteMap.get(noteId);
+    byte[] raw = readNoteData(rw, noteDataId);
+    ExternalId extId = ExternalId.parse(noteId.name(), raw, noteDataId);
+    if (expectedAccountId != null) {
+      checkState(
+          expectedAccountId.equals(extId.accountId()),
+          "external id %s should be removed for account %s,"
+              + " but external id belongs to account %s",
+          extIdKey.get(),
+          expectedAccountId.get(),
+          extId.accountId().get());
+    }
+    noteMap.remove(noteId);
+    return extId;
+  }
+
+  private void checkExternalIdsDontExist(Collection<ExternalId> extIds)
+      throws DuplicateExternalIdKeyException, IOException {
+    checkExternalIdKeysDontExist(ExternalId.Key.from(extIds));
+  }
+
+  private void checkExternalIdKeysDontExist(
+      Collection<ExternalId.Key> extIdKeysToAdd, Collection<ExternalId.Key> extIdKeysToDelete)
+      throws DuplicateExternalIdKeyException, IOException {
+    HashSet<ExternalId.Key> newKeys = new HashSet<>(extIdKeysToAdd);
+    newKeys.removeAll(extIdKeysToDelete);
+    checkExternalIdKeysDontExist(newKeys);
+  }
+
+  private void checkExternalIdKeysDontExist(Collection<ExternalId.Key> extIdKeys)
+      throws IOException, DuplicateExternalIdKeyException {
+    for (ExternalId.Key extIdKey : extIdKeys) {
+      if (noteMap.contains(extIdKey.sha1())) {
+        throw new DuplicateExternalIdKeyException(extIdKey);
+      }
+    }
+  }
+
+  private void checkLoaded() {
+    checkState(noteMap != null, "External IDs not loaded yet");
+  }
+
+  @FunctionalInterface
+  private interface NoteMapUpdate {
+    void execute(RevWalk rw, NoteMap noteMap)
+        throws IOException, ConfigInvalidException, DuplicateExternalIdKeyException;
+  }
+
+  @FunctionalInterface
+  private interface CacheUpdate {
+    void execute(ExternalIdCacheUpdates cacheUpdates) throws IOException;
+  }
+
+  private static class ExternalIdCacheUpdates {
+    private final Set<ExternalId> added = new HashSet<>();
+    private final Set<ExternalId> removed = new HashSet<>();
+
+    ExternalIdCacheUpdates add(Collection<ExternalId> extIds) {
+      this.added.addAll(extIds);
+      return this;
+    }
+
+    public Set<ExternalId> getAdded() {
+      return ImmutableSet.copyOf(added);
+    }
+
+    ExternalIdCacheUpdates remove(Collection<ExternalId> extIds) {
+      this.removed.addAll(extIds);
+      return this;
+    }
+
+    public Set<ExternalId> getRemoved() {
+      return ImmutableSet.copyOf(removed);
+    }
+  }
+}

java/com/google/gerrit/server/account/externalids/ExternalIdReader.java

@@ -14,10 +14,7 @@
 
 package com.google.gerrit.server.account.externalids;
 
-import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
-
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableSet;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.metrics.Description;
 import com.google.gerrit.metrics.Description.Units;
@@ -29,17 +26,13 @@ import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 import java.io.IOException;
-import java.util.HashSet;
 import java.util.Set;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.notes.Note;
 import org.eclipse.jgit.notes.NoteMap;
 import org.eclipse.jgit.revwalk.RevWalk;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  * Class to read external IDs from NoteDb.
@@ -58,10 +51,6 @@ import org.slf4j.LoggerFactory;
  */
 @Singleton
 public class ExternalIdReader {
-  private static final Logger log = LoggerFactory.getLogger(ExternalIdReader.class);
-
-  public static final int MAX_NOTE_SZ = 1 << 19;
-
   public static ObjectId readRevision(Repository repo) throws IOException {
     Ref ref = repo.exactRef(RefNames.REFS_EXTERNAL_IDS);
     return ref != null ? ref.getObjectId() : ObjectId.zeroId();
@@ -104,11 +93,12 @@ public class ExternalIdReader {
   }
 
   /** Reads and returns all external IDs. */
-  Set<ExternalId> all() throws IOException {
+  Set<ExternalId> all() throws IOException, ConfigInvalidException {
     checkReadEnabled();
 
-    try (Repository repo = repoManager.openRepository(allUsersName)) {
+    try (Timer0.Context ctx = readAllLatency.start();
-      return all(repo, readRevision(repo));
+        Repository repo = repoManager.openRepository(allUsersName)) {
+      return ExternalIdNotes.loadReadOnly(repo).all();
     }
   }
 
@@ -116,34 +106,12 @@ public class ExternalIdReader {
    * Reads and returns all external IDs from the specified revision of the refs/meta/external-ids
    * branch.
    */
-  Set<ExternalId> all(ObjectId rev) throws IOException {
+  Set<ExternalId> all(ObjectId rev) throws IOException, ConfigInvalidException {
     checkReadEnabled();
 
-    try (Repository repo = repoManager.openRepository(allUsersName)) {
-      return all(repo, rev);
-    }
-  }
-
-  /** Reads and returns all external IDs. */
-  private Set<ExternalId> all(Repository repo, ObjectId rev) throws IOException {
-    if (rev.equals(ObjectId.zeroId())) {
-      return ImmutableSet.of();
-    }
-
     try (Timer0.Context ctx = readAllLatency.start();
-        RevWalk rw = new RevWalk(repo)) {
+        Repository repo = repoManager.openRepository(allUsersName)) {
-      NoteMap noteMap = readNoteMap(rw, rev);
+      return ExternalIdNotes.loadReadOnly(repo, rev).all();
-      Set<ExternalId> extIds = new HashSet<>();
-      for (Note note : noteMap) {
-        byte[] raw =
-            rw.getObjectReader().open(note.getData(), OBJ_BLOB).getCachedBytes(MAX_NOTE_SZ);
-        try {
-          extIds.add(ExternalId.parse(note.getName(), raw, note.getData()));
-        } catch (Exception e) {
-          log.error(String.format("Ignoring invalid external ID note %s", note.getName()), e);
-        }
-      }
-      return extIds;
     }
   }
 
@@ -152,14 +120,8 @@ public class ExternalIdReader {
   ExternalId get(ExternalId.Key key) throws IOException, ConfigInvalidException {
     checkReadEnabled();
 
-    try (Repository repo = repoManager.openRepository(allUsersName);
+    try (Repository repo = repoManager.openRepository(allUsersName)) {
-        RevWalk rw = new RevWalk(repo)) {
+      return ExternalIdNotes.loadReadOnly(repo).get(key).orElse(null);
-      ObjectId rev = readRevision(repo);
-      if (rev.equals(ObjectId.zeroId())) {
-        return null;
-      }
-
-      return parse(key, rw, rev);
     }
   }
 
@@ -168,27 +130,9 @@ public class ExternalIdReader {
   ExternalId get(ExternalId.Key key, ObjectId rev) throws IOException, ConfigInvalidException {
     checkReadEnabled();
 
-    if (rev.equals(ObjectId.zeroId())) {
+    try (Repository repo = repoManager.openRepository(allUsersName)) {
-      return null;
+      return ExternalIdNotes.loadReadOnly(repo, rev).get(key).orElse(null);
     }
-
-    try (Repository repo = repoManager.openRepository(allUsersName);
-        RevWalk rw = new RevWalk(repo)) {
-      return parse(key, rw, rev);
-    }
-  }
-
-  private static ExternalId parse(ExternalId.Key key, RevWalk rw, ObjectId rev)
-      throws IOException, ConfigInvalidException {
-    NoteMap noteMap = readNoteMap(rw, rev);
-    ObjectId noteId = key.sha1();
-    if (!noteMap.contains(noteId)) {
-      return null;
-    }
-
-    ObjectId noteData = noteMap.get(noteId);
-    byte[] raw = rw.getObjectReader().open(noteData, OBJ_BLOB).getCachedBytes(MAX_NOTE_SZ);
-    return ExternalId.parse(noteId.name(), raw, noteData);
   }
 
   private void checkReadEnabled() throws IOException {

java/com/google/gerrit/server/account/externalids/ExternalIds.java

@@ -43,12 +43,12 @@ public class ExternalIds {
   }
 
   /** Returns all external IDs. */
-  public Set<ExternalId> all() throws IOException {
+  public Set<ExternalId> all() throws IOException, ConfigInvalidException {
     return externalIdReader.all();
   }
 
   /** Returns all external IDs from the specified revision of the refs/meta/external-ids branch. */
-  public Set<ExternalId> all(ObjectId rev) throws IOException {
+  public Set<ExternalId> all(ObjectId rev) throws IOException, ConfigInvalidException {
     return externalIdReader.all(rev);
   }
 

java/com/google/gerrit/server/account/externalids/ExternalIdsBatchUpdate.java

@@ -1,129 +0,0 @@
-// Copyright (C) 2016 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server.account.externalids;
-
-import com.google.common.collect.ImmutableSet;
-import com.google.gerrit.server.GerritPersonIdent;
-import com.google.gerrit.server.config.AllUsersName;
-import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gwtorm.server.OrmException;
-import com.google.inject.Inject;
-import java.io.IOException;
-import java.util.HashSet;
-import java.util.Set;
-import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectInserter;
-import org.eclipse.jgit.lib.PersonIdent;
-import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.notes.NoteMap;
-import org.eclipse.jgit.revwalk.RevWalk;
-
-/**
- * This class allows to do batch updates to external IDs.
- *
- * <p>For NoteDb all updates will result in a single commit to the refs/meta/external-ids branch.
- * This means callers can prepare many updates by invoking {@link #replace(ExternalId, ExternalId)}
- * multiple times and when {@link ExternalIdsBatchUpdate#commit(String)} is invoked a single NoteDb
- * commit is created that contains all the prepared updates.
- */
-public class ExternalIdsBatchUpdate {
-  private final GitRepositoryManager repoManager;
-  private final GitReferenceUpdated gitRefUpdated;
-  private final AllUsersName allUsersName;
-  private final PersonIdent serverIdent;
-  private final ExternalIdCache externalIdCache;
-  private final Set<ExternalId> toAdd = new HashSet<>();
-  private final Set<ExternalId> toDelete = new HashSet<>();
-
-  @Inject
-  public ExternalIdsBatchUpdate(
-      GitRepositoryManager repoManager,
-      GitReferenceUpdated gitRefUpdated,
-      AllUsersName allUsersName,
-      @GerritPersonIdent PersonIdent serverIdent,
-      ExternalIdCache externalIdCache) {
-    this.repoManager = repoManager;
-    this.gitRefUpdated = gitRefUpdated;
-    this.allUsersName = allUsersName;
-    this.serverIdent = serverIdent;
-    this.externalIdCache = externalIdCache;
-  }
-
-  /**
-   * Adds an external ID replacement to the batch.
-   *
-   * <p>The actual replacement is only done when {@link #commit(String)} is invoked.
-   */
-  public void replace(ExternalId extIdToDelete, ExternalId extIdToAdd) {
-    ExternalIdsUpdate.checkSameAccount(ImmutableSet.of(extIdToDelete, extIdToAdd));
-    toAdd.add(extIdToAdd);
-    toDelete.add(extIdToDelete);
-  }
-
-  /**
-   * Commits this batch.
-   *
-   * <p>This means external ID replacements which were prepared by invoking {@link
-   * #replace(ExternalId, ExternalId)} are now executed. Deletion of external IDs is done before
-   * adding the new external IDs. This means if an external ID is specified for deletion and an
-   * external ID with the same key is specified to be added, the old external ID with that key is
-   * deleted first and then the new external ID is added (so the external ID for that key is
-   * replaced).
-   *
-   * <p>For NoteDb a single commit is created that contains all the external ID updates.
-   */
-  public void commit(String commitMessage)
-      throws IOException, OrmException, ConfigInvalidException {
-    if (toDelete.isEmpty() && toAdd.isEmpty()) {
-      return;
-    }
-
-    try (Repository repo = repoManager.openRepository(allUsersName);
-        RevWalk rw = new RevWalk(repo);
-        ObjectInserter ins = repo.newObjectInserter()) {
-      ObjectId rev = ExternalIdReader.readRevision(repo);
-
-      NoteMap noteMap = ExternalIdReader.readNoteMap(rw, rev);
-
-      for (ExternalId extId : toDelete) {
-        ExternalIdsUpdate.remove(rw, noteMap, extId);
-      }
-
-      for (ExternalId extId : toAdd) {
-        ExternalIdsUpdate.insert(rw, ins, noteMap, extId);
-      }
-
-      ObjectId newRev =
-          ExternalIdsUpdate.commit(
-              allUsersName,
-              repo,
-              rw,
-              ins,
-              rev,
-              noteMap,
-              commitMessage,
-              serverIdent,
-              serverIdent,
-              null,
-              gitRefUpdated);
-      externalIdCache.onReplace(rev, newRev, toDelete, toAdd);
-    }
-
-    toAdd.clear();
-    toDelete.clear();
-  }
-}

java/com/google/gerrit/server/account/externalids/ExternalIdsConsistencyChecker.java

@@ -16,7 +16,6 @@ package com.google.gerrit.server.account.externalids;
 
 import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME;
 import static java.util.stream.Collectors.joining;
-import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
 
 import com.google.common.collect.ListMultimap;
 import com.google.common.collect.MultimapBuilder;
@@ -58,31 +57,29 @@ public class ExternalIdsConsistencyChecker {
     this.validator = validator;
   }
 
-  public List<ConsistencyProblemInfo> check() throws IOException {
+  public List<ConsistencyProblemInfo> check() throws IOException, ConfigInvalidException {
     try (Repository repo = repoManager.openRepository(allUsers)) {
-      return check(repo, ExternalIdReader.readRevision(repo));
+      return check(ExternalIdNotes.loadReadOnly(repo));
     }
   }
 
-  public List<ConsistencyProblemInfo> check(ObjectId rev) throws IOException {
+  public List<ConsistencyProblemInfo> check(ObjectId rev)
+      throws IOException, ConfigInvalidException {
     try (Repository repo = repoManager.openRepository(allUsers)) {
-      return check(repo, rev);
+      return check(ExternalIdNotes.loadReadOnly(repo, rev));
     }
   }
 
-  private List<ConsistencyProblemInfo> check(Repository repo, ObjectId commit) throws IOException {
+  private List<ConsistencyProblemInfo> check(ExternalIdNotes extIdNotes) throws IOException {
     List<ConsistencyProblemInfo> problems = new ArrayList<>();
 
     ListMultimap<String, ExternalId.Key> emails =
         MultimapBuilder.hashKeys().arrayListValues().build();
 
-    try (RevWalk rw = new RevWalk(repo)) {
+    try (RevWalk rw = new RevWalk(extIdNotes.getRepository())) {
-      NoteMap noteMap = ExternalIdReader.readNoteMap(rw, commit);
+      NoteMap noteMap = extIdNotes.getNoteMap();
       for (Note note : noteMap) {
-        byte[] raw =
+        byte[] raw = ExternalIdNotes.readNoteData(rw, note.getData());
-            rw.getObjectReader()
-                .open(note.getData(), OBJ_BLOB)
-                .getCachedBytes(ExternalIdReader.MAX_NOTE_SZ);
         try {
           ExternalId extId = ExternalId.parse(note.getName(), raw, note.getData());
           problems.addAll(validateExternalId(extId));

java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java

@@ -15,35 +15,18 @@
 package com.google.gerrit.server.account.externalids;
 
 import static com.google.common.base.Preconditions.checkNotNull;
-import static com.google.common.base.Preconditions.checkState;
-import static com.google.gerrit.server.account.externalids.ExternalIdReader.MAX_NOTE_SZ;
-import static com.google.gerrit.server.account.externalids.ExternalIdReader.readNoteMap;
-import static com.google.gerrit.server.account.externalids.ExternalIdReader.readRevision;
-import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.stream.Collectors.toSet;
-import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
-import static org.eclipse.jgit.lib.Constants.OBJ_TREE;
 
-import com.google.auto.value.AutoValue;
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Streams;
 import com.google.common.util.concurrent.Runnables;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.metrics.Counter0;
 import com.google.gerrit.metrics.Description;
 import com.google.gerrit.metrics.MetricMaker;
 import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.client.RefNames;
-import com.google.gerrit.server.GerritPersonIdent;
-import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.config.AllUsersName;
-import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
 import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.LockFailureException;
+import com.google.gerrit.server.git.MetaDataUpdate;
 import com.google.gerrit.server.update.RetryHelper;
 import com.google.gwtorm.server.OrmDuplicateKeyException;
 import com.google.gwtorm.server.OrmException;
@@ -53,20 +36,8 @@ import com.google.inject.Singleton;
 import java.io.IOException;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashSet;
-import java.util.Set;
-import java.util.stream.Stream;
 import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.CommitBuilder;
-import org.eclipse.jgit.lib.Config;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectInserter;
-import org.eclipse.jgit.lib.PersonIdent;
-import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.notes.NoteMap;
-import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.revwalk.RevWalk;
 
 /**
  * Updates externalIds in ReviewDb and NoteDb.
@@ -89,8 +60,6 @@ import org.eclipse.jgit.revwalk.RevWalk;
  * cache and thus triggers reindex for them.
  */
 public class ExternalIdsUpdate {
-  private static final String COMMIT_MSG = "Update external IDs";
-
   /**
    * Factory to create an ExternalIdsUpdate instance for updating external IDs by the Gerrit server.
    *
@@ -100,50 +69,43 @@ public class ExternalIdsUpdate {
   @Singleton
   public static class Server {
     private final GitRepositoryManager repoManager;
+    private final Provider<MetaDataUpdate.Server> metaDataUpdateServerFactory;
     private final AccountCache accountCache;
     private final AllUsersName allUsersName;
     private final MetricMaker metricMaker;
     private final ExternalIds externalIds;
     private final ExternalIdCache externalIdCache;
-    private final Provider<PersonIdent> serverIdent;
-    private final GitReferenceUpdated gitRefUpdated;
     private final RetryHelper retryHelper;
 
     @Inject
     public Server(
         GitRepositoryManager repoManager,
+        Provider<MetaDataUpdate.Server> metaDataUpdateServerFactory,
         AccountCache accountCache,
         AllUsersName allUsersName,
         MetricMaker metricMaker,
         ExternalIds externalIds,
         ExternalIdCache externalIdCache,
-        @GerritPersonIdent Provider<PersonIdent> serverIdent,
-        GitReferenceUpdated gitRefUpdated,
         RetryHelper retryHelper) {
       this.repoManager = repoManager;
+      this.metaDataUpdateServerFactory = metaDataUpdateServerFactory;
       this.accountCache = accountCache;
       this.allUsersName = allUsersName;
       this.metricMaker = metricMaker;
       this.externalIds = externalIds;
       this.externalIdCache = externalIdCache;
-      this.serverIdent = serverIdent;
-      this.gitRefUpdated = gitRefUpdated;
       this.retryHelper = retryHelper;
     }
 
     public ExternalIdsUpdate create() {
-      PersonIdent i = serverIdent.get();
       return new ExternalIdsUpdate(
           repoManager,
+          () -> metaDataUpdateServerFactory.get().create(allUsersName),
           accountCache,
           allUsersName,
           metricMaker,
           externalIds,
           externalIdCache,
-          i,
-          i,
-          null,
-          gitRefUpdated,
           retryHelper);
     }
   }
@@ -160,47 +122,40 @@ public class ExternalIdsUpdate {
   @Singleton
   public static class ServerNoReindex {
     private final GitRepositoryManager repoManager;
+    private final Provider<MetaDataUpdate.Server> metaDataUpdateServerFactory;
     private final AllUsersName allUsersName;
     private final MetricMaker metricMaker;
     private final ExternalIds externalIds;
     private final ExternalIdCache externalIdCache;
-    private final Provider<PersonIdent> serverIdent;
-    private final GitReferenceUpdated gitRefUpdated;
     private final RetryHelper retryHelper;
 
     @Inject
     public ServerNoReindex(
         GitRepositoryManager repoManager,
+        Provider<MetaDataUpdate.Server> metaDataUpdateServerFactory,
         AllUsersName allUsersName,
         MetricMaker metricMaker,
         ExternalIds externalIds,
         ExternalIdCache externalIdCache,
-        @GerritPersonIdent Provider<PersonIdent> serverIdent,
-        GitReferenceUpdated gitRefUpdated,
         RetryHelper retryHelper) {
       this.repoManager = repoManager;
+      this.metaDataUpdateServerFactory = metaDataUpdateServerFactory;
       this.allUsersName = allUsersName;
       this.metricMaker = metricMaker;
       this.externalIds = externalIds;
       this.externalIdCache = externalIdCache;
-      this.serverIdent = serverIdent;
-      this.gitRefUpdated = gitRefUpdated;
       this.retryHelper = retryHelper;
     }
 
     public ExternalIdsUpdate create() {
-      PersonIdent i = serverIdent.get();
       return new ExternalIdsUpdate(
           repoManager,
+          () -> metaDataUpdateServerFactory.get().create(allUsersName),
           null,
           allUsersName,
           metricMaker,
           externalIds,
           externalIdCache,
-          i,
-          i,
-          null,
-          gitRefUpdated,
           retryHelper);
     }
   }
@@ -214,98 +169,74 @@ public class ExternalIdsUpdate {
   @Singleton
   public static class User {
     private final GitRepositoryManager repoManager;
+    private final Provider<MetaDataUpdate.User> metaDataUpdateUserFactory;
     private final AccountCache accountCache;
     private final AllUsersName allUsersName;
     private final MetricMaker metricMaker;
     private final ExternalIds externalIds;
     private final ExternalIdCache externalIdCache;
-    private final Provider<PersonIdent> serverIdent;
-    private final Provider<IdentifiedUser> identifiedUser;
-    private final GitReferenceUpdated gitRefUpdated;
     private final RetryHelper retryHelper;
 
     @Inject
     public User(
         GitRepositoryManager repoManager,
+        Provider<MetaDataUpdate.User> metaDataUpdateUserFactory,
         AccountCache accountCache,
         AllUsersName allUsersName,
         MetricMaker metricMaker,
         ExternalIds externalIds,
         ExternalIdCache externalIdCache,
-        @GerritPersonIdent Provider<PersonIdent> serverIdent,
-        Provider<IdentifiedUser> identifiedUser,
-        GitReferenceUpdated gitRefUpdated,
         RetryHelper retryHelper) {
       this.repoManager = repoManager;
+      this.metaDataUpdateUserFactory = metaDataUpdateUserFactory;
       this.accountCache = accountCache;
       this.allUsersName = allUsersName;
       this.metricMaker = metricMaker;
       this.externalIds = externalIds;
       this.externalIdCache = externalIdCache;
-      this.serverIdent = serverIdent;
-      this.identifiedUser = identifiedUser;
-      this.gitRefUpdated = gitRefUpdated;
       this.retryHelper = retryHelper;
     }
 
     public ExternalIdsUpdate create() {
-      IdentifiedUser user = identifiedUser.get();
-      PersonIdent i = serverIdent.get();
       return new ExternalIdsUpdate(
           repoManager,
+          () -> metaDataUpdateUserFactory.get().create(allUsersName),
           accountCache,
           allUsersName,
           metricMaker,
           externalIds,
           externalIdCache,
-          createPersonIdent(i, user),
-          i,
-          user,
-          gitRefUpdated,
           retryHelper);
     }
-
-    private PersonIdent createPersonIdent(PersonIdent ident, IdentifiedUser user) {
-      return user.newCommitterIdent(ident.getWhen(), ident.getTimeZone());
-    }
   }
 
   private final GitRepositoryManager repoManager;
+  private final MetaDataUpdateFactory metaDataUpdateFactory;
   @Nullable private final AccountCache accountCache;
   private final AllUsersName allUsersName;
   private final ExternalIds externalIds;
   private final ExternalIdCache externalIdCache;
-  private final PersonIdent committerIdent;
-  private final PersonIdent authorIdent;
-  @Nullable private final IdentifiedUser currentUser;
-  private final GitReferenceUpdated gitRefUpdated;
   private final RetryHelper retryHelper;
   private final Runnable afterReadRevision;
   private final Counter0 updateCount;
 
   private ExternalIdsUpdate(
       GitRepositoryManager repoManager,
+      MetaDataUpdateFactory metaDataUpdateFactory,
       @Nullable AccountCache accountCache,
       AllUsersName allUsersName,
       MetricMaker metricMaker,
       ExternalIds externalIds,
       ExternalIdCache externalIdCache,
-      PersonIdent committerIdent,
-      PersonIdent authorIdent,
-      @Nullable IdentifiedUser currentUser,
-      GitReferenceUpdated gitRefUpdated,
       RetryHelper retryHelper) {
     this(
         repoManager,
+        metaDataUpdateFactory,
         accountCache,
         allUsersName,
         metricMaker,
         externalIds,
         externalIdCache,
-        committerIdent,
-        authorIdent,
-        currentUser,
-        gitRefUpdated,
         retryHelper,
         Runnables.doNothing());
   }
@@ -313,26 +244,20 @@ public class ExternalIdsUpdate {
   @VisibleForTesting
   public ExternalIdsUpdate(
       GitRepositoryManager repoManager,
+      MetaDataUpdateFactory metaDataUpdateFactory,
       @Nullable AccountCache accountCache,
       AllUsersName allUsersName,
       MetricMaker metricMaker,
       ExternalIds externalIds,
       ExternalIdCache externalIdCache,
-      PersonIdent committerIdent,
-      PersonIdent authorIdent,
-      @Nullable IdentifiedUser currentUser,
-      GitReferenceUpdated gitRefUpdated,
       RetryHelper retryHelper,
       Runnable afterReadRevision) {
     this.repoManager = checkNotNull(repoManager, "repoManager");
+    this.metaDataUpdateFactory = checkNotNull(metaDataUpdateFactory, "metaDataUpdateFactory");
     this.accountCache = accountCache;
     this.allUsersName = checkNotNull(allUsersName, "allUsersName");
-    this.committerIdent = checkNotNull(committerIdent, "committerIdent");
     this.externalIds = checkNotNull(externalIds, "externalIds");
     this.externalIdCache = checkNotNull(externalIdCache, "externalIdCache");
-    this.authorIdent = checkNotNull(authorIdent, "authorIdent");
-    this.currentUser = currentUser;
-    this.gitRefUpdated = checkNotNull(gitRefUpdated, "gitRefUpdated");
     this.retryHelper = checkNotNull(retryHelper, "retryHelper");
     this.afterReadRevision = checkNotNull(afterReadRevision, "afterReadRevision");
     this.updateCount =
@@ -358,18 +283,7 @@ public class ExternalIdsUpdate {
    */
   public void insert(Collection<ExternalId> extIds)
       throws IOException, ConfigInvalidException, OrmException {
-    RefsMetaExternalIdsUpdate u =
+    updateNoteMap(n -> n.insert(extIds));
-        updateNoteMap(
-            o -> {
-              UpdatedExternalIds updatedExtIds = new UpdatedExternalIds();
-              for (ExternalId extId : extIds) {
-                ExternalId insertedExtId = insert(o.rw(), o.ins(), o.noteMap(), extId);
-                updatedExtIds.onUpdate(insertedExtId);
-              }
-              return updatedExtIds;
-            });
-    externalIdCache.onCreate(u.oldRev(), u.newRev(), u.updatedExtIds().getUpdated());
-    evictAccounts(u);
   }
 
   /**
@@ -388,18 +302,7 @@ public class ExternalIdsUpdate {
    */
   public void upsert(Collection<ExternalId> extIds)
       throws IOException, ConfigInvalidException, OrmException {
-    RefsMetaExternalIdsUpdate u =
+    updateNoteMap(n -> n.upsert(extIds));
-        updateNoteMap(
-            o -> {
-              UpdatedExternalIds updatedExtIds = new UpdatedExternalIds();
-              for (ExternalId extId : extIds) {
-                ExternalId updatedExtId = upsert(o.rw(), o.ins(), o.noteMap(), extId);
-                updatedExtIds.onUpdate(updatedExtId);
-              }
-              return updatedExtIds;
-            });
-    externalIdCache.onUpdate(u.oldRev(), u.newRev(), u.updatedExtIds().getUpdated());
-    evictAccounts(u);
   }
 
   /**
@@ -421,18 +324,7 @@ public class ExternalIdsUpdate {
    */
   public void delete(Collection<ExternalId> extIds)
       throws IOException, ConfigInvalidException, OrmException {
-    RefsMetaExternalIdsUpdate u =
+    updateNoteMap(n -> n.delete(extIds));
-        updateNoteMap(
-            o -> {
-              UpdatedExternalIds updatedExtIds = new UpdatedExternalIds();
-              for (ExternalId extId : extIds) {
-                ExternalId removedExtId = remove(o.rw(), o.noteMap(), extId);
-                updatedExtIds.onRemove(removedExtId);
-              }
-              return updatedExtIds;
-            });
-    externalIdCache.onRemove(u.oldRev(), u.newRev(), u.updatedExtIds().getRemoved());
-    evictAccounts(u);
   }
 
   /**
@@ -454,18 +346,7 @@ public class ExternalIdsUpdate {
    */
   public void delete(Account.Id accountId, Collection<ExternalId.Key> extIdKeys)
       throws IOException, ConfigInvalidException, OrmException {
-    RefsMetaExternalIdsUpdate u =
+    updateNoteMap(n -> n.delete(accountId, extIdKeys));
-        updateNoteMap(
-            o -> {
-              UpdatedExternalIds updatedExtIds = new UpdatedExternalIds();
-              for (ExternalId.Key extIdKey : extIdKeys) {
-                ExternalId removedExtId = remove(o.rw(), o.noteMap(), extIdKey, accountId);
-                updatedExtIds.onRemove(removedExtId);
-              }
-              return updatedExtIds;
-            });
-    externalIdCache.onRemove(u.oldRev(), u.newRev(), u.updatedExtIds().getRemoved());
-    evictAccount(accountId);
   }
 
   /**
@@ -475,18 +356,7 @@ public class ExternalIdsUpdate {
    */
   public void deleteByKeys(Collection<ExternalId.Key> extIdKeys)
       throws IOException, ConfigInvalidException, OrmException {
-    RefsMetaExternalIdsUpdate u =
+    updateNoteMap(n -> n.deleteByKeys(extIdKeys));
-        updateNoteMap(
-            o -> {
-              UpdatedExternalIds updatedExtIds = new UpdatedExternalIds();
-              for (ExternalId.Key extIdKey : extIdKeys) {
-                ExternalId extId = remove(o.rw(), o.noteMap(), extIdKey, null);
-                updatedExtIds.onRemove(extId);
-              }
-              return updatedExtIds;
-            });
-    externalIdCache.onRemove(u.oldRev(), u.newRev(), u.updatedExtIds().getRemoved());
-    evictAccounts(u);
   }
 
   /** Deletes all external IDs of the specified account. */
@@ -509,30 +379,7 @@ public class ExternalIdsUpdate {
   public void replace(
       Account.Id accountId, Collection<ExternalId.Key> toDelete, Collection<ExternalId> toAdd)
       throws IOException, ConfigInvalidException, OrmException {
-    checkSameAccount(toAdd, accountId);
+    updateNoteMap(n -> n.replace(accountId, toDelete, toAdd));
-
-    RefsMetaExternalIdsUpdate u =
-        updateNoteMap(
-            o -> {
-              UpdatedExternalIds updatedExtIds = new UpdatedExternalIds();
-              for (ExternalId.Key extIdKey : toDelete) {
-                ExternalId removedExtId = remove(o.rw(), o.noteMap(), extIdKey, accountId);
-                updatedExtIds.onRemove(removedExtId);
-              }
-
-              for (ExternalId extId : toAdd) {
-                ExternalId insertedExtId = insert(o.rw(), o.ins(), o.noteMap(), extId);
-                updatedExtIds.onUpdate(insertedExtId);
-              }
-              return updatedExtIds;
-            });
-    externalIdCache.onReplace(
-        u.oldRev(),
-        u.newRev(),
-        accountId,
-        u.updatedExtIds().getRemoved(),
-        u.updatedExtIds().getUpdated());
-    evictAccount(accountId);
   }
 
   /**
@@ -547,24 +394,7 @@ public class ExternalIdsUpdate {
    */
   public void replaceByKeys(Collection<ExternalId.Key> toDelete, Collection<ExternalId> toAdd)
       throws IOException, ConfigInvalidException, OrmException {
-    RefsMetaExternalIdsUpdate u =
+    updateNoteMap(n -> n.replaceByKeys(toDelete, toAdd));
-        updateNoteMap(
-            o -> {
-              UpdatedExternalIds updatedExtIds = new UpdatedExternalIds();
-              for (ExternalId.Key extIdKey : toDelete) {
-                ExternalId removedExtId = remove(o.rw(), o.noteMap(), extIdKey, null);
-                updatedExtIds.onRemove(removedExtId);
-              }
-
-              for (ExternalId extId : toAdd) {
-                ExternalId insertedExtId = insert(o.rw(), o.ins(), o.noteMap(), extId);
-                updatedExtIds.onUpdate(insertedExtId);
-              }
-              return updatedExtIds;
-            });
-    externalIdCache.onReplace(
-        u.oldRev(), u.newRev(), u.updatedExtIds().getRemoved(), u.updatedExtIds().getUpdated());
-    evictAccounts(u);
   }
 
   /**
@@ -591,334 +421,38 @@ public class ExternalIdsUpdate {
    */
   public void replace(Collection<ExternalId> toDelete, Collection<ExternalId> toAdd)
       throws IOException, ConfigInvalidException, OrmException {
-    Account.Id accountId = checkSameAccount(Iterables.concat(toDelete, toAdd));
+    updateNoteMap(n -> n.replace(toDelete, toAdd));
-    if (accountId == null) {
-      // toDelete and toAdd are empty -> nothing to do
-      return;
   }
 
-    replace(accountId, toDelete.stream().map(e -> e.key()).collect(toSet()), toAdd);
+  private void updateNoteMap(ExternalIdUpdater updater)
-  }
-
-  /**
-   * Checks that all specified external IDs belong to the same account.
-   *
-   * @return the ID of the account to which all specified external IDs belong.
-   */
-  public static Account.Id checkSameAccount(Iterable<ExternalId> extIds) {
-    return checkSameAccount(extIds, null);
-  }
-
-  /**
-   * Checks that all specified external IDs belong to specified account. If no account is specified
-   * it is checked that all specified external IDs belong to the same account.
-   *
-   * @return the ID of the account to which all specified external IDs belong.
-   */
-  public static Account.Id checkSameAccount(
-      Iterable<ExternalId> extIds, @Nullable Account.Id accountId) {
-    for (ExternalId extId : extIds) {
-      if (accountId == null) {
-        accountId = extId.accountId();
-        continue;
-      }
-      checkState(
-          accountId.equals(extId.accountId()),
-          "external id %s belongs to account %s, expected account %s",
-          extId.key().get(),
-          extId.accountId().get(),
-          accountId.get());
-    }
-    return accountId;
-  }
-
-  /**
-   * Inserts a new external ID and sets it in the note map.
-   *
-   * <p>If the external ID already exists, the insert fails with {@link OrmDuplicateKeyException}.
-   */
-  public static ExternalId insert(RevWalk rw, ObjectInserter ins, NoteMap noteMap, ExternalId extId)
-      throws OrmDuplicateKeyException, ConfigInvalidException, IOException {
-    if (noteMap.contains(extId.key().sha1())) {
-      throw new OrmDuplicateKeyException(
-          String.format("external id %s already exists", extId.key().get()));
-    }
-    return upsert(rw, ins, noteMap, extId);
-  }
-
-  /**
-   * Insert or updates an new external ID and sets it in the note map.
-   *
-   * <p>If the external ID already exists it is overwritten.
-   */
-  public static ExternalId upsert(RevWalk rw, ObjectInserter ins, NoteMap noteMap, ExternalId extId)
-      throws IOException, ConfigInvalidException {
-    ObjectId noteId = extId.key().sha1();
-    Config c = new Config();
-    if (noteMap.contains(extId.key().sha1())) {
-      byte[] raw =
-          rw.getObjectReader().open(noteMap.get(noteId), OBJ_BLOB).getCachedBytes(MAX_NOTE_SZ);
-      try {
-        c.fromText(new String(raw, UTF_8));
-      } catch (ConfigInvalidException e) {
-        throw new ConfigInvalidException(
-            String.format("Invalid external id config for note %s: %s", noteId, e.getMessage()));
-      }
-    }
-    extId.writeToConfig(c);
-    byte[] raw = c.toText().getBytes(UTF_8);
-    ObjectId noteData = ins.insert(OBJ_BLOB, raw);
-    noteMap.set(noteId, noteData);
-    return ExternalId.create(extId, noteData);
-  }
-
-  /**
-   * Removes an external ID from the note map.
-   *
-   * @throws IllegalStateException is thrown if there is an existing external ID that has the same
-   *     key, but otherwise doesn't match the specified external ID.
-   */
-  public static ExternalId remove(RevWalk rw, NoteMap noteMap, ExternalId extId)
-      throws IOException, ConfigInvalidException {
-    ObjectId noteId = extId.key().sha1();
-    if (!noteMap.contains(noteId)) {
-      return null;
-    }
-
-    ObjectId noteData = noteMap.get(noteId);
-    byte[] raw = rw.getObjectReader().open(noteData, OBJ_BLOB).getCachedBytes(MAX_NOTE_SZ);
-    ExternalId actualExtId = ExternalId.parse(noteId.name(), raw, noteData);
-    checkState(
-        extId.equals(actualExtId),
-        "external id %s should be removed, but it's not matching the actual external id %s",
-        extId.toString(),
-        actualExtId.toString());
-    noteMap.remove(noteId);
-    return actualExtId;
-  }
-
-  /**
-   * Removes an external ID from the note map by external ID key.
-   *
-   * @throws IllegalStateException is thrown if an expected account ID is provided and an external
-   *     ID with the specified key exists, but belongs to another account.
-   * @return the external ID that was removed, {@code null} if no external ID with the specified key
-   *     exists
-   */
-  private static ExternalId remove(
-      RevWalk rw, NoteMap noteMap, ExternalId.Key extIdKey, Account.Id expectedAccountId)
-      throws IOException, ConfigInvalidException {
-    ObjectId noteId = extIdKey.sha1();
-    if (!noteMap.contains(noteId)) {
-      return null;
-    }
-
-    ObjectId noteData = noteMap.get(noteId);
-    byte[] raw = rw.getObjectReader().open(noteData, OBJ_BLOB).getCachedBytes(MAX_NOTE_SZ);
-    ExternalId extId = ExternalId.parse(noteId.name(), raw, noteData);
-    if (expectedAccountId != null) {
-      checkState(
-          expectedAccountId.equals(extId.accountId()),
-          "external id %s should be removed for account %s,"
-              + " but external id belongs to account %s",
-          extIdKey.get(),
-          expectedAccountId.get(),
-          extId.accountId().get());
-    }
-    noteMap.remove(noteId);
-    return extId;
-  }
-
-  private RefsMetaExternalIdsUpdate updateNoteMap(ExternalIdUpdater updater)
       throws IOException, ConfigInvalidException, OrmException {
-    return retryHelper.execute(
+    retryHelper.execute(
         () -> {
-          try (Repository repo = repoManager.openRepository(allUsersName);
+          try (Repository repo = repoManager.openRepository(allUsersName)) {
-              ObjectInserter ins = repo.newObjectInserter()) {
+            ExternalIdNotes extIdNotes =
-            ObjectId rev = readRevision(repo);
+                new ExternalIdNotes(externalIdCache, accountCache, repo)
-
+                    .setAfterReadRevision(afterReadRevision)
-            afterReadRevision.run();
+                    .load();
-
+            updater.update(extIdNotes);
-            try (RevWalk rw = new RevWalk(repo)) {
+            try (MetaDataUpdate metaDataUpdate = metaDataUpdateFactory.create()) {
-              NoteMap noteMap = readNoteMap(rw, rev);
+              extIdNotes.commit(metaDataUpdate);
-              UpdatedExternalIds updatedExtIds =
-                  updater.update(OpenRepo.create(repo, rw, ins, noteMap));
-
-              return commit(repo, rw, ins, rev, noteMap, updatedExtIds);
             }
+            extIdNotes.updateCaches();
+            updateCount.increment();
+            return null;
           }
         });
   }
 
-  private RefsMetaExternalIdsUpdate commit(
-      Repository repo,
-      RevWalk rw,
-      ObjectInserter ins,
-      ObjectId rev,
-      NoteMap noteMap,
-      UpdatedExternalIds updatedExtIds)
-      throws IOException {
-    ObjectId newRev =
-        commit(
-            allUsersName,
-            repo,
-            rw,
-            ins,
-            rev,
-            noteMap,
-            COMMIT_MSG,
-            committerIdent,
-            authorIdent,
-            currentUser,
-            gitRefUpdated);
-    updateCount.increment();
-    return RefsMetaExternalIdsUpdate.create(rev, newRev, updatedExtIds);
-  }
-
-  /** Commits updates to the external IDs. */
-  public static ObjectId commit(
-      Project.NameKey project,
-      Repository repo,
-      RevWalk rw,
-      ObjectInserter ins,
-      ObjectId rev,
-      NoteMap noteMap,
-      String commitMessage,
-      PersonIdent committerIdent,
-      PersonIdent authorIdent,
-      @Nullable IdentifiedUser user,
-      GitReferenceUpdated gitRefUpdated)
-      throws IOException {
-    CommitBuilder cb = new CommitBuilder();
-    cb.setMessage(commitMessage);
-    cb.setTreeId(noteMap.writeTree(ins));
-    cb.setAuthor(authorIdent);
-    cb.setCommitter(committerIdent);
-    if (!rev.equals(ObjectId.zeroId())) {
-      cb.setParentId(rev);
-    } else {
-      cb.setParentIds(); // Ref is currently nonexistent, commit has no parents.
-    }
-    if (cb.getTreeId() == null) {
-      if (rev.equals(ObjectId.zeroId())) {
-        cb.setTreeId(emptyTree(ins)); // No parent, assume empty tree.
-      } else {
-        RevCommit p = rw.parseCommit(rev);
-        cb.setTreeId(p.getTree()); // Copy tree from parent.
-      }
-    }
-    ObjectId commitId = ins.insert(cb);
-    ins.flush();
-
-    RefUpdate u = repo.updateRef(RefNames.REFS_EXTERNAL_IDS);
-    u.setRefLogIdent(committerIdent);
-    u.setRefLogMessage("Update external IDs", false);
-    u.setExpectedOldObjectId(rev);
-    u.setNewObjectId(commitId);
-    RefUpdate.Result res = u.update();
-    switch (res) {
-      case NEW:
-      case FAST_FORWARD:
-      case NO_CHANGE:
-      case RENAMED:
-      case FORCED:
-        break;
-      case LOCK_FAILURE:
-        throw new LockFailureException("Updating external IDs failed with " + res, u);
-      case IO_FAILURE:
-      case NOT_ATTEMPTED:
-      case REJECTED:
-      case REJECTED_CURRENT_BRANCH:
-      case REJECTED_MISSING_OBJECT:
-      case REJECTED_OTHER_REASON:
-      default:
-        throw new IOException("Updating external IDs failed with " + res);
-    }
-    gitRefUpdated.fire(project, u, user != null ? user.getAccount() : null);
-    return rw.parseCommit(commitId);
-  }
-
-  private static ObjectId emptyTree(ObjectInserter ins) throws IOException {
-    return ins.insert(OBJ_TREE, new byte[] {});
-  }
-
-  private void evictAccount(Account.Id accountId) throws IOException {
-    if (accountCache != null) {
-      accountCache.evict(accountId);
-    }
-  }
-
-  private void evictAccounts(RefsMetaExternalIdsUpdate u) throws IOException {
-    if (accountCache != null) {
-      for (Account.Id id : u.updatedExtIds().all().map(ExternalId::accountId).collect(toSet())) {
-        accountCache.evict(id);
-      }
-    }
-  }
-
   @FunctionalInterface
   private static interface ExternalIdUpdater {
-    UpdatedExternalIds update(OpenRepo openRepo)
+    void update(ExternalIdNotes extIdsNotes)
         throws IOException, ConfigInvalidException, OrmException;
   }
 
-  @AutoValue
-  abstract static class OpenRepo {
-    static OpenRepo create(Repository repo, RevWalk rw, ObjectInserter ins, NoteMap noteMap) {
-      return new AutoValue_ExternalIdsUpdate_OpenRepo(repo, rw, ins, noteMap);
-    }
-
-    abstract Repository repo();
-
-    abstract RevWalk rw();
-
-    abstract ObjectInserter ins();
-
-    abstract NoteMap noteMap();
-  }
-
   @VisibleForTesting
-  @AutoValue
+  @FunctionalInterface
-  public abstract static class RefsMetaExternalIdsUpdate {
+  public static interface MetaDataUpdateFactory {
-    static RefsMetaExternalIdsUpdate create(
+    MetaDataUpdate create() throws IOException;
-        ObjectId oldRev, ObjectId newRev, UpdatedExternalIds updatedExtIds) {
-      return new AutoValue_ExternalIdsUpdate_RefsMetaExternalIdsUpdate(
-          oldRev, newRev, updatedExtIds);
-    }
-
-    abstract ObjectId oldRev();
-
-    abstract ObjectId newRev();
-
-    abstract UpdatedExternalIds updatedExtIds();
-  }
-
-  public static class UpdatedExternalIds {
-    private Set<ExternalId> updated = new HashSet<>();
-    private Set<ExternalId> removed = new HashSet<>();
-
-    public void onUpdate(ExternalId extId) {
-      if (extId != null) {
-        updated.add(extId);
-      }
-    }
-
-    public void onRemove(ExternalId extId) {
-      if (extId != null) {
-        removed.add(extId);
-      }
-    }
-
-    public Set<ExternalId> getUpdated() {
-      return ImmutableSet.copyOf(updated);
-    }
-
-    public Set<ExternalId> getRemoved() {
-      return ImmutableSet.copyOf(removed);
-    }
-
-    public Stream<ExternalId> all() {
-      return Streams.concat(removed.stream(), updated.stream());
-    }
   }
 }

java/com/google/gerrit/server/git/receive/ReceiveCommits.java

@@ -2932,16 +2932,17 @@ class ReceiveCommits {
           accountsUpdate
               .create()
               .update(
+                  "Set Full Name on Receive Commits",
                   user.getAccountId(),
-                  a -> {
+                  (a, u) -> {
                     if (Strings.isNullOrEmpty(a.getFullName())) {
-                      a.setFullName(setFullNameTo);
+                      u.setFullName(setFullNameTo);
                     }
                   });
       if (account != null) {
         user.getAccount().setFullName(account.getFullName());
       }
-    } catch (IOException | ConfigInvalidException e) {
+    } catch (OrmException | IOException | ConfigInvalidException e) {
       logWarn("Failed to update full name of caller", e);
     }
   }

java/com/google/gerrit/server/git/validators/CommitValidators.java

@@ -718,7 +718,7 @@ public class CommitValidators {
             throw new CommitValidationException("invalid external IDs", msgs);
           }
           return msgs;
-        } catch (IOException e) {
+        } catch (IOException | ConfigInvalidException e) {
           String m = "error validating external IDs";
           log.warn(m, e);
           throw new CommitValidationException(m, e);

java/com/google/gerrit/server/schema/Schema_144.java

@@ -18,11 +18,11 @@ import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.account.externalids.ExternalId;
-import com.google.gerrit.server.account.externalids.ExternalIdReader;
+import com.google.gerrit.server.account.externalids.ExternalIdNotes;
-import com.google.gerrit.server.account.externalids.ExternalIdsUpdate;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
 import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.MetaDataUpdate;
 import com.google.gwtorm.jdbc.JdbcSchema;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -34,12 +34,8 @@ import java.sql.Statement;
 import java.util.HashSet;
 import java.util.Set;
 import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.notes.NoteMap;
-import org.eclipse.jgit.revwalk.RevWalk;
 
 public class Schema_144 extends SchemaVersion {
   private static final String COMMIT_MSG = "Import external IDs from ReviewDb";
@@ -83,29 +79,16 @@ public class Schema_144 extends SchemaVersion {
     }
 
     try {
-      try (Repository repo = repoManager.openRepository(allUsersName);
+      try (Repository repo = repoManager.openRepository(allUsersName)) {
-          RevWalk rw = new RevWalk(repo);
+        ExternalIdNotes extIdNotes = ExternalIdNotes.loadNoCacheUpdate(repo);
-          ObjectInserter ins = repo.newObjectInserter()) {
+        extIdNotes.upsert(toAdd);
-        ObjectId rev = ExternalIdReader.readRevision(repo);
+        try (MetaDataUpdate metaDataUpdate =
-
+            new MetaDataUpdate(GitReferenceUpdated.DISABLED, allUsersName, repo)) {
-        NoteMap noteMap = ExternalIdReader.readNoteMap(rw, rev);
+          metaDataUpdate.getCommitBuilder().setAuthor(serverIdent);
-
+          metaDataUpdate.getCommitBuilder().setCommitter(serverIdent);
-        for (ExternalId extId : toAdd) {
+          metaDataUpdate.getCommitBuilder().setMessage(COMMIT_MSG);
-          ExternalIdsUpdate.upsert(rw, ins, noteMap, extId);
+          extIdNotes.commit(metaDataUpdate);
         }
-
-        ExternalIdsUpdate.commit(
-            allUsersName,
-            repo,
-            rw,
-            ins,
-            rev,
-            noteMap,
-            COMMIT_MSG,
-            serverIdent,
-            serverIdent,
-            null,
-            GitReferenceUpdated.DISABLED);
       }
     } catch (IOException | ConfigInvalidException e) {
       throw new OrmException("Failed to migrate external IDs to NoteDb", e);

java/com/google/gerrit/server/schema/Schema_148.java

@@ -14,17 +14,15 @@
 
 package com.google.gerrit.server.schema;
 
-import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
-
 import com.google.common.primitives.Ints;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.account.externalids.ExternalId;
-import com.google.gerrit.server.account.externalids.ExternalIdReader;
+import com.google.gerrit.server.account.externalids.ExternalIdNotes;
-import com.google.gerrit.server.account.externalids.ExternalIdsUpdate;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
 import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.MetaDataUpdate;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -32,13 +30,8 @@ import java.io.IOException;
 import java.sql.SQLException;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.Config;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.notes.Note;
-import org.eclipse.jgit.notes.NoteMap;
-import org.eclipse.jgit.revwalk.RevWalk;
 
 public class Schema_148 extends SchemaVersion {
   private static final String COMMIT_MSG = "Make account IDs of external IDs human-readable";
@@ -61,44 +54,22 @@ public class Schema_148 extends SchemaVersion {
 
   @Override
   protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException, SQLException {
-    try (Repository repo = repoManager.openRepository(allUsersName);
+    try (Repository repo = repoManager.openRepository(allUsersName)) {
-        RevWalk rw = new RevWalk(repo);
+      ExternalIdNotes extIdNotes = ExternalIdNotes.loadNoCacheUpdate(repo);
-        ObjectInserter ins = repo.newObjectInserter()) {
+      for (ExternalId extId : extIdNotes.all()) {
-      ObjectId rev = ExternalIdReader.readRevision(repo);
-      NoteMap noteMap = ExternalIdReader.readNoteMap(rw, rev);
-      boolean dirty = false;
-      for (Note note : noteMap) {
-        byte[] raw =
-            rw.getObjectReader()
-                .open(note.getData(), OBJ_BLOB)
-                .getCachedBytes(ExternalIdReader.MAX_NOTE_SZ);
-        try {
-          ExternalId extId = ExternalId.parse(note.getName(), raw, note.getData());
-
         if (needsUpdate(extId)) {
-            ExternalIdsUpdate.upsert(rw, ins, noteMap, extId);
+          extIdNotes.upsert(extId);
-            dirty = true;
-          }
-        } catch (ConfigInvalidException e) {
-          ui.message(
-              String.format("Warning: Ignoring invalid external ID note %s", note.getName()));
         }
       }
-      if (dirty) {
+
-        ExternalIdsUpdate.commit(
+      try (MetaDataUpdate metaDataUpdate =
-            allUsersName,
+          new MetaDataUpdate(GitReferenceUpdated.DISABLED, allUsersName, repo)) {
-            repo,
+        metaDataUpdate.getCommitBuilder().setAuthor(serverUser);
-            rw,
+        metaDataUpdate.getCommitBuilder().setCommitter(serverUser);
-            ins,
+        metaDataUpdate.getCommitBuilder().setMessage(COMMIT_MSG);
-            rev,
+        extIdNotes.commit(metaDataUpdate);
-            noteMap,
-            COMMIT_MSG,
-            serverUser,
-            serverUser,
-            null,
-            GitReferenceUpdated.DISABLED);
       }
-    } catch (IOException e) {
+    } catch (IOException | ConfigInvalidException e) {
       throw new OrmException("Failed to update external IDs", e);
     }
   }

javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java

@@ -31,10 +31,12 @@ import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_GPG
 import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.concurrent.TimeUnit.SECONDS;
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toSet;
 import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
 
+import com.github.rholder.retry.StopStrategies;
 import com.google.common.cache.LoadingCache;
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableList;
@@ -54,6 +56,7 @@ import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.common.data.GlobalCapability;
 import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.extensions.api.accounts.AccountInput;
 import com.google.gerrit.extensions.api.accounts.EmailInput;
 import com.google.gerrit.extensions.api.changes.AddReviewerInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
@@ -64,6 +67,7 @@ import com.google.gerrit.extensions.api.config.ConsistencyCheckInput;
 import com.google.gerrit.extensions.api.config.ConsistencyCheckInput.CheckAccountsInput;
 import com.google.gerrit.extensions.common.AccountInfo;
 import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.EmailInfo;
 import com.google.gerrit.extensions.common.GpgKeyInfo;
 import com.google.gerrit.extensions.common.SshKeyInfo;
 import com.google.gerrit.extensions.events.AccountIndexedListener;
@@ -75,6 +79,7 @@ import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.gpg.Fingerprint;
 import com.google.gerrit.gpg.PublicKeyStore;
 import com.google.gerrit.gpg.testing.TestKey;
@@ -90,18 +95,25 @@ import com.google.gerrit.server.account.Emails;
 import com.google.gerrit.server.account.WatchConfig;
 import com.google.gerrit.server.account.WatchConfig.NotifyType;
 import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.account.externalids.ExternalIdNotes;
 import com.google.gerrit.server.account.externalids.ExternalIds;
-import com.google.gerrit.server.account.externalids.ExternalIdsUpdate;
+import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.server.git.LockFailureException;
+import com.google.gerrit.server.git.MetaDataUpdate;
 import com.google.gerrit.server.git.ProjectConfig;
 import com.google.gerrit.server.index.account.AccountIndexer;
 import com.google.gerrit.server.index.account.StalenessChecker;
 import com.google.gerrit.server.mail.Address;
+import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
 import com.google.gerrit.server.notedb.rebuild.ChangeRebuilderImpl;
 import com.google.gerrit.server.project.RefPattern;
 import com.google.gerrit.server.query.account.InternalAccountQuery;
+import com.google.gerrit.server.update.RetryHelper;
 import com.google.gerrit.server.util.MagicBranch;
 import com.google.gerrit.testing.ConfigSuite;
 import com.google.gerrit.testing.FakeEmailSender.Message;
+import com.google.gerrit.testing.TestTimeUtil;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.name.Named;
@@ -119,10 +131,12 @@ import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
 import org.bouncycastle.bcpg.ArmoredOutputStream;
 import org.bouncycastle.openpgp.PGPPublicKey;
 import org.bouncycastle.openpgp.PGPPublicKeyRing;
 import org.eclipse.jgit.api.errors.TransportException;
+import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.CommitBuilder;
@@ -163,10 +177,6 @@ public class AccountIT extends AbstractDaemonTest {
 
   @Inject private ExternalIds externalIds;
 
-  @Inject private ExternalIdsUpdate.User externalIdsUpdateFactory;
-
-  @Inject private ExternalIdsUpdate.ServerNoReindex externalIdsUpdateNoReindexFactory;
-
   @Inject private DynamicSet<AccountIndexedListener> accountIndexedListeners;
 
   @Inject private DynamicSet<GitReferenceUpdatedListener> refUpdateListeners;
@@ -181,6 +191,16 @@ public class AccountIT extends AbstractDaemonTest {
 
   @Inject private AccountIndexer accountIndexer;
 
+  @Inject private OutgoingEmailValidator emailValidator;
+
+  @Inject private GitReferenceUpdated gitReferenceUpdated;
+
+  @Inject private RetryHelper.Metrics retryMetrics;
+
+  @Inject private Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory;
+
+  @Inject private ExternalIdNotes.Factory extIdNotesFactory;
+
   @Inject
   @Named("accounts")
   private LoadingCache<Account.Id, Optional<AccountState>> accountsCache;
@@ -189,8 +209,6 @@ public class AccountIT extends AbstractDaemonTest {
   private RegistrationHandle accountIndexEventCounterHandle;
   private RefUpdateCounter refUpdateCounter;
   private RegistrationHandle refUpdateCounterHandle;
-  private ExternalIdsUpdate externalIdsUpdate;
-  private List<ExternalId> savedExternalIds;
 
   @Before
   public void addAccountIndexEventCounter() {
@@ -218,27 +236,6 @@ public class AccountIT extends AbstractDaemonTest {
     }
   }
 
-  @Before
-  public void saveExternalIds() throws Exception {
-    externalIdsUpdate = externalIdsUpdateFactory.create();
-
-    savedExternalIds = new ArrayList<>();
-    savedExternalIds.addAll(externalIds.byAccount(admin.id));
-    savedExternalIds.addAll(externalIds.byAccount(user.id));
-  }
-
-  @After
-  public void restoreExternalIds() throws Exception {
-    if (savedExternalIds != null) {
-      // savedExternalIds is null when we don't run SSH tests and the assume in
-      // @Before in AbstractDaemonTest prevents this class' @Before method from
-      // being executed.
-      externalIdsUpdate.delete(externalIds.byAccount(admin.id));
-      externalIdsUpdate.delete(externalIds.byAccount(user.id));
-      externalIdsUpdate.insert(savedExternalIds);
-    }
-  }
-
   @After
   public void clearPublicKeyStore() throws Exception {
     try (Repository repo = repoManager.openRepository(allUsers)) {
@@ -266,8 +263,8 @@ public class AccountIT extends AbstractDaemonTest {
   }
 
   @Test
-  public void create() throws Exception {
+  public void createByAccountCreator() throws Exception {
-    Account.Id accountId = create(2); // account creation + external ID creation
+    Account.Id accountId = createByAccountCreator(2); // account creation + external ID creation
     refUpdateCounter.assertRefUpdateFor(
         RefUpdateCounter.projectRef(allUsers, RefNames.refsUsers(accountId)),
         RefUpdateCounter.projectRef(allUsers, RefNames.REFS_EXTERNAL_IDS),
@@ -276,8 +273,9 @@ public class AccountIT extends AbstractDaemonTest {
 
   @Test
   @UseSsh
-  public void createWithSshKeys() throws Exception {
+  public void createWithSshKeysByAccountCreator() throws Exception {
-    Account.Id accountId = create(3); // account creation + external ID creation + adding SSH keys
+    Account.Id accountId =
+        createByAccountCreator(3); // account creation + external ID creation + adding SSH keys
     refUpdateCounter.assertRefUpdateFor(
         ImmutableMap.of(
             RefUpdateCounter.projectRef(allUsers, RefNames.refsUsers(accountId)),
@@ -289,7 +287,7 @@ public class AccountIT extends AbstractDaemonTest {
             1));
   }
 
-  private Account.Id create(int expectedAccountReindexCalls) throws Exception {
+  private Account.Id createByAccountCreator(int expectedAccountReindexCalls) throws Exception {
     String name = "foo";
     TestAccount foo = accountCreator.create(name);
     AccountInfo info = gApi.accounts().id(foo.id.get()).get();
@@ -301,18 +299,115 @@ public class AccountIT extends AbstractDaemonTest {
   }
 
   @Test
-  public void createAnonymousCoward() throws Exception {
+  public void createAnonymousCowardByAccountCreator() throws Exception {
     TestAccount anonymousCoward = accountCreator.create();
     accountIndexedCounter.assertReindexOf(anonymousCoward);
     assertUserBranchWithoutAccountConfig(anonymousCoward.getId());
   }
 
+  @Test
+  public void create() throws Exception {
+    AccountInput input = new AccountInput();
+    input.username = "foo";
+    input.name = "Foo";
+    input.email = "foo@example.com";
+    AccountInfo accountInfo = gApi.accounts().create(input).get();
+    assertThat(accountInfo._accountId).isNotNull();
+    assertThat(accountInfo.username).isEqualTo(input.username);
+    assertThat(accountInfo.name).isEqualTo(input.name);
+    assertThat(accountInfo.email).isEqualTo(input.email);
+    assertThat(accountInfo.status).isNull();
+
+    Account.Id accountId = new Account.Id(accountInfo._accountId);
+    accountIndexedCounter.assertReindexOf(accountId, 2); // account creation + external ID creation
+    assertThat(externalIds.byAccount(accountId))
+        .containsExactly(
+            ExternalId.createUsername(input.username, accountId, null),
+            ExternalId.createEmail(accountId, input.email));
+  }
+
+  @Test
+  public void createAccountUsernameAlreadyTaken() throws Exception {
+    AccountInput input = new AccountInput();
+    input.username = admin.username;
+
+    exception.expect(ResourceConflictException.class);
+    exception.expectMessage("username '" + admin.username + "' already exists");
+    gApi.accounts().create(input);
+  }
+
+  @Test
+  public void createAccountEmailAlreadyTaken() throws Exception {
+    AccountInput input = new AccountInput();
+    input.username = "foo";
+    input.email = admin.email;
+
+    exception.expect(UnprocessableEntityException.class);
+    exception.expectMessage("email '" + admin.email + "' already exists");
+    gApi.accounts().create(input);
+  }
+
+  @Test
+  public void commitMessageOnAccountUpdates() throws Exception {
+    AccountsUpdate au = accountsUpdate.create();
+    Account.Id accountId = new Account.Id(seq.nextAccountId());
+    au.insert("Create Test Account", accountId, u -> {});
+    assertLastCommitMessageOfUserBranch(accountId, "Create Test Account");
+
+    au.update("Set Status", accountId, u -> u.setStatus("Foo"));
+    assertLastCommitMessageOfUserBranch(accountId, "Set Status");
+  }
+
+  private void assertLastCommitMessageOfUserBranch(Account.Id accountId, String expectedMessage)
+      throws Exception {
+    try (Repository repo = repoManager.openRepository(allUsers);
+        RevWalk rw = new RevWalk(repo)) {
+      Ref exactRef = repo.exactRef(RefNames.refsUsers(accountId));
+      assertThat(rw.parseCommit(exactRef.getObjectId()).getShortMessage())
+          .isEqualTo(expectedMessage);
+    }
+  }
+
+  @Test
+  public void createAtomically() throws Exception {
+    TestTimeUtil.resetWithClockStep(1, SECONDS);
+    try {
+      Account.Id accountId = new Account.Id(seq.nextAccountId());
+      String fullName = "Foo";
+      ExternalId extId = ExternalId.createEmail(accountId, "foo@example.com");
+      Account account =
+          accountsUpdate
+              .create()
+              .insert(
+                  "Create Account Atomically",
+                  accountId,
+                  u -> u.setFullName(fullName).addExternalId(extId));
+      assertThat(account.getFullName()).isEqualTo(fullName);
+
+      AccountInfo info = gApi.accounts().id(accountId.get()).get();
+      assertThat(info.name).isEqualTo(fullName);
+
+      List<EmailInfo> emails = gApi.accounts().id(accountId.get()).getEmails();
+      assertThat(emails.stream().map(e -> e.email).collect(toSet())).containsExactly(extId.email());
+
+      RevCommit commitUserBranch = getRemoteHead(allUsers, RefNames.refsUsers(accountId));
+      RevCommit commitRefsMetaExternalIds = getRemoteHead(allUsers, RefNames.REFS_EXTERNAL_IDS);
+      assertThat(commitUserBranch.getCommitTime())
+          .isEqualTo(commitRefsMetaExternalIds.getCommitTime());
+    } finally {
+      TestTimeUtil.useSystemTime();
+    }
+  }
+
   @Test
   public void updateNonExistingAccount() throws Exception {
     Account.Id nonExistingAccountId = new Account.Id(999999);
     AtomicBoolean consumerCalled = new AtomicBoolean();
     Account account =
-        accountsUpdate.create().update(nonExistingAccountId, a -> consumerCalled.set(true));
+        accountsUpdate
+            .create()
+            .update(
+                "Update Non-Existing Account", nonExistingAccountId, a -> consumerCalled.set(true));
     assertThat(account).isNull();
     assertThat(consumerCalled.get()).isFalse();
   }
@@ -324,7 +419,9 @@ public class AccountIT extends AbstractDaemonTest {
 
     String status = "OOO";
     Account account =
-        accountsUpdate.create().update(anonymousCoward.getId(), a -> a.setStatus(status));
+        accountsUpdate
+            .create()
+            .update("Set status", anonymousCoward.getId(), u -> u.setStatus(status));
     assertThat(account).isNotNull();
     assertThat(account.getFullName()).isNull();
     assertThat(account.getStatus()).isEqualTo(status);
@@ -771,11 +868,16 @@ public class AccountIT extends AbstractDaemonTest {
     String email = "foo.bar@example.com";
     String extId1 = "foo:bar";
     String extId2 = "foo:baz";
-    List<ExternalId> extIds =
+    accountsUpdate
-        ImmutableList.of(
+        .create()
-            ExternalId.createWithEmail(ExternalId.Key.parse(extId1), admin.id, email),
+        .update(
-            ExternalId.createWithEmail(ExternalId.Key.parse(extId2), admin.id, email));
+            "Add External IDs",
-    externalIdsUpdateFactory.create().insert(extIds);
+            admin.id,
+            u ->
+                u.addExternalId(
+                        ExternalId.createWithEmail(ExternalId.Key.parse(extId1), admin.id, email))
+                    .addExternalId(
+                        ExternalId.createWithEmail(ExternalId.Key.parse(extId2), admin.id, email)));
     accountIndexedCounter.assertReindexOf(admin);
     assertThat(
             gApi.accounts().self().getExternalIds().stream().map(e -> e.identity).collect(toSet()))
@@ -827,9 +929,14 @@ public class AccountIT extends AbstractDaemonTest {
 
     // exact match with other scheme
     String email = "foo.bar@example.com";
-    externalIdsUpdateFactory
+    accountsUpdate
         .create()
-        .insert(ExternalId.createWithEmail(ExternalId.Key.parse("foo:bar"), admin.id, email));
+        .update(
+            "Add Email",
+            admin.id,
+            u ->
+                u.addExternalId(
+                    ExternalId.createWithEmail(ExternalId.Key.parse("foo:bar"), admin.id, email)));
     assertEmail(emails.getAccountFor(email), admin);
 
     // wrong case doesn't match
@@ -854,7 +961,9 @@ 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("Set Preferred Email", 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 +1439,9 @@ 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("Set Preferred Email", foo.id, u -> u.setPreferredEmail(noEmail));
     accountIndexedCounter.clear();
 
     grant(allUsers, userRef, Permission.PUSH, false, REGISTERED_USERS);
@@ -1591,7 +1702,7 @@ public class AccountIT extends AbstractDaemonTest {
     assertIteratorSize(2, getOnlyKeyFromStore(key).getUserIDs());
 
     pk = PGPPublicKey.removeCertification(pk, "foo:myId");
-    info = addGpgKey(armor(pk)).get(id);
+    info = addGpgKeyNoReindex(armor(pk)).get(id);
     assertThat(info.userIds).hasSize(1);
     assertIteratorSize(1, getOnlyKeyFromStore(key).getUserIDs());
   }
@@ -1600,7 +1711,12 @@ public class AccountIT extends AbstractDaemonTest {
   public void addOtherUsersGpgKey_Conflict() throws Exception {
     // Both users have a matching external ID for this key.
     addExternalIdEmail(admin, "test5@example.com");
-    externalIdsUpdate.insert(ExternalId.create("foo", "myId", user.getId()));
+    accountsUpdate
+        .create()
+        .update(
+            "Add External ID",
+            user.getId(),
+            u -> u.addExternalId(ExternalId.create("foo", "myId", user.getId())));
     accountIndexedCounter.assertReindexOf(user);
 
     TestKey key = validKeyWithSecondUserId();
@@ -1774,7 +1890,12 @@ public class AccountIT extends AbstractDaemonTest {
 
     // Delete the external ID for the preferred email. This makes the account inconsistent since it
     // now doesn't have an external ID for its preferred email.
-    externalIdsUpdate.delete(ExternalId.createEmail(account.getId(), email));
+    accountsUpdate
+        .create()
+        .update(
+            "Delete External ID",
+            account.getId(),
+            u -> u.deleteExternalId(ExternalId.createEmail(account.getId(), email)));
     expectedProblems.add(
         new ConsistencyProblemInfo(
             ConsistencyProblemInfo.Status.ERROR,
@@ -1812,11 +1933,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("Create Test Account", 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("Set Full Name", accountId, u -> u.setFullName("foo"));
     assertThat(account.getMetaId()).isNotEqualTo(updatedAccount.getMetaId());
     assertThat(updatedAccount.getMetaId()).isEqualTo(getMetaId(accountId));
   }
@@ -1880,6 +2001,115 @@ public class AccountIT extends AbstractDaemonTest {
         Permission.CREATE);
   }
 
+  @Test
+  public void retryOnLockFailure() throws Exception {
+    String status = "happy";
+    String fullName = "Foo";
+    AtomicBoolean doneBgUpdate = new AtomicBoolean(false);
+    PersonIdent ident = serverIdent.get();
+    AccountsUpdate update =
+        new AccountsUpdate(
+            repoManager,
+            gitReferenceUpdated,
+            null,
+            allUsers,
+            emailValidator,
+            metaDataUpdateInternalFactory,
+            new RetryHelper(
+                cfg,
+                retryMetrics,
+                null,
+                null,
+                null,
+                r -> r.withBlockStrategy(noSleepBlockStrategy)),
+            extIdNotesFactory,
+            ident,
+            ident,
+            () -> {
+              if (!doneBgUpdate.getAndSet(true)) {
+                try {
+                  accountsUpdate.create().update("Set Status", admin.id, u -> u.setStatus(status));
+                } catch (IOException | ConfigInvalidException | OrmException e) {
+                  // Ignore, the successful update of the account is asserted later
+                }
+              }
+            });
+    assertThat(doneBgUpdate.get()).isFalse();
+    AccountInfo accountInfo = gApi.accounts().id(admin.id.get()).get();
+    assertThat(accountInfo.status).isNull();
+    assertThat(accountInfo.name).isNotEqualTo(fullName);
+
+    Account updatedAccount = update.update("Set Full Name", admin.id, u -> u.setFullName(fullName));
+    assertThat(doneBgUpdate.get()).isTrue();
+
+    assertThat(updatedAccount.getStatus()).isEqualTo(status);
+    assertThat(updatedAccount.getFullName()).isEqualTo(fullName);
+
+    accountInfo = gApi.accounts().id(admin.id.get()).get();
+    assertThat(accountInfo.status).isEqualTo(status);
+    assertThat(accountInfo.name).isEqualTo(fullName);
+  }
+
+  @Test
+  public void failAfterRetryerGivesUp() throws Exception {
+    List<String> status = ImmutableList.of("foo", "bar", "baz");
+    String fullName = "Foo";
+    AtomicInteger bgCounter = new AtomicInteger(0);
+    PersonIdent ident = serverIdent.get();
+    AccountsUpdate update =
+        new AccountsUpdate(
+            repoManager,
+            gitReferenceUpdated,
+            null,
+            allUsers,
+            emailValidator,
+            metaDataUpdateInternalFactory,
+            new RetryHelper(
+                cfg,
+                retryMetrics,
+                null,
+                null,
+                null,
+                r ->
+                    r.withStopStrategy(StopStrategies.stopAfterAttempt(status.size()))
+                        .withBlockStrategy(noSleepBlockStrategy)),
+            extIdNotesFactory,
+            ident,
+            ident,
+            () -> {
+              try {
+                accountsUpdate
+                    .create()
+                    .update(
+                        "Set Status",
+                        admin.id,
+                        u -> u.setStatus(status.get(bgCounter.getAndAdd(1))));
+              } catch (IOException | ConfigInvalidException | OrmException e) {
+                // Ignore, the expected exception is asserted later
+              }
+            });
+    assertThat(bgCounter.get()).isEqualTo(0);
+    AccountInfo accountInfo = gApi.accounts().id(admin.id.get()).get();
+    assertThat(accountInfo.status).isNull();
+    assertThat(accountInfo.name).isNotEqualTo(fullName);
+
+    try {
+      update.update("Set Full Name", admin.id, u -> u.setFullName(fullName));
+      fail("expected LockFailureException");
+    } catch (LockFailureException e) {
+      // Ignore, expected
+    }
+    assertThat(bgCounter.get()).isEqualTo(status.size());
+
+    Account updatedAccount = accounts.get(admin.id);
+    assertThat(updatedAccount.getStatus()).isEqualTo(Iterables.getLast(status));
+    assertThat(updatedAccount.getFullName()).isEqualTo(admin.fullName);
+
+    accountInfo = gApi.accounts().id(admin.id.get()).get();
+    assertThat(accountInfo.status).isEqualTo(Iterables.getLast(status));
+    assertThat(accountInfo.name).isEqualTo(admin.fullName);
+  }
+
   @Test
   public void stalenessChecker() throws Exception {
     // Newly created account is not stale.
@@ -1912,17 +2142,28 @@ public class AccountIT extends AbstractDaemonTest {
 
     // Manually inserting/updating/deleting an external ID of the user makes the index document
     // stale.
-    ExternalIdsUpdate externalIdsUpdateNoReindex = externalIdsUpdateNoReindexFactory.create();
+    try (Repository repo = repoManager.openRepository(allUsers)) {
+      ExternalIdNotes extIdNotes = ExternalIdNotes.loadNoCacheUpdate(repo);
+
       ExternalId.Key key = ExternalId.Key.create("foo", "foo");
-    externalIdsUpdateNoReindex.insert(ExternalId.create(key, accountId));
+      extIdNotes.insert(ExternalId.create(key, accountId));
+      try (MetaDataUpdate update = metaDataUpdateFactory.create(allUsers)) {
+        extIdNotes.commit(update);
+      }
       assertStaleAccountAndReindex(accountId);
 
-    externalIdsUpdateNoReindex.upsert(
+      extIdNotes.upsert(ExternalId.createWithEmail(key, accountId, "foo@example.com"));
-        ExternalId.createWithEmail(key, accountId, "foo@example.com"));
+      try (MetaDataUpdate update = metaDataUpdateFactory.create(allUsers)) {
+        extIdNotes.commit(update);
+      }
       assertStaleAccountAndReindex(accountId);
 
-    externalIdsUpdateNoReindex.delete(accountId, key);
+      extIdNotes.delete(accountId, key);
+      try (MetaDataUpdate update = metaDataUpdateFactory.create(allUsers)) {
+        extIdNotes.commit(update);
+      }
       assertStaleAccountAndReindex(accountId);
+    }
 
     // Manually delete account
     try (Repository repo = repoManager.openRepository(allUsers);
@@ -2047,8 +2288,14 @@ public class AccountIT extends AbstractDaemonTest {
 
   private void addExternalIdEmail(TestAccount account, String email) throws Exception {
     checkNotNull(email);
-    externalIdsUpdate.insert(
+    accountsUpdate
-        ExternalId.createWithEmail(name("test"), email, account.getId(), email));
+        .create()
+        .update(
+            "Add Email",
+            account.getId(),
+            u ->
+                u.addExternalId(
+                    ExternalId.createWithEmail(name("test"), email, account.getId(), email)));
     accountIndexedCounter.assertReindexOf(account);
     setApiUser(account);
   }
@@ -2060,6 +2307,10 @@ public class AccountIT extends AbstractDaemonTest {
     return gpgKeys;
   }
 
+  private Map<String, GpgKeyInfo> addGpgKeyNoReindex(String armored) throws Exception {
+    return gApi.accounts().self().putGpgKeys(ImmutableList.of(armored), ImmutableList.<String>of());
+  }
+
   private void assertUser(AccountInfo info, TestAccount account) throws Exception {
     assertUser(info, account, null);
   }

javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java

@@ -24,6 +24,7 @@ import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.stream.Collectors.toList;
 import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
+import static org.eclipse.jgit.lib.Constants.OBJ_TREE;
 
 import com.github.rholder.retry.StopStrategies;
 import com.google.common.collect.ImmutableList;
@@ -43,20 +44,23 @@ import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.metrics.MetricMaker;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.account.AccountsUpdate;
 import com.google.gerrit.server.account.externalids.DisabledExternalIdCache;
 import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.account.externalids.ExternalIdNotes;
 import com.google.gerrit.server.account.externalids.ExternalIdReader;
 import com.google.gerrit.server.account.externalids.ExternalIds;
 import com.google.gerrit.server.account.externalids.ExternalIdsUpdate;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
 import com.google.gerrit.server.git.LockFailureException;
+import com.google.gerrit.server.git.MetaDataUpdate;
 import com.google.gerrit.server.update.RetryHelper;
 import com.google.gson.reflect.TypeToken;
 import com.google.gwtorm.server.OrmDuplicateKeyException;
-import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
@@ -69,11 +73,14 @@ import org.eclipse.jgit.api.errors.TransportException;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
 import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.CommitBuilder;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.notes.NoteMap;
+import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.transport.PushResult;
 import org.eclipse.jgit.transport.RemoteRefUpdate;
@@ -82,11 +89,12 @@ import org.eclipse.jgit.util.MutableInteger;
 import org.junit.Test;
 
 public class ExternalIdIT extends AbstractDaemonTest {
-  @Inject private ExternalIdsUpdate.Server extIdsUpdate;
+  @Inject private AccountsUpdate.Server accountsUpdate;
   @Inject private ExternalIds externalIds;
   @Inject private ExternalIdReader externalIdReader;
   @Inject private MetricMaker metricMaker;
   @Inject private RetryHelper.Metrics retryMetrics;
+  @Inject private ExternalIdNotes.Factory externalIdNotesFactory;
 
   @Test
   public void getExternalIds() throws Exception {
@@ -454,31 +462,28 @@ public class ExternalIdIT extends AbstractDaemonTest {
     return new ConsistencyProblemInfo(ConsistencyProblemInfo.Status.ERROR, message);
   }
 
-  private void insertValidExternalIds() throws IOException, ConfigInvalidException, OrmException {
+  private void insertValidExternalIds() throws Exception {
     MutableInteger i = new MutableInteger();
     String scheme = "valid";
-    ExternalIdsUpdate u = extIdsUpdate.create();
 
     // create valid external IDs
-    u.insert(
+    insertExtId(
         ExternalId.createWithPassword(
             ExternalId.Key.parse(nextId(scheme, i)),
             admin.id,
             "admin.other@example.com",
             "secret-password"));
-    u.insert(createExternalIdWithOtherCaseEmail(nextId(scheme, i)));
+    insertExtId(createExternalIdWithOtherCaseEmail(nextId(scheme, i)));
   }
 
-  private Set<ConsistencyProblemInfo> insertInvalidButParsableExternalIds()
+  private Set<ConsistencyProblemInfo> insertInvalidButParsableExternalIds() throws Exception {
-      throws IOException, ConfigInvalidException, OrmException {
     MutableInteger i = new MutableInteger();
     String scheme = "invalid";
-    ExternalIdsUpdate u = extIdsUpdate.create();
 
     Set<ConsistencyProblemInfo> expectedProblems = new HashSet<>();
     ExternalId extIdForNonExistingAccount =
         createExternalIdForNonExistingAccount(nextId(scheme, i));
-    u.insert(extIdForNonExistingAccount);
+    insertExtIdForNonExistingAccount(extIdForNonExistingAccount);
     expectedProblems.add(
         consistencyError(
             "External ID '"
@@ -487,7 +492,7 @@ public class ExternalIdIT extends AbstractDaemonTest {
                 + extIdForNonExistingAccount.accountId().get()));
 
     ExternalId extIdWithInvalidEmail = createExternalIdWithInvalidEmail(nextId(scheme, i));
-    u.insert(extIdWithInvalidEmail);
+    insertExtId(extIdWithInvalidEmail);
     expectedProblems.add(
         consistencyError(
             "External ID '"
@@ -496,7 +501,7 @@ public class ExternalIdIT extends AbstractDaemonTest {
                 + extIdWithInvalidEmail.email()));
 
     ExternalId extIdWithDuplicateEmail = createExternalIdWithDuplicateEmail(nextId(scheme, i));
-    u.insert(extIdWithDuplicateEmail);
+    insertExtId(extIdWithDuplicateEmail);
     expectedProblems.add(
         consistencyError(
             "Email '"
@@ -508,7 +513,7 @@ public class ExternalIdIT extends AbstractDaemonTest {
                 + "'"));
 
     ExternalId extIdWithBadPassword = createExternalIdWithBadPassword("admin-username");
-    u.insert(extIdWithBadPassword);
+    insertExtId(extIdWithBadPassword);
     expectedProblems.add(
         consistencyError(
             "External ID '"
@@ -570,12 +575,11 @@ public class ExternalIdIT extends AbstractDaemonTest {
 
   private String insertExternalIdWithoutAccountId(Repository repo, RevWalk rw, String externalId)
       throws IOException {
-    ObjectId rev = ExternalIdReader.readRevision(repo);
+    return insertExternalId(
-    NoteMap noteMap = ExternalIdReader.readNoteMap(rw, rev);
+        repo,
-
+        rw,
+        (ins, noteMap) -> {
           ExternalId extId = ExternalId.create(ExternalId.Key.parse(externalId), admin.id);
-
-    try (ObjectInserter ins = repo.newObjectInserter()) {
           ObjectId noteId = extId.key().sha1();
           Config c = new Config();
           extId.writeToConfig(c);
@@ -583,104 +587,105 @@ public class ExternalIdIT extends AbstractDaemonTest {
           byte[] raw = c.toText().getBytes(UTF_8);
           ObjectId dataBlob = ins.insert(OBJ_BLOB, raw);
           noteMap.set(noteId, dataBlob);
-
+          return noteId;
-      ExternalIdsUpdate.commit(
+        });
-          allUsers,
-          repo,
-          rw,
-          ins,
-          rev,
-          noteMap,
-          "Add external ID",
-          admin.getIdent(),
-          admin.getIdent(),
-          null,
-          GitReferenceUpdated.DISABLED);
-      return noteId.getName();
-    }
   }
 
   private String insertExternalIdWithKeyThatDoesntMatchNoteId(
       Repository repo, RevWalk rw, String externalId) throws IOException {
-    ObjectId rev = ExternalIdReader.readRevision(repo);
+    return insertExternalId(
-    NoteMap noteMap = ExternalIdReader.readNoteMap(rw, rev);
+        repo,
-
+        rw,
+        (ins, noteMap) -> {
           ExternalId extId = ExternalId.create(ExternalId.Key.parse(externalId), admin.id);
-
-    try (ObjectInserter ins = repo.newObjectInserter()) {
           ObjectId noteId = ExternalId.Key.parse(externalId + "x").sha1();
           Config c = new Config();
           extId.writeToConfig(c);
           byte[] raw = c.toText().getBytes(UTF_8);
           ObjectId dataBlob = ins.insert(OBJ_BLOB, raw);
           noteMap.set(noteId, dataBlob);
-
+          return noteId;
-      ExternalIdsUpdate.commit(
+        });
-          allUsers,
-          repo,
-          rw,
-          ins,
-          rev,
-          noteMap,
-          "Add external ID",
-          admin.getIdent(),
-          admin.getIdent(),
-          null,
-          GitReferenceUpdated.DISABLED);
-      return noteId.getName();
-    }
   }
 
   private String insertExternalIdWithInvalidConfig(Repository repo, RevWalk rw, String externalId)
       throws IOException {
-    ObjectId rev = ExternalIdReader.readRevision(repo);
+    return insertExternalId(
-    NoteMap noteMap = ExternalIdReader.readNoteMap(rw, rev);
+        repo,
-
+        rw,
-    try (ObjectInserter ins = repo.newObjectInserter()) {
+        (ins, noteMap) -> {
           ObjectId noteId = ExternalId.Key.parse(externalId).sha1();
           byte[] raw = "bad-config".getBytes(UTF_8);
           ObjectId dataBlob = ins.insert(OBJ_BLOB, raw);
           noteMap.set(noteId, dataBlob);
-
+          return noteId;
-      ExternalIdsUpdate.commit(
+        });
-          allUsers,
-          repo,
-          rw,
-          ins,
-          rev,
-          noteMap,
-          "Add external ID",
-          admin.getIdent(),
-          admin.getIdent(),
-          null,
-          GitReferenceUpdated.DISABLED);
-      return noteId.getName();
-    }
   }
 
   private String insertExternalIdWithEmptyNote(Repository repo, RevWalk rw, String externalId)
       throws IOException {
+    return insertExternalId(
+        repo,
+        rw,
+        (ins, noteMap) -> {
+          ObjectId noteId = ExternalId.Key.parse(externalId).sha1();
+          byte[] raw = "".getBytes(UTF_8);
+          ObjectId dataBlob = ins.insert(OBJ_BLOB, raw);
+          noteMap.set(noteId, dataBlob);
+          return noteId;
+        });
+  }
+
+  private String insertExternalId(Repository repo, RevWalk rw, ExternalIdInserter extIdInserter)
+      throws IOException {
     ObjectId rev = ExternalIdReader.readRevision(repo);
     NoteMap noteMap = ExternalIdReader.readNoteMap(rw, rev);
 
     try (ObjectInserter ins = repo.newObjectInserter()) {
-      ObjectId noteId = ExternalId.Key.parse(externalId).sha1();
+      ObjectId noteId = extIdInserter.addNote(ins, noteMap);
-      byte[] raw = "".getBytes(UTF_8);
-      ObjectId dataBlob = ins.insert(OBJ_BLOB, raw);
-      noteMap.set(noteId, dataBlob);
 
-      ExternalIdsUpdate.commit(
+      CommitBuilder cb = new CommitBuilder();
-          allUsers,
+      cb.setMessage("Update external IDs");
-          repo,
+      cb.setTreeId(noteMap.writeTree(ins));
-          rw,
+      cb.setAuthor(admin.getIdent());
-          ins,
+      cb.setCommitter(admin.getIdent());
-          rev,
+      if (!rev.equals(ObjectId.zeroId())) {
-          noteMap,
+        cb.setParentId(rev);
-          "Add external ID",
+      } else {
-          admin.getIdent(),
+        cb.setParentIds(); // Ref is currently nonexistent, commit has no parents.
-          admin.getIdent(),
+      }
-          null,
+      if (cb.getTreeId() == null) {
-          GitReferenceUpdated.DISABLED);
+        if (rev.equals(ObjectId.zeroId())) {
+          cb.setTreeId(ins.insert(OBJ_TREE, new byte[] {})); // No parent, assume empty tree.
+        } else {
+          RevCommit p = rw.parseCommit(rev);
+          cb.setTreeId(p.getTree()); // Copy tree from parent.
+        }
+      }
+      ObjectId commitId = ins.insert(cb);
+      ins.flush();
+
+      RefUpdate u = repo.updateRef(RefNames.REFS_EXTERNAL_IDS);
+      u.setExpectedOldObjectId(rev);
+      u.setNewObjectId(commitId);
+      RefUpdate.Result res = u.update();
+      switch (res) {
+        case NEW:
+        case FAST_FORWARD:
+        case NO_CHANGE:
+        case RENAMED:
+        case FORCED:
+          break;
+        case LOCK_FAILURE:
+        case IO_FAILURE:
+        case NOT_ATTEMPTED:
+        case REJECTED:
+        case REJECTED_CURRENT_BRANCH:
+        case REJECTED_MISSING_OBJECT:
+        case REJECTED_OTHER_REASON:
+        default:
+          throw new IOException("Updating external IDs failed with " + res);
+      }
       return noteId.getName();
     }
   }
@@ -718,15 +723,12 @@ public class ExternalIdIT extends AbstractDaemonTest {
     ExternalIdsUpdate update =
         new ExternalIdsUpdate(
             repoManager,
+            () -> metaDataUpdateFactory.create(allUsers),
             accountCache,
             allUsers,
             metricMaker,
             externalIds,
             new DisabledExternalIdCache(),
-            serverIdent.get(),
-            serverIdent.get(),
-            null,
-            GitReferenceUpdated.DISABLED,
             new RetryHelper(
                 cfg,
                 retryMetrics,
@@ -737,8 +739,8 @@ public class ExternalIdIT extends AbstractDaemonTest {
             () -> {
               if (!doneBgUpdate.getAndSet(true)) {
                 try {
-                  extIdsUpdate.create().insert(ExternalId.create(barId, admin.id));
+                  insertExtId(ExternalId.create(barId, admin.id));
-                } catch (IOException | ConfigInvalidException | OrmException e) {
+                } catch (Exception e) {
                   // Ignore, the successful insertion of the external ID is asserted later
                 }
               }
@@ -762,15 +764,12 @@ public class ExternalIdIT extends AbstractDaemonTest {
     ExternalIdsUpdate update =
         new ExternalIdsUpdate(
             repoManager,
+            () -> metaDataUpdateFactory.create(allUsers),
             accountCache,
             allUsers,
             metricMaker,
             externalIds,
             new DisabledExternalIdCache(),
-            serverIdent.get(),
-            serverIdent.get(),
-            null,
-            GitReferenceUpdated.DISABLED,
             new RetryHelper(
                 cfg,
                 retryMetrics,
@@ -782,10 +781,8 @@ public class ExternalIdIT extends AbstractDaemonTest {
                         .withBlockStrategy(noSleepBlockStrategy)),
             () -> {
               try {
-                extIdsUpdate
+                insertExtId(ExternalId.create(extIdsKeys[bgCounter.getAndAdd(1)], admin.id));
-                    .create()
+              } catch (Exception e) {
-                    .insert(ExternalId.create(extIdsKeys[bgCounter.getAndAdd(1)], admin.id));
-              } catch (IOException | ConfigInvalidException | OrmException e) {
                 // Ignore, the successful insertion of the external ID is asserted later
               }
             });
@@ -806,7 +803,12 @@ public class ExternalIdIT extends AbstractDaemonTest {
   public void readExternalIdWithAccountIdThatCanBeExpressedInKiB() throws Exception {
     ExternalId.Key extIdKey = ExternalId.Key.parse("foo:bar");
     Account.Id accountId = new Account.Id(1024 * 100);
-    extIdsUpdate.create().insert(ExternalId.create(extIdKey, accountId));
+    accountsUpdate
+        .create()
+        .insert(
+            "Create Account with Bad External ID",
+            accountId,
+            u -> u.addExternalId(ExternalId.create(extIdKey, accountId)));
     ExternalId extId = externalIds.get(extIdKey);
     assertThat(extId.accountId()).isEqualTo(accountId);
   }
@@ -817,20 +819,24 @@ public class ExternalIdIT extends AbstractDaemonTest {
     try (AutoCloseable ctx = createFailOnLoadContext()) {
       // insert external ID
       ExternalId extId = ExternalId.create("foo", "bar", admin.id);
-      extIdsUpdate.create().insert(extId);
+      insertExtId(extId);
       expectedExtIds.add(extId);
       assertThat(externalIds.byAccount(admin.id)).containsExactlyElementsIn(expectedExtIds);
 
       // update external ID
       expectedExtIds.remove(extId);
-      extId = ExternalId.createWithEmail("foo", "bar", admin.id, "foo.bar@example.com");
+      ExternalId extId2 = ExternalId.createWithEmail("foo", "bar", admin.id, "foo.bar@example.com");
-      extIdsUpdate.create().upsert(extId);
+      accountsUpdate
-      expectedExtIds.add(extId);
+          .create()
+          .update("Update External ID", admin.id, u -> u.updateExternalId(extId2));
+      expectedExtIds.add(extId2);
       assertThat(externalIds.byAccount(admin.id)).containsExactlyElementsIn(expectedExtIds);
 
       // delete external ID
-      extIdsUpdate.create().delete(extId);
+      accountsUpdate
-      expectedExtIds.remove(extId);
+          .create()
+          .update("Delete External ID", admin.id, u -> u.deleteExternalId(extId));
+      expectedExtIds.remove(extId2);
       assertThat(externalIds.byAccount(admin.id)).containsExactlyElementsIn(expectedExtIds);
     }
   }
@@ -866,50 +872,47 @@ public class ExternalIdIT extends AbstractDaemonTest {
     assertThat(externalIds.byAccount(admin.id)).containsExactlyElementsIn(expectedExternalIds);
   }
 
-  private void insertExtIdBehindGerritsBack(ExternalId extId) throws Exception {
+  private void insertExtId(ExternalId extId) throws Exception {
+    accountsUpdate
+        .create()
+        .update("Add External ID", extId.accountId(), u -> u.addExternalId(extId));
+  }
+
+  private void insertExtIdForNonExistingAccount(ExternalId extId) throws Exception {
+    // Cannot use AccountsUpdate to insert an external ID for a non-existing account.
     try (Repository repo = repoManager.openRepository(allUsers);
-        RevWalk rw = new RevWalk(repo);
+        MetaDataUpdate update = metaDataUpdateFactory.create(allUsers)) {
-        ObjectInserter ins = repo.newObjectInserter()) {
+      ExternalIdNotes extIdNotes = externalIdNotesFactory.load(repo);
-      ObjectId rev = ExternalIdReader.readRevision(repo);
+      extIdNotes.insert(extId);
-      NoteMap noteMap = ExternalIdReader.readNoteMap(rw, rev);
+      extIdNotes.commit(update);
-      ExternalIdsUpdate.insert(rw, ins, noteMap, extId);
+      extIdNotes.updateCaches();
-      ExternalIdsUpdate.commit(
+    }
-          allUsers,
+  }
-          repo,
+
-          rw,
+  private void insertExtIdBehindGerritsBack(ExternalId extId) throws Exception {
-          ins,
+    try (Repository repo = repoManager.openRepository(allUsers)) {
-          rev,
+      // Inserting an external ID "behind Gerrit's back" means that the caches are not updated.
-          noteMap,
+      ExternalIdNotes extIdNotes = ExternalIdNotes.loadNoCacheUpdate(repo);
-          "insert new ID",
+      extIdNotes.insert(extId);
-          serverIdent.get(),
+      try (MetaDataUpdate metaDataUpdate =
-          serverIdent.get(),
+          new MetaDataUpdate(GitReferenceUpdated.DISABLED, null, repo)) {
-          null,
+        metaDataUpdate.getCommitBuilder().setAuthor(admin.getIdent());
-          GitReferenceUpdated.DISABLED);
+        metaDataUpdate.getCommitBuilder().setCommitter(admin.getIdent());
+        extIdNotes.commit(metaDataUpdate);
+      }
     }
   }
 
   private void addExtId(TestRepository<?> testRepo, ExternalId... extIds)
       throws IOException, OrmDuplicateKeyException, ConfigInvalidException {
-    ObjectId rev = ExternalIdReader.readRevision(testRepo.getRepository());
+    ExternalIdNotes extIdNotes = externalIdNotesFactory.load(testRepo.getRepository());
-
+    extIdNotes.insert(Arrays.asList(extIds));
-    try (ObjectInserter ins = testRepo.getRepository().newObjectInserter()) {
+    try (MetaDataUpdate metaDataUpdate =
-      NoteMap noteMap = ExternalIdReader.readNoteMap(testRepo.getRevWalk(), rev);
+        new MetaDataUpdate(GitReferenceUpdated.DISABLED, null, testRepo.getRepository())) {
-      for (ExternalId extId : extIds) {
+      metaDataUpdate.getCommitBuilder().setAuthor(admin.getIdent());
-        ExternalIdsUpdate.insert(testRepo.getRevWalk(), ins, noteMap, extId);
+      metaDataUpdate.getCommitBuilder().setCommitter(admin.getIdent());
-      }
+      extIdNotes.commit(metaDataUpdate);
-
+      extIdNotes.updateCaches();
-      ExternalIdsUpdate.commit(
-          allUsers,
-          testRepo.getRepository(),
-          testRepo.getRevWalk(),
-          ins,
-          rev,
-          noteMap,
-          "Add external ID",
-          admin.getIdent(),
-          admin.getIdent(),
-          null,
-          GitReferenceUpdated.DISABLED);
     }
   }
 
@@ -950,4 +953,9 @@ public class ExternalIdIT extends AbstractDaemonTest {
       }
     };
   }
+
+  @FunctionalInterface
+  private interface ExternalIdInserter {
+    public ObjectId addNote(ObjectInserter ins, NoteMap noteMap) throws IOException;
+  }
 }

javatests/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java

@@ -40,7 +40,7 @@ import com.google.gerrit.server.account.AccountManager;
 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;
+import com.google.gerrit.server.account.externalids.ExternalIds;
 import com.google.gerrit.server.schema.SchemaCreator;
 import com.google.gerrit.server.util.RequestContext;
 import com.google.gerrit.server.util.ThreadLocalRequestContext;
@@ -55,6 +55,7 @@ import com.google.inject.util.Providers;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Set;
 import org.bouncycastle.openpgp.PGPPublicKey;
 import org.bouncycastle.openpgp.PGPPublicKeyRing;
 import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
@@ -84,7 +85,7 @@ public class GerritPublicKeyCheckerTest {
 
   @Inject private ThreadLocalRequestContext requestContext;
 
-  @Inject private ExternalIdsUpdate.Server externalIdsUpdateFactory;
+  @Inject private ExternalIds externalIds;
 
   private LifecycleManager lifecycle;
   private ReviewDb db;
@@ -116,7 +117,9 @@ public class GerritPublicKeyCheckerTest {
     schemaCreator.create(db);
     userId = accountManager.authenticate(AuthRequest.forUser("user")).getAccountId();
     // Note: does not match any key in TestKeys.
-    accountsUpdate.create().update(userId, a -> a.setPreferredEmail("user@example.com"));
+    accountsUpdate
+        .create()
+        .update("Set Preferred Email", userId, u -> u.setPreferredEmail("user@example.com"));
     user = reloadUser();
 
     requestContext.setContext(
@@ -219,8 +222,10 @@ public class GerritPublicKeyCheckerTest {
 
   @Test
   public void noExternalIds() throws Exception {
-    ExternalIdsUpdate externalIdsUpdate = externalIdsUpdateFactory.create();
+    Set<ExternalId> extIds = externalIds.byAccount(user.getAccountId());
-    externalIdsUpdate.deleteAll(user.getAccountId());
+    accountsUpdate
+        .create()
+        .update("Delete External IDs", user.getAccountId(), u -> u.deleteExternalIds(extIds));
     reloadUser();
 
     TestKey key = validKeyWithSecondUserId();
@@ -233,9 +238,7 @@ public class GerritPublicKeyCheckerTest {
     checker = checkerFactory.create().setStore(store).disableTrust();
     assertProblems(
         checker.check(key.getPublicKey()), Status.BAD, "Key is not associated with any users");
-    externalIdsUpdate.insert(
+    insertExtId(ExternalId.create(toExtIdKey(key.getPublicKey()), user.getAccountId()));
-        ExternalId.create(toExtIdKey(key.getPublicKey()), user.getAccountId()));
-    reloadUser();
     assertProblems(checker.check(key.getPublicKey()), Status.BAD, "No identities found for user");
   }
 
@@ -402,7 +405,7 @@ public class GerritPublicKeyCheckerTest {
     cb.setCommitter(ident);
     assertThat(store.save(cb)).isAnyOf(NEW, FAST_FORWARD, FORCED);
 
-    externalIdsUpdateFactory.create().insert(newExtIds);
+    accountsUpdate.create().update("Add External IDs", id, u -> u.addExternalIds(newExtIds));
   }
 
   private TestKey add(TestKey k, IdentifiedUser user) throws Exception {
@@ -425,9 +428,13 @@ public class GerritPublicKeyCheckerTest {
   }
 
   private void addExternalId(String scheme, String id, String email) throws Exception {
-    externalIdsUpdateFactory
+    insertExtId(ExternalId.createWithEmail(scheme, id, user.getAccountId(), email));
+  }
+
+  private void insertExtId(ExternalId extId) throws Exception {
+    accountsUpdate
         .create()
-        .insert(ExternalId.createWithEmail(scheme, id, user.getAccountId(), email));
+        .update("Add External ID", extId.accountId(), u -> u.addExternalId(extId));
     reloadUser();
   }
 }

javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java

@@ -46,6 +46,7 @@ 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.account.InternalAccountUpdate;
 import com.google.gerrit.server.account.externalids.ExternalId;
 import com.google.gerrit.server.account.externalids.ExternalIds;
 import com.google.gerrit.server.config.AllProjectsName;
@@ -417,7 +418,7 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests {
       md.getCommitBuilder().setCommitter(ident);
       AccountConfig accountConfig = new AccountConfig(null, accountId);
       accountConfig.load(repo);
-      accountConfig.getLoadedAccount().get().setFullName(newName);
+      accountConfig.setAccountUpdate(InternalAccountUpdate.builder().setFullName(newName).build());
       accountConfig.commit(md);
     }
 
@@ -541,11 +542,10 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests {
       accountsUpdate
           .create()
           .update(
+              "Update Test Account",
               id,
-              a -> {
+              u -> {
-                a.setFullName(fullName);
+                u.setFullName(fullName).setPreferredEmail(email).setActive(active);
-                a.setPreferredEmail(email);
-                a.setActive(active);
               });
       return id;
     }

javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java

@@ -84,7 +84,6 @@ 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;
 import com.google.gerrit.server.change.ChangeInserter;
 import com.google.gerrit.server.change.ChangeTriplet;
 import com.google.gerrit.server.change.PatchSetInserter;
@@ -172,7 +171,6 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Inject protected ThreadLocalRequestContext requestContext;
   @Inject protected ProjectCache projectCache;
   @Inject protected MetaDataUpdate.Server metaDataUpdateFactory;
-  @Inject protected ExternalIdsUpdate.Server externalIdsUpdate;
 
   // Only for use in setting up/tearing down injector; other users should use schemaFactory.
   @Inject private InMemoryDatabase inMemoryDatabase;
@@ -223,8 +221,12 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
 
     userId = accountManager.authenticate(AuthRequest.forUser("user")).getAccountId();
     String email = "user@example.com";
-    externalIdsUpdate.create().insert(ExternalId.createEmail(userId, email));
+    accountsUpdate
-    accountsUpdate.create().update(userId, a -> a.setPreferredEmail(email));
+        .create()
+        .update(
+            "Add Email",
+            userId,
+            u -> u.addExternalId(ExternalId.createEmail(userId, email)).setPreferredEmail(email));
     user = userFactory.create(userId);
     requestContext.setContext(newRequestContext(userId));
   }
@@ -2729,11 +2731,10 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
       accountsUpdate
           .create()
           .update(
+              "Update Test Account",
               id,
-              a -> {
+              u -> {
-                a.setFullName(fullName);
+                u.setFullName(fullName).setPreferredEmail(email).setActive(active);
-                a.setPreferredEmail(email);
-                a.setActive(active);
               });
       return id;
     }

javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java

@@ -409,11 +409,10 @@ public abstract class AbstractQueryGroupsTest extends GerritServerTests {
       accountsUpdate
           .create()
           .update(
+              "Update Test Account",
               id,
-              a -> {
+              u -> {
-                a.setFullName(fullName);
+                u.setFullName(fullName).setPreferredEmail(email).setActive(active);
-                a.setPreferredEmail(email);
-                a.setActive(active);
               });
       return id;
     }

javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java

@@ -264,11 +264,10 @@ public abstract class AbstractQueryProjectsTest extends GerritServerTests {
       accountsUpdate
           .create()
           .update(
+              "Update Test Account",
               id,
-              a -> {
+              u -> {
-                a.setFullName(fullName);
+                u.setFullName(fullName).setPreferredEmail(email).setActive(active);
-                a.setPreferredEmail(email);
-                a.setActive(active);
               });
       return id;
     }