Convert ProjectControl.isVisible to PermissionBackend

Remove isReadable and isVisible, relying only on PermissionBackend.

Change-Id: I478119601acfe661da4c164a55e3642b020c4cd7

gerrit-server/src/main/java/com/google/gerrit/common/EventBroker.java

@@ -16,6 +16,7 @@ package com.google.gerrit.common;
 
 import com.google.gerrit.extensions.registration.DynamicItem;
 import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.lifecycle.LifecycleModule;
 import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Change;
@@ -28,6 +29,9 @@ import com.google.gerrit.server.events.Event;
 import com.google.gerrit.server.events.ProjectEvent;
 import com.google.gerrit.server.events.RefEvent;
 import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.permissions.ProjectPermission;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.gerrit.server.project.ProjectState;
@@ -54,6 +58,7 @@ public class EventBroker implements EventDispatcher {
   /** Listeners to receive all changes as they happen. */
   protected final DynamicSet<EventListener> unrestrictedListeners;
 
+  private final PermissionBackend permissionBackend;
   protected final ProjectCache projectCache;
 
   protected final ChangeNotes.Factory notesFactory;
@@ -64,11 +69,13 @@ public class EventBroker implements EventDispatcher {
   public EventBroker(
       DynamicSet<UserScopedEventListener> listeners,
       DynamicSet<EventListener> unrestrictedListeners,
+      PermissionBackend permissionBackend,
       ProjectCache projectCache,
       ChangeNotes.Factory notesFactory,
       Provider<ReviewDb> dbProvider) {
     this.listeners = listeners;
     this.unrestrictedListeners = unrestrictedListeners;
+    this.permissionBackend = permissionBackend;
     this.projectCache = projectCache;
     this.notesFactory = notesFactory;
     this.dbProvider = dbProvider;
@@ -137,11 +144,12 @@ public class EventBroker implements EventDispatcher {
   }
 
   protected boolean isVisibleTo(Project.NameKey project, CurrentUser user) {
-    ProjectState pe = projectCache.get(project);
+    try {
-    if (pe == null) {
+      permissionBackend.user(user).project(project).check(ProjectPermission.ACCESS);
+      return true;
+    } catch (AuthException | PermissionBackendException e) {
       return false;
     }
-    return pe.controlFor(user).isVisible();
   }
 
   protected boolean isVisibleTo(Change change, CurrentUser user) throws OrmException {

gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java

@@ -476,7 +476,7 @@ public class ReceiveCommits {
     // If the user lacks READ permission, some references may be filtered and hidden from view.
     // Check objects mentioned inside the incoming pack file are reachable from visible refs.
     try {
-      permissions.check(ProjectPermission.READ);
+      permissionBackend.user(user).project(project.getNameKey()).check(ProjectPermission.READ);
     } catch (AuthException e) {
       rp.setCheckReferencedObjectsAreReachable(receiveConfig.checkReferencedObjectsAreReachable);
     }

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

@@ -76,9 +76,6 @@ import org.slf4j.LoggerFactory;
 
 /** Access control management for a user accessing a project's data. */
 public class ProjectControl {
-  public static final int VISIBLE = 1 << 0;
-  public static final int OWNER = 1 << 1;
-
   private static final Logger log = LoggerFactory.getLogger(ProjectControl.class);
 
   public static class GenericFactory {
@@ -97,18 +94,6 @@ public class ProjectControl {
       }
       return p.controlFor(user);
     }
-
-    public ProjectControl validateFor(Project.NameKey nameKey, int need, CurrentUser user)
-        throws NoSuchProjectException, IOException {
-      final ProjectControl c = controlFor(nameKey, user);
-      if ((need & VISIBLE) == VISIBLE && c.isVisible()) {
-        return c;
-      }
-      if ((need & OWNER) == OWNER && c.isOwner()) {
-        return c;
-      }
-      throw new NoSuchProjectException(nameKey);
-    }
   }
 
   public static class Factory {
@@ -122,26 +107,6 @@ public class ProjectControl {
     public ProjectControl controlFor(final Project.NameKey nameKey) throws NoSuchProjectException {
       return userCache.get().get(nameKey);
     }
-
-    public ProjectControl validateFor(final Project.NameKey nameKey) throws NoSuchProjectException {
-      return validateFor(nameKey, VISIBLE);
-    }
-
-    public ProjectControl ownerFor(final Project.NameKey nameKey) throws NoSuchProjectException {
-      return validateFor(nameKey, OWNER);
-    }
-
-    public ProjectControl validateFor(final Project.NameKey nameKey, final int need)
-        throws NoSuchProjectException {
-      final ProjectControl c = controlFor(nameKey);
-      if ((need & VISIBLE) == VISIBLE && c.isVisible()) {
-        return c;
-      }
-      if ((need & OWNER) == OWNER && c.isOwner()) {
-        return c;
-      }
-      throw new NoSuchProjectException(nameKey);
-    }
   }
 
   public interface AssistedFactory {
@@ -280,21 +245,6 @@ public class ProjectControl {
     return getProject().getState().equals(com.google.gerrit.extensions.client.ProjectState.HIDDEN);
   }
 
-  /**
-   * Returns whether the project is readable to the current user. Note that the project could still
-   * be hidden.
-   */
-  public boolean isReadable() {
-    return (user.isInternalUser() || canPerformOnAnyRef(Permission.READ));
-  }
-
-  /**
-   * Returns whether the project is accessible to the current user, i.e. readable and not hidden.
-   */
-  public boolean isVisible() {
-    return isReadable() && !isHidden();
-  }
-
   public boolean canAddRefs() {
     return (canPerformOnAnyRef(Permission.CREATE) || isOwnerAnyRef());
   }
@@ -312,16 +262,11 @@ public class ProjectControl {
     return false;
   }
 
-  /** Can this user see all the refs in this projects? */
-  public boolean allRefsAreVisible() {
-    return allRefsAreVisible(Collections.<String>emptySet());
-  }
-
   public boolean allRefsAreVisible(Set<String> ignore) {
     return user.isInternalUser() || canPerformOnAllRefs(Permission.READ, ignore);
   }
 
-  /** Is this user a project owner? Ownership does not imply {@link #isVisible()} */
+  /** Is this user a project owner? */
   public boolean isOwner() {
     return (isDeclaredOwner() && !controlForRef("refs/*").isBlocked(Permission.OWNER))
         || user.getCapabilities().isAdmin_DoNotUse();
@@ -609,10 +554,11 @@ public class ProjectControl {
     private boolean can(ProjectPermission perm) throws PermissionBackendException {
       switch (perm) {
         case ACCESS:
-          return (!isHidden() && isReadable()) || isOwner();
+          return (!isHidden() && (user.isInternalUser() || canPerformOnAnyRef(Permission.READ)))
+              || isOwner();
 
         case READ:
-          return (!isHidden() && allRefsAreVisible()) || isOwner();
+          return (!isHidden() && allRefsAreVisible(Collections.emptySet())) || isOwner();
       }
       throw new PermissionBackendException(perm + " unsupported");
     }

gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java

@@ -58,6 +58,7 @@ import com.google.gerrit.server.config.AllUsersNameProvider;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.git.ProjectConfig;
 import com.google.gerrit.server.index.SingleVersionModule.SingleVersionListener;
+import com.google.gerrit.server.permissions.ProjectPermission;
 import com.google.gerrit.server.permissions.RefPermission;
 import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.gerrit.server.schema.SchemaCreator;
@@ -110,12 +111,14 @@ public class RefControlTest {
     assertThat(u.controlForRef(ref).isOwner()).named("NOT OWN " + ref).isFalse();
   }
 
-  private void assertCanRead(ProjectControl u) {
+  private void assertCanAccess(ProjectControl u) {
-    assertThat(u.isVisible()).named("can read").isTrue();
+    boolean access = u.asForProject().testOrFalse(ProjectPermission.ACCESS);
+    assertThat(access).named("can access").isTrue();
   }
 
-  private void assertCannotRead(ProjectControl u) {
+  private void assertAccessDenied(ProjectControl u) {
-    assertThat(u.isVisible()).named("cannot read").isFalse();
+    boolean access = u.asForProject().testOrFalse(ProjectPermission.ACCESS);
+    assertThat(access).named("cannot access").isFalse();
   }
 
   private void assertCanRead(String ref, ProjectControl u) {
@@ -443,13 +446,13 @@ public class RefControlTest {
   public void inheritDuplicateSections() throws Exception {
     allow(parent, READ, ADMIN, "refs/*");
     allow(local, READ, DEVS, "refs/heads/*");
-    assertCanRead(user(local, "a", ADMIN));
+    assertCanAccess(user(local, "a", ADMIN));
 
     local = new ProjectConfig(localKey);
     local.load(newRepository(localKey));
     local.getProject().setParentName(parentKey);
     allow(local, READ, DEVS, "refs/*");
-    assertCanRead(user(local, "d", DEVS));
+    assertCanAccess(user(local, "d", DEVS));
   }
 
   @Test
@@ -457,7 +460,7 @@ public class RefControlTest {
     allow(parent, READ, REGISTERED_USERS, "refs/*");
     deny(local, READ, REGISTERED_USERS, "refs/*");
 
-    assertCannotRead(user(local));
+    assertAccessDenied(user(local));
   }
 
   @Test
@@ -466,7 +469,7 @@ public class RefControlTest {
     deny(local, READ, REGISTERED_USERS, "refs/heads/*");
 
     ProjectControl u = user(local);
-    assertCanRead(u);
+    assertCanAccess(u);
     assertCanRead("refs/master", u);
     assertCanRead("refs/tags/foobar", u);
     assertCanRead("refs/heads/master", u);
@@ -479,7 +482,7 @@ public class RefControlTest {
     allow(local, READ, REGISTERED_USERS, "refs/heads/*");
 
     ProjectControl u = user(local);
-    assertCanRead(u);
+    assertCanAccess(u);
     assertCannotRead("refs/foobar", u);
     assertCannotRead("refs/tags/foobar", u);
     assertCanRead("refs/heads/foobar", u);