Merge changes from topic "boolcond"

* changes:
  UiActions: defer GlobalPermission checks on views
  CherryPick: use BooleanCondition for visible
  PermissionBackend: support bulk evaluation of UiAction
  PermissionBackend: support testCond for UiAction
  Delay UiAction visible and enabled with BooleanCondition

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/conditions/BooleanCondition.java

@@ -0,0 +1,217 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.extensions.conditions;
+
+import com.google.common.collect.Iterables;
+import java.util.Collections;
+
+/** Delayed evaluation of a boolean condition. */
+public abstract class BooleanCondition {
+  public static final BooleanCondition TRUE = new Value(true);
+  public static final BooleanCondition FALSE = new Value(false);
+
+  public static BooleanCondition valueOf(boolean a) {
+    return a ? TRUE : FALSE;
+  }
+
+  public static BooleanCondition and(BooleanCondition a, BooleanCondition b) {
+    return a == FALSE || b == FALSE ? FALSE : new And(a, b);
+  }
+
+  public static BooleanCondition and(boolean a, BooleanCondition b) {
+    return and(valueOf(a), b);
+  }
+
+  public static BooleanCondition or(BooleanCondition a, BooleanCondition b) {
+    return a == TRUE || b == TRUE ? TRUE : new Or(a, b);
+  }
+
+  public static BooleanCondition or(boolean a, BooleanCondition b) {
+    return or(valueOf(a), b);
+  }
+
+  public static BooleanCondition not(BooleanCondition bc) {
+    return bc == TRUE ? FALSE : bc == FALSE ? TRUE : new Not(bc);
+  }
+
+  BooleanCondition() {}
+
+  /** @return evaluate the condition and return its value. */
+  public abstract boolean value();
+
+  /**
+   * Recursively collect all children of type {@code type}.
+   *
+   * @param type implementation type of the conditions to collect and return.
+   * @return non-null, unmodifiable iteration of children of type {@code type}.
+   */
+  public abstract <T> Iterable<T> children(Class<T> type);
+
+  private static final class And extends BooleanCondition {
+    private final BooleanCondition a;
+    private final BooleanCondition b;
+
+    And(BooleanCondition a, BooleanCondition b) {
+      this.a = a;
+      this.b = b;
+    }
+
+    @Override
+    public boolean value() {
+      return a.value() && b.value();
+    }
+
+    @Override
+    public <T> Iterable<T> children(Class<T> type) {
+      return Iterables.concat(a.children(type), b.children(type));
+    }
+
+    @Override
+    public int hashCode() {
+      return a.hashCode() * 31 + b.hashCode();
+    }
+
+    @Override
+    public boolean equals(Object other) {
+      if (other instanceof And) {
+        And o = (And) other;
+        return a.equals(o.a) && b.equals(o.b);
+      }
+      return false;
+    }
+
+    @Override
+    public String toString() {
+      return "(" + maybeTrim(a, getClass()) + " && " + maybeTrim(a, getClass()) + ")";
+    }
+  }
+
+  private static final class Or extends BooleanCondition {
+    private final BooleanCondition a;
+    private final BooleanCondition b;
+
+    Or(BooleanCondition a, BooleanCondition b) {
+      this.a = a;
+      this.b = b;
+    }
+
+    @Override
+    public boolean value() {
+      return a.value() || b.value();
+    }
+
+    @Override
+    public <T> Iterable<T> children(Class<T> type) {
+      return Iterables.concat(a.children(type), b.children(type));
+    }
+
+    @Override
+    public int hashCode() {
+      return a.hashCode() * 31 + b.hashCode();
+    }
+
+    @Override
+    public boolean equals(Object other) {
+      if (other instanceof Or) {
+        Or o = (Or) other;
+        return a.equals(o.a) && b.equals(o.b);
+      }
+      return false;
+    }
+
+    @Override
+    public String toString() {
+      return "(" + maybeTrim(a, getClass()) + " || " + maybeTrim(a, getClass()) + ")";
+    }
+  }
+
+  private static final class Not extends BooleanCondition {
+    private final BooleanCondition cond;
+
+    Not(BooleanCondition bc) {
+      cond = bc;
+    }
+
+    @Override
+    public boolean value() {
+      return !cond.value();
+    }
+
+    @Override
+    public <T> Iterable<T> children(Class<T> type) {
+      return cond.children(type);
+    }
+
+    @Override
+    public int hashCode() {
+      return cond.hashCode() * 31;
+    }
+
+    @Override
+    public boolean equals(Object other) {
+      return other instanceof Not ? cond.equals(((Not) other).cond) : false;
+    }
+
+    @Override
+    public String toString() {
+      return "!" + cond;
+    }
+  }
+
+  private static final class Value extends BooleanCondition {
+    private final boolean value;
+
+    Value(boolean v) {
+      value = v;
+    }
+
+    @Override
+    public boolean value() {
+      return value;
+    }
+
+    @Override
+    public <T> Iterable<T> children(Class<T> type) {
+      return Collections.emptyList();
+    }
+
+    @Override
+    public int hashCode() {
+      return value ? 1 : 0;
+    }
+
+    @Override
+    public boolean equals(Object other) {
+      return other instanceof Value ? value == ((Value) other).value : false;
+    }
+
+    @Override
+    public String toString() {
+      return Boolean.toString(value);
+    }
+  }
+
+  /** Remove leading '(' and trailing ')' if the type is the same as the parent. */
+  static String maybeTrim(BooleanCondition cond, Class<? extends BooleanCondition> type) {
+    String s = cond.toString();
+    if (cond.getClass() == type
+        && s.length() > 2
+        && s.charAt(0) == '('
+        && s.charAt(s.length() - 1) == ')') {
+      s = s.substring(1, s.length() - 1);
+    }
+    return s;
+  }
+}

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/conditions/PrivateInternals_BooleanCondition.java

