Merge "AccountConfig: Return loaded account as Optional"

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

@@ -32,6 +32,7 @@ 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;
 import org.eclipse.jgit.lib.Config;
@@ -78,8 +79,7 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
   private final Account.Id accountId;
   private final String ref;
 
-  private boolean isLoaded;
-  private Account account;
+  private Optional<Account> loadedAccount;
   private Timestamp registeredOn;
   private List<ValidationError> validationErrors;
 
@@ -97,14 +97,13 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
   /**
    * Get the loaded account.
    *
-   * @return the loaded account, {@code null} if load didn't find the account because it doesn't
-   *     exist
+   * @return the loaded account, {@link Optional#empty()} if load didn't find the account because it
+   *     doesn't exist
    * @throws IllegalStateException if the account was not loaded yet
    */
-  @Nullable
-  public Account getAccount() {
+  public Optional<Account> getLoadedAccount() {
     checkLoaded();
-    return account;
+    return loadedAccount;
   }
 
   /**
@@ -117,7 +116,7 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
    */
   public void setAccount(Account account) {
     checkLoaded();
-    this.account = account;
+    this.loadedAccount = Optional.of(account);
     this.registeredOn = account.getRegisteredOn();
   }
 
@@ -133,8 +132,8 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
       throw new OrmDuplicateKeyException(String.format("account %s already exists", accountId));
     }
     this.registeredOn = TimeUtil.nowTs();
-    this.account = new Account(accountId, registeredOn);
-    return account;
+    this.loadedAccount = Optional.of(new Account(accountId, registeredOn));
+    return loadedAccount.get();
   }
 
   @Override
@@ -147,14 +146,13 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
 
       Config cfg = readConfig(ACCOUNT_CONFIG);
 
-      account = parse(cfg);
-      account.setMetaId(revision.name());
+      loadedAccount = Optional.of(parse(cfg, revision.name()));
+    } else {
+      loadedAccount = Optional.empty();
     }
-
-    isLoaded = true;
   }
 
-  private Account parse(Config cfg) {
+  private Account parse(Config cfg, String metaId) {
     Account account = new Account(accountId, registeredOn);
     account.setActive(cfg.getBoolean(ACCOUNT, null, KEY_ACTIVE, true));
     account.setFullName(get(cfg, KEY_FULL_NAME));
@@ -168,13 +166,14 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
     }
 
     account.setStatus(get(cfg, KEY_STATUS));
+    account.setMetaId(metaId);
     return account;
   }
 
   @Override
   public RevCommit commit(MetaDataUpdate update) throws IOException {
     RevCommit c = super.commit(update);
-    account.setMetaId(c.name());
+    loadedAccount.get().setMetaId(c.name());
     return c;
   }
 
@@ -182,16 +181,20 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
   protected boolean onSave(CommitBuilder commit) throws IOException, ConfigInvalidException {
     checkLoaded();
 
+    if (!loadedAccount.isPresent()) {
+      return false;
+    }
+
     if (revision != null) {
       commit.setMessage("Update account\n");
-    } else if (account != null) {
+    } else {
       commit.setMessage("Create account\n");
       commit.setAuthor(new PersonIdent(commit.getAuthor(), registeredOn));
       commit.setCommitter(new PersonIdent(commit.getCommitter(), registeredOn));
     }
 
     Config cfg = readConfig(ACCOUNT_CONFIG);
-    writeToConfig(account, cfg);
+    writeToConfig(loadedAccount.get(), cfg);
     saveConfig(ACCOUNT_CONFIG, cfg);
     return true;
   }
@@ -253,7 +256,7 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
   }
 
   private void checkLoaded() {
-    checkState(isLoaded, "Account %s not loaded yet", accountId.get());
+    checkState(loadedAccount != null, "Account %s not loaded yet", accountId.get());
   }
 
   /**

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

@@ -31,6 +31,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Stream;
 import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -60,7 +61,7 @@ public class Accounts {
   @Nullable
   public Account get(Account.Id accountId) throws IOException, ConfigInvalidException {
     try (Repository repo = repoManager.openRepository(allUsersName)) {
-      return read(repo, accountId);
+      return read(repo, accountId).orElse(null);
     }
   }
 
@@ -69,7 +70,7 @@ public class Accounts {
     List<Account> accounts = new ArrayList<>(accountIds.size());
     try (Repository repo = repoManager.openRepository(allUsersName)) {
       for (Account.Id accountId : accountIds) {
-        accounts.add(read(repo, accountId));
+        read(repo, accountId).ifPresent(accounts::add);
       }
     }
     return accounts;
@@ -86,7 +87,7 @@ public class Accounts {
     try (Repository repo = repoManager.openRepository(allUsersName)) {
       for (Account.Id accountId : accountIds) {
         try {
-          accounts.add(read(repo, accountId));
+          read(repo, accountId).ifPresent(accounts::add);
         } catch (Exception e) {
           log.error(String.format("Ignoring invalid account %s", accountId.get()), e);
         }
@@ -135,12 +136,11 @@ public class Accounts {
     }
   }
 
-  @Nullable
-  private Account read(Repository allUsersRepository, Account.Id accountId)
+  private Optional<Account> read(Repository allUsersRepository, Account.Id accountId)
       throws IOException, ConfigInvalidException {
     AccountConfig accountConfig = new AccountConfig(emailValidator, accountId);
     accountConfig.load(allUsersRepository);
-    return accountConfig.getAccount();
+    return accountConfig.getLoadedAccount();
   }
 
   public static Stream<Account.Id> readUserRefs(Repository repo) throws IOException {

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

@@ -35,6 +35,7 @@ import com.google.inject.Provider;
 import com.google.inject.Singleton;
 import java.io.IOException;
 import java.util.List;
+import java.util.Optional;
 import java.util.function.Consumer;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.ObjectId;
@@ -231,6 +232,7 @@ public class AccountsUpdate {
    * @throws IOException if updating the user branch fails
    * @throws ConfigInvalidException if any of the account fields has an invalid value
    */
+  @Nullable
   public Account update(Account.Id accountId, List<Consumer<Account>> consumers)
       throws IOException, ConfigInvalidException {
     if (consumers.isEmpty()) {
@@ -238,13 +240,13 @@ public class AccountsUpdate {
     }
 
     AccountConfig accountConfig = read(accountId);
-    Account account = accountConfig.getAccount();
-    if (account != null) {
-      consumers.stream().forEach(c -> c.accept(account));
+    Optional<Account> account = accountConfig.getLoadedAccount();
+    if (account.isPresent()) {
+      consumers.stream().forEach(c -> c.accept(account.get()));
       commit(accountConfig);
     }
 
-    return account;
+    return account.orElse(null);
   }
 
   /**