Add test API to create/update accounts

The new API simplifies setting up account data for tests and makes the
tests more readable.

Only a few callers have been adapted to demonstrate the usage of the new
API. Follow-up changes should adapt all tests to the new API and remove
the AccountCreator and the old TestAccount class.

Some methods in AbstractDaemonTest are duplicated for now. The old
methods should be removed when all tests have been migrated to the new
API.

Ideally TestAccount shouldn't know about SSH specifics, but we didn’t
find a good way to keep this concern separate yet.

To execute account operations we directly use the internal API.
One benefit of this is that we can also support operations like
updates without index updates or deletions even though the REST or
Java API don't allow them. Another benefit is that we bypass
permissions. This means that tests need not adjust permissions just
to create the correct test setup and thus hopefully will lead to
simpler and more readable tests.

Similar test APIs should be added to create/update groups, changes etc.
As result of this the setup of test data will become more consistent.

Change-Id: I8911660b027d09309879918543c063d51465b0bf
Signed-off-by: Edwin Kempin <ekempin@google.com>

java/com/google/gerrit/acceptance/AbstractDaemonTest.java

@@ -1462,15 +1462,21 @@ public abstract class AbstractDaemonTest {
   }
 
   protected void assertNotifyTo(TestAccount expected) {
-    assertNotifyTo(expected.emailAddress);
+    assertNotifyTo(expected.email, expected.fullName);
   }
 
