Fix ReturnValueIgnored bug pattern flagged by error prone Details: https://errorprone.info/bugpattern/ReturnValueIgnored Change-Id: Ieef7cb2f10df0d904d131c8a1311e2f7f942c2bb
diff --git a/java/com/google/gerrit/server/account/AccountManager.java b/java/com/google/gerrit/server/account/AccountManager.java index 407d2f7..891a467 100644 --- a/java/com/google/gerrit/server/account/AccountManager.java +++ b/java/com/google/gerrit/server/account/AccountManager.java
@@ -264,14 +264,16 @@ } 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 4b68198..5babebd 100644 --- a/java/com/google/gerrit/server/account/SetInactiveFlag.java +++ b/java/com/google/gerrit/server/account/SetInactiveFlag.java
@@ -55,26 +55,30 @@ 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 @@ 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 332aba7..8794f66 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.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 @@ () -> { 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 04b5a9f..1d999dd 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 @@ 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); } @@ -170,7 +175,11 @@ 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 aee0b78..9a11891 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.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 @@ 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 02c2ff0..97f866b 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.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 @@ } 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(