Change visibility: prefer test instead of check

Do not rely to throwing AuthExceptions for checking
change visibility but ForChange.test instead, which
returns a boolean.

Relying to exceptions to be thrown and catching them
for a simple change filtering would cause the JVM to
trigger multiple context switching and slowing down
the overall ACL evaluation.

A slow ACL evaluation for changes filtering may
result in the increase of threads utilisation
while the change query is executed.

Release-Notes: skip
Change-Id: Ie488a41b28b5c19710a11c2840793f04b1d3861b

1 files changed, 4 insertions, 5 deletions

diff --git a/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java b/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
index e3e0312cbc..60587da27b 100644
--- a/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
@@ -17,7 +17,6 @@ package com.google.gerrit.server.query.change;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.entities.Change;
 import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.index.query.IsVisibleToPredicate;
 import com.google.gerrit.server.AnonymousUser;
 import com.google.gerrit.server.CurrentUser;
@@ -87,7 +86,10 @@ public class ChangeIsVisibleToPredicate extends IsVisibleToPredicate<ChangeData>
                     .filter(u -> u instanceof GroupBackedUser || u instanceof InternalUser)
                     .orElseGet(anonymousUserProvider::get));
     try {
-      withUser.change(cd).check(ChangePermission.READ);
+      if (!withUser.change(cd).test(ChangePermission.READ)) {
+        logger.atFine().log("Filter out non-visisble change: %s", cd);
+        return false;
+      }
     } catch (PermissionBackendException e) {
       Throwable cause = e.getCause();
       if (cause instanceof RepositoryNotFoundException) {
@@ -97,9 +99,6 @@ public class ChangeIsVisibleToPredicate extends IsVisibleToPredicate<ChangeData>
         return false;
       }
       throw new StorageException("unable to check permissions on change " + cd.getId(), e);
-    } catch (AuthException e) {
-      logger.atFine().log("Filter out non-visisble change: %s", cd);
-      return false;
     }
 
     cd.cacheVisibleTo(user);