Fix ReturnValueIgnored bug pattern flagged by error prone

Release-Notes: skip
Details: https://errorprone.info/bugpattern/ReturnValueIgnored
Change-Id: Ieef7cb2f10df0d904d131c8a1311e2f7f942c2bb

6 files changed, 155 insertions, 124 deletions

diff --git a/java/com/google/gerrit/server/account/AccountManager.java b/java/com/google/gerrit/server/account/AccountManager.java
index 407d2f7a1a..891a467577 100644
--- a/java/com/google/gerrit/server/account/AccountManager.java
+++ b/java/com/google/gerrit/server/account/AccountManager.java
@@ -264,14 +264,16 @@ public class AccountManager {
     }
 
     if (!accountUpdates.isEmpty()) {
-      accountsUpdateProvider
-          .get()
-          .update(
-              "Update Account on Login",
-              user.getAccountId(),
-              AccountsUpdate.joinConsumers(accountUpdates))
-          .orElseThrow(
-              () -> new StorageException("Account " + user.getAccountId() + " has been deleted"));
+      Optional<AccountState> updatedAccount =
+          accountsUpdateProvider
+              .get()
+              .update(
+                  "Update Account on Login",
+                  user.getAccountId(),
+                  AccountsUpdate.joinConsumers(accountUpdates));
+      if (!updatedAccount.isPresent()) {
+        throw new StorageException("Account " + user.getAccountId() + " has been deleted");
+      }
     }
   }
 
