commit a42f8b5e753003f61cea043a00f7a1f8ebede0ac
author Shawn Pearce <sop@google.com> Fri Aug 11 23:12:16 2017 -0700
committer Shawn Pearce <sop@google.com> Fri Aug 11 23:23:19 2017 -0700
tree 70c3dd02d4b1832d936ca20795fedd8a28609d53
parent 2c76a76449b94574040c5fc6c53cd4684d1717f5

UiActions: defer GlobalPermission checks on views

Instead of running the global permission check immediately, always get
the view's UiAction.Description and chain the global permissions
checks into visible's BooleanCondition tree.  This allows global
permissions and plugin-specific permissions to evaluate through
bulkEvaluateTest.

Change-Id: I80d7f08d56526838d5a0990e91ba586ca0d7c9d3

gerrit-server/src/main/java/com/google/gerrit/server/extensions/webui/UiActions.java

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/webui/UiActions.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/webui/UiActions.java
index 2993fa8..79a5d4c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/webui/UiActions.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/webui/UiActions.java
@@ -14,12 +14,15 @@
 
 package com.google.gerrit.server.extensions.webui;
 
+import static com.google.gerrit.extensions.conditions.BooleanCondition.and;
+import static com.google.gerrit.extensions.conditions.BooleanCondition.or;
 import static java.util.stream.Collectors.toList;
 
 import com.google.common.base.Predicate;
 import com.google.common.collect.Streams;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission;
+import com.google.gerrit.extensions.conditions.BooleanCondition;
 import com.google.gerrit.extensions.registration.DynamicMap;
 import com.google.gerrit.extensions.restapi.RestCollection;
 import com.google.gerrit.extensions.restapi.RestResource;
@@ -35,6 +38,7 @@
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Objects;
 import java.util.Set;
@@ -110,22 +114,27 @@
       return null;
     }
 
+    UiAction.Description dsc = ((UiAction<R>) view).getDescription(resource);
+    if (dsc == null) {
+      return null;
+    }
+
+    Set<GlobalOrPluginPermission> globalRequired;
     try {
-      Set<GlobalOrPluginPermission> need =
-          GlobalPermission.fromAnnotation(e.getPluginName(), view.getClass());
-      if (!need.isEmpty() && permissionBackend.user(userProvider).test(need).isEmpty()) {
-        // A permission is required, but test returned no candidates.
-        return null;
-      }
+      globalRequired = GlobalPermission.fromAnnotation(e.getPluginName(), view.getClass());
     } catch (PermissionBackendException err) {
       log.error(
           String.format("exception testing view %s.%s", e.getPluginName(), e.getExportName()), err);
       return null;
     }
-
-    UiAction.Description dsc = ((UiAction<R>) view).getDescription(resource);
-    if (dsc == null) {
-      return null;
+    if (!globalRequired.isEmpty()) {
+      PermissionBackend.WithUser withUser = permissionBackend.user(userProvider);
+      Iterator<GlobalOrPluginPermission> i = globalRequired.iterator();
+      BooleanCondition p = withUser.testCond(i.next());
+      while (i.hasNext()) {
+        p = or(p, withUser.testCond(i.next()));
+      }
+      dsc.setVisible(and(p, dsc.getVisibleCondition()));
     }
 
     String name = e.getExportName().substring(d + 1);