Merge changes I1f530f02,I66fb4509

* changes:
  Add @Nullable annotation to AccessSection
  Remove RefControl#isBlocked.

java/com/google/gerrit/common/data/AccessSection.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.common.data;
 
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.reviewdb.client.Project;
 import java.util.ArrayList;
 import java.util.HashSet;
@@ -52,10 +53,12 @@ public class AccessSection extends RefConfigSection implements Comparable<Access
     permissions = list;
   }
 
+  @Nullable
   public Permission getPermission(String name) {
     return getPermission(name, false);
   }
 
+  @Nullable
   public Permission getPermission(String name, boolean create) {
     for (Permission p : getPermissions()) {
       if (p.getName().equalsIgnoreCase(name)) {

java/com/google/gerrit/server/permissions/ProjectControl.java

@@ -146,7 +146,7 @@ class ProjectControl {
 
   /** Is this user a project owner? */
   boolean isOwner() {
-    return (isDeclaredOwner() && !controlForRef("refs/*").isBlocked(Permission.OWNER)) || isAdmin();
+    return (isDeclaredOwner() && controlForRef("refs/*").canPerform(Permission.OWNER)) || isAdmin();
   }
 
   /**

java/com/google/gerrit/server/permissions/RefControl.java

@@ -145,11 +145,6 @@ class RefControl {
     return null;
   }
 
-  /** True if the user is blocked from using this permission. */
-  boolean isBlocked(String permissionName) {
-    return !doCanPerform(permissionName, false, true);
-  }
-
   /** True if the user has this permission. Works only for non labels. */
   boolean canPerform(String permissionName) {
     return canPerform(permissionName, false);
@@ -159,10 +154,6 @@ class RefControl {
     return new ForRefImpl();
   }
 
-  private boolean canPerform(String permissionName, boolean isChangeOwner) {
-    return doCanPerform(permissionName, isChangeOwner, false);
-  }
-
   private boolean canUpload() {
     return projectControl.controlForRef("refs/for/" + refName).canPerform(Permission.PUSH);
   }
@@ -339,7 +330,7 @@ class RefControl {
     return new PermissionRange(permissionName, min, max);
   }
 
-  private boolean doCanPerform(String permissionName, boolean isChangeOwner, boolean blockOnly) {
+  private boolean canPerform(String permissionName, boolean isChangeOwner) {
     List<PermissionRule> access = access(permissionName, isChangeOwner);
     List<PermissionRule> overridden = relevant.getOverridden(permissionName);
     Set<ProjectRef> allows = new HashSet<>();
@@ -355,7 +346,7 @@ class RefControl {
       blocks.remove(relevant.getRuleProps(rule));
     }
     blocks.removeAll(allows);
-    return blocks.isEmpty() && (!allows.isEmpty() || blockOnly);
+    return blocks.isEmpty() && !allows.isEmpty();
   }
 
   /** True if the user has force this permission. Works only for non labels. */

javatests/com/google/gerrit/server/permissions/RefControlTest.java

@@ -152,12 +152,8 @@ public class RefControlTest {
     assertThat(create).named("cannot create change " + ref).isFalse();
   }
 
-  private void assertBlocked(String p, String ref, ProjectControl u) {
-    assertThat(u.controlForRef(ref).isBlocked(p)).named(p + " is blocked for " + ref).isTrue();
-  }
-
   private void assertNotBlocked(String p, String ref, ProjectControl u) {
-    assertThat(u.controlForRef(ref).isBlocked(p)).named(p + " is blocked for " + ref).isFalse();
+    assertThat(u.controlForRef(ref).canPerform(p)).named(p + " is blocked for " + ref).isTrue();
   }
 
   private void assertCanUpdate(String ref, ProjectControl u) {
@@ -410,21 +406,28 @@ public class RefControlTest {
   public void blockPushDrafts() {
     allow(parent, PUSH, REGISTERED_USERS, "refs/for/refs/*");
     block(parent, PUSH, ANONYMOUS_USERS, "refs/drafts/*");
+    allow(local, PUSH, REGISTERED_USERS, "refs/drafts/*");
 
     ProjectControl u = user(local);
     assertCreateChange("refs/heads/master", u);
-    assertBlocked(PUSH, "refs/drafts/refs/heads/master", u);
+    assertThat(u.controlForRef("refs/drafts/master").canPerform(PUSH)).isFalse();
   }
 
   @Test
   public void blockPushDraftsUnblockAdmin() {
     block(parent, PUSH, ANONYMOUS_USERS, "refs/drafts/*");
     allow(parent, PUSH, ADMIN, "refs/drafts/*");
+    allow(local, PUSH, REGISTERED_USERS, "refs/drafts/*");
 
     ProjectControl u = user(local);
     ProjectControl a = user(local, "a", ADMIN);
-    assertBlocked(PUSH, "refs/drafts/refs/heads/master", u);
-    assertNotBlocked(PUSH, "refs/drafts/refs/heads/master", a);
+
+    assertThat(a.controlForRef("refs/drafts/master").canPerform(PUSH))
+        .named("push is allowed")
+        .isTrue();
+    assertThat(u.controlForRef("refs/drafts/master").canPerform(PUSH))
+        .named("push is not allowed")
+        .isFalse();
   }
 
   @Test
@@ -610,7 +613,9 @@ public class RefControlTest {
     allow(local, SUBMIT, REGISTERED_USERS, "refs/heads/*");
 
     ProjectControl u = user(local);
-    assertNotBlocked(SUBMIT, "refs/heads/master", u);
+    assertThat(u.controlForRef("refs/heads/master").canPerform(SUBMIT))
+        .named("submit is allowed")
+        .isTrue();
   }
 
   @Test