diff --git a/java/com/google/gerrit/server/account/SetInactiveFlag.java b/java/com/google/gerrit/server/account/SetInactiveFlag.java
index 4b68198dca..5babebdfcb 100644
--- a/java/com/google/gerrit/server/account/SetInactiveFlag.java
+++ b/java/com/google/gerrit/server/account/SetInactiveFlag.java
@@ -55,26 +55,30 @@ public class SetInactiveFlag {
       throws RestApiException, IOException, ConfigInvalidException {
     AtomicBoolean alreadyInactive = new AtomicBoolean(false);
     AtomicReference<Optional<RestApiException>> exception = new AtomicReference<>(Optional.empty());
-    accountsUpdateProvider
-        .get()
-        .update(
-            "Deactivate Account via API",
-            accountId,
-            (a, u) -> {
-              if (!a.account().isActive()) {
-                alreadyInactive.set(true);
-              } else {
-                try {
-                  accountActivationValidationListeners.runEach(
-                      l -> l.validateDeactivation(a), ValidationException.class);
-                } catch (ValidationException e) {
-                  exception.set(Optional.of(new ResourceConflictException(e.getMessage(), e)));
-                  return;
-                }
-                u.setActive(false);
-              }
-            })
-        .orElseThrow(() -> new ResourceNotFoundException("account not found"));
+    Optional<AccountState> updatedAccount =
+        accountsUpdateProvider
+            .get()
+            .update(
+                "Deactivate Account via API",
+                accountId,
+                (a, u) -> {
+                  if (!a.account().isActive()) {
+                    alreadyInactive.set(true);
+                  } else {
+                    try {
+                      accountActivationValidationListeners.runEach(
+                          l -> l.validateDeactivation(a), ValidationException.class);
+                    } catch (ValidationException e) {
+                      exception.set(Optional.of(new ResourceConflictException(e.getMessage(), e)));
+                      return;
+                    }
+                    u.setActive(false);
+                  }
+                });
+    if (!updatedAccount.isPresent()) {
+      throw new ResourceNotFoundException("account not found");
+    }
+
     if (exception.get().isPresent()) {
       throw exception.get().get();
     }
@@ -94,26 +98,30 @@ public class SetInactiveFlag {
       throws RestApiException, IOException, ConfigInvalidException {
     AtomicBoolean alreadyActive = new AtomicBoolean(false);
     AtomicReference<Optional<RestApiException>> exception = new AtomicReference<>(Optional.empty());
-    accountsUpdateProvider
-        .get()
-        .update(
-            "Activate Account via API",
-            accountId,
-            (a, u) -> {
-              if (a.account().isActive()) {
-                alreadyActive.set(true);
-              } else {
-                try {
-                  accountActivationValidationListeners.runEach(
-                      l -> l.validateActivation(a), ValidationException.class);
-                } catch (ValidationException e) {
-                  exception.set(Optional.of(new ResourceConflictException(e.getMessage(), e)));
-                  return;
-                }
-                u.setActive(true);
-              }
-            })
-        .orElseThrow(() -> new ResourceNotFoundException("account not found"));
+    Optional<AccountState> updatedAccount =
+        accountsUpdateProvider
+            .get()
+            .update(
+                "Activate Account via API",
+                accountId,
+                (a, u) -> {
+                  if (a.account().isActive()) {
+                    alreadyActive.set(true);
+                  } else {
+                    try {
+                      accountActivationValidationListeners.runEach(
+                          l -> l.validateActivation(a), ValidationException.class);
+                    } catch (ValidationException e) {
+                      exception.set(Optional.of(new ResourceConflictException(e.getMessage(), e)));
+                      return;
+                    }
+                    u.setActive(true);
+                  }
+                });
+    if (!updatedAccount.isPresent()) {
+      throw new ResourceNotFoundException("account not found");
+    }
+
     if (exception.get().isPresent()) {
       throw exception.get().get();
     }
diff --git a/java/com/google/gerrit/server/project/ProjectCacheWarmer.java b/java/com/google/gerrit/server/project/ProjectCacheWarmer.java
index 332aba71f2..8794f6675b 100644
--- a/java/com/google/gerrit/server/project/ProjectCacheWarmer.java
+++ b/java/com/google/gerrit/server/project/ProjectCacheWarmer.java
@@ -22,6 +22,7 @@ import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.logging.LoggingContextAwareExecutorService;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
+import java.util.Optional;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
@@ -54,15 +55,15 @@ public class ProjectCacheWarmer implements LifecycleListener {
               () -> {
                 for (Project.NameKey name : cache.all()) {
                   pool.execute(
-                      () ->
-                          cache
-                              .get(name)
-                              .orElseThrow(
-                                  () ->
-                                      new IllegalStateException(
-                                          "race while traversing projects. got "
-                                              + name
-                                              + " when loading all projects, but can't load it now")));
+                      () -> {
+                        Optional<ProjectState> project = cache.get(name);
+                        if (!project.isPresent()) {
+                          throw new IllegalStateException(
+                              "race while traversing projects. got "
+                                  + name
+                                  + " when loading all projects, but can't load it now");
+                        }
+                      });
                 }
                 pool.shutdown();
                 try {
diff --git a/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java b/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
index 6c5559ce9f..2c935c566d 100644
--- a/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
+++ b/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
@@ -15,11 +15,11 @@
 package com.google.gerrit.server.project;
 
 import static com.google.common.collect.ImmutableList.toImmutableList;
-import static com.google.gerrit.server.project.ProjectCache.noSuchProject;
 
 import com.google.common.collect.Streams;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.Project;
 import com.google.gerrit.entities.SubmitRecord;
 import com.google.gerrit.entities.SubmitTypeRecord;
 import com.google.gerrit.exceptions.StorageException;
@@ -115,7 +115,12 @@ public class SubmitRuleEvaluator {
           throw new StorageException("Change not found");
         }
 
-        projectState = projectCache.get(cd.project()).orElseThrow(noSuchProject(cd.project()));
+        Project.NameKey name = cd.project();
+        Optional<ProjectState> projectStateOptional = projectCache.get(name);
+        if (!projectStateOptional.isPresent()) {
+          throw new NoSuchProjectException(name);
+        }
+        projectState = projectStateOptional.get();
       } catch (NoSuchProjectException e) {
         throw new IllegalStateException("Unable to find project while evaluating submit rule", e);
       }
@@ -168,7 +173,11 @@ public class SubmitRuleEvaluator {
   public SubmitTypeRecord getSubmitType(ChangeData cd) {
     try (Timer0.Context ignored = submitTypeEvaluationLatency.start()) {
       try {
-        projectCache.get(cd.project()).orElseThrow(noSuchProject(cd.project()));
+        Project.NameKey name = cd.project();
+        Optional<ProjectState> project = projectCache.get(name);
+        if (!project.isPresent()) {
+          throw new NoSuchProjectException(name);
+        }
       } catch (NoSuchProjectException e) {
         throw new IllegalStateException("Unable to find project while evaluating submit rule", e);
       }
diff --git a/java/com/google/gerrit/server/restapi/account/PutPreferred.java b/java/com/google/gerrit/server/restapi/account/PutPreferred.java
index aee0b78441..9a11891117 100644
--- a/java/com/google/gerrit/server/restapi/account/PutPreferred.java
+++ b/java/com/google/gerrit/server/restapi/account/PutPreferred.java
@@ -28,6 +28,7 @@ import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.ServerInitiated;
 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.ExternalIdFactory;
@@ -92,70 +93,75 @@ public class PutPreferred implements RestModifyView<AccountResource.Email, Input
       throws RestApiException, IOException, ConfigInvalidException {
     AtomicReference<Optional<RestApiException>> exception = new AtomicReference<>(Optional.empty());
     AtomicBoolean alreadyPreferred = new AtomicBoolean(false);
-    accountsUpdateProvider
-        .get()
-        .update(
-            "Set Preferred Email via API",
-            user.getAccountId(),
-            (a, u) -> {
-              if (preferredEmail.equals(a.account().preferredEmail())) {
-                alreadyPreferred.set(true);
-              } else {
-                // check if the user has a matching email
-                String matchingEmail = null;
-                for (String email :
-                    a.externalIds().stream()
-                        .map(ExternalId::email)
-                        .filter(Objects::nonNull)
-                        .collect(toSet())) {
-                  if (email.equals(preferredEmail)) {
-                    // we have an email that matches exactly, prefer this one
-                    matchingEmail = email;
-                    break;
-                  } else if (matchingEmail == null && email.equalsIgnoreCase(preferredEmail)) {
-                    // we found an email that matches but has a different case
-                    matchingEmail = email;
-                  }
-                }
-
-                if (matchingEmail == null) {
-                  // user doesn't have an external ID for this email
-                  if (user.hasEmailAddress(preferredEmail)) {
-                    // but Realm says the user is allowed to use this email
-                    Set<ExternalId> existingExtIdsWithThisEmail =
-                        externalIds.byEmail(preferredEmail);
-                    if (!existingExtIdsWithThisEmail.isEmpty()) {
-                      // but the email is already assigned to another account
-                      logger.atWarning().log(
-                          "Cannot set preferred email %s for account %s because it is owned"
-                              + " by the following account(s): %s",
-                          preferredEmail,
-                          user.getAccountId(),
-                          existingExtIdsWithThisEmail.stream()
-                              .map(ExternalId::accountId)
-                              .collect(toList()));
-                      exception.set(
-                          Optional.of(
-                              new ResourceConflictException("email in use by another account")));
-                      return;
+    Optional<AccountState> updatedAccount =
+        accountsUpdateProvider
+            .get()
+            .update(
+                "Set Preferred Email via API",
+                user.getAccountId(),
+                (a, u) -> {
+                  if (preferredEmail.equals(a.account().preferredEmail())) {
+                    alreadyPreferred.set(true);
+                  } else {
+                    // check if the user has a matching email
+                    String matchingEmail = null;
+                    for (String email :
+                        a.externalIds().stream()
+                            .map(ExternalId::email)
+                            .filter(Objects::nonNull)
+                            .collect(toSet())) {
+                      if (email.equals(preferredEmail)) {
+                        // we have an email that matches exactly, prefer this one
+                        matchingEmail = email;
+                        break;
+                      } else if (matchingEmail == null && email.equalsIgnoreCase(preferredEmail)) {
+                        // we found an email that matches but has a different case
+                        matchingEmail = email;
+                      }
                     }
 
-                    // claim the email now
-                    u.addExternalId(
-                        externalIdFactory.createEmail(a.account().id(), preferredEmail));
-                    matchingEmail = preferredEmail;
-                  } else {
-                    // Realm says that the email doesn't belong to the user. This can only happen as
-                    // a race condition because EmailsCollection would have thrown
-                    // ResourceNotFoundException already before invoking this REST endpoint.
-                    exception.set(Optional.of(new ResourceNotFoundException(preferredEmail)));
-                    return;
+                    if (matchingEmail == null) {
+                      // user doesn't have an external ID for this email
+                      if (user.hasEmailAddress(preferredEmail)) {
+                        // but Realm says the user is allowed to use this email
+                        Set<ExternalId> existingExtIdsWithThisEmail =
+                            externalIds.byEmail(preferredEmail);
+                        if (!existingExtIdsWithThisEmail.isEmpty()) {
+                          // but the email is already assigned to another account
+                          logger.atWarning().log(
+                              "Cannot set preferred email %s for account %s because it is owned"
+                                  + " by the following account(s): %s",
+                              preferredEmail,
+                              user.getAccountId(),
+                              existingExtIdsWithThisEmail.stream()
+                                  .map(ExternalId::accountId)
+                                  .collect(toList()));
+                          exception.set(
+                              Optional.of(
+                                  new ResourceConflictException(
+                                      "email in use by another account")));
+                          return;
+                        }
+
+                        // claim the email now
+                        u.addExternalId(
+                            externalIdFactory.createEmail(a.account().id(), preferredEmail));
+                        matchingEmail = preferredEmail;
+                      } else {
+                        // Realm says that the email doesn't belong to the user. This can only
+                        // happen as
+                        // a race condition because EmailsCollection would have thrown
+                        // ResourceNotFoundException already before invoking this REST endpoint.
+                        exception.set(Optional.of(new ResourceNotFoundException(preferredEmail)));
+                        return;
+                      }
+                    }
+                    u.setPreferredEmail(matchingEmail);
                   }
-                }
-                u.setPreferredEmail(matchingEmail);
-              }
-            })
-        .orElseThrow(() -> new ResourceNotFoundException("account not found"));
+                });
+    if (!updatedAccount.isPresent()) {
+      throw new ResourceNotFoundException("account not found");
+    }
     if (exception.get().isPresent()) {
       throw exception.get().get();
     }
diff --git a/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java b/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java
index 02c2ff0280..97f866b991 100644
--- a/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java
+++ b/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java
@@ -15,6 +15,7 @@
 package com.google.gerrit.server.restapi.change;
 
 import com.google.common.base.MoreObjects;
+import com.google.gerrit.entities.Project;
 import com.google.gerrit.entities.SubmitRecord;
 import com.google.gerrit.extensions.common.AccountInfo;
 import com.google.gerrit.extensions.common.TestSubmitRuleInfo;
@@ -28,12 +29,14 @@ import com.google.gerrit.server.account.AccountLoader;
 import com.google.gerrit.server.change.RevisionResource;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.rules.PrologOptions;
 import com.google.gerrit.server.rules.PrologRule;
 import com.google.gerrit.server.rules.RulesCache;
 import com.google.inject.Inject;
 import java.util.LinkedHashMap;
+import java.util.Optional;
 import org.kohsuke.args4j.Option;
 
 public class TestSubmitRule implements RestModifyView<RevisionResource, TestSubmitRuleInput> {
@@ -74,9 +77,11 @@ public class TestSubmitRule implements RestModifyView<RevisionResource, TestSubm
     }
     input.filters = MoreObjects.firstNonNull(input.filters, filters);
 
-    projectCache
-        .get(rsrc.getProject())
-        .orElseThrow(() -> new BadRequestException("project not found " + rsrc.getProject()));
+    Project.NameKey name = rsrc.getProject();
+    Optional<ProjectState> project = projectCache.get(name);
+    if (!project.isPresent()) {
+      throw new BadRequestException("project not found " + name);
+    }
     ChangeData cd = changeDataFactory.create(rsrc.getNotes());
     SubmitRecord record =
         prologRule.evaluate(