@@ -0,0 +1,33 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.extensions.conditions;
+
+import java.util.Collections;
+
+/** <b>DO NOT USE</b> */
+public final class PrivateInternals_BooleanCondition {
+  private PrivateInternals_BooleanCondition() {}
+
+  public abstract static class SubclassOnlyInCoreServer extends BooleanCondition {
+    @SuppressWarnings("unchecked")
+    @Override
+    public <T> Iterable<T> children(Class<T> type) {
+      if (type.isAssignableFrom(getClass())) {
+        return Collections.singleton((T) this);
+      }
+      return Collections.emptyList();
+    }
+  }
+}

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/webui/UiAction.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.extensions.webui;
 
+import com.google.gerrit.extensions.conditions.BooleanCondition;
 import com.google.gerrit.extensions.restapi.RestResource;
 import com.google.gerrit.extensions.restapi.RestView;
 
@@ -35,8 +36,8 @@ public interface UiAction<R extends RestResource> extends RestView<R> {
     private String id;
     private String label;
     private String title;
-    private boolean visible = true;
-    private boolean enabled = true;
+    private BooleanCondition visible = BooleanCondition.TRUE;
+    private BooleanCondition enabled = BooleanCondition.TRUE;
 
     public String getMethod() {
       return method;
@@ -77,6 +78,10 @@ public interface UiAction<R extends RestResource> extends RestView<R> {
     }
 
     public boolean isVisible() {
+      return getVisibleCondition().value();
+    }
+
+    public BooleanCondition getVisibleCondition() {
       return visible;
     }
 
@@ -85,16 +90,33 @@ public interface UiAction<R extends RestResource> extends RestView<R> {
      * action description may not be sent to the client.
      */
     public Description setVisible(boolean visible) {
+      return setVisible(BooleanCondition.valueOf(visible));
+    }
+
+    /**
+     * Set if the action's button is visible on screen for the current client. If not visible the
+     * action description may not be sent to the client.
+     */
+    public Description setVisible(BooleanCondition visible) {
       this.visible = visible;
       return this;
     }
 
     public boolean isEnabled() {
-      return enabled && isVisible();
+      return getEnabledCondition().value();
+    }
+
+    public BooleanCondition getEnabledCondition() {
+      return BooleanCondition.and(enabled, visible);
     }
 
     /** Set if the button should be invokable (true), or greyed out (false). */
     public Description setEnabled(boolean enabled) {
+      return setEnabled(BooleanCondition.valueOf(enabled));
+    }
+
+    /** Set if the button should be invokable (true), or greyed out (false). */
+    public Description setEnabled(BooleanCondition enabled) {
       this.enabled = enabled;
       return this;
     }

gerrit-server/src/main/java/com/google/gerrit/server/change/ActionJson.java

@@ -16,7 +16,6 @@ package com.google.gerrit.server.change;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
-import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
@@ -36,6 +35,7 @@ import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -162,7 +162,7 @@ public class ActionJson {
       return out;
     }
 
-    FluentIterable<UiAction.Description> descs =
+    Iterable<UiAction.Description> descs =
         uiActions.from(changeViews, changeResourceFactory.create(ctl));
 
     // The followup action is a client-side only operation that does not
@@ -174,7 +174,7 @@ public class ActionJson {
       PrivateInternals_UiActionDescription.setMethod(descr, "POST");
       descr.setTitle("Create follow-up change");
       descr.setLabel("Follow-Up");
-      descs = descs.append(descr);
+      descs = Iterables.concat(descs, Collections.singleton(descr));
     }
 
     ACTION:

gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java

@@ -14,6 +14,8 @@
 
 package com.google.gerrit.server.change;
 
+import static com.google.gerrit.extensions.conditions.BooleanCondition.and;
+
 import com.google.gerrit.extensions.api.changes.CherryPickInput;
 import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -106,10 +108,11 @@ public class CherryPick
         .setLabel("Cherry Pick")
         .setTitle("Cherry pick change to a different branch")
         .setVisible(
-            rsrc.isCurrent()
-                && permissionBackend
+            and(
+                rsrc.isCurrent(),
+                permissionBackend
                     .user(user)
                     .project(rsrc.getProject())
-                    .testOrFalse(ProjectPermission.CREATE_CHANGE));
+                    .testCond(ProjectPermission.CREATE_CHANGE)));
   }
 }

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

