Merge "Introduced a new PermissionRule.Action: BLOCK."

6 files changed, 98 insertions, 13 deletions

diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/PermissionRule.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/PermissionRule.java
index 6deb14f0..2ddd8a7a 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/PermissionRule.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/PermissionRule.java
@@ -16,7 +16,7 @@ package com.google.gerrit.common.data;
 
 public class PermissionRule implements Comparable<PermissionRule> {
   public static enum Action {
-    ALLOW, DENY,
+    ALLOW, DENY, BLOCK,
 
     INTERACTIVE, BATCH;
   }
@@ -53,6 +53,14 @@ public class PermissionRule implements Comparable<PermissionRule> {
     action = Action.DENY;
   }
 
+  public boolean isBlock() {
+    return action == Action.BLOCK;
+  }
+
+  public void setBlock() {
+    action = Action.BLOCK;
+  }
+
   public Boolean getForce() {
     return force;
   }
@@ -97,7 +105,10 @@ public class PermissionRule implements Comparable<PermissionRule> {
 
   void mergeFrom(PermissionRule src) {
     if (getAction() != src.getAction()) {
-      if (getAction() == Action.DENY || src.getAction() == Action.DENY) {
+      if (getAction() == Action.BLOCK || src.getAction() == Action.BLOCK) {
+        setAction(Action.BLOCK);
+
+      } else if (getAction() == Action.DENY || src.getAction() == Action.DENY) {
         setAction(Action.DENY);
 
       } else if (getAction() == Action.BATCH || src.getAction() == Action.BATCH) {
@@ -151,6 +162,10 @@ public class PermissionRule implements Comparable<PermissionRule> {
         r.append("deny ");
         break;
 
+      case BLOCK:
+        r.append("block ");
+        break;
+
       case INTERACTIVE:
         r.append("interactive ");
         break;
@@ -189,6 +204,10 @@ public class PermissionRule implements Comparable<PermissionRule> {
       rule.setAction(Action.DENY);
       src = src.substring("deny ".length()).trim();
 
+    } else if (src.startsWith("block ")) {
+      rule.setAction(Action.BLOCK);
+      src = src.substring("block ".length()).trim();
+
     } else if (src.startsWith("interactive ")) {
       rule.setAction(Action.INTERACTIVE);
       src = src.substring("interactive ".length()).trim();
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionRuleEditor.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionRuleEditor.java
index 98e62834..7f5b5738 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionRuleEditor.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionRuleEditor.java
@@ -124,7 +124,8 @@ public class PermissionRuleEditor extends Composite implements
         action.setValue(PermissionRule.Action.ALLOW);
         action.setAcceptableValues(Arrays.asList(
             PermissionRule.Action.ALLOW,
-            PermissionRule.Action.DENY));
+            PermissionRule.Action.DENY,
+            PermissionRule.Action.BLOCK));
       }
     }
 
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java
index 1196b023..02f56c5d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java
@@ -99,19 +99,25 @@ public class PermissionCollection {
       sorter.sort(ref, sections);
 
       Set<SeenRule> seen = new HashSet<SeenRule>();
+      Set<SeenRule> seenBlockingRules = new HashSet<SeenRule>();
       Set<String> exclusiveGroupPermissions = new HashSet<String>();
 
       HashMap<String, List<PermissionRule>> permissions =
           new HashMap<String, List<PermissionRule>>();
       for (AccessSection section : sections) {
         for (Permission permission : section.getPermissions()) {
-          if (exclusiveGroupPermissions.contains(permission.getName())) {
-            continue;
-          }
+          boolean exclusivePermissionExists =
+              exclusiveGroupPermissions.contains(permission.getName());
 
           for (PermissionRule rule : permission.getRules()) {
             SeenRule s = new SeenRule(section, permission, rule);
-            if (seen.add(s) && !rule.isDeny()) {
+            boolean addRule = false;
+            if (rule.isBlock()) {
+              addRule = seenBlockingRules.add(s);
+            } else {
+              addRule = seen.add(s) && !rule.isDeny() && !exclusivePermissionExists;
+            }
+            if (addRule) {
               List<PermissionRule> r = permissions.get(permission.getName());
               if (r == null) {
                 r = new ArrayList<PermissionRule>(2);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
index 1024f9af..ee428333 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
@@ -19,6 +19,7 @@ import com.google.gerrit.common.data.AccessSection;
 import com.google.gerrit.common.data.Capable;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.common.data.PermissionRule;
+import com.google.gerrit.common.data.PermissionRule.Action;
 import com.google.gerrit.reviewdb.AbstractAgreement;
 import com.google.gerrit.reviewdb.AccountAgreement;
 import com.google.gerrit.reviewdb.AccountGroup;
@@ -393,7 +394,7 @@ public class ProjectControl {
       }
 
       for (PermissionRule rule : permission.getRules()) {
-        if (rule.isDeny() || !match(rule)) {
+        if (rule.isBlock() || rule.isDeny() || !match(rule)) {
           continue;
         }
 
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
index 984623d5..9c60133b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
@@ -163,12 +163,16 @@ public class RefControl {
       // granting of powers beyond pushing to the configuration.
       return false;
     }
+    boolean result = false;
     for (PermissionRule rule : access(Permission.PUSH)) {
+      if (rule.isBlock()) {
+        return false;
+      }
       if (rule.getForce()) {
-        return true;
+        result = true;
       }
     }
-    return false;
+    return result;
   }
 
   /**
@@ -313,16 +317,35 @@ public class RefControl {
       List<PermissionRule> ruleList) {
     int min = 0;
     int max = 0;
+    int blockMin = Integer.MIN_VALUE;
+    int blockMax = Integer.MAX_VALUE;
     for (PermissionRule rule : ruleList) {
-      min = Math.min(min, rule.getMin());
-      max = Math.max(max, rule.getMax());
+      if (rule.isBlock()) {
+        blockMin = Math.max(blockMin, rule.getMin());
+        blockMax = Math.min(blockMax, rule.getMax());
+      } else {
+        min = Math.min(min, rule.getMin());
+        max = Math.max(max, rule.getMax());
+      }
+    }
+    if (blockMin > Integer.MIN_VALUE) {
+      min = Math.max(min, blockMin + 1);
+    }
+    if (blockMax < Integer.MAX_VALUE) {
+      max = Math.min(max, blockMax - 1);
     }
     return new PermissionRange(permissionName, min, max);
   }
 
   /** True if the user has this permission. Works only for non labels. */
   boolean canPerform(String permissionName) {
-    return !access(permissionName).isEmpty();
+    List<PermissionRule> access = access(permissionName);
+    for (PermissionRule rule : access) {
+      if (rule.isBlock() && !rule.getForce()) {
+        return false;
+      }
+    }
+    return !access.isEmpty();
   }
 
   /** Rules for the given permission, or the empty list. */
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
index f829ba01..29b27628 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.project;
 
+import static com.google.gerrit.common.data.Permission.LABEL;
 import static com.google.gerrit.common.data.Permission.OWNER;
 import static com.google.gerrit.common.data.Permission.PUSH;
 import static com.google.gerrit.common.data.Permission.READ;
@@ -21,6 +22,7 @@ import static com.google.gerrit.common.data.Permission.SUBMIT;
 
 import com.google.gerrit.common.data.Capable;
 import com.google.gerrit.common.data.GroupReference;
+import com.google.gerrit.common.data.PermissionRange;
 import com.google.gerrit.common.data.PermissionRule;
 import com.google.gerrit.reviewdb.AccountGroup;
 import com.google.gerrit.reviewdb.AccountProjectWatch;
@@ -221,6 +223,26 @@ public class RefControlTest extends TestCase {
     assertTrue("d can read", d.controlForRef("refs/heads/foo-QA-bar").isVisible());
   }
 
+  public void testBlockRule_ParentBlocksChild() {
+    grant(local, PUSH, devs, "refs/tags/*");
+    grant(parent, PUSH, anonymous, "refs/tags/*").setBlock();
+
+    ProjectControl u = user(devs);
+    assertFalse("u can't force update tag", u.controlForRef("refs/tags/V10").canForceUpdate());
+  }
+
+  public void testBlockLabelRange_ParentBlocksChild() {
+    grant(local, LABEL + "Code-Review", -2, +2, devs, "refs/heads/*");
+    grant(parent, LABEL + "Code-Review", -2, +2, devs, "refs/heads/*").setBlock();
+
+    ProjectControl u = user(devs);
+
+    PermissionRange range = u.controlForRef("refs/heads/master").getRange(LABEL + "Code-Review");
+    assertTrue("u can vote -1", range.contains(-1));
+    assertTrue("u can vote +1", range.contains(1));
+    assertFalse("u can't vote -2", range.contains(-2));
+    assertFalse("u can't vote 2", range.contains(2));
+  }
   // -----------------------------------------------------------------------
 
   private final Map<Project.NameKey, ProjectState> all;
@@ -317,7 +339,20 @@ public class RefControlTest extends TestCase {
 
   private PermissionRule grant(ProjectConfig project, String permissionName,
       AccountGroup.UUID group, String ref) {
+    return grant(project, permissionName, newRule(project, group), ref);
+  }
+
+  private PermissionRule grant(ProjectConfig project, String permissionName,
+      int min, int max, AccountGroup.UUID group, String ref) {
     PermissionRule rule = newRule(project, group);
+    rule.setMin(min);
+    rule.setMax(max);
+    return grant(project, permissionName, rule, ref);
+  }
+
+
+  private PermissionRule grant(ProjectConfig project, String permissionName,
+      PermissionRule rule, String ref) {
     project.getAccessSection(ref, true) //
         .getPermission(permissionName, true) //
         .add(rule);