opendev/gerrit
Code Issues Proposed changes

Fix DefaultPermissionBackend#absentUser to work outside the request scope

Browse Source 
A recent optimization has caused this operation to fail when called
from outside the request scope. This fix makes sure getting a user
is optional and the code still works if that isn't possible.

Change-Id: I963b073bb82d2a1014971fe170804e058373f0f7
This commit is contained in:
Patrick Hiesel
2020-10-21 18:17:41 +02:00
parent ee3012b743
commit e5b6e2ea19
1 changed files with 19 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
View File

@@ -42,6 +42,7 @@ import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.Set;
@Singleton
@@ -82,12 +83,12 @@ public class DefaultPermissionBackend extends PermissionBackend {
@Override
public WithUser absentUser(Account.Id id) {
requireNonNull(id, "user");
CurrentUser user = currentUser.get();
if (user.isIdentifiedUser() && id.equals(user.asIdentifiedUser().getAccountId())) {
Optional<Account.Id> user = getAccountIdOfIdentifiedUser();
if (user.isPresent() && id.equals(user.get())) {
// What looked liked an absent user is actually the current caller. Use the per-request
// singleton IdentifiedUser instead of constructing a new object to leverage caching in member
// variables of IdentifiedUser.
return new WithUserImpl(user.asIdentifiedUser());
return new WithUserImpl(currentUser.get().asIdentifiedUser());
}
return new WithUserImpl(identifiedUserFactory.create(requireNonNull(id, "user")));
}
@@ -97,6 +98,21 @@ public class DefaultPermissionBackend extends PermissionBackend {
return true;
}
/**
* Returns the {@link Account.Id} of the current user if a user is signed in. Catches exceptions
* so that background jobs don't get impacted.
*/
private Optional<Account.Id> getAccountIdOfIdentifiedUser() {
try {
return currentUser.get().isIdentifiedUser()
? Optional.of(currentUser.get().getAccountId())
: Optional.empty();
} catch (Exception e) {
logger.atFine().withCause(e).log("Unable to get current user");
return Optional.empty();
}
}
class WithUserImpl extends WithUser {
private final CurrentUser user;
private Boolean admin;