jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From tri...@apache.org
Subject svn commit: r1528897 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/ oak-core/src/main/resources/org/apache/jackrabbit/oak/plugins/nodetype/write/ oak-core/src/test/java/org/apache/jackrab...
Date Thu, 03 Oct 2013 14:43:57 GMT
Author: tripod
Date: Thu Oct  3 14:43:57 2013
New Revision: 1528897

URL: http://svn.apache.org/r1528897
Log:
OAK-527 Implement Permission evaluation

Added:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStore.java
      - copied, changed from r1527460, jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java
Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionUtil.java
    jackrabbit/oak/trunk/oak-core/src/main/resources/org/apache/jackrabbit/oak/plugins/nodetype/write/builtin_nodetypes.cnd
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractPermissionHookTest.java
    jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java?rev=1528897&r1=1528896&r2=1528897&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java Thu Oct  3 14:43:57 2013
@@ -18,39 +18,29 @@ package org.apache.jackrabbit.oak.securi
 
 import java.security.Principal;
 import java.security.acl.Group;
-import java.util.Collection;
-import java.util.Comparator;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
-import java.util.NoSuchElementException;
 import java.util.Set;
-import javax.annotation.CheckForNull;
+
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
-import com.google.common.base.Predicate;
-import com.google.common.base.Strings;
-import com.google.common.collect.ImmutableSortedSet;
-import com.google.common.collect.Iterators;
-import com.google.common.primitives.Longs;
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.api.Tree;
-import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.core.ImmutableTree;
 import org.apache.jackrabbit.oak.spi.security.authorization.permission.PermissionConstants;
 import org.apache.jackrabbit.oak.spi.security.authorization.permission.Permissions;
 import org.apache.jackrabbit.oak.spi.security.authorization.permission.ReadStatus;
-import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionPattern;
 import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionProvider;
 import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeBits;
 import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeBitsProvider;
 import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
-import org.apache.jackrabbit.oak.util.TreeUtil;
 import org.apache.jackrabbit.util.Text;
 