@@ -14,23 +14,32 @@
 
 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.FluentIterable;
+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;
 import com.google.gerrit.extensions.restapi.RestView;
 import com.google.gerrit.extensions.webui.PrivateInternals_UiActionDescription;
 import com.google.gerrit.extensions.webui.UiAction;
+import com.google.gerrit.extensions.webui.UiAction.Description;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.permissions.GlobalPermission;
 import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendCondition;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 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;
 import org.slf4j.Logger;
@@ -53,16 +62,35 @@ public class UiActions {
     this.userProvider = userProvider;
   }
 
-  public <R extends RestResource> FluentIterable<UiAction.Description> from(
+  public <R extends RestResource> Iterable<UiAction.Description> from(
       RestCollection<?, R> collection, R resource) {
     return from(collection.views(), resource);
   }
 
-  public <R extends RestResource> FluentIterable<UiAction.Description> from(
+  public <R extends RestResource> Iterable<UiAction.Description> from(
       DynamicMap<RestView<R>> views, R resource) {
-    return FluentIterable.from(views)
-        .transform((e) -> describe(e, resource))
-        .filter(Objects::nonNull);
+    List<UiAction.Description> descs =
+        Streams.stream(views)
+            .map(e -> describe(e, resource))
+            .filter(Objects::nonNull)
+            .collect(toList());
+
+    List<PermissionBackendCondition> conds =
+        Streams.concat(
+                descs.stream().flatMap(u -> Streams.stream(visibleCondition(u))),
+                descs.stream().flatMap(u -> Streams.stream(enabledCondition(u))))
+            .collect(toList());
+    permissionBackend.bulkEvaluateTest(conds);
+
+    return descs.stream().filter(u -> u.isVisible()).collect(toList());
+  }
+
+  private static Iterable<PermissionBackendCondition> visibleCondition(Description u) {
+    return u.getVisibleCondition().children(PermissionBackendCondition.class);
+  }
+
+  private static Iterable<PermissionBackendCondition> enabledCondition(Description u) {
+    return u.getEnabledCondition().children(PermissionBackendCondition.class);
   }
 
   @Nullable
@@ -86,22 +114,27 @@ public class UiActions {
       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 || !dsc.isVisible()) {
-      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);

gerrit-server/src/main/java/com/google/gerrit/server/permissions/PermissionBackend.java

@@ -20,6 +20,7 @@ import static java.util.stream.Collectors.toSet;
 import com.google.common.collect.Sets;
 import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission;
+import com.google.gerrit.extensions.conditions.BooleanCondition;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Project;
@@ -78,7 +79,7 @@ import org.slf4j.LoggerFactory;
  *   public UiAction.Description getDescription(ChangeResource rsrc) {
  *     return new UiAction.Description()
  *       .setLabel("Submit")
- *       .setVisible(rsrc.permissions().testOrFalse(ChangePermission.SUBMIT));
+ *       .setVisible(rsrc.permissions().testCond(ChangePermission.SUBMIT));
  * }
  * </pre>
  */
@@ -94,6 +95,24 @@ public abstract class PermissionBackend {
     return user(checkNotNull(user, "Provider<CurrentUser>").get());
   }
 
+  /**
+   * Bulk evaluate a collection of {@link PermissionBackendCondition} for view handling.
+   *
+   * <p>Overridden implementations should call {@link PermissionBackendCondition#set(boolean)} to
+   * cache the result of {@code testOrFalse} in the condition for later evaluation. Caching the
+   * result will bypass the usual invocation of {@code testOrFalse}.
+   *
+   * <p>{@code conds} may contain duplicate entries (such as same user, resource, permission
+   * triplet). When duplicates exist, implementations should set a result into all instances to
+   * ensure {@code testOrFalse} does not get invoked during evaluation of the containing condition.
+   *
+   * @param conds conditions to consider.
+   */
+  public void bulkEvaluateTest(Collection<PermissionBackendCondition> conds) {
+    // Do nothing by default. The default implementation of PermissionBackendCondition
+    // delegates to the appropriate testOrFalse method in PermissionBackend.
+  }
+
   /** PermissionBackend with an optional per-request ReviewDb handle. */
   public abstract static class AcceptsReviewDb<T> {
     protected Provider<ReviewDb> db;
@@ -198,6 +217,10 @@ public abstract class PermissionBackend {
       }
     }
 
+    public BooleanCondition testCond(GlobalOrPluginPermission perm) {
+      return new PermissionBackendCondition.WithUser(this, perm);
+    }
+
     /**
      * Filter a set of projects using {@code check(perm)}.
      *
@@ -265,6 +288,10 @@ public abstract class PermissionBackend {
         return false;
       }
     }
+
+    public BooleanCondition testCond(ProjectPermission perm) {
+      return new PermissionBackendCondition.ForProject(this, perm);
+    }
   }
 
   /** PermissionBackend scoped to a user, project and reference. */
@@ -313,6 +340,10 @@ public abstract class PermissionBackend {
         return false;
       }
     }
+
+    public BooleanCondition testCond(RefPermission perm) {
+      return new PermissionBackendCondition.ForRef(this, perm);
+    }
   }
 
   /** PermissionBackend scoped to a user, project, reference and change. */
