Fix nits in accounts test API to set secondary emails

* TestAccountCreation:
  Accept any Set in setter for secondary emails. This way callers are
  not forced to create an ImmutableSet.

* AccountOperationsImpl:
  Factor out some code from the fillBuilder(
  InternalAccountUpdate.Builder, TestAccountUpdate, AccountState) method
  into separate methods to improve code readability.

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

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

@@ -177,24 +177,33 @@ public class AccountOperationsImpl implements AccountOperations {
       accountUpdate.status().ifPresent(builder::setStatus);
       accountUpdate.active().ifPresent(builder::setActive);
 
-      ImmutableSet<String> secondaryEmails;
+      ImmutableSet<String> secondaryEmails = getSecondaryEmails(accountUpdate, accountState);
-      ImmutableSet<String> allEmails =
-          ExternalId.getEmails(accountState.externalIds()).collect(toImmutableSet());
-      if (accountUpdate.preferredEmail().isPresent()) {
-        secondaryEmails =
-            ImmutableSet.copyOf(
-                Sets.difference(allEmails, ImmutableSet.of(accountUpdate.preferredEmail().get())));
-      } else if (accountState.account().preferredEmail() != null) {
-        secondaryEmails =
-            ImmutableSet.copyOf(
-                Sets.difference(
-                    allEmails, ImmutableSet.of(accountState.account().preferredEmail())));
-      } else {
-        secondaryEmails = allEmails;
-      }
       ImmutableSet<String> newSecondaryEmails =
           ImmutableSet.copyOf(accountUpdate.secondaryEmailsModification().apply(secondaryEmails));
       if (!secondaryEmails.equals(newSecondaryEmails)) {
+        setSecondaryEmails(builder, accountUpdate, accountState, newSecondaryEmails);
+      }
+    }
+
+    private ImmutableSet<String> getSecondaryEmails(
+        TestAccountUpdate accountUpdate, AccountState accountState) {
+      ImmutableSet<String> allEmails =
+          ExternalId.getEmails(accountState.externalIds()).collect(toImmutableSet());
+      if (accountUpdate.preferredEmail().isPresent()) {
+        return ImmutableSet.copyOf(
+            Sets.difference(allEmails, ImmutableSet.of(accountUpdate.preferredEmail().get())));
+      } else if (accountState.account().preferredEmail() != null) {
+        return ImmutableSet.copyOf(
+            Sets.difference(allEmails, ImmutableSet.of(accountState.account().preferredEmail())));
+      }
+      return allEmails;
+    }
+
+    private void setSecondaryEmails(
+        InternalAccountUpdate.Builder builder,
+        TestAccountUpdate accountUpdate,
+        AccountState accountState,
+        ImmutableSet<String> newSecondaryEmails) {
       // delete all external IDs of SCHEME_MAILTO scheme, then add back SCHEME_MAILTO external IDs
       // for the new secondary emails and the preferred email
       builder.deleteExternalIds(
@@ -213,7 +222,6 @@ public class AccountOperationsImpl implements AccountOperations {
             ExternalId.createEmail(accountId, accountState.account().preferredEmail()));
       }
     }
-    }
 
     @Override
     public TestAccountInvalidation.Builder forInvalidation() {

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

@@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableSet;
 import com.google.gerrit.acceptance.testsuite.ThrowingFunction;
 import com.google.gerrit.entities.Account;
 import java.util.Optional;
+import java.util.Set;
 
 @AutoValue
 public abstract class TestAccountCreation {
@@ -88,7 +89,7 @@ public abstract class TestAccountCreation {
       return active(false);
     }
 
-    public abstract Builder secondaryEmails(ImmutableSet<String> secondaryEmails);
+    public abstract Builder secondaryEmails(Set<String> secondaryEmails);
 
     abstract ImmutableSet.Builder<String> secondaryEmailsBuilder();