Convert SuggestParentCandidates to PermissionBackend

Define two ProjectPermissions:

  ACCESS - Caller can see at least one reference (or change) and
  should be able to learn the project exists, and use APIs that
  depend on that fact. Hidden projects are not accessible unless
  the user is an owner.

  READ - Caller can see all references in the project and can
  use things like GitwebServlet where filtering doesn't happen.
  Hidden projects are not readable unless the user is an owner.

Add a filter() method in PermissionBackend to support checking
a permission across many projects at once, and use this inside
of SuggestParentCandidates to filter results to only projects
that the caller has ACCESS permission to.

Change-Id: I1329a8df1e7858e02379b7a1a526ad4954f0e42a

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

@@ -17,6 +17,7 @@ package com.google.gerrit.server.permissions;
 import static com.google.common.base.Preconditions.checkNotNull;
 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.restapi.AuthException;
 import com.google.gerrit.reviewdb.client.Branch;
@@ -183,6 +184,30 @@ public abstract class PermissionBackend {
         return false;
       }
     }
+
+    /**
+     * Filter a set of projects using {@code check(perm)}.
+     *
+     * @param perm required permission in a project to be included in result.
+     * @param projects candidate set of projects; may be empty.
+     * @return filtered set of {@code projects} where {@code check(perm)} was successful.
+     * @throws PermissionBackendException backend cannot access its internal state.
+     */
+    public Set<Project.NameKey> filter(ProjectPermission perm, Collection<Project.NameKey> projects)
+        throws PermissionBackendException {
+      checkNotNull(perm, "ProjectPermission");
+      checkNotNull(projects, "projects");
+      Set<Project.NameKey> allowed = Sets.newHashSetWithExpectedSize(projects.size());
+      for (Project.NameKey project : projects) {
+        try {
+          project(project).check(perm);
+          allowed.add(project);
+        } catch (AuthException e) {
+          // Do not include this project in allowed.
+        }
+      }
+      return allowed;
+    }
   }
 
   /** PermissionBackend scoped to a user and project. */
@@ -204,6 +229,15 @@ public abstract class PermissionBackend {
     public boolean test(ProjectPermission perm) throws PermissionBackendException {
       return test(EnumSet.of(perm)).contains(perm);
     }
+
+    public boolean testOrFalse(ProjectPermission perm) {
+      try {
+        return test(perm);
+      } catch (PermissionBackendException e) {
+        logger.warn("Cannot test " + perm + "; assuming false", e);
+        return false;
+      }
+    }
   }
 
   /** PermissionBackend scoped to a user, project and reference. */

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

@@ -19,6 +19,19 @@ import java.util.Locale;
 import java.util.Optional;
 
 public enum ProjectPermission {
+  /**
+   * Can access at least one reference or change within the repository.
+   *
+   * <p>Checking this permission instead of {@link #READ} may require filtering to hide specific
+   * references or changes, which can be expensive.
+   */
+  ACCESS,
+
+  /**
+   * Can read all references in the repository.
+   *
+   * <p>This is a stronger form of {@link #ACCESS} where no filtering is required.
+   */
   READ(Permission.READ);
 
   private final String name;

gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java

@@ -608,8 +608,11 @@ public class ProjectControl {
 
     private boolean can(ProjectPermission perm) throws PermissionBackendException {
       switch (perm) {
+        case ACCESS:
+          return (!isHidden() && isReadable()) || isOwner();
+
         case READ:
-          return isReadable();
+          return (!isHidden() && allRefsAreVisible()) || isOwner();
       }
       throw new PermissionBackendException(perm + " unsupported");
     }

gerrit-server/src/main/java/com/google/gerrit/server/project/SuggestParentCandidates.java

@@ -14,65 +14,61 @@
 
 package com.google.gerrit.server.project;
 
+import static java.util.stream.Collectors.toList;
+
 import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.permissions.ProjectPermission;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 import com.google.inject.Singleton;
-import java.util.ArrayList;
-import java.util.Comparator;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
-import java.util.TreeSet;
 
 @Singleton
 public class SuggestParentCandidates {
-  private final ProjectControl.Factory projectControlFactory;
   private final ProjectCache projectCache;
-  private final AllProjectsName allProject;
+  private final PermissionBackend permissionBackend;
+  private final Provider<CurrentUser> user;
+  private final AllProjectsName allProjects;
 
   @Inject
   SuggestParentCandidates(
-      final ProjectControl.Factory projectControlFactory,
-      final ProjectCache projectCache,
-      final AllProjectsName allProject) {
-    this.projectControlFactory = projectControlFactory;
+      ProjectCache projectCache,
+      PermissionBackend permissionBackend,
+      Provider<CurrentUser> user,
+      AllProjectsName allProjects) {
     this.projectCache = projectCache;
-    this.allProject = allProject;
+    this.permissionBackend = permissionBackend;
+    this.user = user;
+    this.allProjects = allProjects;
   }
 
-  public List<Project.NameKey> getNameKeys() throws NoSuchProjectException {
-    List<Project> pList = getProjects();
-    final List<Project.NameKey> nameKeys = new ArrayList<>(pList.size());
-    for (Project p : pList) {
-      nameKeys.add(p.getNameKey());
-    }
-    return nameKeys;
+  public List<Project.NameKey> getNameKeys() throws PermissionBackendException {
+    return permissionBackend
+        .user(user)
+        .filter(ProjectPermission.ACCESS, parents())
+        .stream()
+        .sorted()
+        .collect(toList());
   }
 
-  public List<Project> getProjects() throws NoSuchProjectException {
-    Set<Project> projects =
-        new TreeSet<>(
-            new Comparator<Project>() {
-              @Override
-              public int compare(Project o1, Project o2) {
-                return o1.getName().compareTo(o2.getName());
-              }
-            });
+  private Set<Project.NameKey> parents() {
+    Set<Project.NameKey> parents = new HashSet<>();
     for (Project.NameKey p : projectCache.all()) {
-      try {
-        final ProjectControl control = projectControlFactory.controlFor(p);
-        final Project.NameKey parentK = control.getProject().getParent();
-        if (parentK != null) {
-          ProjectControl pControl = projectControlFactory.controlFor(parentK);
-          if (pControl.isVisible() || pControl.isOwner()) {
-            projects.add(pControl.getProject());
-          }
+      ProjectState ps = projectCache.get(p);
+      if (ps != null) {
+        Project.NameKey parent = ps.getProject().getParent();
+        if (parent != null) {
+          parents.add(parent);
         }
-      } catch (NoSuchProjectException e) {
-        continue;
       }
     }
-    projects.add(projectControlFactory.controlFor(allProject).getProject());
-    return new ArrayList<>(projects);
+    parents.add(allProjects);
+    return parents;
   }
 }