@@ -354,6 +385,10 @@ public abstract class PermissionBackend {
       }
     }
 
+    public BooleanCondition testCond(ChangePermissionOrLabel perm) {
+      return new PermissionBackendCondition.ForChange(this, perm);
+    }
+
     /**
      * Test which values of a label the user may be able to set.
      *

gerrit-server/src/main/java/com/google/gerrit/server/permissions/PermissionBackendCondition.java

@@ -0,0 +1,152 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.permissions;
+
+import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission;
+import com.google.gerrit.extensions.conditions.BooleanCondition;
+import com.google.gerrit.extensions.conditions.PrivateInternals_BooleanCondition;
+
+/** {@link BooleanCondition} to evaluate a permission. */
+public abstract class PermissionBackendCondition
+    extends PrivateInternals_BooleanCondition.SubclassOnlyInCoreServer {
+  Boolean value;
+
+  /**
+   * Assign a specific {@code testOrFalse} result to this condition.
+   *
+   * <p>By setting the condition to a specific value the condition will bypass calling {@link
+   * PermissionBackend} during {@code value()}, and immediately return the set value instead.
+   *
+   * @param val value to return from {@code value()}.
+   */
+  public void set(boolean val) {
+    value = val;
+  }
+
+  @Override
+  public abstract String toString();
+
+  public static class WithUser extends PermissionBackendCondition {
+    private final PermissionBackend.WithUser impl;
+    private final GlobalOrPluginPermission perm;
+
+    WithUser(PermissionBackend.WithUser impl, GlobalOrPluginPermission perm) {
+      this.impl = impl;
+      this.perm = perm;
+    }
+
+    public PermissionBackend.WithUser withUser() {
+      return impl;
+    }
+
+    public GlobalOrPluginPermission permission() {
+      return perm;
+    }
+
+    @Override
+    public boolean value() {
+      return value != null ? value : impl.testOrFalse(perm);
+    }
+
+    @Override
+    public String toString() {
+      return "PermissionBackendCondition.WithUser(" + perm + ")";
+    }
+  }
+
+  public static class ForProject extends PermissionBackendCondition {
+    private final PermissionBackend.ForProject impl;
+    private final ProjectPermission perm;
+
+    ForProject(PermissionBackend.ForProject impl, ProjectPermission perm) {
+      this.impl = impl;
+      this.perm = perm;
+    }
+
+    public PermissionBackend.ForProject project() {
+      return impl;
+    }
+
+    public ProjectPermission permission() {
+      return perm;
+    }
+
+    @Override
+    public boolean value() {
+      return value != null ? value : impl.testOrFalse(perm);
+    }
+
+    @Override
+    public String toString() {
+      return "PermissionBackendCondition.ForProject(" + perm + ")";
+    }
+  }
+
+  public static class ForRef extends PermissionBackendCondition {
+    private final PermissionBackend.ForRef impl;
+    private final RefPermission perm;
+
+    ForRef(PermissionBackend.ForRef impl, RefPermission perm) {
+      this.impl = impl;
+      this.perm = perm;
+    }
+
+    public PermissionBackend.ForRef ref() {
+      return impl;
+    }
+
+    public RefPermission permission() {
+      return perm;
+    }
+
+    @Override
+    public boolean value() {
+      return value != null ? value : impl.testOrFalse(perm);
+    }
+
+    @Override
+    public String toString() {
+      return "PermissionBackendCondition.ForRef(" + perm + ")";
+    }
+  }
+
+  public static class ForChange extends PermissionBackendCondition {
+    private final PermissionBackend.ForChange impl;
+    private final ChangePermissionOrLabel perm;
+
+    ForChange(PermissionBackend.ForChange impl, ChangePermissionOrLabel perm) {
+      this.impl = impl;
+      this.perm = perm;
+    }
+
+    public PermissionBackend.ForChange change() {
+      return impl;
+    }
+
+    public ChangePermissionOrLabel permission() {
+      return perm;
+    }
+
+    @Override
+    public boolean value() {
+      return value != null ? value : impl.testOrFalse(perm);
+    }
+
+    @Override
+    public String toString() {
+      return "PermissionBackendCondition.ForChange(" + perm + ")";
+    }
+  }
+}