+import com.google.common.collect.Iterators;
+
 import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkNotNull;
 
 /**
  * TODO: WIP
@@ -63,10 +53,13 @@ class CompiledPermissionImpl implements 
     private final Map<String, Tree> userTrees;
     private final Map<String, Tree> groupTrees;
 
-    private final Set<String> readPaths;
+    private final String[] readPathsCheckList;
 
     private PrivilegeBitsProvider bitsProvider;
 
+    private final PermissionStore userStore;
+    private final PermissionStore groupStore;
+
     CompiledPermissionImpl(@Nonnull Set<Principal> principals,
                            @Nonnull ImmutableTree permissionsTree,
                            @Nonnull PrivilegeBitsProvider bitsProvider,
@@ -76,7 +69,12 @@ class CompiledPermissionImpl implements 
         this.principals = principals;
         this.restrictionProvider = restrictionProvider;
         this.bitsProvider = bitsProvider;
-        this.readPaths = readPaths;
+        readPathsCheckList = new String[readPaths.size() * 2];
+        int i=0;
+        for (String p: readPaths) {
+            readPathsCheckList[i++] = p;
+            readPathsCheckList[i++] = p + "/";
+        }
 
         userTrees = new HashMap<String, Tree>(principals.size());
         groupTrees = new HashMap<String, Tree>(principals.size());
@@ -91,6 +89,9 @@ class CompiledPermissionImpl implements 
                 }
             }
         }
+
+        userStore = new PermissionStore(userTrees, restrictionProvider);
+        groupStore = new PermissionStore(groupTrees, restrictionProvider);
     }
 
     //------------------------------------------------< CompiledPermissions >---
@@ -111,6 +112,9 @@ class CompiledPermissionImpl implements 
                 target.remove(pName);
             }
         }
+        // todo, improve flush stores
+        userStore.flush();
+        groupStore.flush();
     }
 
     @Override
@@ -119,9 +123,9 @@ class CompiledPermissionImpl implements 
             return ReadStatus.ALLOW_ALL_REGULAR;
         }
         long permission = (property == null) ? Permissions.READ_NODE : Permissions.READ_PROPERTY;
-        Iterator<PermissionEntry> it = getEntryIterator(new EntryPredicate(tree, property));
+        Iterator<PermissionStore.PermissionEntry> it = getEntryIterator(new PermissionStore.EntryPredicate(tree, property));
         while (it.hasNext()) {
-            PermissionEntry entry = it.next();
+            PermissionStore.PermissionEntry entry = it.next();
             if (entry.readStatus != null) {
                 return entry.readStatus;
             } else if (entry.privilegeBits.includesRead(permission)) {
@@ -133,18 +137,18 @@ class CompiledPermissionImpl implements 
 
     @Override
     public boolean isGranted(long permissions) {
-        return hasPermissions(getEntryIterator(new EntryPredicate()), permissions, null, null);
+        return hasPermissions(getEntryIterator(new PermissionStore.EntryPredicate()), permissions, null, null);
     }
 
     @Override
     public boolean isGranted(@Nonnull Tree tree, @Nullable PropertyState property, long permissions) {
-        Iterator<PermissionEntry> it = getEntryIterator(new EntryPredicate(tree, property));
+        Iterator<PermissionStore.PermissionEntry> it = getEntryIterator(new PermissionStore.EntryPredicate(tree, property));
         return hasPermissions(it, permissions, tree, null);
     }
 
     @Override
     public boolean isGranted(@Nonnull String path, long permissions) {
-        Iterator<PermissionEntry> it = getEntryIterator(new EntryPredicate(path));
+        Iterator<PermissionStore.PermissionEntry> it = getEntryIterator(new PermissionStore.EntryPredicate(path));
         return hasPermissions(it, permissions, null, path);
     }
 
@@ -169,7 +173,7 @@ class CompiledPermissionImpl implements 
         return (principal instanceof Group) ? groupTrees : userTrees;
     }
 
-    private boolean hasPermissions(@Nonnull Iterator<PermissionEntry> entries,
+    private boolean hasPermissions(@Nonnull Iterator<PermissionStore.PermissionEntry> entries,
                                    long permissions, @Nullable Tree tree, @Nullable String path) {
         // calculate readable paths if the given permissions includes any read permission.
         boolean isReadable = Permissions.diff(Permissions.READ, permissions) != Permissions.READ && isReadablePath(tree, path);
@@ -192,14 +196,12 @@ class CompiledPermissionImpl implements 
         PrivilegeBits denyBits = PrivilegeBits.getInstance();
         PrivilegeBits parentAllowBits;
         PrivilegeBits parentDenyBits;
-        String parentPath = null;
+        String parentPath;
 
         if (respectParent) {
             parentAllowBits = PrivilegeBits.getInstance();
             parentDenyBits = PrivilegeBits.getInstance();
-            if (path != null || tree != null) {
-                parentPath = PermissionUtil.getParentPathOrNull((path != null) ? path : tree.getPath());
-            }
+            parentPath = PermissionUtil.getParentPathOrNull((path != null) ? path : tree.getPath());
         } else {
             parentAllowBits = PrivilegeBits.EMPTY;
             parentDenyBits = PrivilegeBits.EMPTY;
@@ -207,7 +209,7 @@ class CompiledPermissionImpl implements 
         }
 
         while (entries.hasNext()) {
-            PermissionEntry entry = entries.next();
+            PermissionStore.PermissionEntry entry = entries.next();
             if (respectParent && (parentPath != null)) {
                 boolean matchesParent = entry.matchesParent(parentPath);
                 if (matchesParent) {
@@ -241,14 +243,16 @@ class CompiledPermissionImpl implements 
 
     @Nonnull
     private PrivilegeBits getPrivilegeBits(@Nullable Tree tree) {
-        EntryPredicate pred = (tree == null) ? new EntryPredicate() : new EntryPredicate(tree, null);
-        Iterator<PermissionEntry> entries = getEntryIterator(pred);
+        PermissionStore.EntryPredicate pred = (tree == null)
+                ? new PermissionStore.EntryPredicate()
+                : new PermissionStore.EntryPredicate(tree, null);
+        Iterator<PermissionStore.PermissionEntry> entries = getEntryIterator(pred);
 
         PrivilegeBits allowBits = PrivilegeBits.getInstance();
         PrivilegeBits denyBits = PrivilegeBits.getInstance();
 
         while (entries.hasNext()) {
-            PermissionEntry entry = entries.next();
+            PermissionStore.PermissionEntry entry = entries.next();
             if (entry.isAllow) {
                 allowBits.addDifference(entry.privilegeBits, denyBits);
             } else {
@@ -264,187 +268,28 @@ class CompiledPermissionImpl implements 
     }
 
     @Nonnull
-    private Iterator<PermissionEntry> getEntryIterator(@Nonnull EntryPredicate predicate) {
-        Iterator<PermissionEntry> userEntries = (userTrees.isEmpty()) ?
-                Iterators.<PermissionEntry>emptyIterator() :
-                new EntryIterator(userTrees, predicate);
-        Iterator<PermissionEntry> groupEntries = (groupTrees.isEmpty()) ?
-                Iterators.<PermissionEntry>emptyIterator():
-                new EntryIterator(groupTrees, predicate);
+    private Iterator<PermissionStore.PermissionEntry> getEntryIterator(@Nonnull PermissionStore.EntryPredicate predicate) {
+        Iterator<PermissionStore.PermissionEntry> userEntries = userStore.getEntryIterator(predicate);
+        Iterator<PermissionStore.PermissionEntry> groupEntries = groupStore.getEntryIterator(predicate);
         return Iterators.concat(userEntries, groupEntries);
     }
 
     private boolean isReadablePath(@Nullable Tree tree, @Nullable String treePath) {
-        if (!readPaths.isEmpty()) {
+        if (readPathsCheckList.length > 0) {
             String targetPath = (tree != null) ? tree.getPath() : treePath;
             if (targetPath != null) {
-                for (String path : readPaths) {
-                    if (Text.isDescendantOrEqual(path, targetPath)) {
+                for (int i=0; i<readPathsCheckList.length; i++) {
+                    if (targetPath.equals(readPathsCheckList[i++])) {
                         return true;
                     }
-                }
-            }
-        }
-        return false;
-    }
-
-    private static final class PermissionEntry {
-
-        private final boolean isAllow;
-        private final PrivilegeBits privilegeBits;
-        private final long index;
-        private final String path;
-        private final RestrictionPattern restriction;
-
-        private ReadStatus readStatus = null; // TODO
-
-        private PermissionEntry(Tree entryTree, RestrictionProvider restrictionsProvider) {
-            isAllow = entryTree.getProperty(REP_IS_ALLOW).getValue(Type.BOOLEAN);
-            privilegeBits = PrivilegeBits.getInstance(entryTree.getProperty(REP_PRIVILEGE_BITS));
-            index = checkNotNull(entryTree.getProperty(REP_INDEX).getValue(Type.LONG)).longValue();
-            path = Strings.emptyToNull(TreeUtil.getString(entryTree, REP_ACCESS_CONTROLLED_PATH));
-            restriction = restrictionsProvider.getPattern(path, entryTree);
-        }
-
-        private boolean matches(@Nonnull Tree tree, @Nullable PropertyState property) {
-            return restriction.matches(tree, property);
-        }
-
-        private boolean matches(@Nonnull String treePath) {
-            return restriction.matches(treePath);
-        }
-
-        private boolean matches() {
-            return restriction.matches();
-        }
-
-        private boolean matchesParent(@Nonnull String parentPath) {
-            if (Text.isDescendantOrEqual(path, parentPath)) {
-                return restriction.matches(parentPath);
-            } else {
-                return false;
-            }
-        }
-    }
-
-    private class EntryIterator implements Iterator<PermissionEntry> {
-
-        private final Collection<Tree> principalTrees;
-        private final EntryPredicate predicate;
-
-        // the next oak path for which to retrieve permission entries
-        private String path;
-        // the ordered permission entries at a given path in the hierarchy
-        private Iterator<PermissionEntry> nextEntries = Iterators.emptyIterator();
-        // the next permission entry
-        private PermissionEntry next;
-
-        private EntryIterator(@Nonnull Map<String, Tree> principalTrees,
-                              @Nonnull EntryPredicate predicate) {
-            this.principalTrees = principalTrees.values();
-            this.predicate = predicate;
-            this.path = Strings.nullToEmpty(predicate.path);
-            next = seekNext();
-        }
-
-        @Override
-        public boolean hasNext() {
-            return next != null;
-        }
-
-        @Override
-        public PermissionEntry next() {
-            if (next == null) {
-                throw new NoSuchElementException();
-            }
-
-            PermissionEntry pe = next;
-            next = seekNext();
-            return pe;
-        }
-
-        @Override
-        public void remove() {
-            throw new UnsupportedOperationException();
-        }
-
-        @CheckForNull
-        private PermissionEntry seekNext() {
-            // calculate the ordered entries for the next hierarchy level.
-            while (!nextEntries.hasNext() && path != null) {
-                nextEntries = getNextEntries();
-                path = PermissionUtil.getParentPathOrNull(path);
-            }
-
-            if (nextEntries.hasNext()) {
-                return nextEntries.next();
-            } else {
-                return null;
-            }
-        }
-
-        @Nonnull
-        private Iterator<PermissionEntry> getNextEntries() {
-            ImmutableSortedSet.Builder<PermissionEntry> entries = new ImmutableSortedSet.Builder(new EntryComparator());
-            for (Tree principalRoot : principalTrees) {
-                String name = PermissionUtil.getEntryName(path);
-                Tree parent = principalRoot;
-                while (parent.hasChild(name)) {
-                    parent = parent.getChild(name);
-                    PermissionEntry pe = new PermissionEntry(parent, restrictionProvider);
-                    if (predicate.apply(pe)) {
-                        entries.add(pe);
+                    if (targetPath.startsWith(readPathsCheckList[i])) {
+                        return true;
                     }
                 }
             }
-            return entries.build().iterator();
-        }
-    }
-
-    private static final class EntryComparator implements Comparator<PermissionEntry> {
-        @Override
-        public int compare(@Nonnull PermissionEntry entry,
-                           @Nonnull PermissionEntry otherEntry) {
-            return Longs.compare(otherEntry.index, entry.index);
         }
+        return false;
     }
 
-    private static final class EntryPredicate implements Predicate<PermissionEntry> {
-
-        private final Tree tree;
-        private final PropertyState property;
-        private final String path;
-
-        private EntryPredicate(@Nonnull Tree tree, @Nullable PropertyState property) {
-            this.tree = tree;
-            this.property = property;
-            this.path = tree.getPath();
-        }
-
-        private EntryPredicate(@Nonnull String path) {
-            this.tree = null;
-            this.property = null;
-            this.path = path;
-        }
-
-        private EntryPredicate() {
-            this.tree = null;
-            this.property = null;
-            this.path = null;
-        }
 
-        @Override
-        public boolean apply(@Nullable PermissionEntry entry) {
-            if (entry == null) {
-                return false;
-            }
-            if (tree != null) {
-                return entry.matches(tree, property);
-            } else if (path != null) {
-                return entry.matches(path);
-            } else {
-                return entry.matches();
-            }
-        }
-    }
 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java?rev=1528897&r1=1528896&r2=1528897&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java Thu Oct  3 14:43:57 2013
@@ -17,19 +17,16 @@
 package org.apache.jackrabbit.oak.security.authorization.permission;
 
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
+
 import javax.annotation.Nonnull;
 
-import com.google.common.base.Objects;
-import com.google.common.base.Strings;
-import com.google.common.collect.Lists;
 import org.apache.jackrabbit.oak.api.CommitFailedException;
-import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.api.Type;
-import org.apache.jackrabbit.oak.commons.PathUtils;
-import org.apache.jackrabbit.oak.core.AbstractTree;
 import org.apache.jackrabbit.oak.core.ImmutableRoot;
 import org.apache.jackrabbit.oak.core.ImmutableTree;
 import org.apache.jackrabbit.oak.core.TreeTypeProvider;
@@ -50,6 +47,9 @@ import org.apache.jackrabbit.util.Text;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.base.Objects;
+import com.google.common.base.Strings;
+
 import static com.google.common.base.Preconditions.checkNotNull;
 import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE;
 import static org.apache.jackrabbit.JcrConstants.JCR_SYSTEM;
@@ -59,6 +59,24 @@ import static org.apache.jackrabbit.oak.
  * {@code CommitHook} implementation that processes any modification made to
  * access control content and updates persisted permission store associated
  * with access control related data stored in the repository.
+ * <p>
+ * The access control entries are grouped by principal and store below the store root based on the hash value of the
+ * access controllable path. hash collisions are handled by adding subnodes accordingly.
+ * <pre>
+ *   /jcr:system/rep:permissionStore/crx.default
+ *      /everyone
+ *          /552423  [rep:PermissionStore]
+ *              /0     [rep:Permissions]
+ *              /1     [rep:Permissions]
+ *              /c0     [rep:PermissionStore]
+ *                  /0      [rep:Permissions]
+ *                  /1      [rep:Permissions]
+ *                  /2      [rep:Permissions]
+ *              /c1     [rep:PermissionStore]
+ *                  /0      [rep:Permissions]
+ *                  /1      [rep:Permissions]
+ *                  /2      [rep:Permissions]
+ * </pre>
  */
 public class PermissionHook implements PostValidationHook, AccessControlConstants, PermissionConstants {
 
@@ -71,6 +89,10 @@ public class PermissionHook implements P
     private ReadOnlyNodeTypeManager ntMgr;
     private PrivilegeBitsProvider bitsProvider;
 
+    private Map<String, Acl> modified = new HashMap<String, Acl>();
+
+    private Map<String, Acl> deleted = new HashMap<String, Acl>();
+
     public PermissionHook(String workspaceName, RestrictionProvider restrictionProvider) {
         this.workspaceName = workspaceName;
         this.restrictionProvider = restrictionProvider;
@@ -85,10 +107,29 @@ public class PermissionHook implements P
         ntMgr = ReadOnlyNodeTypeManager.getInstance(before);
         bitsProvider = new PrivilegeBitsProvider(new ImmutableRoot(before));
 
-        after.compareAgainstBaseState(before, new Diff(new BeforeNode(before), new AfterNode(rootAfter)));
+        Diff diff = new Diff("");
+        after.compareAgainstBaseState(before, diff);
+        apply();
         return rootAfter.getNodeState();
     }
 
+    private void apply() {
+        for (Map.Entry<String, Acl> entry:deleted.entrySet()) {
+            entry.getValue().remove();
+        }
+        for (Map.Entry<String, Acl> entry:modified.entrySet()) {
+            entry.getValue().update();
+        }
+    }
+
+    private boolean isACL(@Nonnull Tree tree) {
+        return ntMgr.isNodeType(tree, NT_REP_ACL);
+    }
+
+    private boolean isACE(@Nonnull Tree tree) {
+        return ntMgr.isNodeType(tree, NT_REP_ACE);
+    }
+
     @Nonnull
     private NodeBuilder getPermissionRoot(NodeBuilder rootBuilder) {
         // permission root has been created during workspace initialization
@@ -99,84 +140,61 @@ public class PermissionHook implements P
         return new ImmutableTree(ImmutableTree.ParentProvider.UNSUPPORTED, name, nodeState, TreeTypeProvider.EMPTY);
     }
 
-    private static List<String> getAceNames(NodeState aclState) {
-        PropertyState ordering = checkNotNull(aclState.getProperty(AbstractTree.OAK_CHILD_ORDER));
-        return Lists.newArrayList(ordering.getValue(Type.STRINGS));
-    }
-
-    private static String getAccessControlledPath(Node acl) {
-        return (REP_REPO_POLICY.equals(acl.getName()) ? "" : Text.getRelativeParent(acl.getPath(), 1));
-    }
-
     private class Diff extends DefaultNodeStateDiff {
 
-        private final Node parentBefore;
-        private final AfterNode parentAfter;
+        private final String parentPath;
 
-        private Diff(@Nonnull Node parentBefore, @Nonnull AfterNode parentAfter) {
-            this.parentBefore = parentBefore;
-            this.parentAfter = parentAfter;
+        private Diff(String parentPath) {
+            this.parentPath = parentPath;
         }
 
         @Override
         public boolean childNodeAdded(String name, NodeState after) {
             if (NodeStateUtils.isHidden(name)) {
                 // ignore hidden nodes
-            } else if (isACL(name, after)) {
-                int index = 0;
-                for (String aceName : getAceNames(after)) {
-                    Tree child = getTree(aceName, after.getChildNode(aceName));
-                    if (isACE(child)) {
-                        PermissionEntry entry = createPermissionEntry(child, index, new AfterNode(parentAfter, name));
-                        entry.add();
-                    }
-                    index++;
-                }
+                return true;
+            }
+            String path = parentPath + "/" + name;
+            Tree tree = getTree(name, after);
+            if (isACL(tree)) {
+                Acl acl = new Acl(parentPath, name, new AfterNode(path, after));
+                modified.put(acl.accessControlledPath, acl);
             } else {
-                Node before = new BeforeNode(parentBefore.getPath(), name, EMPTY_NODE);
-                AfterNode node = new AfterNode(parentAfter, name);
-                after.compareAgainstBaseState(before.getNodeState(), new Diff(before, node));
+                after.compareAgainstBaseState(EMPTY_NODE, new Diff(path));
             }
             return true;
         }
 
         @Override
-        public boolean childNodeChanged(String name, final NodeState before, NodeState after) {
+        public boolean childNodeChanged(String name, NodeState before, NodeState after) {
             if (NodeStateUtils.isHidden(name)) {
                 // ignore hidden nodes
-            } else if (isACL(name, before) || isACL(name, after)) {
-                List<AcEntry> entriesBefore = createEntries(new BeforeNode(parentBefore, name, before));
-                List<AcEntry> entriesAfter = createEntries(new AfterNode(parentAfter, name));
-
-                for (int indexBefore = 0; indexBefore < entriesBefore.size(); indexBefore++) {
-                    AcEntry ace = entriesBefore.get(indexBefore);
-                    if (entriesAfter.isEmpty() || !entriesAfter.contains(ace)) {
-                        new PermissionEntry(ace, indexBefore).remove();
+                return true;
+            }
+            String path = parentPath + "/" + name;
+            Tree beforeTree = getTree(name, before);
+            Tree afterTree = getTree(name, after);
+            if (isACL(beforeTree)) {
+                if (isACL(afterTree)) {
+                    Acl acl = new Acl(parentPath, name, new AfterNode(path, after));
+                    modified.put(acl.accessControlledPath, acl);
+
+                    // also consider to remove the ACL from removed entries of other principals
+                    Acl beforeAcl = new Acl(parentPath, name, new BeforeNode(path, before));
+                    beforeAcl.entries.keySet().removeAll(acl.entries.keySet());
+                    if (!beforeAcl.entries.isEmpty()) {
+                        deleted.put(parentPath, beforeAcl);
                     }
-                }
 
-                List<PermissionEntry> toRemove = new ArrayList<PermissionEntry>();
-                List<PermissionEntry> toAdd = new ArrayList<PermissionEntry>();
-                for (int indexAfter = 0; indexAfter < entriesAfter.size(); indexAfter++) {
-                    AcEntry ace = entriesAfter.get(indexAfter);
-                    int indexBefore = entriesBefore.indexOf(ace);
-                    if (indexBefore == -1) {
-                        toAdd.add(new PermissionEntry(ace, indexAfter));
-                    } else if (indexBefore != indexAfter) {
-                        toRemove.add(new PermissionEntry(entriesBefore.get(indexBefore), indexBefore));
-                        toAdd.add(new PermissionEntry(ace, indexAfter));
-                    } // else: nothing to do
-                }
-                for (PermissionEntry pe : toRemove) {
-                    pe.remove();
-                }
-                for (PermissionEntry pe : toAdd) {
-                    pe.add();
+                } else {
+                    Acl acl = new Acl(parentPath, name, new BeforeNode(path, before));
+                    deleted.put(acl.accessControlledPath, acl);
                 }
+            } else if (isACL(afterTree)) {
+                Acl acl = new Acl(parentPath, name, new AfterNode(path, after));
+                modified.put(acl.accessControlledPath, acl);
             } else {
-                BeforeNode nodeBefore = new BeforeNode(parentBefore.getPath(), name, before);
-                AfterNode nodeAfter = new AfterNode(parentAfter, name);
-                after.compareAgainstBaseState(before, new Diff(nodeBefore, nodeAfter));
+                after.compareAgainstBaseState(before, new Diff(path));
             }
             return true;
         }
@@ -185,55 +203,18 @@ public class PermissionHook implements P
         public boolean childNodeDeleted(String name, NodeState before) {
             if (NodeStateUtils.isHidden(name)) {
                 // ignore hidden nodes
-            } else if (isACL(name, before)) {
-                int index = 0;
-                for (String aceName : getAceNames(before)) {
-                    Tree child = getTree(aceName, before.getChildNode(aceName));
-                    if (isACE(child)) {
-                        PermissionEntry entry = createPermissionEntry(child, index,
-                                new BeforeNode(parentBefore, name, before));
-                        entry.remove();
-                    }
-                    index++;
-                }
+                return true;
+            }
+            String path = parentPath + "/" + name;
+            Tree tree = getTree(name, before);
+            if (isACL(tree)) {
+                Acl acl = new Acl(parentPath, name, new BeforeNode(path, before));
+                deleted.put(acl.accessControlledPath, acl);
             } else {
-                BeforeNode nodeBefore = new BeforeNode(parentBefore.getPath(), name, before);
-                AfterNode after = new AfterNode(parentAfter.getPath(), name, EMPTY_NODE);
-                after.getNodeState().compareAgainstBaseState(before, new Diff(nodeBefore, after));
+                EMPTY_NODE.compareAgainstBaseState(before, new Diff(path));
             }
             return true;
         }
-
-        //--------------------------------------------------------< private >---
-
-        private boolean isACL(@Nonnull String name, @Nonnull NodeState nodeState) {
-            return ntMgr.isNodeType(getTree(name, nodeState), NT_REP_ACL);
-        }
-
-        private boolean isACE(@Nonnull Tree tree) {
-            return ntMgr.isNodeType(tree, NT_REP_ACE);
-        }
-
-        @Nonnull
-        private PermissionEntry createPermissionEntry(@Nonnull Tree ace,
-                                                      int index,
-                                                      @Nonnull Node acl) {
-            String accessControlledPath = getAccessControlledPath(acl);
-            return new PermissionEntry(ace, accessControlledPath, index);
-        }
-
-        @Nonnull
-        private List<AcEntry> createEntries(Node acl) {
-            List<AcEntry> acEntries = new ArrayList<AcEntry>();
-            NodeState aclState = acl.getNodeState();
-            for (String aceName : getAceNames(aclState)) {
-                Tree child = getTree(aceName, aclState.getChildNode(aceName));
-                if (isACE(child)) {
-                    acEntries.add(new AcEntry(child, getAccessControlledPath(acl)));
-                }
-            }
-            return acEntries;
-        }
     }
 
     private abstract static class Node {
@@ -244,18 +225,10 @@ public class PermissionHook implements P
             this.path = path;
         }
 
-        private Node(String parentPath, String name) {
-            this.path = PathUtils.concat(parentPath, new String[]{name});
-        }
-
         String getName() {
             return Text.getName(path);
         }
 
-        String getPath() {
-            return path;
-        }
-
         abstract NodeState getNodeState();
     }
 
@@ -263,18 +236,8 @@ public class PermissionHook implements P
 
         private final NodeState nodeState;
 
-        BeforeNode(NodeState root) {
-            super("/");
-            this.nodeState = root;
-        }
-
-        BeforeNode(String parentPath, String name, NodeState nodeState) {
-            super(parentPath, name);
-            this.nodeState = nodeState;
-        }
-
-        BeforeNode(Node parent, String name, NodeState nodeState) {
-            super(parent.getPath(), name);
+        BeforeNode(String parentPath, NodeState nodeState) {
+            super(parentPath);
             this.nodeState = nodeState;
         }
 
@@ -288,23 +251,165 @@ public class PermissionHook implements P
 
         private final NodeBuilder builder;
 
-        private AfterNode(NodeBuilder rootBuilder) {
-            super("/");
-            this.builder = rootBuilder;
+        private AfterNode(String path, NodeState state) {
+            super(path);
+            this.builder = state.builder();
         }
 
-        private AfterNode(String parentPath, String name, NodeState state) {
-            super(parentPath, name);
-            this.builder = state.builder();
+        @Override
+        NodeState getNodeState() {
+            return builder.getNodeState();
         }
+    }
+
+    private class Acl {
+
+        private final String accessControlledPath;
+
+        private final String nodeName;
+
+        private final Map<String, List<AcEntry>> entries = new HashMap<String, List<AcEntry>>();
 
-        private AfterNode(AfterNode parent, String name) {
-            super(parent.getPath(), name);
-            this.builder = parent.builder.child(name);
+        private Acl(String aclPath, String name, @Nonnull Node node) {
+            if (name.equals(REP_REPO_POLICY)) {
+                this.accessControlledPath = "";
+            } else {
+                this.accessControlledPath = aclPath.length() == 0 ? "/" : aclPath;
+            }
+            nodeName = PermissionUtil.getEntryName(accessControlledPath);
+            int index = 0;
+            Tree aclTree = getTree(node.getName(), node.getNodeState());
+            for (Tree child : aclTree.getChildren()) {
+                if (isACE(child)) {
+                    AcEntry entry = new AcEntry(child, accessControlledPath, index);
+                    List<AcEntry> list = entries.get(entry.principalName);
+                    if (list == null) {
+                        list = new ArrayList<AcEntry>();
+                        entries.put(entry.principalName, list);
+                    }
+                    list.add(entry);
+                    index++;
+                }
+            }
         }
 
-        NodeState getNodeState() {
-            return builder.getNodeState();
+        private void remove() {
+            String msg = "Unable to remove permission entry";
+            for (String principalName: entries.keySet()) {
+                if (permissionRoot.hasChildNode(principalName)) {
+                    NodeBuilder principalRoot = permissionRoot.getChildNode(principalName);
+
+                    // find the ACL node that for this path and principal
+                    NodeBuilder parent = principalRoot.getChildNode(nodeName);
+                    if (!parent.exists()) {
+                        continue;
+                    }
+
+                    // check if the node is the correct one
+                    if (PermissionUtil.checkACLPath(parent, accessControlledPath)) {
+                        // remove and reconnect child nodes
+                        NodeBuilder newParent = null;
+                        for (String childName : parent.getChildNodeNames()) {
+                            if (childName.charAt(0) != 'c') {
+                                continue;
+                            }
+                            NodeBuilder child = parent.getChildNode(childName);
+                            if (newParent == null) {
+                                newParent = child;
+                            } else {
+                                newParent.setChildNode(childName, child.getNodeState());
+                                child.remove();
+                            }
+                        }
+                        parent.remove();
+                        if (newParent != null) {
+                            principalRoot.setChildNode(nodeName, newParent.getNodeState());
+                        }
+                    } else {
+                        // check if any of the child nodes match
+                        for (String childName : parent.getChildNodeNames()) {
+                            if (childName.charAt(0) != 'c') {
+                                continue;
+                            }
+                            NodeBuilder child = parent.getChildNode(childName);
+                            if (PermissionUtil.checkACLPath(child, accessControlledPath)) {
+                                // remove child
+                                child.remove();
+                            }
+                        }
+                    }
+                } else {
+                    log.error("{} {}: Principal root missing.", msg, this);
+                }
+            }
+        }
+
+        private void update() {
+            for (String principalName: entries.keySet()) {
+                NodeBuilder principalRoot = permissionRoot.child(principalName);
+                if (!principalRoot.hasProperty(JCR_PRIMARYTYPE)) {
+                    principalRoot.setProperty(JCR_PRIMARYTYPE, NT_REP_PERMISSION_STORE, Type.NAME);
+                }
+                NodeBuilder parent = principalRoot.child(nodeName);
+                if (!parent.hasProperty(JCR_PRIMARYTYPE)) {
+                    parent.setProperty(JCR_PRIMARYTYPE, NT_REP_PERMISSION_STORE, Type.NAME);
+                }
+
+                // check if current parent already has the correct path
+                if (parent.hasProperty(REP_ACCESS_CONTROLLED_PATH)) {
+                    if (!PermissionUtil.checkACLPath(parent, accessControlledPath)) {
+                        // hash collision, find a new child
+                        NodeBuilder child = null;
+                        int idx = 0;
+                        for (String childName : parent.getChildNodeNames()) {
+                            if (childName.charAt(0) != 'c') {
+                                continue;
+                            }
+                            child = parent.getChildNode(childName);
+                            if (PermissionUtil.checkACLPath(child, accessControlledPath)) {
+                                break;
+                            }
+                            child = null;
+                            idx++;
+                        }
+                        while (child == null) {
+                            String name = "c" + String.valueOf(idx++);
+                            child = parent.getChildNode(name);
+                            if (child.exists()) {
+                                child = null;
+                            } else {
+                                child = parent.child(name);
+                                child.setProperty(JCR_PRIMARYTYPE, NT_REP_PERMISSION_STORE, Type.NAME);
+                            }
+                        }
+                        parent = child;
+                        parent.setProperty(REP_ACCESS_CONTROLLED_PATH, accessControlledPath);
+                    }
+                } else {
+                    // new parent
+                    parent.setProperty(REP_ACCESS_CONTROLLED_PATH, accessControlledPath);
+                }
+                updateEntries(parent, entries.get(principalName));
+            }
+        }
+
+        private void updateEntries(NodeBuilder parent, List<AcEntry> list) {
+            // remove old entries
+            for (String childName : parent.getChildNodeNames()) {
+                if (childName.charAt(0) != 'c') {
+                    parent.getChildNode(childName).remove();
+                }
+            }
+            for (AcEntry ace: list) {
+                NodeBuilder n = parent.child(String.valueOf(ace.index))
+                        .setProperty(JCR_PRIMARYTYPE, NT_REP_PERMISSIONS, Type.NAME)
+                        .setProperty(REP_IS_ALLOW, ace.isAllow)
+                        .setProperty(REP_INDEX, ace.index)
+                        .setProperty(ace.privilegeBits.asPropertyState(REP_PRIVILEGE_BITS));
+                for (Restriction restriction : ace.restrictions) {
+                    n.setProperty(restriction.getProperty());
+                }
+            }
         }
     }
 
@@ -315,15 +420,16 @@ public class PermissionHook implements P
         private final PrivilegeBits privilegeBits;
         private final boolean isAllow;
         private final Set<Restriction> restrictions;
-
+        private final long index;
         private int hashCode = -1;
 
-        private AcEntry(@Nonnull Tree aceTree, @Nonnull String accessControlledPath) {
+        private AcEntry(@Nonnull Tree aceTree, @Nonnull String accessControlledPath, long index) {
             this.accessControlledPath = accessControlledPath;
             principalName = Text.escapeIllegalJcrChars(checkNotNull(TreeUtil.getString(aceTree, REP_PRINCIPAL_NAME)));
             privilegeBits = bitsProvider.getBits(TreeUtil.getStrings(aceTree, REP_PRIVILEGES));
             isAllow = NT_REP_GRANT_ACE.equals(TreeUtil.getPrimaryTypeName(aceTree));
             restrictions = restrictionProvider.readRestrictions(Strings.emptyToNull(accessControlledPath), aceTree);
+            this.index = index;
         }
 
         @Override
@@ -361,120 +467,4 @@ public class PermissionHook implements P
             return sb.toString();
         }
     }
-
-    private final class PermissionEntry {
-
-        private final AcEntry ace;
-        private final long index;
-        private final String nodeName;
-
-        private int hashCode = -1;
-
-        private PermissionEntry(@Nonnull Tree aceTree, @Nonnull String accessControlledPath, long index) {
-            this(new AcEntry(aceTree, accessControlledPath), index);
-        }
-
-        private PermissionEntry(@Nonnull AcEntry ace, long index) {
-            this.ace = ace;
-            this.index = index;
-
-            // create node name from ace definition
-            nodeName = PermissionUtil.getEntryName(ace.accessControlledPath);
-        }
-
-        private void add() {
-            NodeBuilder principalRoot = permissionRoot.child(ace.principalName);
-            if (!principalRoot.hasProperty(JCR_PRIMARYTYPE)) {
-                principalRoot.setProperty(JCR_PRIMARYTYPE, NT_REP_PERMISSION_STORE, Type.NAME);
-            }
-            NodeBuilder parent = getParent(principalRoot);
-            // create the new entry using a unique name to avoid overwriting
-            // entries with the same access controlled path
-            String tmpName = String.valueOf(hashCode());
-            NodeBuilder toAdd = parent.child(tmpName)
-                    .setProperty(JCR_PRIMARYTYPE, NT_REP_PERMISSIONS, Type.NAME)
-                    .setProperty(REP_ACCESS_CONTROLLED_PATH, ace.accessControlledPath)
-                    .setProperty(REP_IS_ALLOW, ace.isAllow)
-                    .setProperty(REP_INDEX, index)
-                    .setProperty(ace.privilegeBits.asPropertyState(REP_PRIVILEGE_BITS));
-            for (Restriction restriction : ace.restrictions) {
-                toAdd.setProperty(restriction.getProperty());
-            }
-
-            // reconnect existing entries for that path
-            if (parent.hasChildNode(nodeName)) {
-                NodeBuilder child = parent.getChildNode(nodeName);
-                toAdd.setChildNode(nodeName, child.getNodeState());
-                child.remove();
-            }
-
-            // finally connect 'toAdd' to the parent using the correct name
-            parent.setChildNode(nodeName, toAdd.getNodeState());
-            toAdd.remove();
-        }
-
-        private void remove() {
-            String msg = "Unable to remove permission entry";
-            if (permissionRoot.hasChildNode(ace.principalName)) {
-                NodeBuilder principalRoot = permissionRoot.getChildNode(ace.principalName);
-                NodeBuilder parent = getParent(principalRoot);
-                if (parent.hasChildNode(nodeName)) {
-                    NodeBuilder target = parent.getChildNode(nodeName);
-                    // reconnect children
-                    for (String childName : target.getChildNodeNames()) {
-                        NodeBuilder child = target.getChildNode(childName);
-                        parent.setChildNode(childName, child.getNodeState());
-                        child.remove();
-                    }
-                    // remove the target node
-                    target.remove();
-                } else {
-                    log.error("{} {}. No corresponding node found in permission store.", msg, this);
-                }
-            } else {
-                log.error("{} {}: Principal root missing.", msg, this);
-            }
-        }
-
-        private NodeBuilder getParent(NodeBuilder principalRoot) {
-            NodeBuilder parent = principalRoot;
-            while (parent.hasChildNode(nodeName)) {
-                NodeBuilder child = parent.getChildNode(nodeName);
-                long childIndex = child.getProperty(REP_INDEX).getValue(Type.LONG);
-                if (index < childIndex) {
-                    parent = child;
-                } else {
-                    break;
-                }
-            }
-            return parent;
-        }
-
-        @Override
-        public int hashCode() {
-            if (hashCode == -1) {
-                hashCode = Objects.hashCode(ace, index);
-            }
-            return hashCode;
-        }
-
-        @Override
-        public boolean equals(Object o) {
-            if (o == this) {
-                return true;
-            }
-            if (o instanceof PermissionEntry) {
-                PermissionEntry other = (PermissionEntry) o;
-                return index == other.index && ace.equals(other.ace);
-            }
-            return false;
-        }
-
-        @Override
-        public String toString() {
-            StringBuilder sb = new StringBuilder();
-            sb.append("permission entry: ").append(ace.toString()).append('-').append(index);
-            return sb.toString();
-        }
-    }
 }

Copied: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStore.java (from r1527460, jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java)
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStore.java?p2=jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStore.java&p1=jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java&r1=1527460&r2=1528897&rev=1528897&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStore.java Thu Oct  3 14:43:57 2013
@@ -16,339 +16,125 @@
  */
 package org.apache.jackrabbit.oak.security.authorization.permission;
 
-import java.security.Principal;
-import java.security.acl.Group;
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.NoSuchElementException;
-import java.util.Set;
+import java.util.TreeSet;
+
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
-import com.google.common.base.Predicate;
-import com.google.common.base.Strings;
-import com.google.common.collect.ImmutableSortedSet;
-import com.google.common.collect.Iterators;
-import com.google.common.primitives.Longs;
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.api.Type;
-import org.apache.jackrabbit.oak.core.ImmutableTree;
 import org.apache.jackrabbit.oak.spi.security.authorization.permission.PermissionConstants;
-import org.apache.jackrabbit.oak.spi.security.authorization.permission.Permissions;
 import org.apache.jackrabbit.oak.spi.security.authorization.permission.ReadStatus;
 import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionPattern;
 import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionProvider;
 import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeBits;
-import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeBitsProvider;
-import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
-import org.apache.jackrabbit.oak.util.TreeUtil;
 import org.apache.jackrabbit.util.Text;
 
-import static com.google.common.base.Preconditions.checkArgument;
+import com.google.common.base.Predicate;
+import com.google.common.base.Strings;
+import com.google.common.collect.Iterators;
+import com.google.common.primitives.Longs;
+
 import static com.google.common.base.Preconditions.checkNotNull;
 
 /**
- * TODO: WIP
+ * The Permission store reads the principal based access control permissions. One store is currently used to handle
+ * 1 set of principal trees (so the compiled permissions use 2 stores, one for the user principals and one for the
+ * group principals)
  */
-class CompiledPermissionImpl implements CompiledPermissions, PermissionConstants {
-
-    private final Set<Principal> principals;
-    private final RestrictionProvider restrictionProvider;
+class PermissionStore implements PermissionConstants {
 
-    private final Map<String, Tree> userTrees;
-    private final Map<String, Tree> groupTrees;
+    private final Map<String, Tree> principalTrees;
 
-    private final Set<String> readPaths;
+    private final RestrictionProvider restrictionProvider;
 
-    private PrivilegeBitsProvider bitsProvider;
+    private final Map<String, Collection<PermissionEntry>> cache = new HashMap<String, Collection<PermissionEntry>>();
 
-    CompiledPermissionImpl(@Nonnull Set<Principal> principals,
-                           @Nonnull ImmutableTree permissionsTree,
-                           @Nonnull PrivilegeBitsProvider bitsProvider,
-                           @Nonnull RestrictionProvider restrictionProvider,
-                           @Nonnull Set<String> readPaths) {
-        checkArgument(!principals.isEmpty());
-        this.principals = principals;
+    PermissionStore(Map<String, Tree> principalTrees, RestrictionProvider restrictionProvider) {
+        this.principalTrees = principalTrees;
         this.restrictionProvider = restrictionProvider;
-        this.bitsProvider = bitsProvider;
-        this.readPaths = readPaths;
-
-        userTrees = new HashMap<String, Tree>(principals.size());
-        groupTrees = new HashMap<String, Tree>(principals.size());
-        if (permissionsTree.exists()) {
-            for (Principal principal : principals) {
-                Tree t = getPrincipalRoot(permissionsTree, principal);
-                Map<String, Tree> target = getTargetMap(principal);
-                if (t.exists()) {
-                    target.put(principal.getName(), t);
-                } else {
-                    target.remove(principal.getName());
-                }
-            }
-        }
-    }
-
-    //------------------------------------------------< CompiledPermissions >---
-    @Override
-    public void refresh(@Nonnull ImmutableTree permissionsTree,
-                 @Nonnull PrivilegeBitsProvider bitsProvider) {
-        this.bitsProvider = bitsProvider;
-        // test if a permission has been added for those principals that didn't have one before
-        for (Principal principal : principals) {
-            Map<String, Tree> target = getTargetMap(principal);
-            Tree principalRoot = getPrincipalRoot(permissionsTree, principal);
-            String pName = principal.getName();
-            if (principalRoot.exists()) {
-                if (!target.containsKey(pName) || !principalRoot.equals(target.get(pName))) {
-                    target.put(pName, principalRoot);
-                }
-            } else {
-                target.remove(pName);
-            }
-        }
-    }
-
-    @Override
-    public ReadStatus getReadStatus(@Nonnull Tree tree, @Nullable PropertyState property) {
-        if (isReadablePath(tree, null)) {
-            return ReadStatus.ALLOW_ALL_REGULAR;
-        }
-        long permission = (property == null) ? Permissions.READ_NODE : Permissions.READ_PROPERTY;
-        Iterator<PermissionEntry> it = getEntryIterator(new EntryPredicate(tree, property));
-        while (it.hasNext()) {
-            PermissionEntry entry = it.next();
-            if (entry.readStatus != null) {
-                return entry.readStatus;
-            } else if (entry.privilegeBits.includesRead(permission)) {
-                return (entry.isAllow) ? ReadStatus.ALLOW_THIS : ReadStatus.DENY_THIS;
-            }
-        }
-        return ReadStatus.DENY_THIS;
-    }
-
-    @Override
-    public boolean isGranted(long permissions) {
-        return hasPermissions(getEntryIterator(new EntryPredicate()), permissions, null, null);
-    }
-
-    @Override
-    public boolean isGranted(@Nonnull Tree tree, @Nullable PropertyState property, long permissions) {
-        Iterator<PermissionEntry> it = getEntryIterator(new EntryPredicate(tree, property));
-        return hasPermissions(it, permissions, tree, null);
-    }
-
-    @Override
-    public boolean isGranted(@Nonnull String path, long permissions) {
-        Iterator<PermissionEntry> it = getEntryIterator(new EntryPredicate(path));
-        return hasPermissions(it, permissions, null, path);
-    }
-
-    @Override
-    public Set<String> getPrivileges(@Nullable Tree tree) {
-        return bitsProvider.getPrivilegeNames(getPrivilegeBits(tree));
-    }
-
-    @Override
-    public boolean hasPrivileges(@Nullable Tree tree, String... privilegeNames) {
-        return getPrivilegeBits(tree).includes(bitsProvider.getBits(privilegeNames));
-    }
-
-    //------------------------------------------------------------< private >---
-    @Nonnull
-    private static Tree getPrincipalRoot(Tree permissionsTree, Principal principal) {
-        return permissionsTree.getChild(Text.escapeIllegalJcrChars(principal.getName()));
     }
 
     @Nonnull
-    private Map<String, Tree> getTargetMap(Principal principal) {
-        return (principal instanceof Group) ? groupTrees : userTrees;
-    }
-
-    private boolean hasPermissions(@Nonnull Iterator<PermissionEntry> entries,
-                                   long permissions, @Nullable Tree tree, @Nullable String path) {
-        // calculate readable paths if the given permissions includes any read permission.
-        boolean isReadable = Permissions.diff(Permissions.READ, permissions) != Permissions.READ && isReadablePath(tree, path);
-        if (!entries.hasNext() && !isReadable) {
-            return false;
-        }
-
-        boolean respectParent = (tree != null || path != null) &&
-                (Permissions.includes(permissions, Permissions.ADD_NODE) ||
-                Permissions.includes(permissions, Permissions.REMOVE_NODE) ||
-                Permissions.includes(permissions, Permissions.MODIFY_CHILD_NODE_COLLECTION));
-
-        long allows = (isReadable) ? Permissions.READ : Permissions.NO_PERMISSION;
-        long denies = Permissions.NO_PERMISSION;
-
-        PrivilegeBits allowBits = PrivilegeBits.getInstance();
-        if (isReadable) {
-            allowBits.add(bitsProvider.getBits(PrivilegeConstants.JCR_READ));
-        }
-        PrivilegeBits denyBits = PrivilegeBits.getInstance();
-        PrivilegeBits parentAllowBits;
-        PrivilegeBits parentDenyBits;
-        String parentPath = null;
-
-        if (respectParent) {
-            parentAllowBits = PrivilegeBits.getInstance();
-            parentDenyBits = PrivilegeBits.getInstance();
-            if (path != null || tree != null) {
-                parentPath = PermissionUtil.getParentPathOrNull((path != null) ? path : tree.getPath());
-            }
-        } else {
-            parentAllowBits = PrivilegeBits.EMPTY;
-            parentDenyBits = PrivilegeBits.EMPTY;
-            parentPath = null;
-        }
-
-        while (entries.hasNext()) {
-            PermissionEntry entry = entries.next();
-            if (respectParent && (parentPath != null)) {
-                boolean matchesParent = entry.matchesParent(parentPath);
-                if (matchesParent) {
-                    if (entry.isAllow) {
-                        parentAllowBits.addDifference(entry.privilegeBits, parentDenyBits);
+    private Collection<PermissionEntry> getEntries(String path) {
+        Collection<PermissionEntry> ret = cache.get(path);
+        if (ret == null) {
+            ret = new TreeSet<PermissionEntry>();
+            String name = PermissionUtil.getEntryName(path);
+            for (Map.Entry<String, Tree> principalRoot : principalTrees.entrySet()) {
+                Tree child = principalRoot.getValue().getChild(name);
+                if (child.exists()) {
+                    if (PermissionUtil.checkACLPath(child, path)) {
+                        loadPermissionsFromNode(path, ret, child);
                     } else {
-                        parentDenyBits.addDifference(entry.privilegeBits, parentAllowBits);
+                        // check for child node
+                        for (Tree node: child.getChildren()) {
+                            if (PermissionUtil.checkACLPath(node, path)) {
+                                loadPermissionsFromNode(path, ret, node);
+                            }
+                        }
                     }
                 }
             }
-
-            if (entry.isAllow) {
-                allowBits.addDifference(entry.privilegeBits, denyBits);
-                long ap = PrivilegeBits.calculatePermissions(allowBits, parentAllowBits, true);
-                allows |= Permissions.diff(ap, denies);
-                if ((allows | ~permissions) == -1) {
-                    return true;
-                }
-            } else {
-                denyBits.addDifference(entry.privilegeBits, allowBits);
-                long dp = PrivilegeBits.calculatePermissions(denyBits, parentDenyBits, false);
-                denies |= Permissions.diff(dp, allows);
-                if (Permissions.includes(denies, permissions)) {
-                    return false;
-                }
-            }
+            //cache.put(path, ret);
         }
-
-        return (allows | ~permissions) == -1;
+        return ret;
     }
 
-    @Nonnull
-    private PrivilegeBits getPrivilegeBits(@Nullable Tree tree) {
-        EntryPredicate pred = (tree == null) ? new EntryPredicate() : new EntryPredicate(tree, null);
-        Iterator<PermissionEntry> entries = getEntryIterator(pred);
-
-        PrivilegeBits allowBits = PrivilegeBits.getInstance();
-        PrivilegeBits denyBits = PrivilegeBits.getInstance();
-
-        while (entries.hasNext()) {
-            PermissionEntry entry = entries.next();
-            if (entry.isAllow) {
-                allowBits.addDifference(entry.privilegeBits, denyBits);
-            } else {
-                denyBits.addDifference(entry.privilegeBits, allowBits);
+    private void loadPermissionsFromNode(String path, Collection<PermissionEntry> ret, Tree node) {
+        for (Tree ace: node.getChildren()) {
+            if (ace.getName().charAt(0) != 'c') {
+                ret.add(new PermissionEntry(path, ace, restrictionProvider));
             }
         }
-
-        // special handling for paths that are always readable
-        if (isReadablePath(tree, null)) {
-            allowBits.add(bitsProvider.getBits(PrivilegeConstants.JCR_READ));
-        }
-        return allowBits;
-    }
-
-    @Nonnull
-    private Iterator<PermissionEntry> getEntryIterator(@Nonnull EntryPredicate predicate) {
-        Iterator<PermissionEntry> userEntries = (userTrees.isEmpty()) ?
-                Iterators.<PermissionEntry>emptyIterator() :
-                new EntryIterator(userTrees, predicate);
-        Iterator<PermissionEntry> groupEntries = (groupTrees.isEmpty()) ?
-                Iterators.<PermissionEntry>emptyIterator():
-                new EntryIterator(groupTrees, predicate);
-        return Iterators.concat(userEntries, groupEntries);
     }
 
-    private boolean isReadablePath(@Nullable Tree tree, @Nullable String treePath) {
-        if (!readPaths.isEmpty()) {
-            String targetPath = (tree != null) ? tree.getPath() : treePath;
-            if (targetPath != null) {
-                for (String path : readPaths) {
-                    if (Text.isDescendantOrEqual(path, targetPath)) {
-                        return true;
-                    }
-                }
-            }
+    public Iterator<PermissionEntry> getEntryIterator(@Nonnull EntryPredicate predicate) {
+        if (principalTrees.isEmpty()) {
+            return Iterators.emptyIterator();
         }
-        return false;
+        return new EntryIterator(predicate);
     }
 
-    private static final class PermissionEntry {
-
-        private final boolean isAllow;
-        private final PrivilegeBits privilegeBits;
-        private final long index;
-        private final String path;
-        private final RestrictionPattern restriction;
-
-        private ReadStatus readStatus = null; // TODO
-
-        private PermissionEntry(Tree entryTree, RestrictionProvider restrictionsProvider) {
-            isAllow = entryTree.getProperty(REP_IS_ALLOW).getValue(Type.BOOLEAN);
-            privilegeBits = PrivilegeBits.getInstance(entryTree.getProperty(REP_PRIVILEGE_BITS));
-            index = checkNotNull(entryTree.getProperty(REP_INDEX).getValue(Type.LONG)).longValue();
-            path = Strings.emptyToNull(TreeUtil.getString(entryTree, REP_ACCESS_CONTROLLED_PATH));
-            restriction = restrictionsProvider.getPattern(path, entryTree);
-        }
-
-        private boolean matches(@Nonnull Tree tree, @Nullable PropertyState property) {
-            return restriction.matches(tree, property);
-        }
-
-        private boolean matches(@Nonnull String treePath) {
-            return restriction.matches(treePath);
-        }
-
-        private boolean matches() {
-            return restriction.matches();
-        }
-
-        private boolean matchesParent(@Nonnull String parentPath) {
-            if (Text.isDescendantOrEqual(path, parentPath)) {
-                return restriction.matches(parentPath);
-            } else {
-                return false;
-            }
-        }
+    public void flush() {
+        cache.clear();
     }
 
     private class EntryIterator implements Iterator<PermissionEntry> {
 
-        private final Collection<Tree> principalTrees;
         private final EntryPredicate predicate;
 
         // the next oak path for which to retrieve permission entries
         private String path;
+
         // the ordered permission entries at a given path in the hierarchy
-        private Iterator<PermissionEntry> nextEntries = Iterators.emptyIterator();
+        private Iterator<PermissionEntry> nextEntries;
+
         // the next permission entry
         private PermissionEntry next;
 
-        private EntryIterator(@Nonnull Map<String, Tree> principalTrees,
-                              @Nonnull EntryPredicate predicate) {
-            this.principalTrees = principalTrees.values();
+        private EntryIterator(@Nonnull EntryPredicate predicate) {
             this.predicate = predicate;
             this.path = Strings.nullToEmpty(predicate.path);
-            next = seekNext();
         }
 
         @Override
         public boolean hasNext() {
+            if (next == null) {
+                // lazy initialization
+                if (nextEntries == null) {
+                    nextEntries = Iterators.emptyIterator();
+                    seekNext();
+                }
+            }
             return next != null;
         }
 
@@ -359,7 +145,7 @@ class CompiledPermissionImpl implements 
             }
 
             PermissionEntry pe = next;
-            next = seekNext();
+            seekNext();
             return pe;
         }
 
@@ -369,65 +155,43 @@ class CompiledPermissionImpl implements 
         }
 
         @CheckForNull
-        private PermissionEntry seekNext() {
-            // calculate the ordered entries for the next hierarchy level.
-            while (!nextEntries.hasNext() && path != null) {
-                nextEntries = getNextEntries();
-                path = PermissionUtil.getParentPathOrNull(path);
-            }
-
-            if (nextEntries.hasNext()) {
-                return nextEntries.next();
-            } else {
-                return null;
-            }
-        }
-
-        @Nonnull
-        private Iterator<PermissionEntry> getNextEntries() {
-            ImmutableSortedSet.Builder<PermissionEntry> entries = new ImmutableSortedSet.Builder(new EntryComparator());
-            for (Tree principalRoot : principalTrees) {
-                String name = PermissionUtil.getEntryName(path);
-                Tree parent = principalRoot;
-                while (parent.hasChild(name)) {
-                    parent = parent.getChild(name);
-                    PermissionEntry pe = new PermissionEntry(parent, restrictionProvider);
+        private void seekNext() {
+            for (next = null; next == null;) {
+                if (nextEntries.hasNext()) {
+                    PermissionEntry pe = nextEntries.next();
                     if (predicate.apply(pe)) {
-                        entries.add(pe);
+                        next = pe;
+                    }
+                } else {
+                    if (path == null) {
+                        break;
                     }
+                    nextEntries = getEntries(path).iterator();
+                    path = PermissionUtil.getParentPathOrNull(path);
                 }
             }
-            return entries.build().iterator();
         }
     }
 
-    private static final class EntryComparator implements Comparator<PermissionEntry> {
-        @Override
-        public int compare(@Nonnull PermissionEntry entry,
-                           @Nonnull PermissionEntry otherEntry) {
-            return Longs.compare(otherEntry.index, entry.index);
-        }
-    }
-
-    private static final class EntryPredicate implements Predicate<PermissionEntry> {
+    public static final class EntryPredicate implements Predicate<PermissionEntry> {
 
         private final Tree tree;
         private final PropertyState property;
         private final String path;
 
-        private EntryPredicate(@Nonnull Tree tree, @Nullable PropertyState property) {
+        public EntryPredicate(@Nonnull Tree tree, @Nullable PropertyState property) {
             this.tree = tree;
             this.property = property;
             this.path = tree.getPath();
         }
 
-        private EntryPredicate(@Nonnull String path) {
+        public EntryPredicate(@Nonnull String path) {
             this.tree = null;
             this.property = null;
             this.path = path;
         }
 
-        private EntryPredicate() {
+        public EntryPredicate() {
             this.tree = null;
             this.property = null;
             this.path = null;
@@ -447,4 +211,95 @@ class CompiledPermissionImpl implements 
             }
         }
     }
+
+
+    public static final class PermissionEntry implements Comparable<PermissionEntry> {
+
+        /**
+         * flag controls if this is an allow or deny entry
+         */
+        public final boolean isAllow;
+
+        /**
+         * the privilege bits
+         */
+        public final PrivilegeBits privilegeBits;
+
+        /**
+         * the index (order) of the original ACE in the ACL.
+         */
+        public final int index;
+
+        /**
+         * the access controlled (node) path
+         */
+        public final String path;
+
+        /**
+         * the restriction pattern for this entry
+         */
+        public final RestrictionPattern restriction;
+
+        /**
+         * pre-evaluated read status
+         */
+        public ReadStatus readStatus = null; // TODO
+
+        private PermissionEntry(String path, Tree entryTree, RestrictionProvider restrictionsProvider) {
+            this.path = path;
+            isAllow = entryTree.getProperty(REP_IS_ALLOW).getValue(Type.BOOLEAN);
+            privilegeBits = PrivilegeBits.getInstance(entryTree.getProperty(REP_PRIVILEGE_BITS));
+            index = (int) (long) checkNotNull(entryTree.getProperty(REP_INDEX).getValue(Type.LONG));
+            restriction = restrictionsProvider.getPattern(path, entryTree);
+        }
+
+        public boolean matches(@Nonnull Tree tree, @Nullable PropertyState property) {
+            return restriction == RestrictionPattern.EMPTY || restriction.matches(tree, property);
+        }
+
+        public boolean matches(@Nonnull String treePath) {
+            return restriction == RestrictionPattern.EMPTY || restriction.matches(treePath);
+        }
+
+        public boolean matches() {
+            return restriction == RestrictionPattern.EMPTY || restriction.matches();
+        }
+
+        public boolean matchesParent(@Nonnull String parentPath) {
+            return Text.isDescendantOrEqual(path, parentPath) && (restriction == RestrictionPattern.EMPTY || restriction.matches(parentPath));
+        }
+
+        @Override
+        public int compareTo(PermissionEntry o) {
+            final int anotherVal = o.index;
+            // reverse order
+            return (index<anotherVal ? 1 : (index==anotherVal ? 0 : -1));
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+
+            PermissionEntry that = (PermissionEntry) o;
+
+            if (index != that.index) return false;
+
+            return true;
+        }
+
+        @Override
+        public int hashCode() {
+            return index;
+        }
+    }
+
+    private static final class EntryComparator implements Comparator<PermissionEntry> {
+        @Override
+        public int compare(@Nonnull PermissionEntry entry,
+                           @Nonnull PermissionEntry otherEntry) {
+            return Longs.compare(otherEntry.index, entry.index);
+        }
+    }
+
 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionUtil.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionUtil.java?rev=1528897&r1=1528896&r2=1528897&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionUtil.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionUtil.java Thu Oct  3 14:43:57 2013
@@ -20,20 +20,33 @@ import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
+import org.apache.jackrabbit.oak.api.PropertyState;
+import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.api.Type;
+import org.apache.jackrabbit.oak.spi.security.authorization.permission.PermissionConstants;
+import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+
 import com.google.common.base.Strings;
-import org.apache.jackrabbit.oak.commons.PathUtils;
-import org.apache.jackrabbit.util.Text;
 
 /**
  * PermissionUtil... TODO
  */
-public final class PermissionUtil {
+public final class PermissionUtil implements PermissionConstants {
 
     private PermissionUtil() {}
 
     @CheckForNull
-    public static String getParentPathOrNull(@Nonnull String path) {
-        return (PathUtils.denotesRoot(path) || path.isEmpty()) ? null : Text.getRelativeParent(path, 1);
+    public static String getParentPathOrNull(@Nonnull final String path) {
+        if (path.length() <= 1) {
+            return null;
+        } else {
+            int idx = path.lastIndexOf('/');
+            if (idx == 0) {
+                return "/";
+            } else {
+                return path.substring(0, idx);
+            }
+        }
     }
 
     @Nonnull
@@ -41,4 +54,14 @@ public final class PermissionUtil {
         String path = Strings.nullToEmpty(accessControlledPath);
         return String.valueOf(path.hashCode());
     }
+
+    public static boolean checkACLPath(NodeBuilder node, String path) {
+        PropertyState property = node.getProperty(REP_ACCESS_CONTROLLED_PATH);
+        return property != null && path.equals(property.getValue(Type.STRING));
+    }
+
+    public static boolean checkACLPath(Tree node, String path) {
+        PropertyState property = node.getProperty(REP_ACCESS_CONTROLLED_PATH);
+        return property != null && path.equals(property.getValue(Type.STRING));
+    }
 }
\ No newline at end of file

Modified: jackrabbit/oak/trunk/oak-core/src/main/resources/org/apache/jackrabbit/oak/plugins/nodetype/write/builtin_nodetypes.cnd
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/resources/org/apache/jackrabbit/oak/plugins/nodetype/write/builtin_nodetypes.cnd?rev=1528897&r1=1528896&r2=1528897&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/resources/org/apache/jackrabbit/oak/plugins/nodetype/write/builtin_nodetypes.cnd (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/resources/org/apache/jackrabbit/oak/plugins/nodetype/write/builtin_nodetypes.cnd Thu Oct  3 14:43:57 2013
@@ -669,6 +669,7 @@
  * @since oak 1.0
  */
 [rep:PermissionStore]
+  - rep:accessControlledPath (STRING) protected
   + * (rep:PermissionStore) = rep:PermissionStore protected IGNORE
   + * (rep:Permissions) = rep:Permissions protected IGNORE
 

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractPermissionHookTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractPermissionHookTest.java?rev=1528897&r1=1528896&r2=1528897&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractPermissionHookTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractPermissionHookTest.java Thu Oct  3 14:43:57 2013
@@ -20,6 +20,7 @@ import java.security.Principal;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
+
 import javax.jcr.RepositoryException;
 import javax.jcr.security.AccessControlEntry;
 import javax.jcr.security.AccessControlManager;
@@ -39,7 +40,6 @@ import org.apache.jackrabbit.oak.spi.sec
 import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeBitsProvider;
 import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
 import org.apache.jackrabbit.oak.util.NodeUtil;
-import org.apache.jackrabbit.util.Text;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -108,24 +108,12 @@ public abstract class AbstractPermission
 
     protected Tree getEntry(String principalName, String accessControlledPath, long index) throws Exception {
         Tree principalRoot = getPrincipalRoot(principalName);
-        return traverse(principalRoot, accessControlledPath, index);
-    }
-
-    protected Tree traverse(Tree parent, String accessControlledPath, long index) throws Exception {
-        for (Tree entry : parent.getChildren()) {
-            String path = entry.getProperty(REP_ACCESS_CONTROLLED_PATH).getValue(Type.STRING);
-            long entryIndex = entry.getProperty(REP_INDEX).getValue(Type.LONG);
-            if (accessControlledPath.equals(path)) {
-                if (index == entryIndex) {
-                    return entry;
-                } else if (index > entryIndex) {
-                    return traverse(entry, accessControlledPath, index);
-                }
-            } else if (Text.isDescendant(path, accessControlledPath)) {
-                return traverse(entry, accessControlledPath, index);
-            }
+        Tree parent = principalRoot.getChild(PermissionUtil.getEntryName(accessControlledPath));
+        Tree entry = parent.getChild(String.valueOf(index));
+        if (!entry.exists()) {
+            throw new RepositoryException("no such entry");
         }
-        throw new RepositoryException("no such entry");
+        return entry;
     }
 
     protected long cntEntries(Tree parent) {
@@ -159,8 +147,9 @@ public abstract class AbstractPermission
         root.commit();
 
         Tree principalRoot = getPrincipalRoot(testPrincipalName);
-        assertEquals(1, cntEntries(principalRoot));
-        assertEquals("*", principalRoot.getChildren().iterator().next().getProperty(REP_GLOB).getValue(Type.STRING));
+        assertEquals(2, cntEntries(principalRoot));
+        Tree parent = principalRoot.getChildren().iterator().next();
+        assertEquals("*", parent.getChildren().iterator().next().getProperty(REP_GLOB).getValue(Type.STRING));
 
         // modify the restrictions node
         Tree restrictionsNode = root.getTree(restrictionsPath);
@@ -168,16 +157,18 @@ public abstract class AbstractPermission
         root.commit();
 
         principalRoot = getPrincipalRoot(testPrincipalName);
-        assertEquals(1, cntEntries(principalRoot));
-        assertEquals("/*/jcr:content/*", principalRoot.getChildren().iterator().next().getProperty(REP_GLOB).getValue(Type.STRING));
+        assertEquals(2, cntEntries(principalRoot));
+        parent = principalRoot.getChildren().iterator().next();
+        assertEquals("/*/jcr:content/*", parent.getChildren().iterator().next().getProperty(REP_GLOB).getValue(Type.STRING));
 
         // remove the restriction again
         root.getTree(restrictionsPath).remove();
         root.commit();
 
         principalRoot = getPrincipalRoot(testPrincipalName);
-        assertEquals(1, cntEntries(principalRoot));
-        assertNull(principalRoot.getChildren().iterator().next().getProperty(REP_GLOB));
+        assertEquals(2, cntEntries(principalRoot));
+        parent = principalRoot.getChildren().iterator().next();
+        assertNull(parent.getChildren().iterator().next().getProperty(REP_GLOB));
     }
 
     @Test
@@ -332,7 +323,7 @@ public abstract class AbstractPermission
         assertTrue(root.getTree(childPath + "/rep:policy").exists());
 
         Tree principalRoot = getPrincipalRoot(EveryonePrincipal.NAME);
-        assertEquals(2, cntEntries(principalRoot));
+        assertEquals(4, cntEntries(principalRoot));
 
         ContentSession testSession = createTestSession();
         Root testRoot = testSession.getLatestRoot();
@@ -350,6 +341,6 @@ public abstract class AbstractPermission
         // aces must be removed in the permission store even if the editing
         // session wasn't able to access them.
         principalRoot = getPrincipalRoot(EveryonePrincipal.NAME);
-        assertEquals(1, cntEntries(principalRoot));
+        assertEquals(2, cntEntries(principalRoot));
     }
 }
\ No newline at end of file

Modified: jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java?rev=1528897&r1=1528896&r2=1528897&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java Thu Oct  3 14:43:57 2013
@@ -514,7 +514,9 @@ public class ReadTest extends AbstractEv
         TraversingItemVisitor v = new TraversingItemVisitor.Default(true, -1) {
             @Override
             protected void entering(Node node, int level) throws RepositoryException {
-                if (node.isNodeType("rep:Permissions") && path.equals(node.getProperty("rep:accessControlledPath").getString())) {
+                if (node.isNodeType("rep:Permissions")
+                        && node.hasProperty("rep:accessControlledPath")
+                        && path.equals(node.getProperty("rep:accessControlledPath").getString())) {
                     assertEquals(index, node.getProperty("rep:index").getLong());
                     assertEquals(isAllow, node.getProperty("rep:isAllow").getBoolean());
                 }



Mime
View raw message