-  protected void assertNotifyTo(Address expected) {
+  protected void assertNotifyTo(
+      com.google.gerrit.acceptance.testsuite.account.TestAccount expected) {
+    assertNotifyTo(expected.preferredEmail().orElse(null), expected.fullname().orElse(null));
+  }
+
+  private void assertNotifyTo(String expectedEmail, String expectedFullname) {
+    Address expectedAddress = new Address(expectedFullname, expectedEmail);
     assertThat(sender.getMessages()).hasSize(1);
     Message m = sender.getMessages().get(0);
-    assertThat(m.rcpt()).containsExactly(expected);
+    assertThat(m.rcpt()).containsExactly(expectedAddress);
     assertThat(((EmailHeader.AddressList) m.headers().get("To")).getAddressList())
-        .containsExactly(expected);
+        .containsExactly(expectedAddress);
     assertThat(m.headers().get("Cc").isEmpty()).isTrue();
   }
 
@@ -1478,13 +1484,23 @@ public abstract class AbstractDaemonTest {
     assertNotifyCc(expected.emailAddress);
   }
 
-  protected void assertNotifyCc(Address expected) {
+  protected void assertNotifyCc(
+      com.google.gerrit.acceptance.testsuite.account.TestAccount expected) {
+    assertNotifyCc(expected.preferredEmail().orElse(null), expected.fullname().orElse(null));
+  }
+
+  protected void assertNotifyCc(String expectedEmail, String expectedFullname) {
+    Address expectedAddress = new Address(expectedFullname, expectedEmail);
+    assertNotifyCc(expectedAddress);
+  }
+
+  protected void assertNotifyCc(Address expectedAddress) {
     assertThat(sender.getMessages()).hasSize(1);
     Message m = sender.getMessages().get(0);
-    assertThat(m.rcpt()).containsExactly(expected);
+    assertThat(m.rcpt()).containsExactly(expectedAddress);
     assertThat(m.headers().get("To").isEmpty()).isTrue();
     assertThat(((EmailHeader.AddressList) m.headers().get("Cc")).getAddressList())
-        .containsExactly(expected);
+        .containsExactly(expectedAddress);
   }
 
   protected void assertNotifyBcc(TestAccount expected) {
@@ -1495,6 +1511,17 @@ public abstract class AbstractDaemonTest {
     assertThat(m.headers().get("Cc").isEmpty()).isTrue();
   }
 
+  protected void assertNotifyBcc(
+      com.google.gerrit.acceptance.testsuite.account.TestAccount expected) {
+    assertThat(sender.getMessages()).hasSize(1);
+    Message m = sender.getMessages().get(0);
+    assertThat(m.rcpt())
+        .containsExactly(
+            new Address(expected.fullname().orElse(null), expected.preferredEmail().orElse(null)));
+    assertThat(m.headers().get("To").isEmpty()).isTrue();
+    assertThat(m.headers().get("Cc").isEmpty()).isTrue();
+  }
+
   protected interface ProjectWatchInfoConfiguration {
     void configure(ProjectWatchInfo pwi);
   }

java/com/google/gerrit/acceptance/GerritServer.java

@@ -22,6 +22,8 @@ import com.google.auto.value.AutoValue;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
+import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
+import com.google.gerrit.acceptance.testsuite.account.AccountOperationsImpl;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.extensions.config.FactoryModule;
 import com.google.gerrit.lucene.LuceneIndexModule;
@@ -430,6 +432,7 @@ public class GerritServer implements AutoCloseable {
           protected void configure() {
             bindConstant().annotatedWith(SshEnabled.class).to(daemon.getEnableSshd());
             bind(AccountCreator.class);
+            bind(AccountOperations.class).to(AccountOperationsImpl.class);
             factory(PushOneCommit.Factory.class);
             install(InProcessProtocol.module());
             install(new NoSshModule());

java/com/google/gerrit/acceptance/testsuite/ThrowingFunction.java

@@ -0,0 +1,21 @@
+// Copyright (C) 2018 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.acceptance.testsuite;
+
+@FunctionalInterface
+public interface ThrowingFunction<T, R> {
+
+  R apply(T value) throws Exception;
+}

java/com/google/gerrit/acceptance/testsuite/account/AccountOperations.java

@@ -0,0 +1,99 @@
+// Copyright (C) 2018 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.acceptance.testsuite.account;
+
+import com.google.gerrit.reviewdb.client.Account;
+
+/**
+ * An aggregation of operations on accounts for test purposes.
+ *
+ * <p>To execute the operations, no Gerrit permissions are necessary.
+ *
+ * <p><strong>Note:</strong> This interface is not implemented using the REST or extension API.
+ * Hence, it cannot be used for testing those APIs.
+ */
+public interface AccountOperations {
+
+  /**
+   * Starts the fluent chain for a querying or modifying an account. Please see the methods of
+   * {@link MoreAccountOperations} for details on possible operations.
+   *
+   * @return an aggregation of operations on a specific account
+   */
+  MoreAccountOperations account(Account.Id accountId);
+
+  /**
+   * Starts the fluent chain to create an account. The returned builder can be used to specify the
+   * attributes of the new account. To create the account for real, {@link
+   * TestAccountCreation.Builder#create()} must be called.
+   *
+   * <p>Example:
+   *
+   * <pre>
+   * TestAccount createdAccount = accountOperations
+   *     .newAccount()
+   *     .username("janedoe")
+   *     .preferredEmail("janedoe@example.com")
+   *     .fullname("Jane Doe")
+   *     .create();
+   * </pre>
+   *
+   * <p><strong>Note:</strong> If another account with the provided user name or preferred email
+   * address already exists, the creation of the account will fail.
+   *
+   * @return a builder to create the new account
+   */
+  TestAccountCreation.Builder newAccount();
+
+  /** An aggregation of methods on a specific account. */
+  interface MoreAccountOperations {
+
+    /**
+     * Checks whether the account exists.
+     *
+     * @return {@code true} if the account exists
+     */
+    boolean exists() throws Exception;
+
+    /**
+     * Retrieves the account.
+     *
+     * <p><strong>Note:</strong> This call will fail with an exception if the requested account
+     * doesn't exist. If you want to check for the existence of an account, use {@link #exists()}
+     * instead.
+     *
+     * @return the corresponding {@code TestAccount}
+     */
+    TestAccount get() throws Exception;
+
+    /**
+     * Starts the fluent chain to update an account. The returned builder can be used to specify how
+     * the attributes of the account should be modified. To update the account for real, {@link
+     * TestAccountUpdate.Builder#update()} must be called.
+     *
+     * <p>Example:
+     *
+     * <pre>
+     * TestAccount updatedAccount = accountOperations.forUpdate().status("on vacation").update();
+     * </pre>
+     *
+     * <p><strong>Note:</strong> The update will fail with an exception if the account to update
+     * doesn't exist. If you want to check for the existence of an account, use {@link #exists()}.
+     *
+     * @return a builder to update the account
+     */
+    TestAccountUpdate.Builder forUpdate();
+  }
+}

java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java

@@ -0,0 +1,211 @@
+// Copyright (C) 2018 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.acceptance.testsuite.account;
+
+import static com.google.common.base.Preconditions.checkState;
+import static java.nio.charset.StandardCharsets.US_ASCII;
+
+import com.google.gerrit.acceptance.SshEnabled;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.server.Sequences;
+import com.google.gerrit.server.ServerInitiated;
+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.InternalAccountUpdate;
+import com.google.gerrit.server.account.VersionedAuthorizedKeys;
+import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.ssh.SshKeyCache;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.jcraft.jsch.JSch;
+import com.jcraft.jsch.JSchException;
+import com.jcraft.jsch.KeyPair;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.UnsupportedEncodingException;
+import java.util.Optional;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+
+/**
+ * The implementation of {@code AccountOperations}.
+ *
+ * <p>There is only one implementation of {@code AccountOperations}. Nevertheless, we keep the
+ * separation between interface and implementation to enhance clarity.
+ */
+public class AccountOperationsImpl implements AccountOperations {
+  private final Accounts accounts;
+  private final AccountsUpdate accountsUpdate;
+  private final Sequences seq;
+  private final SshKeyCache sshKeyCache;
+  private final VersionedAuthorizedKeys.Accessor authorizedKeys;
+  private final boolean sshEnabled;
+
+  @Inject
+  public AccountOperationsImpl(
+      Accounts accounts,
+      @ServerInitiated AccountsUpdate accountsUpdate,
+      Sequences seq,
+      SshKeyCache sshKeyCache,
+      VersionedAuthorizedKeys.Accessor authorizedKeys,
+      // TODO(ekempin,aliceks): Find a way not to use this config parameter here. Ideally,
+      // completely factor out SSH from this class.
+      @SshEnabled boolean sshEnabled) {
+    this.accounts = accounts;
+    this.accountsUpdate = accountsUpdate;
+    this.seq = seq;
+    this.sshKeyCache = sshKeyCache;
+    this.authorizedKeys = authorizedKeys;
+    this.sshEnabled = sshEnabled;
+  }
+
+  @Override
+  public MoreAccountOperations account(Account.Id accountId) {
+    return new MoreAccountOperationsImpl(accountId);
+  }
+
+  @Override
+  public TestAccountCreation.Builder newAccount() {
+    return TestAccountCreation.builder(this::createAccount);
+  }
+
+  private TestAccount createAccount(TestAccountCreation accountCreation) throws Exception {
+    AccountsUpdate.AccountUpdater accountUpdater =
+        (account, updateBuilder) ->
+            fillBuilder(updateBuilder, accountCreation, account.getAccount().getId());
+    AccountState createdAccount = createAccount(accountUpdater);
+
+    TestAccount.Builder builder = toTestAccount(createdAccount);
+    Optional<String> userName = createdAccount.getUserName();
+    if (sshEnabled && userName.isPresent()) {
+      addSshKeyPair(builder, createdAccount.getAccount(), userName.get());
+    }
+    return builder.build();
+  }
+
+  private AccountState createAccount(AccountsUpdate.AccountUpdater accountUpdater)
+      throws OrmException, IOException, ConfigInvalidException {
+    Account.Id accountId = new Account.Id(seq.nextAccountId());
+    return accountsUpdate.insert("Create Test Account", accountId, accountUpdater);
+  }
+
+  private static void fillBuilder(
+      InternalAccountUpdate.Builder builder,
+      TestAccountCreation accountCreation,
+      Account.Id accountId) {
+    accountCreation.fullname().ifPresent(builder::setFullName);
+    accountCreation.preferredEmail().ifPresent(e -> setPreferredEmail(builder, accountId, e));
+    String httpPassword = accountCreation.httpPassword().orElse(null);
+    accountCreation.username().ifPresent(u -> setUsername(builder, accountId, u, httpPassword));
+    accountCreation.status().ifPresent(builder::setStatus);
+  }
+
+  private void addSshKeyPair(TestAccount.Builder builder, Account account, String username)
+      throws Exception {
+    KeyPair sshKey = genSshKey();
+    authorizedKeys.addKey(account.getId(), publicKey(sshKey, account.getPreferredEmail()));
+    sshKeyCache.evict(username);
+    builder.sshKeyPair(sshKey);
+  }
+
+  private static KeyPair genSshKey() throws JSchException {
+    JSch jsch = new JSch();
+    return KeyPair.genKeyPair(jsch, KeyPair.RSA);
+  }
+
+  private static String publicKey(KeyPair sshKey, String comment)
+      throws UnsupportedEncodingException {
+    ByteArrayOutputStream out = new ByteArrayOutputStream();
+    sshKey.writePublicKey(out, comment);
+    return out.toString(US_ASCII.name()).trim();
+  }
+
+  private static TestAccount.Builder toTestAccount(AccountState accountState) {
+    Account createdAccount = accountState.getAccount();
+    return TestAccount.builder()
+        .accountId(createdAccount.getId())
+        .preferredEmail(Optional.ofNullable(createdAccount.getPreferredEmail()))
+        .fullname(Optional.ofNullable(createdAccount.getFullName()))
+        .username(accountState.getUserName());
+  }
+
+  private static InternalAccountUpdate.Builder setPreferredEmail(
+      InternalAccountUpdate.Builder builder, Account.Id accountId, String preferredEmail) {
+    return builder
+        .setPreferredEmail(preferredEmail)
+        .addExternalId(ExternalId.createEmail(accountId, preferredEmail));
+  }
+
+  private static InternalAccountUpdate.Builder setUsername(
+      InternalAccountUpdate.Builder builder,
+      Account.Id accountId,
+      String username,
+      String httpPassword) {
+    return builder.addExternalId(ExternalId.createUsername(username, accountId, httpPassword));
+  }
+
+  private class MoreAccountOperationsImpl implements MoreAccountOperations {
+    private final Account.Id accountId;
+
+    MoreAccountOperationsImpl(Account.Id accountId) {
+      this.accountId = accountId;
+    }
+
+    @Override
+    public boolean exists() throws Exception {
+      return accounts.get(accountId).isPresent();
+    }
+
+    @Override
+    public TestAccount get() throws Exception {
+      AccountState account =
+          accounts
+              .get(accountId)
+              .orElseThrow(
+                  () -> new IllegalStateException("Tried to get non-existing test account"));
+      return toTestAccount(account).build();
+    }
+
+    @Override
+    public TestAccountUpdate.Builder forUpdate() {
+      return TestAccountUpdate.builder(this::updateAccount);
+    }
+
+    private TestAccount updateAccount(TestAccountUpdate accountUpdate)
+        throws OrmException, IOException, ConfigInvalidException {
+      AccountsUpdate.AccountUpdater accountUpdater =
+          (account, updateBuilder) -> fillBuilder(updateBuilder, accountUpdate, accountId);
+      Optional<AccountState> updatedAccount = updateAccount(accountUpdater);
+      checkState(updatedAccount.isPresent(), "Tried to update non-existing test account");
+      return toTestAccount(updatedAccount.get()).build();
+    }
+
+    private Optional<AccountState> updateAccount(AccountsUpdate.AccountUpdater accountUpdater)
+        throws OrmException, IOException, ConfigInvalidException {
+      return accountsUpdate.update("Update Test Account", accountId, accountUpdater);
+    }
+
+    private void fillBuilder(
+        InternalAccountUpdate.Builder builder,
+        TestAccountUpdate accountUpdate,
+        Account.Id accountId) {
+      accountUpdate.fullname().ifPresent(builder::setFullName);
+      accountUpdate.preferredEmail().ifPresent(e -> setPreferredEmail(builder, accountId, e));
+      String httpPassword = accountUpdate.httpPassword().orElse(null);
+      accountUpdate.username().ifPresent(u -> setUsername(builder, accountId, u, httpPassword));
+      accountUpdate.status().ifPresent(builder::setStatus);
+    }
+  }
+}

java/com/google/gerrit/acceptance/testsuite/account/TestAccount.java

@@ -0,0 +1,53 @@
+// Copyright (C) 2018 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.acceptance.testsuite.account;
+
+import com.google.auto.value.AutoValue;
+import com.google.gerrit.reviewdb.client.Account;
+import com.jcraft.jsch.KeyPair;
+import java.util.Optional;
+
+@AutoValue
+public abstract class TestAccount {
+  public abstract Account.Id accountId();
+
+  public abstract Optional<String> fullname();
+
+  public abstract Optional<String> preferredEmail();
+
+  public abstract Optional<String> username();
+
+  // TODO(ekempin,aliceks): Factor out SSH key handling from the class.
+  public abstract Optional<KeyPair> sshKeyPair();
+
+  static Builder builder() {
+    return new AutoValue_TestAccount.Builder();
+  }
+
+  @AutoValue.Builder
+  abstract static class Builder {
+    abstract Builder accountId(Account.Id accountId);
+
+    abstract Builder fullname(Optional<String> fullname);
+
+    abstract Builder preferredEmail(Optional<String> fullname);
+
+    abstract Builder username(Optional<String> username);
+
+    abstract Builder sshKeyPair(KeyPair keyPair);
+
+    abstract TestAccount build();
+  }
+}

java/com/google/gerrit/acceptance/testsuite/account/TestAccountCreation.java

@@ -0,0 +1,83 @@
+// Copyright (C) 2018 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.acceptance.testsuite.account;
+
+import com.google.auto.value.AutoValue;
+import com.google.gerrit.acceptance.testsuite.ThrowingFunction;
+import java.util.Optional;
+
+@AutoValue
+public abstract class TestAccountCreation {
+  public abstract Optional<String> fullname();
+
+  public abstract Optional<String> httpPassword();
+
+  public abstract Optional<String> preferredEmail();
+
+  public abstract Optional<String> username();
+
+  public abstract Optional<String> status();
+
+  abstract ThrowingFunction<TestAccountCreation, TestAccount> accountCreator();
+
+  public static Builder builder(ThrowingFunction<TestAccountCreation, TestAccount> accountCreator) {
+    return new AutoValue_TestAccountCreation.Builder()
+        .accountCreator(accountCreator)
+        .httpPassword("http-pass");
+  }
+
+  @AutoValue.Builder
+  public abstract static class Builder {
+    public abstract Builder fullname(String fullname);
+
+    public Builder clearFullname() {
+      return fullname("");
+    }
+
+    public abstract Builder httpPassword(String httpPassword);
+
+    public Builder clearHttpPassword() {
+      return httpPassword("");
+    }
+
+    public abstract Builder preferredEmail(String preferredEmail);
+
+    public Builder clearPreferredEmail() {
+      return preferredEmail("");
+    }
+
+    public abstract Builder username(String username);
+
+    public Builder clearUsername() {
+      return username("");
+    }
+
+    public abstract Builder status(String status);
+
+    public Builder clearStatus() {
+      return status("");
+    }
+
+    abstract Builder accountCreator(
+        ThrowingFunction<TestAccountCreation, TestAccount> accountCreator);
+
+    abstract TestAccountCreation autoBuild();
+
+    public TestAccount create() throws Exception {
+      TestAccountCreation accountUpdate = autoBuild();
+      return accountUpdate.accountCreator().apply(accountUpdate);
+    }
+  }
+}

java/com/google/gerrit/acceptance/testsuite/account/TestAccountUpdate.java

@@ -0,0 +1,83 @@
+// Copyright (C) 2018 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.acceptance.testsuite.account;
+
+import com.google.auto.value.AutoValue;
+import com.google.gerrit.acceptance.testsuite.ThrowingFunction;
+import java.util.Optional;
+
+@AutoValue
+public abstract class TestAccountUpdate {
+  public abstract Optional<String> fullname();
+
+  public abstract Optional<String> httpPassword();
+
+  public abstract Optional<String> preferredEmail();
+
+  public abstract Optional<String> username();
+
+  public abstract Optional<String> status();
+
+  abstract ThrowingFunction<TestAccountUpdate, TestAccount> accountUpdater();
+
+  public static Builder builder(ThrowingFunction<TestAccountUpdate, TestAccount> accountUpdater) {
+    return new AutoValue_TestAccountUpdate.Builder()
+        .accountUpdater(accountUpdater)
+        .httpPassword("http-pass");
+  }
+
+  @AutoValue.Builder
+  public abstract static class Builder {
+    public abstract Builder fullname(String fullname);
+
+    public Builder clearFullname() {
+      return fullname("");
+    }
+
+    public abstract Builder httpPassword(String httpPassword);
+
+    public Builder clearHttpPassword() {
+      return httpPassword("");
+    }
+
+    public abstract Builder preferredEmail(String preferredEmail);
+
+    public Builder clearPreferredEmail() {
+      return preferredEmail("");
+    }
+
+    public abstract Builder username(String username);
+
+    public Builder clearUsername() {
+      return username("");
+    }
+
+    public abstract Builder status(String status);
+
+    public Builder clearStatus() {
+      return status("");
+    }
+
+    abstract Builder accountUpdater(
+        ThrowingFunction<TestAccountUpdate, TestAccount> accountUpdater);
+
+    abstract TestAccountUpdate autoBuild();
+
+    public TestAccount update() throws Exception {
+      TestAccountUpdate accountUpdate = autoBuild();
+      return accountUpdate.accountUpdater().apply(accountUpdate);
+    }
+  }
+}

javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java

@@ -65,8 +65,9 @@ import com.google.gerrit.acceptance.GerritConfig;
 import com.google.gerrit.acceptance.GitUtil;
 import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.PushOneCommit;
-import com.google.gerrit.acceptance.TestAccount;
 import com.google.gerrit.acceptance.TestProjectInput;
+import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
+import com.google.gerrit.acceptance.testsuite.account.TestAccount;
 import com.google.gerrit.common.FooterConstants;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.common.data.LabelFunction;
@@ -135,6 +136,7 @@ import com.google.gerrit.server.StarredChangesUtil;
 import com.google.gerrit.server.change.ChangeResource;
 import com.google.gerrit.server.git.ChangeMessageModifier;
 import com.google.gerrit.server.group.SystemGroupBackend;
+import com.google.gerrit.server.mail.Address;
 import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
 import com.google.gerrit.server.project.testing.Util;
 import com.google.gerrit.server.restapi.change.PostReview;
@@ -178,6 +180,8 @@ public class ChangeIT extends AbstractDaemonTest {
 
   @Inject private DynamicSet<ChangeIndexedListener> changeIndexedListeners;
 
+  @Inject private AccountOperations accountOperations;
+
   private ChangeIndexedCounter changeIndexedCounter;
   private RegistrationHandle changeIndexedCounterHandle;
 
@@ -476,20 +480,40 @@ public class ChangeIT extends AbstractDaemonTest {
     assertThat(gApi.changes().id(changeId).get().pendingReviewers).isEmpty();
 
     // Add some pending reviewers.
-    TestAccount user1 =
+    String email1 = name("user1") + "@example.com";
-        accountCreator.create(name("user1"), name("user1") + "@example.com", "User 1");
+    String email2 = name("user2") + "@example.com";
-    TestAccount user2 =
+    String email3 = name("user3") + "@example.com";
-        accountCreator.create(name("user2"), name("user2") + "@example.com", "User 2");
+    String email4 = name("user4") + "@example.com";
-    TestAccount user3 =
+    accountOperations
-        accountCreator.create(name("user3"), name("user3") + "@example.com", "User 3");
+        .newAccount()
-    TestAccount user4 =
+        .username(name("user1"))
-        accountCreator.create(name("user4"), name("user4") + "@example.com", "User 4");
+        .preferredEmail(email1)
+        .fullname("User 1")
+        .create();
+    accountOperations
+        .newAccount()
+        .username(name("user2"))
+        .preferredEmail(email2)
+        .fullname("User 2")
+        .create();
+    accountOperations
+        .newAccount()
+        .username(name("user3"))
+        .preferredEmail(email3)
+        .fullname("User 3")
+        .create();
+    accountOperations
+        .newAccount()
+        .username(name("user4"))
+        .preferredEmail(email4)
+        .fullname("User 4")
+        .create();
     ReviewInput in =
         ReviewInput.noScore()
-            .reviewer(user1.email)
+            .reviewer(email1)
-            .reviewer(user2.email)
+            .reviewer(email2)
-            .reviewer(user3.email, CC, false)
+            .reviewer(email3, CC, false)
-            .reviewer(user4.email, CC, false)
+            .reviewer(email4, CC, false)
             .reviewer("byemail1@example.com")
             .reviewer("byemail2@example.com")
             .reviewer("byemail3@example.com", CC, false)
@@ -501,43 +525,43 @@ public class ChangeIT extends AbstractDaemonTest {
         ais -> ais.stream().map(ai -> ai.email).collect(toSet());
     assertThat(toEmails.apply(info.pendingReviewers.get(REVIEWER)))
         .containsExactly(
-            admin.email, user1.email, user2.email, "byemail1@example.com", "byemail2@example.com");
+            admin.email, email1, email2, "byemail1@example.com", "byemail2@example.com");
     assertThat(toEmails.apply(info.pendingReviewers.get(CC)))
-        .containsExactly(user3.email, user4.email, "byemail3@example.com", "byemail4@example.com");
+        .containsExactly(email3, email4, "byemail3@example.com", "byemail4@example.com");
     assertThat(info.pendingReviewers.get(REMOVED)).isNull();
 
     // Stage some pending reviewer removals.
-    gApi.changes().id(changeId).reviewer(user1.email).remove();
+    gApi.changes().id(changeId).reviewer(email1).remove();
-    gApi.changes().id(changeId).reviewer(user3.email).remove();
+    gApi.changes().id(changeId).reviewer(email3).remove();
     gApi.changes().id(changeId).reviewer("byemail1@example.com").remove();
     gApi.changes().id(changeId).reviewer("byemail3@example.com").remove();
     info = gApi.changes().id(changeId).get();
     assertThat(toEmails.apply(info.pendingReviewers.get(REVIEWER)))
-        .containsExactly(admin.email, user2.email, "byemail2@example.com");
+        .containsExactly(admin.email, email2, "byemail2@example.com");
     assertThat(toEmails.apply(info.pendingReviewers.get(CC)))
-        .containsExactly(user4.email, "byemail4@example.com");
+        .containsExactly(email4, "byemail4@example.com");
     assertThat(toEmails.apply(info.pendingReviewers.get(REMOVED)))
-        .containsExactly(user1.email, user3.email, "byemail1@example.com", "byemail3@example.com");
+        .containsExactly(email1, email3, "byemail1@example.com", "byemail3@example.com");
 
     // "Undo" a removal.
-    in = ReviewInput.noScore().reviewer(user1.email);
+    in = ReviewInput.noScore().reviewer(email1);
     gApi.changes().id(changeId).revision("current").review(in);
     info = gApi.changes().id(changeId).get();
     assertThat(toEmails.apply(info.pendingReviewers.get(REVIEWER)))
-        .containsExactly(admin.email, user1.email, user2.email, "byemail2@example.com");
+        .containsExactly(admin.email, email1, email2, "byemail2@example.com");
     assertThat(toEmails.apply(info.pendingReviewers.get(CC)))
-        .containsExactly(user4.email, "byemail4@example.com");
+        .containsExactly(email4, "byemail4@example.com");
     assertThat(toEmails.apply(info.pendingReviewers.get(REMOVED)))
-        .containsExactly(user3.email, "byemail1@example.com", "byemail3@example.com");
+        .containsExactly(email3, "byemail1@example.com", "byemail3@example.com");
 
     // "Commit" by moving out of WIP.
     gApi.changes().id(changeId).setReadyForReview();
     info = gApi.changes().id(changeId).get();
     assertThat(info.pendingReviewers).isEmpty();
     assertThat(toEmails.apply(info.reviewers.get(REVIEWER)))
-        .containsExactly(admin.email, user1.email, user2.email, "byemail2@example.com");
+        .containsExactly(admin.email, email1, email2, "byemail2@example.com");
     assertThat(toEmails.apply(info.reviewers.get(CC)))
-        .containsExactly(user4.email, "byemail4@example.com");
+        .containsExactly(email4, "byemail4@example.com");
     assertThat(info.reviewers.get(REMOVED)).isNull();
   }
 
@@ -1543,7 +1567,7 @@ public class ChangeIT extends AbstractDaemonTest {
 
     // Change status of reviewer and ensure ETag is updated.
     oldETag = rsrc.getETag();
-    gApi.accounts().id(user.id.get()).setStatus("new status");
+    accountOperations.account(user.id).forUpdate().status("new status").update();
     rsrc = parseResource(r);
     assertThat(rsrc.getETag()).isNotEqualTo(oldETag);
   }
@@ -1591,7 +1615,15 @@ public class ChangeIT extends AbstractDaemonTest {
     Timestamp oldTs = rsrc.getChange().getLastUpdatedOn();
 
     // create a group named "ab" with one user: testUser
-    TestAccount testUser = accountCreator.create("abcd", "abcd@test.com", "abcd");
+    String email = "abcd@test.com";
+    String fullname = "abcd";
+    TestAccount testUser =
+        accountOperations
+            .newAccount()
+            .username("abcd")
+            .preferredEmail(email)
+            .fullname(fullname)
+            .create();
     String testGroup = createGroupWithRealName("ab");
     GroupApi groupApi = gApi.groups().id(testGroup);
     groupApi.description("test group");
@@ -1604,11 +1636,11 @@ public class ChangeIT extends AbstractDaemonTest {
     List<Message> messages = sender.getMessages();
     assertThat(messages).hasSize(1);
     Message m = messages.get(0);
-    assertThat(m.rcpt()).containsExactly(testUser.emailAddress);
+    assertThat(m.rcpt()).containsExactly(new Address(fullname, email));
-    assertThat(m.body()).contains("Hello " + testUser.fullName + ",\n");
+    assertThat(m.body()).contains("Hello " + fullname + ",\n");
     assertThat(m.body()).contains("I'd like you to do a code review.");
     assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
-    assertMailReplyTo(m, testUser.email);
+    assertMailReplyTo(m, email);
     ChangeInfo c = gApi.changes().id(r.getChangeId()).get();
 
     // When NoteDb is enabled adding a reviewer records that user as reviewer
@@ -1618,7 +1650,7 @@ public class ChangeIT extends AbstractDaemonTest {
     Collection<AccountInfo> reviewers = c.reviewers.get(REVIEWER);
     assertThat(reviewers).isNotNull();
     assertThat(reviewers).hasSize(1);
-    assertThat(reviewers.iterator().next()._accountId).isEqualTo(testUser.getId().get());
+    assertThat(reviewers.iterator().next()._accountId).isEqualTo(testUser.accountId().get());
 
     // Ensure ETag and lastUpdatedOn are updated.
     rsrc = parseResource(r);
@@ -1635,16 +1667,31 @@ public class ChangeIT extends AbstractDaemonTest {
     Timestamp oldTs = rsrc.getChange().getLastUpdatedOn();
 
     // create a group named "kobe" with one user: lee
-    TestAccount testUser = accountCreator.create("kobebryant", "kobebryant@test.com", "kobebryant");
+    String testUserFullname = "kobebryant";
-    TestAccount myGroupUser = accountCreator.create("lee", "lee@test.com", "lee");
+    accountOperations
+        .newAccount()
+        .username("kobebryant")
+        .preferredEmail("kobebryant@test.com")
+        .fullname(testUserFullname)
+        .create();
+
+    String myGroupUserEmail = "lee@test.com";
+    String myGroupUserFullname = "lee";
+    TestAccount myGroupUser =
+        accountOperations
+            .newAccount()
+            .username("lee")
+            .preferredEmail(myGroupUserEmail)
+            .fullname(myGroupUserFullname)
+            .create();
 
     String testGroup = createGroupWithRealName("kobe");
     GroupApi groupApi = gApi.groups().id(testGroup);
     groupApi.description("test group");
-    groupApi.addMembers(myGroupUser.fullName);
+    groupApi.addMembers(myGroupUserFullname);
 
     // ensure that user "user" is not in the group
-    groupApi.removeMembers(testUser.fullName);
+    groupApi.removeMembers(testUserFullname);
 
     AddReviewerInput in = new AddReviewerInput();
     in.reviewer = testGroup;
@@ -1653,11 +1700,11 @@ public class ChangeIT extends AbstractDaemonTest {
     List<Message> messages = sender.getMessages();
     assertThat(messages).hasSize(1);
     Message m = messages.get(0);
-    assertThat(m.rcpt()).containsExactly(myGroupUser.emailAddress);
+    assertThat(m.rcpt()).containsExactly(new Address(myGroupUserFullname, myGroupUserEmail));
-    assertThat(m.body()).contains("Hello " + myGroupUser.fullName + ",\n");
+    assertThat(m.body()).contains("Hello " + myGroupUserFullname + ",\n");
     assertThat(m.body()).contains("I'd like you to do a code review.");
     assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
-    assertMailReplyTo(m, myGroupUser.email);
+    assertMailReplyTo(m, myGroupUserEmail);
     ChangeInfo c = gApi.changes().id(r.getChangeId()).get();
 
     // When NoteDb is enabled adding a reviewer records that user as reviewer
@@ -1667,7 +1714,7 @@ public class ChangeIT extends AbstractDaemonTest {
     Collection<AccountInfo> reviewers = c.reviewers.get(REVIEWER);
     assertThat(reviewers).isNotNull();
     assertThat(reviewers).hasSize(1);
-    assertThat(reviewers.iterator().next()._accountId).isEqualTo(myGroupUser.getId().get());
+    assertThat(reviewers.iterator().next()._accountId).isEqualTo(myGroupUser.accountId().get());
 
     // Ensure ETag and lastUpdatedOn are updated.
     rsrc = parseResource(r);
@@ -1734,12 +1781,13 @@ public class ChangeIT extends AbstractDaemonTest {
 
   @Test
   public void implicitlyCcOnNonVotingReviewForUserWithoutUserNamePgStyle() throws Exception {
-    TestAccount accountWithoutUsername = accountCreator.create();
+    com.google.gerrit.acceptance.TestAccount accountWithoutUsername = accountCreator.create();
     assertThat(accountWithoutUsername.username).isNull();
     testImplicitlyCcOnNonVotingReviewPgStyle(accountWithoutUsername);
   }
 
-  private void testImplicitlyCcOnNonVotingReviewPgStyle(TestAccount testAccount) throws Exception {
+  private void testImplicitlyCcOnNonVotingReviewPgStyle(
+      com.google.gerrit.acceptance.TestAccount testAccount) throws Exception {
     PushOneCommit.Result r = createChange();
     setApiUser(testAccount);
     assertThat(getReviewerState(r.getChangeId(), testAccount.id)).isEmpty();
@@ -1764,12 +1812,13 @@ public class ChangeIT extends AbstractDaemonTest {
 
   @Test
   public void implicitlyCcOnNonVotingReviewForUserWithoutUserNameGwtStyle() throws Exception {
-    TestAccount accountWithoutUsername = accountCreator.create();
+    com.google.gerrit.acceptance.TestAccount accountWithoutUsername = accountCreator.create();
     assertThat(accountWithoutUsername.username).isNull();
     testImplicitlyCcOnNonVotingReviewGwtStyle(accountWithoutUsername);
   }
 
-  private void testImplicitlyCcOnNonVotingReviewGwtStyle(TestAccount testAccount) throws Exception {
+  private void testImplicitlyCcOnNonVotingReviewGwtStyle(
+      com.google.gerrit.acceptance.TestAccount testAccount) throws Exception {
     PushOneCommit.Result r = createChange();
     setApiUser(testAccount);
     assertThat(getReviewerState(r.getChangeId(), testAccount.id)).isEmpty();
@@ -2094,13 +2143,20 @@ public class ChangeIT extends AbstractDaemonTest {
     in.notify = NotifyHandling.NONE;
 
     // notify unrelated account as TO
-    TestAccount user2 = accountCreator.user2();
+    String email = "user2@example.com";
+    TestAccount user2 =
+        accountOperations
+            .newAccount()
+            .username("user2")
+            .preferredEmail(email)
+            .fullname("User2")
+            .create();
     setApiUser(user);
     recommend(r.getChangeId());
     setApiUser(admin);
     sender.clear();
     in.notifyDetails = new HashMap<>();
-    in.notifyDetails.put(RecipientType.TO, new NotifyInfo(ImmutableList.of(user2.email)));
+    in.notifyDetails.put(RecipientType.TO, new NotifyInfo(ImmutableList.of(email)));
     gApi.changes().id(r.getChangeId()).reviewer(user.getId().toString()).deleteVote(in);
     assertNotifyTo(user2);
 
@@ -2110,7 +2166,7 @@ public class ChangeIT extends AbstractDaemonTest {
     setApiUser(admin);
     sender.clear();
     in.notifyDetails = new HashMap<>();
-    in.notifyDetails.put(RecipientType.CC, new NotifyInfo(ImmutableList.of(user2.email)));
+    in.notifyDetails.put(RecipientType.CC, new NotifyInfo(ImmutableList.of(email)));
     gApi.changes().id(r.getChangeId()).reviewer(user.getId().toString()).deleteVote(in);
     assertNotifyCc(user2);
 
@@ -2120,7 +2176,7 @@ public class ChangeIT extends AbstractDaemonTest {
     setApiUser(admin);
     sender.clear();
     in.notifyDetails = new HashMap<>();
-    in.notifyDetails.put(RecipientType.BCC, new NotifyInfo(ImmutableList.of(user2.email)));
+    in.notifyDetails.put(RecipientType.BCC, new NotifyInfo(ImmutableList.of(email)));
     gApi.changes().id(r.getChangeId()).reviewer(user.getId().toString()).deleteVote(in);
     assertNotifyBcc(user2);
   }
@@ -3275,7 +3331,7 @@ public class ChangeIT extends AbstractDaemonTest {
     assertThat(getCommitMessage(r.getChangeId()))
         .isEqualTo("test commit\n\nChange-Id: " + r.getChangeId() + "\n");
 
-    for (TestAccount acc : ImmutableList.of(admin, user)) {
+    for (com.google.gerrit.acceptance.TestAccount acc : ImmutableList.of(admin, user)) {
       setApiUser(acc);
       String newMessage =
           "modified commit by " + acc.username + "\n\nChange-Id: " + r.getChangeId() + "\n";
@@ -3737,7 +3793,14 @@ public class ChangeIT extends AbstractDaemonTest {
 
   @Test
   public void ignore() throws Exception {
-    TestAccount user2 = accountCreator.user2();
+    String email = "user2@example.com";
+    String fullname = "User2";
+    accountOperations
+        .newAccount()
+        .username("user2")
+        .preferredEmail(email)
+        .fullname(fullname)
+        .create();
 
     PushOneCommit.Result r = createChange();
 
@@ -3746,7 +3809,7 @@ public class ChangeIT extends AbstractDaemonTest {
     gApi.changes().id(r.getChangeId()).addReviewer(in);
 
     in = new AddReviewerInput();
-    in.reviewer = user2.email;
+    in.reviewer = email;
     gApi.changes().id(r.getChangeId()).addReviewer(in);
 
     setApiUser(user);
@@ -3758,7 +3821,7 @@ public class ChangeIT extends AbstractDaemonTest {
     gApi.changes().id(r.getChangeId()).abandon();
     List<Message> messages = sender.getMessages();
     assertThat(messages).hasSize(1);
-    assertThat(messages.get(0).rcpt()).containsExactly(user2.emailAddress);
+    assertThat(messages.get(0).rcpt()).containsExactly(new Address(fullname, email));
 
     setApiUser(user);
     gApi.changes().id(r.getChangeId()).ignore(false);
@@ -3812,7 +3875,7 @@ public class ChangeIT extends AbstractDaemonTest {
 
   @Test
   public void markAsReviewed() throws Exception {
-    TestAccount user2 = accountCreator.user2();
+    com.google.gerrit.acceptance.TestAccount user2 = accountCreator.user2();
 
     PushOneCommit.Result r = createChange();