Merge changes I656a4f40,I8946f0de,I108f1428,Iac5cd922,I4f80207c, ...

* changes:
  AccountConfig: Remove unneeded validation of preferred email
  Remove ExternalIdsUpdate
  Use AccountsUpdate to delete/update GPG keys
  AccountManager: Use AccountsUpdate to delete ext IDs on update link
  AccountManager: Make unlinking ext IDs atomic
  AccountsUpdate: Remove methods to delete account
  AccountManager: Create account and username atomically

java/com/google/gerrit/gpg/server/DeleteGpgKey.java

@@ -24,8 +24,9 @@ import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.gpg.PublicKeyStore;
 import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.account.AccountsUpdate;
 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.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -41,16 +42,19 @@ public class DeleteGpgKey implements RestModifyView<GpgKey, Input> {
 
   private final Provider<PersonIdent> serverIdent;
   private final Provider<PublicKeyStore> storeProvider;
-  private final ExternalIdsUpdate.User externalIdsUpdateFactory;
+  private final AccountsUpdate.User accountsUpdateFactory;
+  private final ExternalIds externalIds;
 
   @Inject
   DeleteGpgKey(
       @GerritPersonIdent Provider<PersonIdent> serverIdent,
       Provider<PublicKeyStore> storeProvider,
-      ExternalIdsUpdate.User externalIdsUpdateFactory) {
+      AccountsUpdate.User accountsUpdateFactory,
+      ExternalIds externalIds) {
     this.serverIdent = serverIdent;
     this.storeProvider = storeProvider;
-    this.externalIdsUpdateFactory = externalIdsUpdateFactory;
+    this.accountsUpdateFactory = accountsUpdateFactory;
+    this.externalIds = externalIds;
   }
 
   @Override
@@ -58,12 +62,16 @@ public class DeleteGpgKey implements RestModifyView<GpgKey, Input> {
       throws ResourceConflictException, PGPException, OrmException, IOException,
           ConfigInvalidException {
     PGPPublicKey key = rsrc.getKeyRing().getPublicKey();
-    externalIdsUpdateFactory
+    ExternalId extId =
-        .create()
+        externalIds.get(
-        .delete(
-            rsrc.getUser().getAccountId(),
             ExternalId.Key.create(
                 SCHEME_GPGKEY, BaseEncoding.base16().encode(key.getFingerprint())));
+    accountsUpdateFactory
+        .create()
+        .update(
+            "Delete GPG Key via API",
+            rsrc.getUser().getAccountId(),
+            u -> u.deleteExternalId(extId));
 
     try (PublicKeyStore store = storeProvider.get()) {
       store.remove(rsrc.getKeyRing().getPublicKey().getFingerprint());

java/com/google/gerrit/gpg/server/GpgKeys.java

@@ -92,7 +92,8 @@ public class GpgKeys implements ChildCollection<AccountResource, GpgKey> {
       throws ResourceNotFoundException, PGPException, OrmException, IOException {
     checkVisible(self, parent);
 
-    byte[] fp = parseFingerprint(id.get(), getGpgExtIds(parent));
+    ExternalId gpgKeyExtId = findGpgKey(id.get(), getGpgExtIds(parent));
+    byte[] fp = parseFingerprint(gpgKeyExtId);
     try (PublicKeyStore store = storeProvider.get()) {
       long keyId = keyId(fp);
       for (PGPPublicKeyRing keyRing : store.get(keyId)) {
@@ -106,30 +107,34 @@ public class GpgKeys implements ChildCollection<AccountResource, GpgKey> {
     throw new ResourceNotFoundException(id);
   }
 
-  static byte[] parseFingerprint(String str, Iterable<ExternalId> existingExtIds)
+  static ExternalId findGpgKey(String str, Iterable<ExternalId> existingExtIds)
       throws ResourceNotFoundException {
     str = CharMatcher.whitespace().removeFrom(str).toUpperCase();
     if ((str.length() != 8 && str.length() != 40)
         || !CharMatcher.anyOf("0123456789ABCDEF").matchesAllOf(str)) {
       throw new ResourceNotFoundException(str);
     }
-    byte[] fp = null;
+    ExternalId gpgKeyExtId = null;
     for (ExternalId extId : existingExtIds) {
       String fpStr = extId.key().id();
       if (!fpStr.endsWith(str)) {
         continue;
-      } else if (fp != null) {
+      } else if (gpgKeyExtId != null) {
         throw new ResourceNotFoundException("Multiple keys found for " + str);
       }
-      fp = BaseEncoding.base16().decode(fpStr);
+      gpgKeyExtId = extId;
       if (str.length() == 40) {
         break;
       }
     }
-    if (fp == null) {
+    if (gpgKeyExtId == null) {
       throw new ResourceNotFoundException(str);
     }
-    return fp;
+    return gpgKeyExtId;
+  }
+
+  static byte[] parseFingerprint(ExternalId gpgKeyExtId) {
+    return BaseEncoding.base16().decode(gpgKeyExtId.key().id());
   }
 
   @Override
@@ -145,8 +150,7 @@ public class GpgKeys implements ChildCollection<AccountResource, GpgKey> {
       Map<String, GpgKeyInfo> keys = new HashMap<>();
       try (PublicKeyStore store = storeProvider.get()) {
         for (ExternalId extId : getGpgExtIds(rsrc)) {
-          String fpStr = extId.key().id();
+          byte[] fp = parseFingerprint(extId);
-          byte[] fp = BaseEncoding.base16().decode(fpStr);
           boolean found = false;
           for (PGPPublicKeyRing keyRing : store.get(keyId(fp))) {
             if (Arrays.equals(keyRing.getPublicKey().getFingerprint(), fp)) {

java/com/google/gerrit/gpg/server/PostGpgKeys.java

@@ -18,14 +18,12 @@ import static com.google.gerrit.gpg.PublicKeyStore.keyIdToString;
 import static com.google.gerrit.gpg.PublicKeyStore.keyToString;
 import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_GPGKEY;
 import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.stream.Collectors.toList;
 
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
-import com.google.common.collect.Sets;
 import com.google.common.io.BaseEncoding;
 import com.google.gerrit.common.errors.EmailException;
 import com.google.gerrit.extensions.api.accounts.GpgKeysInput;
@@ -45,9 +43,9 @@ import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountResource;
 import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.account.AccountsUpdate;
 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.mail.send.AddKeySender;
 import com.google.gerrit.server.query.account.InternalAccountQuery;
 import com.google.gwtorm.server.OrmException;
@@ -61,7 +59,6 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import org.bouncycastle.bcpg.ArmoredInputStream;
 import org.bouncycastle.openpgp.PGPException;
 import org.bouncycastle.openpgp.PGPPublicKey;
@@ -85,7 +82,7 @@ public class PostGpgKeys implements RestModifyView<AccountResource, GpgKeysInput
   private final AddKeySender.Factory addKeyFactory;
   private final Provider<InternalAccountQuery> accountQueryProvider;
   private final ExternalIds externalIds;
-  private final ExternalIdsUpdate.User externalIdsUpdateFactory;
+  private final AccountsUpdate.User accountsUpdateFactory;
 
   @Inject
   PostGpgKeys(
@@ -96,7 +93,7 @@ public class PostGpgKeys implements RestModifyView<AccountResource, GpgKeysInput
       AddKeySender.Factory addKeyFactory,
       Provider<InternalAccountQuery> accountQueryProvider,
       ExternalIds externalIds,
-      ExternalIdsUpdate.User externalIdsUpdateFactory) {
+      AccountsUpdate.User accountsUpdateFactory) {
     this.serverIdent = serverIdent;
     this.self = self;
     this.storeProvider = storeProvider;
@@ -104,7 +101,7 @@ public class PostGpgKeys implements RestModifyView<AccountResource, GpgKeysInput
     this.addKeyFactory = addKeyFactory;
     this.accountQueryProvider = accountQueryProvider;
     this.externalIds = externalIds;
-    this.externalIdsUpdateFactory = externalIdsUpdateFactory;
+    this.accountsUpdateFactory = accountsUpdateFactory;
   }
 
   @Override
@@ -116,8 +113,9 @@ public class PostGpgKeys implements RestModifyView<AccountResource, GpgKeysInput
     Collection<ExternalId> existingExtIds =
         externalIds.byAccount(rsrc.getUser().getAccountId(), SCHEME_GPGKEY);
     try (PublicKeyStore store = storeProvider.get()) {
-      Set<Fingerprint> toRemove = readKeysToRemove(input, existingExtIds);
+      Map<ExternalId, Fingerprint> toRemove = readKeysToRemove(input, existingExtIds);
-      List<PGPPublicKeyRing> newKeys = readKeysToAdd(input, toRemove);
+      Collection<Fingerprint> fingerprintsToRemove = toRemove.values();
+      List<PGPPublicKeyRing> newKeys = readKeysToAdd(input, fingerprintsToRemove);
       List<ExternalId> newExtIds = new ArrayList<>(existingExtIds.size());
 
       for (PGPPublicKeyRing keyRing : newKeys) {
@@ -133,26 +131,29 @@ public class PostGpgKeys implements RestModifyView<AccountResource, GpgKeysInput
         }
       }
 
-      storeKeys(rsrc, newKeys, toRemove);
+      storeKeys(rsrc, newKeys, fingerprintsToRemove);
 
-      List<ExternalId.Key> extIdKeysToRemove =
+      accountsUpdateFactory
-          toRemove.stream().map(fp -> toExtIdKey(fp.get())).collect(toList());
-      externalIdsUpdateFactory
           .create()
-          .replace(rsrc.getUser().getAccountId(), extIdKeysToRemove, newExtIds);
+          .update(
-      return toJson(newKeys, toRemove, store, rsrc.getUser());
+              "Update GPG Keys via API",
+              rsrc.getUser().getAccountId(),
+              u -> u.replaceExternalIds(toRemove.keySet(), newExtIds));
+      return toJson(newKeys, fingerprintsToRemove, store, rsrc.getUser());
     }
   }
 
-  private Set<Fingerprint> readKeysToRemove(
+  private Map<ExternalId, Fingerprint> readKeysToRemove(
       GpgKeysInput input, Collection<ExternalId> existingExtIds) {
     if (input.delete == null || input.delete.isEmpty()) {
-      return ImmutableSet.of();
+      return ImmutableMap.of();
     }
-    Set<Fingerprint> fingerprints = Sets.newHashSetWithExpectedSize(input.delete.size());
+    Map<ExternalId, Fingerprint> fingerprints =
+        Maps.newHashMapWithExpectedSize(input.delete.size());
     for (String id : input.delete) {
       try {
-        fingerprints.add(new Fingerprint(GpgKeys.parseFingerprint(id, existingExtIds)));
+        ExternalId gpgKeyExtId = GpgKeys.findGpgKey(id, existingExtIds);
+        fingerprints.put(gpgKeyExtId, new Fingerprint(GpgKeys.parseFingerprint(gpgKeyExtId)));
       } catch (ResourceNotFoundException e) {
         // Skip removal.
       }
@@ -160,7 +161,7 @@ public class PostGpgKeys implements RestModifyView<AccountResource, GpgKeysInput
     return fingerprints;
   }
 
-  private List<PGPPublicKeyRing> readKeysToAdd(GpgKeysInput input, Set<Fingerprint> toRemove)
+  private List<PGPPublicKeyRing> readKeysToAdd(GpgKeysInput input, Collection<Fingerprint> toRemove)
       throws BadRequestException, IOException {
     if (input.add == null || input.add.isEmpty()) {
       return ImmutableList.of();
@@ -188,7 +189,7 @@ public class PostGpgKeys implements RestModifyView<AccountResource, GpgKeysInput
   }
 
   private void storeKeys(
-      AccountResource rsrc, List<PGPPublicKeyRing> keyRings, Set<Fingerprint> toRemove)
+      AccountResource rsrc, List<PGPPublicKeyRing> keyRings, Collection<Fingerprint> toRemove)
       throws BadRequestException, ResourceConflictException, PGPException, IOException {
     try (PublicKeyStore store = storeProvider.get()) {
       List<String> addedKeys = new ArrayList<>();
@@ -269,7 +270,7 @@ public class PostGpgKeys implements RestModifyView<AccountResource, GpgKeysInput
 
   private Map<String, GpgKeyInfo> toJson(
       Collection<PGPPublicKeyRing> keys,
-      Set<Fingerprint> deleted,
+      Collection<Fingerprint> deleted,
       PublicKeyStore store,
       IdentifiedUser user)
       throws IOException {

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

@@ -18,20 +18,14 @@ import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 
 import com.google.common.base.Strings;
-import com.google.common.collect.ImmutableList;
-import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.git.MetaDataUpdate;
-import com.google.gerrit.server.git.ValidationError;
 import com.google.gerrit.server.git.VersionedMetaData;
-import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
 import com.google.gwtorm.server.OrmDuplicateKeyException;
 import java.io.IOException;
 import java.sql.Timestamp;
-import java.util.ArrayList;
-import java.util.List;
 import java.util.Optional;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.CommitBuilder;
@@ -67,7 +61,7 @@ import org.eclipse.jgit.revwalk.RevSort;
  * account. The first commit may be an empty commit (if no properties were set and 'account.config'
  * doesn't exist).
  */
-public class AccountConfig extends VersionedMetaData implements ValidationError.Sink {
+public class AccountConfig extends VersionedMetaData {
   public static final String ACCOUNT_CONFIG = "account.config";
   public static final String ACCOUNT = "account";
   public static final String KEY_ACTIVE = "active";
@@ -75,17 +69,14 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
   public static final String KEY_PREFERRED_EMAIL = "preferredEmail";
   public static final String KEY_STATUS = "status";
 
-  @Nullable private final OutgoingEmailValidator emailValidator;
   private final Account.Id accountId;
   private final String ref;
 
   private Optional<Account> loadedAccount;
   private Optional<InternalAccountUpdate> accountUpdate = Optional.empty();
   private Timestamp registeredOn;
-  private List<ValidationError> validationErrors;
 
-  public AccountConfig(@Nullable OutgoingEmailValidator emailValidator, Account.Id accountId) {
+  public AccountConfig(Account.Id accountId) {
-    this.emailValidator = emailValidator;
     this.accountId = checkNotNull(accountId);
     this.ref = RefNames.refsUsers(accountId);
   }
@@ -182,11 +173,6 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
 
     String preferredEmail = get(cfg, KEY_PREFERRED_EMAIL);
     account.setPreferredEmail(preferredEmail);
-    if (emailValidator != null && !emailValidator.isValid(preferredEmail)) {
-      error(
-          new ValidationError(
-              ACCOUNT_CONFIG, String.format("Invalid preferred email: %s", preferredEmail)));
-    }
 
     account.setStatus(get(cfg, KEY_STATUS));
     account.setMetaId(metaId);
@@ -296,24 +282,4 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
   private void checkLoaded() {
     checkState(loadedAccount != null, "Account %s not loaded yet", accountId.get());
   }
-
-  /**
-   * Get the validation errors, if any were discovered during load.
-   *
-   * @return list of errors; empty list if there are no errors.
-   */
-  public List<ValidationError> getValidationErrors() {
-    if (validationErrors != null) {
-      return ImmutableList.copyOf(validationErrors);
-    }
-    return ImmutableList.of();
-  }
-
-  @Override
-  public void error(ValidationError error) {
-    if (validationErrors == null) {
-      validationErrors = new ArrayList<>(4);
-    }
-    validationErrors.add(error);
-  }
 }

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

@@ -14,6 +14,9 @@
 
 package com.google.gerrit.server.account;
 
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME;
+
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
@@ -21,7 +24,6 @@ import com.google.common.collect.Sets;
 import com.google.gerrit.common.data.AccessSection;
 import com.google.gerrit.common.data.GlobalCapability;
 import com.google.gerrit.common.data.Permission;
-import com.google.gerrit.common.errors.NameAlreadyUsedException;
 import com.google.gerrit.common.errors.NoSuchGroupException;
 import com.google.gerrit.extensions.client.AccountFieldName;
 import com.google.gerrit.reviewdb.client.Account;
@@ -33,12 +35,12 @@ 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;
 import com.google.gerrit.server.auth.NoSuchUserException;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.group.db.GroupsUpdate;
 import com.google.gerrit.server.group.db.InternalGroupUpdate;
 import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.ssh.SshKeyCache;
 import com.google.gwtorm.server.OrmException;
 import com.google.gwtorm.server.SchemaFactory;
 import com.google.inject.Inject;
@@ -67,11 +69,10 @@ public class AccountManager {
   private final AccountCache byIdCache;
   private final Realm realm;
   private final IdentifiedUser.GenericFactory userFactory;
-  private final ChangeUserName.Factory changeUserNameFactory;
+  private final SshKeyCache sshKeyCache;
   private final ProjectCache projectCache;
   private final AtomicBoolean awaitsFirstAccountCheck;
   private final ExternalIds externalIds;
-  private final ExternalIdsUpdate.Server externalIdsUpdateFactory;
   private final GroupsUpdate.Factory groupsUpdateFactory;
   private final boolean autoUpdateAccountActiveStatus;
   private final SetInactiveFlag setInactiveFlag;
@@ -86,10 +87,9 @@ public class AccountManager {
       AccountCache byIdCache,
       Realm accountMapper,
       IdentifiedUser.GenericFactory userFactory,
-      ChangeUserName.Factory changeUserNameFactory,
+      SshKeyCache sshKeyCache,
       ProjectCache projectCache,
       ExternalIds externalIds,
-      ExternalIdsUpdate.Server externalIdsUpdateFactory,
       GroupsUpdate.Factory groupsUpdateFactory,
       SetInactiveFlag setInactiveFlag) {
     this.schema = schema;
@@ -99,12 +99,11 @@ public class AccountManager {
     this.byIdCache = byIdCache;
     this.realm = accountMapper;
     this.userFactory = userFactory;
-    this.changeUserNameFactory = changeUserNameFactory;
+    this.sshKeyCache = sshKeyCache;
     this.projectCache = projectCache;
     this.awaitsFirstAccountCheck =
         new AtomicBoolean(cfg.getBoolean("capability", "makeFirstUserAdmin", true));
     this.externalIds = externalIds;
-    this.externalIdsUpdateFactory = externalIdsUpdateFactory;
     this.groupsUpdateFactory = groupsUpdateFactory;
     this.autoUpdateAccountActiveStatus =
         cfg.getBoolean("auth", "autoUpdateAccountActiveStatus", false);
@@ -262,6 +261,8 @@ public class AccountManager {
 
     ExternalId extId =
         ExternalId.createWithEmail(who.getExternalIdKey(), newId, who.getEmailAddress());
+    ExternalId userNameExtId =
+        !Strings.isNullOrEmpty(who.getUserName()) ? createUsername(newId, who.getUserName()) : null;
 
     boolean isFirstAccount = awaitsFirstAccountCheck.getAndSet(false) && !accounts.hasAnyAccount();
 
@@ -273,10 +274,14 @@ public class AccountManager {
               .insert(
                   "Create Account on First Login",
                   newId,
-                  u ->
+                  u -> {
-                      u.setFullName(who.getDisplayName())
+                    u.setFullName(who.getDisplayName())
-                          .setPreferredEmail(extId.email())
+                        .setPreferredEmail(extId.email())
-                          .addExternalId(extId));
+                        .addExternalId(extId);
+                    if (userNameExtId != null) {
+                      u.addExternalId(userNameExtId);
+                    }
+                  });
     } catch (DuplicateExternalIdKeyException e) {
       throw new AccountException(
           "Cannot assign external ID \""
@@ -291,6 +296,10 @@ public class AccountManager {
       awaitsFirstAccountCheck.set(isFirstAccount);
     }
 
+    if (userNameExtId != null) {
+      sshKeyCache.evict(who.getUserName());
+    }
+
     IdentifiedUser user = userFactory.create(newId);
 
     if (isFirstAccount) {
@@ -309,37 +318,23 @@ public class AccountManager {
       addGroupMember(db, adminGroupUuid, user);
     }
 
-    if (who.getUserName() != null) {
-      // Only set if the name hasn't been used yet, but was given to us.
-      //
-      try {
-        changeUserNameFactory.create("Set Username on Login", user, who.getUserName()).call();
-      } catch (NameAlreadyUsedException e) {
-        String message =
-            "Cannot assign user name \""
-                + who.getUserName()
-                + "\" to account "
-                + newId
-                + "; name already in use.";
-        handleSettingUserNameFailure(account, extId, message, e, false);
-      } catch (InvalidUserNameException e) {
-        String message =
-            "Cannot assign user name \""
-                + who.getUserName()
-                + "\" to account "
-                + newId
-                + "; name does not conform.";
-        handleSettingUserNameFailure(account, extId, message, e, false);
-      } catch (OrmException e) {
-        String message = "Cannot assign user name";
-        handleSettingUserNameFailure(account, extId, message, e, true);
-      }
-    }
-
     realm.onCreateAccount(who, account);
     return new AuthResult(newId, extId.key(), true);
   }
 
+  private ExternalId createUsername(Account.Id accountId, String username)
+      throws AccountUserNameException {
+    checkArgument(!Strings.isNullOrEmpty(username));
+
+    if (!ExternalId.isValidUsername(username)) {
+      throw new AccountUserNameException(
+          String.format(
+              "Cannot assign user name \"%s\" to account %s; name does not conform.",
+              username, accountId));
+    }
+    return ExternalId.create(SCHEME_USERNAME, username, accountId);
+  }
+
   private void addGroupMember(ReviewDb db, AccountGroup.UUID groupUuid, IdentifiedUser user)
       throws OrmException, IOException, ConfigInvalidException, AccountException {
     // The user initiated this request by logging in. -> Attribute all modifications to that user.
@@ -356,43 +351,6 @@ public class AccountManager {
     }
   }
 
-  /**
-   * This method handles an exception that occurred during the setting of the user name for a newly
-   * created account. If the realm does not allow the user to set a user name manually this method
-   * deletes the newly created account and throws an {@link AccountUserNameException}. In any case
-   * the error message is logged.
-   *
-   * @param account the newly created account
-   * @param extId the newly created external id
-   * @param errorMessage the error message
-   * @param e the exception that occurred during the setting of the user name for the new account
-   * @param logException flag that decides whether the exception should be included into the log
-   * @throws AccountUserNameException thrown if the realm does not allow the user to manually set
-   *     the user name
-   * @throws OrmException thrown if cleaning the database failed
-   */
-  private void handleSettingUserNameFailure(
-      Account account, ExternalId extId, String errorMessage, Exception e, boolean logException)
-      throws AccountUserNameException, OrmException, IOException, ConfigInvalidException {
-    if (logException) {
-      log.error(errorMessage, e);
-    } else {
-      log.error(errorMessage);
-    }
-    if (!realm.allowsEdit(AccountFieldName.USER_NAME)) {
-      // setting the given user name has failed, but the realm does not
-      // allow the user to manually set a user name,
-      // this means we would end with an account without user name
-      // (without 'username:<USERNAME>' external ID),
-      // such an account cannot be used for uploading changes,
-      // this is why the best we can do here is to fail early and cleanup
-      // the database
-      accountsUpdateFactory.create().delete(account);
-      externalIdsUpdateFactory.create().delete(extId);
-      throw new AccountUserNameException(errorMessage, e);
-    }
-  }
-
   /**
    * Link another authentication identity to an existing account.
    *
@@ -453,7 +411,12 @@ public class AccountManager {
                 .filter(e -> e.key().equals(who.getExternalIdKey()))
                 .findAny()
                 .isPresent())) {
-      externalIdsUpdateFactory.create().delete(filteredExtIdsByScheme);
+      accountsUpdateFactory
+          .create()
+          .update(
+              "Delete External IDs on Update Link",
+              to,
+              u -> u.deleteExternalIds(filteredExtIdsByScheme));
     }
     return link(to, who);
   }
@@ -498,27 +461,17 @@ public class AccountManager {
       }
     }
 
-    externalIdsUpdateFactory.create().delete(extIds);
+    accountsUpdateFactory
-
+        .create()
-    if (extIds.stream().anyMatch(e -> e.email() != null)) {
+        .update(
-      accountsUpdateFactory
+            "Unlink External ID" + (extIds.size() > 1 ? "s" : ""),
-          .create()
+            from,
-          .update(
+            (a, u) -> {
-              "Clear Preferred Email on Unlinking External ID\n"
+              u.deleteExternalIds(extIds);
-                  + "\n"
+              if (a.getPreferredEmail() != null
-                  + "The preferred email is cleared because the corresponding external ID\n"
+                  && extIds.stream().anyMatch(e -> a.getPreferredEmail().equals(e.email()))) {
-                  + "was removed.",
+                u.setPreferredEmail(null);
-              from,
+              }
-              (a, u) -> {
+            });
-                if (a.getPreferredEmail() != null) {
-                  for (ExternalId extId : extIds) {
-                    if (a.getPreferredEmail().equals(extId.email())) {
-                      u.setPreferredEmail(null);
-                      break;
-                    }
-                  }
-                }
-              });
-    }
   }
 }

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

@@ -21,6 +21,10 @@ package com.google.gerrit.server.account;
 public class AccountUserNameException extends AccountException {
   private static final long serialVersionUID = 1L;
 
+  public AccountUserNameException(String message) {
+    super(message);
+  }
+
   public AccountUserNameException(String message, Throwable why) {
     super(message, why);
   }

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

@@ -23,7 +23,6 @@ import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 import java.io.IOException;
@@ -46,16 +45,11 @@ public class Accounts {
 
   private final GitRepositoryManager repoManager;
   private final AllUsersName allUsersName;
-  private final OutgoingEmailValidator emailValidator;
 
   @Inject
-  Accounts(
+  Accounts(GitRepositoryManager repoManager, AllUsersName allUsersName) {
-      GitRepositoryManager repoManager,
-      AllUsersName allUsersName,
-      OutgoingEmailValidator emailValidator) {
     this.repoManager = repoManager;
     this.allUsersName = allUsersName;
-    this.emailValidator = emailValidator;
   }
 
   @Nullable
@@ -138,7 +132,7 @@ public class Accounts {
 
   private Optional<Account> read(Repository allUsersRepository, Account.Id accountId)
       throws IOException, ConfigInvalidException {
-    AccountConfig accountConfig = new AccountConfig(emailValidator, accountId);
+    AccountConfig accountConfig = new AccountConfig(accountId);
     accountConfig.load(allUsersRepository);
     return accountConfig.getLoadedAccount();
   }

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

@@ -24,8 +24,6 @@ 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;
@@ -35,7 +33,6 @@ 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.gerrit.server.update.RetryHelper.ActionType;
@@ -51,11 +48,7 @@ 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;
-import org.eclipse.jgit.lib.RefUpdate;
-import org.eclipse.jgit.lib.RefUpdate.Result;
 import org.eclipse.jgit.lib.Repository;
 
 /**
@@ -119,7 +112,6 @@ public class AccountsUpdate {
     private final GitRepositoryManager repoManager;
     private final GitReferenceUpdated gitRefUpdated;
     private final AllUsersName allUsersName;
-    private final OutgoingEmailValidator emailValidator;
     private final Provider<PersonIdent> serverIdentProvider;
     private final Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory;
     private final RetryHelper retryHelper;
@@ -130,7 +122,6 @@ public class AccountsUpdate {
         GitRepositoryManager repoManager,
         GitReferenceUpdated gitRefUpdated,
         AllUsersName allUsersName,
-        OutgoingEmailValidator emailValidator,
         @GerritPersonIdent Provider<PersonIdent> serverIdentProvider,
         Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory,
         RetryHelper retryHelper,
@@ -138,7 +129,6 @@ public class AccountsUpdate {
       this.repoManager = repoManager;
       this.gitRefUpdated = gitRefUpdated;
       this.allUsersName = allUsersName;
-      this.emailValidator = emailValidator;
       this.serverIdentProvider = serverIdentProvider;
       this.metaDataUpdateInternalFactory = metaDataUpdateInternalFactory;
       this.retryHelper = retryHelper;
@@ -152,7 +142,6 @@ public class AccountsUpdate {
           gitRefUpdated,
           null,
           allUsersName,
-          emailValidator,
           metaDataUpdateInternalFactory,
           retryHelper,
           extIdNotesFactory,
@@ -175,7 +164,6 @@ public class AccountsUpdate {
     private final GitRepositoryManager repoManager;
     private final GitReferenceUpdated gitRefUpdated;
     private final AllUsersName allUsersName;
-    private final OutgoingEmailValidator emailValidator;
     private final Provider<PersonIdent> serverIdentProvider;
     private final Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory;
     private final RetryHelper retryHelper;
@@ -186,7 +174,6 @@ public class AccountsUpdate {
         GitRepositoryManager repoManager,
         GitReferenceUpdated gitRefUpdated,
         AllUsersName allUsersName,
-        OutgoingEmailValidator emailValidator,
         @GerritPersonIdent Provider<PersonIdent> serverIdentProvider,
         Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory,
         RetryHelper retryHelper,
@@ -194,7 +181,6 @@ public class AccountsUpdate {
       this.repoManager = repoManager;
       this.gitRefUpdated = gitRefUpdated;
       this.allUsersName = allUsersName;
-      this.emailValidator = emailValidator;
       this.serverIdentProvider = serverIdentProvider;
       this.metaDataUpdateInternalFactory = metaDataUpdateInternalFactory;
       this.retryHelper = retryHelper;
@@ -208,7 +194,6 @@ public class AccountsUpdate {
           gitRefUpdated,
           null,
           allUsersName,
-          emailValidator,
           metaDataUpdateInternalFactory,
           retryHelper,
           extIdNotesFactory,
@@ -228,7 +213,6 @@ public class AccountsUpdate {
     private final GitRepositoryManager repoManager;
     private final GitReferenceUpdated gitRefUpdated;
     private final AllUsersName allUsersName;
-    private final OutgoingEmailValidator emailValidator;
     private final Provider<PersonIdent> serverIdentProvider;
     private final Provider<IdentifiedUser> identifiedUser;
     private final Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory;
@@ -240,7 +224,6 @@ public class AccountsUpdate {
         GitRepositoryManager repoManager,
         GitReferenceUpdated gitRefUpdated,
         AllUsersName allUsersName,
-        OutgoingEmailValidator emailValidator,
         @GerritPersonIdent Provider<PersonIdent> serverIdentProvider,
         Provider<IdentifiedUser> identifiedUser,
         Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory,
@@ -250,7 +233,6 @@ public class AccountsUpdate {
       this.gitRefUpdated = gitRefUpdated;
       this.allUsersName = allUsersName;
       this.serverIdentProvider = serverIdentProvider;
-      this.emailValidator = emailValidator;
       this.identifiedUser = identifiedUser;
       this.metaDataUpdateInternalFactory = metaDataUpdateInternalFactory;
       this.retryHelper = retryHelper;
@@ -266,7 +248,6 @@ public class AccountsUpdate {
           gitRefUpdated,
           user,
           allUsersName,
-          emailValidator,
           metaDataUpdateInternalFactory,
           retryHelper,
           extIdNotesFactory,
@@ -283,7 +264,6 @@ public class AccountsUpdate {
   private final GitReferenceUpdated gitRefUpdated;
   @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 ExternalIdNotesLoader extIdNotesLoader;
@@ -296,7 +276,6 @@ public class AccountsUpdate {
       GitReferenceUpdated gitRefUpdated,
       @Nullable IdentifiedUser currentUser,
       AllUsersName allUsersName,
-      OutgoingEmailValidator emailValidator,
       Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory,
       RetryHelper retryHelper,
       ExternalIdNotesLoader extIdNotesLoader,
@@ -307,7 +286,6 @@ public class AccountsUpdate {
         gitRefUpdated,
         currentUser,
         allUsersName,
-        emailValidator,
         metaDataUpdateInternalFactory,
         retryHelper,
         extIdNotesLoader,
@@ -322,7 +300,6 @@ public class AccountsUpdate {
       GitReferenceUpdated gitRefUpdated,
       @Nullable IdentifiedUser currentUser,
       AllUsersName allUsersName,
-      OutgoingEmailValidator emailValidator,
       Provider<MetaDataUpdate.InternalFactory> metaDataUpdateInternalFactory,
       RetryHelper retryHelper,
       ExternalIdNotesLoader extIdNotesLoader,
@@ -333,7 +310,6 @@ public class AccountsUpdate {
     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");
@@ -446,77 +422,9 @@ public class AccountsUpdate {
         });
   }
 
-  /**
-   * Deletes the account.
-   *
-   * @param account the account that should be deleted
-   * @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, OrmException, ConfigInvalidException {
-    deleteByKey(account.getId());
-  }
-
-  /**
-   * Deletes the account.
-   *
-   * @param accountId the ID of the account that should be deleted
-   * @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, OrmException, ConfigInvalidException {
-    deleteAccount(accountId);
-  }
-
-  private Account deleteAccount(Account.Id accountId)
-      throws IOException, OrmException, ConfigInvalidException {
-    return retryHelper.execute(
-        ActionType.ACCOUNT_UPDATE,
-        () -> {
-          deleteUserBranch(accountId);
-          return null;
-        });
-  }
-
-  private void deleteUserBranch(Account.Id accountId) throws IOException {
-    try (Repository repo = repoManager.openRepository(allUsersName)) {
-      deleteUserBranch(repo, allUsersName, gitRefUpdated, currentUser, authorIdent, accountId);
-    }
-  }
-
-  public static void deleteUserBranch(
-      Repository repo,
-      Project.NameKey project,
-      GitReferenceUpdated gitRefUpdated,
-      @Nullable IdentifiedUser user,
-      PersonIdent refLogIdent,
-      Account.Id accountId)
-      throws IOException {
-    String refName = RefNames.refsUsers(accountId);
-    Ref ref = repo.exactRef(refName);
-    if (ref == null) {
-      return;
-    }
-
-    RefUpdate ru = repo.updateRef(refName);
-    ru.setExpectedOldObjectId(ref.getObjectId());
-    ru.setNewObjectId(ObjectId.zeroId());
-    ru.setForceUpdate(true);
-    ru.setRefLogIdent(refLogIdent);
-    ru.setRefLogMessage("Delete Account", true);
-    Result result = ru.delete();
-    if (result != Result.FORCED) {
-      throw new IOException(String.format("Failed to delete ref %s: %s", refName, result.name()));
-    }
-    gitRefUpdated.fire(project, ru, user != null ? user.getAccount() : null);
-  }
-
   private AccountConfig read(Repository allUsersRepo, Account.Id accountId)
       throws IOException, ConfigInvalidException {
-    AccountConfig accountConfig = new AccountConfig(emailValidator, accountId);
+    AccountConfig accountConfig = new AccountConfig(accountId);
     accountConfig.load(allUsersRepo);
 
     afterReadRevision.run();

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

@@ -1,112 +0,0 @@
-// Copyright (C) 2009 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;
-
-import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME;
-
-import com.google.gerrit.common.Nullable;
-import com.google.gerrit.common.errors.NameAlreadyUsedException;
-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.ssh.SshKeyCache;
-import com.google.gwtjsonrpc.common.VoidResult;
-import com.google.gwtorm.server.OrmDuplicateKeyException;
-import com.google.gwtorm.server.OrmException;
-import com.google.inject.Inject;
-import com.google.inject.assistedinject.Assisted;
-import java.io.IOException;
-import java.util.concurrent.Callable;
-import java.util.regex.Pattern;
-import org.eclipse.jgit.errors.ConfigInvalidException;
-
-/** Operation to change the username of an account. */
-public class ChangeUserName implements Callable<VoidResult> {
-  public static final String USERNAME_CANNOT_BE_CHANGED = "Username cannot be changed.";
-
-  private static final Pattern USER_NAME_PATTERN = Pattern.compile(Account.USER_NAME_PATTERN);
-
-  /** Generic factory to change any user's username. */
-  public interface Factory {
-    ChangeUserName create(
-        @Assisted("message") String message,
-        IdentifiedUser user,
-        @Assisted("newUsername") String newUsername);
-  }
-
-  private final SshKeyCache sshKeyCache;
-  private final ExternalIds externalIds;
-  private final AccountsUpdate.Server accountsUpdate;
-
-  private final String message;
-  private final IdentifiedUser user;
-  private final String newUsername;
-
-  @Inject
-  ChangeUserName(
-      SshKeyCache sshKeyCache,
-      ExternalIds externalIds,
-      AccountsUpdate.Server accountsUpdate,
-      @Assisted("message") String message,
-      @Assisted IdentifiedUser user,
-      @Nullable @Assisted("newUsername") String newUsername) {
-    this.sshKeyCache = sshKeyCache;
-    this.externalIds = externalIds;
-    this.accountsUpdate = accountsUpdate;
-    this.message = message;
-    this.user = user;
-    this.newUsername = newUsername;
-  }
-
-  @Override
-  public VoidResult call()
-      throws OrmException, NameAlreadyUsedException, InvalidUserNameException, IOException,
-          ConfigInvalidException {
-    if (!externalIds.byAccount(user.getAccountId(), SCHEME_USERNAME).isEmpty()) {
-      throw new IllegalStateException(USERNAME_CANNOT_BE_CHANGED);
-    }
-
-    if (newUsername != null && !newUsername.isEmpty()) {
-      if (!USER_NAME_PATTERN.matcher(newUsername).matches()) {
-        throw new InvalidUserNameException();
-      }
-
-      ExternalId.Key key = ExternalId.Key.create(SCHEME_USERNAME, newUsername);
-      try {
-        accountsUpdate
-            .create()
-            .update(
-                message,
-                user.getAccountId(),
-                u -> u.addExternalId(ExternalId.create(key, user.getAccountId(), null, null)));
-      } catch (OrmDuplicateKeyException dupeErr) {
-        // If we are using this identity, don't report the exception.
-        //
-        ExternalId other = externalIds.get(key);
-        if (other != null && other.accountId().equals(user.getAccountId())) {
-          return VoidResult.INSTANCE;
-        }
-
-        // Otherwise, someone else has this identity.
-        //
-        throw new NameAlreadyUsedException(newUsername);
-      }
-    }
-
-    sshKeyCache.evict(newUsername);
-    return VoidResult.INSTANCE;
-  }
-}

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

@@ -33,6 +33,7 @@ import java.io.Serializable;
 import java.util.Collection;
 import java.util.Objects;
 import java.util.Set;
+import java.util.regex.Pattern;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.Constants;
@@ -40,6 +41,12 @@ import org.eclipse.jgit.lib.ObjectId;
 
 @AutoValue
 public abstract class ExternalId implements Serializable {
+  private static final Pattern USER_NAME_PATTERN = Pattern.compile(Account.USER_NAME_PATTERN);
+
+  public static boolean isValidUsername(String username) {
+    return USER_NAME_PATTERN.matcher(username).matches();
+  }
+
   private static final long serialVersionUID = 1L;
 
   private static final String EXTERNAL_ID_SECTION = "externalId";

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

@@ -1,460 +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 static com.google.common.base.Preconditions.checkNotNull;
-
-import com.google.common.annotations.VisibleForTesting;
-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.server.account.AccountCache;
-import com.google.gerrit.server.config.AllUsersName;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.MetaDataUpdate;
-import com.google.gerrit.server.update.RetryHelper;
-import com.google.gerrit.server.update.RetryHelper.ActionType;
-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.util.Collection;
-import java.util.Collections;
-import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.Repository;
-
-/**
- * Updates externalIds in ReviewDb and NoteDb.
- *
- * <p>In NoteDb external IDs are stored in the All-Users repository in a Git Notes branch called
- * refs/meta/external-ids where the sha1 of the external ID is used as note name. Each note content
- * is a git config file that contains an external ID. It has exactly one externalId subsection with
- * an accountId and optionally email and password:
- *
- * <pre>
- * [externalId "username:jdoe"]
- *   accountId = 1003407
- *   email = jdoe@example.com
- *   password = bcrypt:4:LCbmSBDivK/hhGVQMfkDpA==:XcWn0pKYSVU/UJgOvhidkEtmqCp6oKB7
- * </pre>
- *
- * For NoteDb each method call results in one commit on refs/meta/external-ids branch.
- *
- * <p>On updating external IDs this class takes care to evict affected accounts from the account
- * cache and thus triggers reindex for them.
- */
-public class ExternalIdsUpdate {
-  /**
-   * Factory to create an ExternalIdsUpdate instance for updating external IDs by the Gerrit server.
-   *
-   * <p>The Gerrit server identity will be used as author and committer for all commits that update
-   * the external IDs.
-   */
-  @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 RetryHelper retryHelper;
-
-    @Inject
-    public Server(
-        GitRepositoryManager repoManager,
-        Provider<MetaDataUpdate.Server> metaDataUpdateServerFactory,
-        AccountCache accountCache,
-        AllUsersName allUsersName,
-        MetricMaker metricMaker,
-        ExternalIds externalIds,
-        ExternalIdCache externalIdCache,
-        RetryHelper retryHelper) {
-      this.repoManager = repoManager;
-      this.metaDataUpdateServerFactory = metaDataUpdateServerFactory;
-      this.accountCache = accountCache;
-      this.allUsersName = allUsersName;
-      this.metricMaker = metricMaker;
-      this.externalIds = externalIds;
-      this.externalIdCache = externalIdCache;
-      this.retryHelper = retryHelper;
-    }
-
-    public ExternalIdsUpdate create() {
-      return new ExternalIdsUpdate(
-          repoManager,
-          () -> metaDataUpdateServerFactory.get().create(allUsersName),
-          accountCache,
-          allUsersName,
-          metricMaker,
-          externalIds,
-          externalIdCache,
-          retryHelper);
-    }
-  }
-
-  /**
-   * Factory to create an ExternalIdsUpdate instance for updating external IDs by the Gerrit server.
-   *
-   * <p>Using this class no reindex will be performed for the affected accounts and they will also
-   * not be evicted from the account cache.
-   *
-   * <p>The Gerrit server identity will be used as author and committer for all commits that update
-   * the external IDs.
-   */
-  @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 RetryHelper retryHelper;
-
-    @Inject
-    public ServerNoReindex(
-        GitRepositoryManager repoManager,
-        Provider<MetaDataUpdate.Server> metaDataUpdateServerFactory,
-        AllUsersName allUsersName,
-        MetricMaker metricMaker,
-        ExternalIds externalIds,
-        ExternalIdCache externalIdCache,
-        RetryHelper retryHelper) {
-      this.repoManager = repoManager;
-      this.metaDataUpdateServerFactory = metaDataUpdateServerFactory;
-      this.allUsersName = allUsersName;
-      this.metricMaker = metricMaker;
-      this.externalIds = externalIds;
-      this.externalIdCache = externalIdCache;
-      this.retryHelper = retryHelper;
-    }
-
-    public ExternalIdsUpdate create() {
-      return new ExternalIdsUpdate(
-          repoManager,
-          () -> metaDataUpdateServerFactory.get().create(allUsersName),
-          null,
-          allUsersName,
-          metricMaker,
-          externalIds,
-          externalIdCache,
-          retryHelper);
-    }
-  }
-
-  /**
-   * Factory to create an ExternalIdsUpdate instance for updating external IDs by the current user.
-   *
-   * <p>The identity of the current user will be used as author for all commits that update the
-   * external IDs. The Gerrit server identity will be used as committer.
-   */
-  @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 RetryHelper retryHelper;
-
-    @Inject
-    public User(
-        GitRepositoryManager repoManager,
-        Provider<MetaDataUpdate.User> metaDataUpdateUserFactory,
-        AccountCache accountCache,
-        AllUsersName allUsersName,
-        MetricMaker metricMaker,
-        ExternalIds externalIds,
-        ExternalIdCache externalIdCache,
-        RetryHelper retryHelper) {
-      this.repoManager = repoManager;
-      this.metaDataUpdateUserFactory = metaDataUpdateUserFactory;
-      this.accountCache = accountCache;
-      this.allUsersName = allUsersName;
-      this.metricMaker = metricMaker;
-      this.externalIds = externalIds;
-      this.externalIdCache = externalIdCache;
-      this.retryHelper = retryHelper;
-    }
-
-    public ExternalIdsUpdate create() {
-      return new ExternalIdsUpdate(
-          repoManager,
-          () -> metaDataUpdateUserFactory.get().create(allUsersName),
-          accountCache,
-          allUsersName,
-          metricMaker,
-          externalIds,
-          externalIdCache,
-          retryHelper);
-    }
-  }
-
-  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 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,
-      RetryHelper retryHelper) {
-    this(
-        repoManager,
-        metaDataUpdateFactory,
-        accountCache,
-        allUsersName,
-        metricMaker,
-        externalIds,
-        externalIdCache,
-        retryHelper,
-        Runnables.doNothing());
-  }
-
-  @VisibleForTesting
-  public ExternalIdsUpdate(
-      GitRepositoryManager repoManager,
-      MetaDataUpdateFactory metaDataUpdateFactory,
-      @Nullable AccountCache accountCache,
-      AllUsersName allUsersName,
-      MetricMaker metricMaker,
-      ExternalIds externalIds,
-      ExternalIdCache externalIdCache,
-      RetryHelper retryHelper,
-      Runnable afterReadRevision) {
-    this.repoManager = checkNotNull(repoManager, "repoManager");
-    this.metaDataUpdateFactory = checkNotNull(metaDataUpdateFactory, "metaDataUpdateFactory");
-    this.accountCache = accountCache;
-    this.allUsersName = checkNotNull(allUsersName, "allUsersName");
-    this.externalIds = checkNotNull(externalIds, "externalIds");
-    this.externalIdCache = checkNotNull(externalIdCache, "externalIdCache");
-    this.retryHelper = checkNotNull(retryHelper, "retryHelper");
-    this.afterReadRevision = checkNotNull(afterReadRevision, "afterReadRevision");
-    this.updateCount =
-        metricMaker.newCounter(
-            "notedb/external_id_update_count",
-            new Description("Total number of external ID updates.").setRate().setUnit("updates"));
-  }
-
-  /**
-   * Inserts a new external ID.
-   *
-   * <p>If the external ID already exists, the insert fails with {@link OrmDuplicateKeyException}.
-   */
-  public void insert(ExternalId extId) throws IOException, ConfigInvalidException, OrmException {
-    insert(Collections.singleton(extId));
-  }
-
-  /**
-   * Inserts new external IDs.
-   *
-   * <p>If any of the external ID already exists, the insert fails with {@link
-   * OrmDuplicateKeyException}.
-   */
-  public void insert(Collection<ExternalId> extIds)
-      throws IOException, ConfigInvalidException, OrmException {
-    updateNoteMap(n -> n.insert(extIds));
-  }
-
-  /**
-   * 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, OrmException {
-    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, OrmException {
-    updateNoteMap(n -> n.upsert(extIds));
-  }
-
-  /**
-   * 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) throws IOException, ConfigInvalidException, OrmException {
-    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)
-      throws IOException, ConfigInvalidException, OrmException {
-    updateNoteMap(n -> n.delete(extIds));
-  }
-
-  /**
-   * 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)
-      throws IOException, ConfigInvalidException, OrmException {
-    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)
-      throws IOException, ConfigInvalidException, OrmException {
-    updateNoteMap(n -> n.delete(accountId, extIdKeys));
-  }
-
-  /**
-   * 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)
-      throws IOException, ConfigInvalidException, OrmException {
-    updateNoteMap(n -> n.deleteByKeys(extIdKeys));
-  }
-
-  /** Deletes all external IDs of the specified account. */
-  public void deleteAll(Account.Id accountId)
-      throws IOException, ConfigInvalidException, OrmException {
-    delete(externalIds.byAccount(accountId));
-  }
-
-  /**
-   * 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, ConfigInvalidException, OrmException {
-    updateNoteMap(n -> n.replace(accountId, toDelete, toAdd));
-  }
-
-  /**
-   * 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, ConfigInvalidException, OrmException {
-    updateNoteMap(n -> n.replaceByKeys(toDelete, toAdd));
-  }
-
-  /**
-   * 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, ConfigInvalidException, OrmException {
-    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, ConfigInvalidException, OrmException {
-    updateNoteMap(n -> n.replace(toDelete, toAdd));
-  }
-
-  private void updateNoteMap(ExternalIdUpdater updater)
-      throws IOException, ConfigInvalidException, OrmException {
-    retryHelper.execute(
-        ActionType.ACCOUNT_UPDATE,
-        () -> {
-          try (Repository repo = repoManager.openRepository(allUsersName)) {
-            ExternalIdNotes extIdNotes =
-                new ExternalIdNotes(externalIdCache, accountCache, repo)
-                    .setAfterReadRevision(afterReadRevision)
-                    .load();
-            updater.update(extIdNotes);
-            try (MetaDataUpdate metaDataUpdate = metaDataUpdateFactory.create()) {
-              extIdNotes.commit(metaDataUpdate);
-            }
-            extIdNotes.updateCaches();
-            updateCount.increment();
-            return null;
-          }
-        });
-  }
-
-  @FunctionalInterface
-  private static interface ExternalIdUpdater {
-    void update(ExternalIdNotes extIdsNotes)
-        throws IOException, ConfigInvalidException, OrmException;
-  }
-
-  @VisibleForTesting
-  @FunctionalInterface
-  public static interface MetaDataUpdateFactory {
-    MetaDataUpdate create() throws IOException;
-  }
-}

java/com/google/gerrit/server/config/GerritGlobalModule.java

@@ -86,7 +86,6 @@ import com.google.gerrit.server.account.AccountManager;
 import com.google.gerrit.server.account.AccountResolver;
 import com.google.gerrit.server.account.AccountVisibilityProvider;
 import com.google.gerrit.server.account.CapabilityCollection;
-import com.google.gerrit.server.account.ChangeUserName;
 import com.google.gerrit.server.account.EmailExpander;
 import com.google.gerrit.server.account.GroupCacheImpl;
 import com.google.gerrit.server.account.GroupControl;
@@ -419,7 +418,6 @@ public class GerritGlobalModule extends FactoryModule {
     factory(VersionedAuthorizedKeys.Factory.class);
 
     bind(AccountManager.class);
-    factory(ChangeUserName.Factory.class);
 
     bind(new TypeLiteral<List<CommentLinkInfo>>() {})
         .toProvider(CommentLinkProvider.class)

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

@@ -90,7 +90,7 @@ public class AccountValidator {
   private Optional<Account> loadAccount(Account.Id accountId, RevWalk rw, ObjectId commit)
       throws IOException, ConfigInvalidException {
     rw.reset();
-    AccountConfig accountConfig = new AccountConfig(null, accountId);
+    AccountConfig accountConfig = new AccountConfig(accountId);
     accountConfig.load(rw, commit);
     return accountConfig.getLoadedAccount();
   }

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

@@ -14,7 +14,9 @@
 
 package com.google.gerrit.server.restapi.account;
 
-import com.google.gerrit.common.errors.NameAlreadyUsedException;
+import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME;
+
+import com.google.common.base.Strings;
 import com.google.gerrit.extensions.api.accounts.UsernameInput;
 import com.google.gerrit.extensions.client.AccountFieldName;
 import com.google.gerrit.extensions.restapi.AuthException;
@@ -22,14 +24,18 @@ import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
+import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.account.AccountResource;
-import com.google.gerrit.server.account.ChangeUserName;
+import com.google.gerrit.server.account.AccountsUpdate;
-import com.google.gerrit.server.account.InvalidUserNameException;
 import com.google.gerrit.server.account.Realm;
+import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.account.externalids.ExternalIds;
 import com.google.gerrit.server.permissions.GlobalPermission;
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
+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;
@@ -40,19 +46,25 @@ import org.eclipse.jgit.errors.ConfigInvalidException;
 @Singleton
 public class PutUsername implements RestModifyView<AccountResource, UsernameInput> {
   private final Provider<CurrentUser> self;
-  private final ChangeUserName.Factory changeUserNameFactory;
   private final PermissionBackend permissionBackend;
+  private final ExternalIds externalIds;
+  private final AccountsUpdate.Server accountsUpdate;
+  private final SshKeyCache sshKeyCache;
   private final Realm realm;
 
   @Inject
   PutUsername(
       Provider<CurrentUser> self,
-      ChangeUserName.Factory changeUserNameFactory,
       PermissionBackend permissionBackend,
+      ExternalIds externalIds,
+      AccountsUpdate.Server accountsUpdate,
+      SshKeyCache sshKeyCache,
       Realm realm) {
     this.self = self;
-    this.changeUserNameFactory = changeUserNameFactory;
     this.permissionBackend = permissionBackend;
+    this.externalIds = externalIds;
+    this.accountsUpdate = accountsUpdate;
+    this.sshKeyCache = sshKeyCache;
     this.realm = realm;
   }
 
@@ -73,19 +85,39 @@ public class PutUsername implements RestModifyView<AccountResource, UsernameInpu
       input = new UsernameInput();
     }
 
-    try {
+    Account.Id accountId = rsrc.getUser().getAccountId();
-      changeUserNameFactory.create("Set Username via API", rsrc.getUser(), input.username).call();
+    if (!externalIds.byAccount(accountId, SCHEME_USERNAME).isEmpty()) {
-    } catch (IllegalStateException e) {
+      throw new MethodNotAllowedException("Username cannot be changed.");
-      if (ChangeUserName.USERNAME_CANNOT_BE_CHANGED.equals(e.getMessage())) {
+    }
-        throw new MethodNotAllowedException(e.getMessage());
+
-      }
+    if (Strings.isNullOrEmpty(input.username)) {
-      throw e;
+      return input.username;
-    } catch (InvalidUserNameException e) {
+    }
+
+    if (!ExternalId.isValidUsername(input.username)) {
       throw new UnprocessableEntityException("invalid username");
-    } catch (NameAlreadyUsedException e) {
+    }
+
+    ExternalId.Key key = ExternalId.Key.create(SCHEME_USERNAME, input.username);
+    try {
+      accountsUpdate
+          .create()
+          .update(
+              "Set Username via API",
+              accountId,
+              u -> u.addExternalId(ExternalId.create(key, accountId, null, null)));
+    } catch (OrmDuplicateKeyException dupeErr) {
+      // If we are using this identity, don't report the exception.
+      ExternalId other = externalIds.get(key);
+      if (other != null && other.accountId().equals(accountId)) {
+        return input.username;
+      }
+
+      // Otherwise, someone else has this identity.
       throw new ResourceConflictException("username already used");
     }
 
+    sshKeyCache.evict(input.username);
     return input.username;
   }
 }

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

@@ -19,10 +19,7 @@ import static java.util.stream.Collectors.toSet;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.GerritPersonIdent;
-import com.google.gerrit.server.account.AccountsUpdate;
 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;
@@ -34,25 +31,23 @@ import java.sql.Statement;
 import java.util.HashSet;
 import java.util.Objects;
 import java.util.Set;
-import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.lib.RefUpdate.Result;
 import org.eclipse.jgit.lib.Repository;
 
 /** Delete user branches for which no account exists. */
 public class Schema_147 extends SchemaVersion {
   private final GitRepositoryManager repoManager;
   private final AllUsersName allUsersName;
-  private final PersonIdent serverIdent;
 
   @Inject
   Schema_147(
-      Provider<Schema_146> prior,
+      Provider<Schema_146> prior, GitRepositoryManager repoManager, AllUsersName allUsersName) {
-      GitRepositoryManager repoManager,
-      AllUsersName allUsersName,
-      @GerritPersonIdent PersonIdent serverIdent) {
     super(prior);
     this.repoManager = repoManager;
     this.allUsersName = allUsersName;
-    this.serverIdent = serverIdent;
   }
 
   @Override
@@ -69,8 +64,7 @@ public class Schema_147 extends SchemaVersion {
               .collect(toSet());
       accountIdsFromUserBranches.removeAll(accountIdsFromReviewDb);
       for (Account.Id accountId : accountIdsFromUserBranches) {
-        AccountsUpdate.deleteUserBranch(
+        deleteUserBranch(repo, accountId);
-            repo, allUsersName, GitReferenceUpdated.DISABLED, null, serverIdent, accountId);
       }
     } catch (IOException e) {
       throw new OrmException("Failed to delete user branches for non-existing accounts.", e);
@@ -87,4 +81,21 @@ public class Schema_147 extends SchemaVersion {
       return ids;
     }
   }
+
+  private void deleteUserBranch(Repository allUsersRepo, Account.Id accountId) throws IOException {
+    String refName = RefNames.refsUsers(accountId);
+    Ref ref = allUsersRepo.exactRef(refName);
+    if (ref == null) {
+      return;
+    }
+
+    RefUpdate ru = allUsersRepo.updateRef(refName);
+    ru.setExpectedOldObjectId(ref.getObjectId());
+    ru.setNewObjectId(ObjectId.zeroId());
+    ru.setForceUpdate(true);
+    Result result = ru.delete();
+    if (result != Result.FORCED) {
+      throw new IOException(String.format("Failed to delete ref %s: %s", refName, result.name()));
+    }
+  }
 }

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

@@ -139,7 +139,7 @@ public class Schema_154 extends SchemaVersion {
     PersonIdent ident = serverIdent.get();
     md.getCommitBuilder().setAuthor(ident);
     md.getCommitBuilder().setCommitter(ident);
-    AccountConfig accountConfig = new AccountConfig(null, account.getId());
+    AccountConfig accountConfig = new AccountConfig(account.getId());
     accountConfig.load(allUsersRepo);
     accountConfig.setAccount(account);
     accountConfig.commit(md);

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

@@ -104,7 +104,6 @@ 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;
@@ -191,8 +190,6 @@ public class AccountIT extends AbstractDaemonTest {
 
   @Inject private AccountIndexer accountIndexer;
 
-  @Inject private OutgoingEmailValidator emailValidator;
-
   @Inject private GitReferenceUpdated gitReferenceUpdated;
 
   @Inject private RetryHelper.Metrics retryMetrics;
@@ -2013,7 +2010,6 @@ public class AccountIT extends AbstractDaemonTest {
             gitReferenceUpdated,
             null,
             allUsers,
-            emailValidator,
             metaDataUpdateInternalFactory,
             new RetryHelper(
                 cfg,
@@ -2062,7 +2058,6 @@ public class AccountIT extends AbstractDaemonTest {
             gitReferenceUpdated,
             null,
             allUsers,
-            emailValidator,
             metaDataUpdateInternalFactory,
             new RetryHelper(
                 cfg,

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

@@ -26,7 +26,6 @@ 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;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
@@ -41,20 +40,15 @@ import com.google.gerrit.extensions.api.config.ConsistencyCheckInput.CheckAccoun
 import com.google.gerrit.extensions.common.AccountExternalIdInfo;
 import com.google.gerrit.extensions.restapi.AuthException;
 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.inject.Inject;
@@ -67,8 +61,6 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Set;
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicInteger;
 import org.eclipse.jgit.api.errors.TransportException;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
@@ -92,8 +84,6 @@ public class ExternalIdIT extends AbstractDaemonTest {
   @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
@@ -714,91 +704,6 @@ public class ExternalIdIT extends AbstractDaemonTest {
     return scheme + ":foo" + ++i.value;
   }
 
-  @Test
-  public void retryOnLockFailure() throws Exception {
-    ExternalId.Key fooId = ExternalId.Key.create("foo", "foo");
-    ExternalId.Key barId = ExternalId.Key.create("bar", "bar");
-
-    final AtomicBoolean doneBgUpdate = new AtomicBoolean(false);
-    ExternalIdsUpdate update =
-        new ExternalIdsUpdate(
-            repoManager,
-            () -> metaDataUpdateFactory.create(allUsers),
-            accountCache,
-            allUsers,
-            metricMaker,
-            externalIds,
-            new DisabledExternalIdCache(),
-            new RetryHelper(
-                cfg,
-                retryMetrics,
-                null,
-                null,
-                null,
-                r -> r.withBlockStrategy(noSleepBlockStrategy)),
-            () -> {
-              if (!doneBgUpdate.getAndSet(true)) {
-                try {
-                  insertExtId(ExternalId.create(barId, admin.id));
-                } catch (Exception e) {
-                  // Ignore, the successful insertion of the external ID is asserted later
-                }
-              }
-            });
-    assertThat(doneBgUpdate.get()).isFalse();
-    update.insert(ExternalId.create(fooId, admin.id));
-    assertThat(doneBgUpdate.get()).isTrue();
-
-    assertThat(externalIds.get(fooId)).isNotNull();
-    assertThat(externalIds.get(barId)).isNotNull();
-  }
-
-  @Test
-  public void failAfterRetryerGivesUp() throws Exception {
-    ExternalId.Key[] extIdsKeys = {
-      ExternalId.Key.create("foo", "foo"),
-      ExternalId.Key.create("bar", "bar"),
-      ExternalId.Key.create("baz", "baz")
-    };
-    final AtomicInteger bgCounter = new AtomicInteger(0);
-    ExternalIdsUpdate update =
-        new ExternalIdsUpdate(
-            repoManager,
-            () -> metaDataUpdateFactory.create(allUsers),
-            accountCache,
-            allUsers,
-            metricMaker,
-            externalIds,
-            new DisabledExternalIdCache(),
-            new RetryHelper(
-                cfg,
-                retryMetrics,
-                null,
-                null,
-                null,
-                r ->
-                    r.withStopStrategy(StopStrategies.stopAfterAttempt(extIdsKeys.length))
-                        .withBlockStrategy(noSleepBlockStrategy)),
-            () -> {
-              try {
-                insertExtId(ExternalId.create(extIdsKeys[bgCounter.getAndAdd(1)], admin.id));
-              } catch (Exception e) {
-                // Ignore, the successful insertion of the external ID is asserted later
-              }
-            });
-    assertThat(bgCounter.get()).isEqualTo(0);
-    try {
-      update.insert(ExternalId.create(ExternalId.Key.create("abc", "abc"), admin.id));
-      fail("expected LockFailureException");
-    } catch (LockFailureException e) {
-      // Ignore, expected
-    }
-    assertThat(bgCounter.get()).isEqualTo(extIdsKeys.length);
-    for (ExternalId.Key extIdKey : extIdsKeys) {
-      assertThat(externalIds.get(extIdKey)).isNotNull();
-    }
-  }
-
   @Test
   public void readExternalIdWithAccountIdThatCanBeExpressedInKiB() throws Exception {
     ExternalId.Key extIdKey = ExternalId.Key.parse("foo:bar");

javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java

@@ -41,7 +41,6 @@ import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.Sequences;
-import com.google.gerrit.server.account.AccountsUpdate;
 import com.google.gerrit.server.change.ChangeInserter;
 import com.google.gerrit.server.change.ConsistencyChecker;
 import com.google.gerrit.server.change.PatchSetInserter;
@@ -67,7 +66,10 @@ import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.lib.RefUpdate.Result;
+import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.junit.Before;
 import org.junit.Test;
@@ -90,8 +92,6 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest {
 
   @Inject private Sequences sequences;
 
-  @Inject private AccountsUpdate.Server accountsUpdate;
-
   private RevCommit tip;
   private Account.Id adminId;
   private ConsistencyChecker checker;
@@ -126,7 +126,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest {
   public void missingOwner() throws Exception {
     TestAccount owner = accountCreator.create("missing");
     ChangeNotes notes = insertChange(owner);
-    accountsUpdate.create().deleteByKey(owner.getId());
+    deleteUserBranch(owner.getId());
 
     assertProblems(notes, null, problem("Missing change owner: " + owner.getId()));
   }
@@ -958,4 +958,23 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest {
   private void assertNoProblems(ChangeNotes notes, @Nullable FixInput fix) throws Exception {
     assertThat(checker.check(notes, fix).problems()).isEmpty();
   }
+
+  private void deleteUserBranch(Account.Id accountId) throws IOException {
+    try (Repository repo = repoManager.openRepository(allUsers)) {
+      String refName = RefNames.refsUsers(accountId);
+      Ref ref = repo.exactRef(refName);
+      if (ref == null) {
+        return;
+      }
+
+      RefUpdate ru = repo.updateRef(refName);
+      ru.setExpectedOldObjectId(ref.getObjectId());
+      ru.setNewObjectId(ObjectId.zeroId());
+      ru.setForceUpdate(true);
+      Result result = ru.delete();
+      if (result != Result.FORCED) {
+        throw new IOException(String.format("Failed to delete ref %s: %s", refName, result.name()));
+      }
+    }
+  }
 }

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

@@ -416,7 +416,7 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests {
       PersonIdent ident = serverIdent.get();
       md.getCommitBuilder().setAuthor(ident);
       md.getCommitBuilder().setCommitter(ident);
-      AccountConfig accountConfig = new AccountConfig(null, accountId);
+      AccountConfig accountConfig = new AccountConfig(accountId);
       accountConfig.load(repo);
       accountConfig.setAccountUpdate(InternalAccountUpdate.builder().setFullName(newName).build());
       accountConfig.commit(md);