jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From tri...@apache.org
Subject svn commit: r1575041 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/ oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/ oak-core/src/main/java/org/apache/jackrabbit/o...
Date Thu, 06 Mar 2014 20:59:29 GMT
Author: tripod
Date: Thu Mar  6 20:59:29 2014
New Revision: 1575041

URL: http://svn.apache.org/r1575041
Log:
OAK-1471 Mod count in permission store requires cluster wide synchronization

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AuthorizationConfigurationImpl.java
    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/PermissionEntryCache.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImpl.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/PermissionProviderImpl.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStore.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStoreImpl.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/java/org/apache/jackrabbit/oak/security/authorization/permission/PrincipalPermissionEntries.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/PermissionConstants.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHookTest.java
    jackrabbit/oak/trunk/oak-run/run_everyone_acl.sh
    jackrabbit/oak/trunk/oak-run/run_writeacl.sh
    jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/benchmark/BenchmarkRunner.java
    jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/benchmark/ConcurrentWriteACLTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AuthorizationConfigurationImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AuthorizationConfigurationImpl.java?rev=1575041&r1=1575040&r2=1575041&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AuthorizationConfigurationImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AuthorizationConfigurationImpl.java Thu Mar  6 20:59:29 2014
@@ -64,8 +64,6 @@ import com.google.common.collect.Immutab
 @Service({AuthorizationConfiguration.class, SecurityConfiguration.class})
 public class AuthorizationConfigurationImpl extends ConfigurationBase implements AuthorizationConfiguration {
 
-    private final PermissionEntryCache permissionEntryCache = new PermissionEntryCache();
-
     public AuthorizationConfigurationImpl() {
         super();
     }
@@ -103,7 +101,7 @@ public class AuthorizationConfigurationI
     public List<? extends CommitHook> getCommitHooks(String workspaceName) {
         return ImmutableList.of(
                 new VersionablePathHook(workspaceName),
-                new PermissionHook(workspaceName, getRestrictionProvider(), permissionEntryCache));
+                new PermissionHook(workspaceName, getRestrictionProvider()));
     }
 
     @Override
@@ -140,7 +138,7 @@ public class AuthorizationConfigurationI
     @Nonnull
     @Override
     public PermissionProvider getPermissionProvider(Root root, String workspaceName, Set<Principal> principals) {
-        return new PermissionProviderImpl(root, workspaceName, principals, this, permissionEntryCache.createLocalCache());
+        return new PermissionProviderImpl(root, workspaceName, principals, this);
     }
 
 }

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=1575041&r1=1575040&r2=1575041&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 Mar  6 20:59:29 2014
@@ -84,8 +84,7 @@ final class CompiledPermissionImpl imple
 
     private final TreeTypeProvider typeProvider;
 
-    private CompiledPermissionImpl(@Nonnull PermissionEntryCache.Local cache,
-                                   @Nonnull Set<Principal> principals,
+    private CompiledPermissionImpl(@Nonnull Set<Principal> principals,
                                    @Nonnull ImmutableRoot root, @Nonnull String workspaceName,
                                    @Nonnull RestrictionProvider restrictionProvider,
                                    @Nonnull AuthorizationConfiguration acConfig) {
@@ -109,6 +108,7 @@ final class CompiledPermissionImpl imple
         }
 
         ConfigurationParameters options = acConfig.getParameters();
+        PermissionEntryCache cache = new PermissionEntryCache();
         userStore = new PermissionEntryProviderImpl(store, cache, userNames, options);
         groupStore = new PermissionEntryProviderImpl(store, cache, groupNames, options);
 
@@ -117,13 +117,12 @@ final class CompiledPermissionImpl imple
 
     static CompiledPermissions create(@Nonnull ImmutableRoot root, @Nonnull String workspaceName,
                                       @Nonnull Set<Principal> principals,
-                                      @Nonnull AuthorizationConfiguration acConfig,
-                                      @Nonnull PermissionEntryCache.Local cache) {
+                                      @Nonnull AuthorizationConfiguration acConfig) {
         Tree permissionsTree = PermissionUtil.getPermissionsRoot(root, workspaceName);
         if (!permissionsTree.exists() || principals.isEmpty()) {
             return NoPermissions.getInstance();
         } else {
-            return new CompiledPermissionImpl(cache, principals, root, workspaceName, acConfig.getRestrictionProvider(), acConfig);
+            return new CompiledPermissionImpl(principals, root, workspaceName, acConfig.getRestrictionProvider(), acConfig);
         }
     }
 

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCache.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCache.java?rev=1575041&r1=1575040&r2=1575041&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCache.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCache.java Thu Mar  6 20:59:29 2014
@@ -17,28 +17,20 @@
 package org.apache.jackrabbit.oak.security.authorization.permission;
 
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
-import java.util.concurrent.ConcurrentHashMap;
-import javax.annotation.Nonnull;
 
-import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
+import javax.annotation.Nonnull;
 
 /**
  * {@code PermissionEntryCache} caches the permission entries of principals.
- * The cache is held globally and contains a version of the principal permission
- * entries of the session that read them last. Each session gets a lazy copy of
- * the cache and needs to verify if each cached principal permission set still
- * reflects the state that the session sees.
- * Every newly loaded principal permission set can be pushed down to the base
- * cache if it does not exist there yet, or if it's newer.
+ * The cache is held locally for each session and contains a version of the principal permission
+ * entries of the session that read them last.
  *
  * TODO:
- * - currently only the entries of 'everyone' are globally cached. this should be improved to dynamically cache those
- *   principals that are used often
  * - report cache usage metrics
  * - limit size of local caches based on ppe sizes. the current implementation loads all ppes. this can get a memory
  *   problem, as well as a performance problem for principals with many entries. principals with many entries must
@@ -47,98 +39,73 @@ import org.apache.jackrabbit.oak.spi.sec
  */
 public class PermissionEntryCache {
 
-    private final Map<String, PrincipalPermissionEntries> base = new ConcurrentHashMap<String, PrincipalPermissionEntries>();
+    private final Map<String, PrincipalPermissionEntries> entries = new HashMap<String, PrincipalPermissionEntries>();
 
     @Nonnull
-    public Local createLocalCache() {
-        return new Local();
-    }
-
-    public void flush(@Nonnull Set<String> principalNames) {
-        base.keySet().removeAll(principalNames);
-    }
-
-    public final class Local {
-
-        private final Map<String, PrincipalPermissionEntries> entries = new HashMap<String, PrincipalPermissionEntries>();
-
-        private final Set<String> verified = new HashSet<String>();
-
-        private Local() {
-            entries.putAll(base);
-        }
-
-        @Nonnull
-        public PrincipalPermissionEntries getEntries(@Nonnull PermissionStore store,
-                                                     @Nonnull String principalName) {
-            PrincipalPermissionEntries ppe = entries.get(principalName);
-            if (ppe == null) {
+    public PrincipalPermissionEntries getEntries(@Nonnull PermissionStore store,
+                                                 @Nonnull String principalName) {
+        PrincipalPermissionEntries ppe = entries.get(principalName);
+        if (ppe == null) {
+            ppe = store.load(principalName);
+            entries.put(principalName, ppe);
+        } else {
+            if (!ppe.isFullyLoaded()) {
                 ppe = store.load(principalName);
                 entries.put(principalName, ppe);
-            } else {
-                if (!verified.contains(principalName)) {
-                    if (store.getModCount(principalName) != ppe.getModCount()) {
-                        ppe = store.load(principalName);
-                        entries.put(principalName, ppe);
-                    }
-                    verified.add(principalName);
-                }
-            }
-
-            /*
-            Currently this cache only handles entries for the Everyone principal.
-            TODO: the cache should dynamically cache the principals that are used often.
-            */
-            if (EveryonePrincipal.NAME.equals(principalName)) {
-                // check if base cache has the entries
-                PrincipalPermissionEntries baseppe = base.get(principalName);
-                if (baseppe == null || ppe.getModCount() > baseppe.getModCount()) {
-                    base.put(principalName, ppe);
-                }
             }
-            return ppe;
         }
+        return ppe;
+    }
 
-        public void load(@Nonnull PermissionStore store,
-                         @Nonnull Map<String, Collection<PermissionEntry>> pathEntryMap,
-                         @Nonnull String principalName) {
-            // todo: conditionally load entries if too many
-            PrincipalPermissionEntries ppe = getEntries(store, principalName);
-            for (Map.Entry<String, Collection<PermissionEntry>> e: ppe.getEntries().entrySet()) {
-                Collection<PermissionEntry> pathEntries = pathEntryMap.get(e.getKey());
-                if (pathEntries == null) {
-                    pathEntries = new TreeSet<PermissionEntry>(e.getValue());
-                    pathEntryMap.put(e.getKey(), pathEntries);
-                } else {
-                    pathEntries.addAll(e.getValue());
-                }
+    public void load(@Nonnull PermissionStore store,
+                     @Nonnull Map<String, Collection<PermissionEntry>> pathEntryMap,
+                     @Nonnull String principalName) {
+        // todo: conditionally load entries if too many
+        PrincipalPermissionEntries ppe = getEntries(store, principalName);
+        for (Map.Entry<String, Collection<PermissionEntry>> e: ppe.getEntries().entrySet()) {
+            Collection<PermissionEntry> pathEntries = pathEntryMap.get(e.getKey());
+            if (pathEntries == null) {
+                pathEntries = new TreeSet<PermissionEntry>(e.getValue());
+                pathEntryMap.put(e.getKey(), pathEntries);
+            } else {
+                pathEntries.addAll(e.getValue());
             }
         }
+    }
 
-        public void load(@Nonnull PermissionStore store,
-                         @Nonnull Collection<PermissionEntry> ret,
-                         @Nonnull String principalName,
-                         @Nonnull String path) {
-            // todo: conditionally load entries if too many
-            PrincipalPermissionEntries ppe = getEntries(store, principalName);
-            ret.addAll(ppe.getEntries(path));
-        }
-
-        public boolean hasEntries(@Nonnull PermissionStore store,
-                                  @Nonnull String principalName) {
-            // todo: conditionally load entries if too many
-            return getNumEntries(store, principalName) > 0;
-        }
-
-        public long getNumEntries(@Nonnull PermissionStore store,
-                                  @Nonnull String principalName) {
-            // todo: conditionally load entries if too many
-            return getEntries(store, principalName).getEntries().size();
+    public void load(@Nonnull PermissionStore store,
+                     @Nonnull Collection<PermissionEntry> ret,
+                     @Nonnull String principalName,
+                     @Nonnull String path) {
+        PrincipalPermissionEntries ppe = entries.get(principalName);
+        if (ppe == null) {
+            ppe = new PrincipalPermissionEntries(principalName);
+            entries.put(principalName, ppe);
+        }
+        Collection<PermissionEntry> pes = ppe.getEntries().get(path);
+        if (pes == null) {
+            pes = store.load(null, principalName, path);
+            if (pes == null) {
+                pes = Collections.emptySet();
+            } else {
+                ret.addAll(pes);
+            }
+            ppe.getEntries().put(path, pes);
+        } else {
+            ret.addAll(pes);
         }
+    }
 
-        public void flush(@Nonnull Set<String> principalNames) {
-            verified.removeAll(principalNames);
-        }
+    public long getNumEntries(@Nonnull PermissionStore store,
+                              @Nonnull String principalName,
+                              long max) {
+        PrincipalPermissionEntries ppe = entries.get(principalName);
+        return ppe == null
+                ? store.getNumEntries(principalName, max)
+                : ppe.getEntries().size();
+    }
 
+    public void flush(@Nonnull Set<String> principalNames) {
+        entries.keySet().removeAll(principalNames);
     }
 }
\ No newline at end of file

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImpl.java?rev=1575041&r1=1575040&r2=1575041&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImpl.java Thu Mar  6 20:59:29 2014
@@ -47,11 +47,11 @@ class PermissionEntryProviderImpl implem
 
     private final PermissionStore store;
 
-    private final PermissionEntryCache.Local cache;
+    private final PermissionEntryCache cache;
 
     private final long maxSize;
 
-    PermissionEntryProviderImpl(@Nonnull PermissionStore store, @Nonnull PermissionEntryCache.Local cache,
+    PermissionEntryProviderImpl(@Nonnull PermissionStore store, @Nonnull PermissionEntryCache cache,
                                 @Nonnull Set<String> principalNames, @Nonnull ConfigurationParameters options) {
         this.store = store;
         this.cache = cache;
@@ -64,16 +64,10 @@ class PermissionEntryProviderImpl implem
         long cnt = 0;
         existingNames.clear();
         for (String name: principalNames) {
-            if (cnt > maxSize) {
-                if (cache.hasEntries(store, name)) {
-                    existingNames.add(name);
-                }
-            } else {
-                long n = cache.getNumEntries(store, name);
-                cnt+= n;
-                if (n > 0) {
-                    existingNames.add(name);
-                }
+            long n = cache.getNumEntries(store, name, maxSize);
+            cnt+= n;
+            if (n > 0) {
+                existingNames.add(name);
             }
         }
         if (cnt < maxSize) {

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=1575041&r1=1575040&r2=1575041&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 Mar  6 20:59:29 2014
@@ -18,22 +18,17 @@ package org.apache.jackrabbit.oak.securi
 
 import java.util.ArrayList;
 import java.util.HashMap;
-import java.util.HashSet;
 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 org.apache.jackrabbit.oak.api.CommitFailedException;
-import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.core.ImmutableRoot;
-import org.apache.jackrabbit.oak.plugins.tree.ImmutableTree;
 import org.apache.jackrabbit.oak.plugins.nodetype.TypePredicate;
+import org.apache.jackrabbit.oak.plugins.tree.ImmutableTree;
 import org.apache.jackrabbit.oak.spi.commit.CommitInfo;
 import org.apache.jackrabbit.oak.spi.commit.PostValidationHook;
 import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants;
@@ -50,22 +45,25 @@ 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.collect.Iterables.addAll;
 import static com.google.common.collect.Sets.newLinkedHashSet;
 import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE;
 import static org.apache.jackrabbit.JcrConstants.JCR_SYSTEM;
-import static org.apache.jackrabbit.oak.plugins.tree.TreeConstants.OAK_CHILD_ORDER;
 import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE;
+import static org.apache.jackrabbit.oak.plugins.tree.TreeConstants.OAK_CHILD_ORDER;
 
 /**
  * {@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
+ * The access control entries are grouped by principal and stored 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
+ *   /jcr:system/rep:permissionStore/workspace-name
  *      /everyone
  *          /552423  [rep:PermissionStore]
  *              /0     [rep:Permissions]
@@ -86,7 +84,6 @@ public class PermissionHook implements P
 
     private final RestrictionProvider restrictionProvider;
     private final String workspaceName;
-    private final PermissionEntryCache cache;
 
     private NodeBuilder permissionRoot;
     private PrivilegeBitsProvider bitsProvider;
@@ -98,10 +95,9 @@ public class PermissionHook implements P
     private Map<String, Acl> modified = new HashMap<String, Acl>();
     private Map<String, Acl> deleted = new HashMap<String, Acl>();
 
-    public PermissionHook(String workspaceName, RestrictionProvider restrictionProvider, PermissionEntryCache cache) {
+    public PermissionHook(String workspaceName, RestrictionProvider restrictionProvider) {
         this.workspaceName = workspaceName;
         this.restrictionProvider = restrictionProvider;
-        this.cache = cache;
     }
 
     @Nonnull
@@ -125,14 +121,12 @@ public class PermissionHook implements P
     }
 
     private void apply() {
-        Set<String> principalNames = new HashSet<String>();
         for (Map.Entry<String, Acl> entry : deleted.entrySet()) {
-            entry.getValue().remove(principalNames);
+            entry.getValue().remove();
         }
         for (Map.Entry<String, Acl> entry : modified.entrySet()) {
-            entry.getValue().update(principalNames);
+            entry.getValue().update();
         }
-        cache.flush(principalNames);
     }
 
     @Nonnull
@@ -253,11 +247,9 @@ public class PermissionHook implements P
             }
         }
 
-        private void remove(Set<String> principalNames) {
-            String msg = "Unable to remove permission entry";
+        private void remove() {
             for (String principalName: entries.keySet()) {
                 if (permissionRoot.hasChildNode(principalName)) {
-                    principalNames.add(principalName);
                     NodeBuilder principalRoot = permissionRoot.getChildNode(principalName);
 
                     // find the ACL node that for this path and principal
@@ -266,15 +258,12 @@ public class PermissionHook implements P
                         continue;
                     }
 
-                    long numEntries = PermissionUtil.getNumPermissions(principalRoot);
-
                     // 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') {
-                                numEntries--;
                                 continue;
                             }
                             NodeBuilder child = parent.getChildNode(childName);
@@ -297,24 +286,18 @@ public class PermissionHook implements P
                             }
                             NodeBuilder child = parent.getChildNode(childName);
                             if (PermissionUtil.checkACLPath(child, accessControlledPath)) {
-                                // remove child
-                                for (String n: child.getChildNodeNames()) {
-                                    numEntries--;
-                                }
                                 child.remove();
                             }
                         }
                     }
-                    touch(principalRoot, numEntries);
                 } else {
-                    log.error("{} {}: Principal root missing.", msg, this);
+                    log.error("Unable to remove permission entry {}: Principal root missing.", this);
                 }
             }
         }
 
-        private void update(Set<String> principalNames) {
+        private void update() {
             for (String principalName: entries.keySet()) {
-                principalNames.add(principalName);
                 NodeBuilder principalRoot = permissionRoot.child(principalName);
                 if (!principalRoot.hasProperty(JCR_PRIMARYTYPE)) {
                     principalRoot.setProperty(JCR_PRIMARYTYPE, NT_REP_PERMISSION_STORE, Type.NAME);
@@ -358,32 +341,20 @@ public class PermissionHook implements P
                     // new parent
                     parent.setProperty(REP_ACCESS_CONTROLLED_PATH, accessControlledPath);
                 }
-                long numEntries = PermissionUtil.getNumPermissions(principalRoot);
-                numEntries+= updateEntries(parent, entries.get(principalName));
-                touch(principalRoot, numEntries);
+                updateEntries(parent, entries.get(principalName));
             }
         }
 
-        private long updateEntries(NodeBuilder parent, List<AcEntry> list) {
+        private void updateEntries(NodeBuilder parent, List<AcEntry> list) {
             // remove old entries
-            long numEntries = 0;
             for (String childName : parent.getChildNodeNames()) {
                 if (childName.charAt(0) != 'c') {
                     parent.getChildNode(childName).remove();
-                    numEntries--;
                 }
             }
             for (AcEntry ace: list) {
                 PermissionEntry.write(parent, ace.isAllow, ace.index, ace.privilegeBits, ace.restrictions);
-                numEntries++;
             }
-            return numEntries;
-        }
-
-        private void touch(NodeBuilder node, long numEntries) {
-            PropertyState ps = node.getProperty(REP_MOD_COUNT);
-            node.setProperty(REP_MOD_COUNT, ps == null ? 1 : ps.getValue(Type.LONG) + 1);
-            node.setProperty(REP_NUM_PERMISSIONS, numEntries);
         }
     }
 

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionProviderImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionProviderImpl.java?rev=1575041&r1=1575040&r2=1575041&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionProviderImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionProviderImpl.java Thu Mar  6 20:59:29 2014
@@ -53,8 +53,7 @@ public class PermissionProviderImpl impl
     private ImmutableRoot immutableRoot;
 
     public PermissionProviderImpl(@Nonnull Root root, @Nonnull String workspaceName, @Nonnull Set<Principal> principals,
-                                  @Nonnull AuthorizationConfiguration acConfig,
-                                  @Nonnull PermissionEntryCache.Local cache) {
+                                  @Nonnull AuthorizationConfiguration acConfig) {
         this.root = root;
         this.workspaceName = workspaceName;
         this.acConfig = acConfig;
@@ -64,7 +63,7 @@ public class PermissionProviderImpl impl
         if (principals.contains(SystemPrincipal.INSTANCE) || isAdmin(principals)) {
             compiledPermissions = AllPermissions.getInstance();
         } else {
-            compiledPermissions = CompiledPermissionImpl.create(immutableRoot, workspaceName, principals, acConfig, cache);
+            compiledPermissions = CompiledPermissionImpl.create(immutableRoot, workspaceName, principals, acConfig);
         }
     }
 

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStore.java?rev=1575041&r1=1575040&r2=1575041&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStore.java Thu Mar  6 20:59:29 2014
@@ -19,7 +19,9 @@ package org.apache.jackrabbit.oak.securi
 import java.util.Collection;
 import java.util.Map;
 
+import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 
 /**
  * The permission store is used to store and provide access control permissions for principals. It is responsible to
@@ -27,16 +29,24 @@ import javax.annotation.Nonnull;
  */
 public interface PermissionStore {
 
-    void load(@Nonnull Collection<PermissionEntry> entries, @Nonnull String principalName, @Nonnull String path);
+    /**
+     * Loads the permission entries for the given principal and path. if the given {@code entries} is {@code null}, it
+     * will be created automatically if needed. If a {@code entries} is given, it will reuse it and the same object is
+     * returned. If no entries can be found for the given principal or path, {@code null} is returned.
+     *
+     * @param entries the permission entries or {@code null}
+     * @param principalName name of the principal
+     * @param path access controlled path.
+     * @return the given {@code entries}, a new collection or {@code null}
+     */
+    @CheckForNull
+    Collection<PermissionEntry> load(@Nullable Collection<PermissionEntry> entries, @Nonnull String principalName, @Nonnull String path);
 
     void load(@Nonnull Map<String, Collection<PermissionEntry>> entries, @Nonnull String principalName);
 
     @Nonnull
     PrincipalPermissionEntries load(@Nonnull String principalName);
 
-    boolean hasPermissionEntries(@Nonnull String principalName);
+    long getNumEntries(@Nonnull String principalName, long max);
 
-    long getNumEntries(@Nonnull String principalName);
-
-    long getModCount(@Nonnull String principalName);
 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStoreImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStoreImpl.java?rev=1575041&r1=1575040&r2=1575041&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStoreImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStoreImpl.java Thu Mar  6 20:59:29 2014
@@ -23,20 +23,26 @@ import java.util.TreeSet;
 
 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.Root;
 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.security.authorization.restriction.RestrictionProvider;
 import org.apache.jackrabbit.oak.util.TreeUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * {@code PermissionStoreImpl}...
  */
 public class PermissionStoreImpl implements PermissionStore {
 
+    /**
+     * default logger
+     */
+    private static final Logger log = LoggerFactory.getLogger(PermissionStoreImpl.class);
+
     private Tree permissionsTree;
 
     private final String workspaceName;
@@ -71,25 +77,26 @@ public class PermissionStoreImpl impleme
     }
 
     @Override
-    public void load(@Nonnull Collection<PermissionEntry> entries, @Nonnull String principalName, @Nonnull String path) {
+    @CheckForNull
+    public Collection<PermissionEntry> load(@Nullable Collection<PermissionEntry> entries, @Nonnull String principalName, @Nonnull String path) {
         Tree principalRoot = getPrincipalRoot(principalName);
-        if (principalRoot == null) {
-            return;
-        }
-        String name = PermissionUtil.getEntryName(path);
-        if (principalRoot.hasChild(name)) {
-            Tree child = principalRoot.getChild(name);
-            if (PermissionUtil.checkACLPath(child, path)) {
-                loadPermissionEntries(path, entries, child, restrictionProvider);
-            } else {
-                // check for child node
-                for (Tree node : child.getChildren()) {
-                    if (PermissionUtil.checkACLPath(node, path)) {
-                        loadPermissionEntries(path, entries, node, restrictionProvider);
+        if (principalRoot != null) {
+            String name = PermissionUtil.getEntryName(path);
+            if (principalRoot.hasChild(name)) {
+                Tree child = principalRoot.getChild(name);
+                if (PermissionUtil.checkACLPath(child, path)) {
+                    entries = loadPermissionEntries(path, entries, child, restrictionProvider);
+                } else {
+                    // check for child node
+                    for (Tree node : child.getChildren()) {
+                        if (PermissionUtil.checkACLPath(node, path)) {
+                            entries = loadPermissionEntries(path, entries, node, restrictionProvider);
+                        }
                     }
                 }
             }
         }
+        return entries == null || entries.isEmpty() ? null : entries;
     }
 
     @Override
@@ -103,39 +110,27 @@ public class PermissionStoreImpl impleme
     }
 
     @Override
-    public boolean hasPermissionEntries(@Nonnull String principalName) {
-        return getPrincipalRoot(principalName) != null;
-    }
-
-    @Override
-    public long getNumEntries(@Nonnull String principalName) {
+    public long getNumEntries(@Nonnull String principalName, long max) {
+        // we ignore the hash-collisions here
         Tree tree = getPrincipalRoot(principalName);
-        return tree == null ? 0 : PermissionUtil.getNumPermissions(tree);
-    }
-
-    @Override
-    public long getModCount(@Nonnull String principalName) {
-        Tree principalRoot = getPrincipalRoot(principalName);
-        if (principalRoot != null) {
-            PropertyState ps = principalRoot.getProperty(PermissionConstants.REP_MOD_COUNT);
-            if (ps != null) {
-                return ps.getValue(Type.LONG);
-            }
-        }
-        return 0;
+        return tree == null ? 0 : tree.getChildrenCount(max);
     }
 
     @Override
     @Nonnull
     public PrincipalPermissionEntries load(@Nonnull String principalName) {
+        long t0 = System.nanoTime();
         PrincipalPermissionEntries ret = new PrincipalPermissionEntries(principalName);
         Tree principalRoot = getPrincipalRoot(principalName);
         if (principalRoot != null) {
             for (Tree entryTree : principalRoot.getChildren()) {
                 loadPermissionEntries(entryTree, ret.getEntries(), restrictionProvider);
             }
-            PropertyState ps = principalRoot.getProperty(PermissionConstants.REP_MOD_COUNT);
-            ret.setModCount(ps == null ? -1 : ps.getValue(Type.LONG));
+        }
+        ret.setFullyLoaded(true);
+        long t1 = System.nanoTime();
+        if (log.isDebugEnabled()) {
+            log.debug(String.format("loaded %d entries in %.2fus for %s.%n", ret.getEntries().size(), (t1 - t0) / 1000.0, principalName));
         }
         return ret;
     }
@@ -158,14 +153,19 @@ public class PermissionStoreImpl impleme
         }
     }
 
-    private static void loadPermissionEntries(@Nonnull String path,
-                                              @Nonnull Collection<PermissionEntry> ret,
+    @CheckForNull
+    private static Collection<PermissionEntry> loadPermissionEntries(@Nonnull String path,
+                                              @Nullable Collection<PermissionEntry> ret,
                                               @Nonnull Tree tree,
                                               @Nonnull RestrictionProvider restrictionProvider) {
         for (Tree ace : tree.getChildren()) {
             if (ace.getName().charAt(0) != 'c') {
+                if (ret == null) {
+                    ret = new TreeSet<PermissionEntry>();
+                }
                 ret.add(new PermissionEntry(path, ace, restrictionProvider));
             }
         }
+        return ret;
     }
 }
\ No newline at end of file

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=1575041&r1=1575040&r2=1575041&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 Mar  6 20:59:29 2014
@@ -56,16 +56,6 @@ public final class PermissionUtil implem
         return String.valueOf(path.hashCode());
     }
 
-    public static long getNumPermissions(@Nonnull NodeBuilder node) {
-        PropertyState property = node.getProperty(REP_NUM_PERMISSIONS);
-        return property == null ? 0 : property.getValue(Type.LONG);
-    }
-
-    public static long getNumPermissions(@Nonnull Tree node) {
-        PropertyState property = node.getProperty(REP_NUM_PERMISSIONS);
-        return property == null ? 0 : property.getValue(Type.LONG);
-    }
-
     public static boolean checkACLPath(@Nonnull NodeBuilder node, @Nonnull String path) {
         PropertyState property = node.getProperty(REP_ACCESS_CONTROLLED_PATH);
         return property != null && path.equals(property.getValue(Type.STRING));

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PrincipalPermissionEntries.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PrincipalPermissionEntries.java?rev=1575041&r1=1575040&r2=1575041&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PrincipalPermissionEntries.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PrincipalPermissionEntries.java Thu Mar  6 20:59:29 2014
@@ -33,9 +33,9 @@ public class PrincipalPermissionEntries 
     private final String name;
 
     /**
-     * timestamp of the per-principal permission store
+     * indicating if all entries were loaded.
      */
-    private long modCount;
+    private boolean fullyLoaded;
 
     /**
      * map of permission entries, accessed by path
@@ -51,12 +51,12 @@ public class PrincipalPermissionEntries 
         return name;
     }
 
-    public long getModCount() {
-        return modCount;
+    public boolean isFullyLoaded() {
+        return fullyLoaded;
     }
 
-    public void setModCount(long modCount) {
-        this.modCount = modCount;
+    public void setFullyLoaded(boolean fullyLoaded) {
+        this.fullyLoaded = fullyLoaded;
     }
 
     @Nonnull

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/PermissionConstants.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/PermissionConstants.java?rev=1575041&r1=1575040&r2=1575041&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/PermissionConstants.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/PermissionConstants.java Thu Mar  6 20:59:29 2014
@@ -37,8 +37,6 @@ public interface PermissionConstants {
     String PERMISSIONS_STORE_PATH = '/' + JcrConstants.JCR_SYSTEM + '/' + REP_PERMISSION_STORE;
 
     String REP_ACCESS_CONTROLLED_PATH = "rep:accessControlledPath";
-    String REP_NUM_PERMISSIONS = "rep:numPermissions";
-    String REP_MOD_COUNT = "rep:modCount";
 	String REP_IS_ALLOW = "rep:isAllow";
     String REP_PRIVILEGE_BITS = "rep:privileges";
 

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHookTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHookTest.java?rev=1575041&r1=1575040&r2=1575041&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHookTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHookTest.java Thu Mar  6 20:59:29 2014
@@ -347,43 +347,4 @@ public class PermissionHookTest extends 
         principalRoot = getPrincipalRoot(EveryonePrincipal.NAME);
         assertEquals(2, cntEntries(principalRoot));
     }
-
-    @Test
-    public void testNumPermissions() throws Exception {
-
-        AccessControlManager acMgr = getAccessControlManager(root);
-        JackrabbitAccessControlList acl = AccessControlUtils.getAccessControlList(acMgr, testPath);
-        acl.addAccessControlEntry(getTestPrincipal(), privilegesFromNames(JCR_READ, REP_WRITE));
-        acMgr.setPolicy(testPath, acl);
-        root.commit();
-
-        assertEquals(1, PermissionUtil.getNumPermissions(getPrincipalRoot(testPrincipalName)));
-        assertEquals(1, PermissionUtil.getNumPermissions(getPrincipalRoot(EveryonePrincipal.NAME)));
-
-        acl = AccessControlUtils.getAccessControlList(acMgr, childPath);
-        acl.addAccessControlEntry(EveryonePrincipal.getInstance(), privilegesFromNames(JCR_READ));
-        acMgr.setPolicy(childPath, acl);
-        root.commit();
-
-        assertEquals(1, PermissionUtil.getNumPermissions(getPrincipalRoot(testPrincipalName)));
-        assertEquals(2, PermissionUtil.getNumPermissions(getPrincipalRoot(EveryonePrincipal.NAME)));
-
-        acl = AccessControlUtils.getAccessControlList(acMgr, testPath);
-        acl.removeAccessControlEntry(acl.getAccessControlEntries()[0]);
-        acMgr.setPolicy(testPath, acl);
-        root.commit();
-
-        assertEquals(0, PermissionUtil.getNumPermissions(getPrincipalRoot(testPrincipalName)));
-        assertEquals(2, PermissionUtil.getNumPermissions(getPrincipalRoot(EveryonePrincipal.NAME)));
-
-        acl = AccessControlUtils.getAccessControlList(acMgr, childPath);
-        acl.removeAccessControlEntry(acl.getAccessControlEntries()[0]);
-        acMgr.setPolicy(childPath, acl);
-        root.commit();
-
-        assertEquals(0, PermissionUtil.getNumPermissions(getPrincipalRoot(testPrincipalName)));
-        assertEquals(1, PermissionUtil.getNumPermissions(getPrincipalRoot(EveryonePrincipal.NAME)));
-    }
-
-
 }
\ No newline at end of file

Modified: jackrabbit/oak/trunk/oak-run/run_everyone_acl.sh
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-run/run_everyone_acl.sh?rev=1575041&r1=1575040&r2=1575041&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-run/run_everyone_acl.sh (original)
+++ jackrabbit/oak/trunk/oak-run/run_everyone_acl.sh Thu Mar  6 20:59:29 2014
@@ -17,7 +17,7 @@
 #
 TITLE=ConcurrentEveryoneACLTest
 BENCH="ConcurrentEveryoneACLTest"
-ADMIN="false true"
+ADMIN="false"
 RUNTIME=5
 RANDOM_USER="true"
 FIXS="Oak-Tar" # Jackrabbit"

Modified: jackrabbit/oak/trunk/oak-run/run_writeacl.sh
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-run/run_writeacl.sh?rev=1575041&r1=1575040&r2=1575041&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-run/run_writeacl.sh (original)
+++ jackrabbit/oak/trunk/oak-run/run_writeacl.sh Thu Mar  6 20:59:29 2014
@@ -18,12 +18,12 @@
 TITLE=ConcurrentWriteACLTest
 BENCH="ConcurrentWriteACLTest"
 ADMIN="true"
-RUNTIME=5
+RUNTIME=10
 RANDOM_USER="true"
 FIXS="Oak-Mongo" # Jackrabbit"
 THREADS="1,2,4,8,10,15,20,50"
 PROFILE=false
-NUM_ITEMS=1000
+NUM_ITEMS=10
 
 LOG=$TITLE"_$(date +'%Y%m%d_%H%M%S').csv"
 echo "Benchmarks: $BENCH" > $LOG

Modified: jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/benchmark/BenchmarkRunner.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/benchmark/BenchmarkRunner.java?rev=1575041&r1=1575040&r2=1575041&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/benchmark/BenchmarkRunner.java (original)
+++ jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/benchmark/BenchmarkRunner.java Thu Mar  6 20:59:29 2014
@@ -175,7 +175,7 @@ public class BenchmarkRunner {
                     itemsToRead.value(options),
                     report.value(options),
                     randomUser.value(options)),
-            new ConcurrentWriteACLTest(),
+            new ConcurrentWriteACLTest(itemsToRead.value(options)),
             new ConcurrentEveryoneACLTest(runAsAdmin.value(options), itemsToRead.value(options)),
             ReadManyTest.linear("LinearReadEmpty", 1, ReadManyTest.EMPTY),
             ReadManyTest.linear("LinearReadFiles", 1, ReadManyTest.FILES),

Modified: jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/benchmark/ConcurrentWriteACLTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/benchmark/ConcurrentWriteACLTest.java?rev=1575041&r1=1575040&r2=1575041&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/benchmark/ConcurrentWriteACLTest.java (original)
+++ jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/benchmark/ConcurrentWriteACLTest.java Thu Mar  6 20:59:29 2014
@@ -19,6 +19,7 @@ package org.apache.jackrabbit.oak.benchm
 
 import java.util.Random;
 
+import javax.jcr.InvalidItemStateException;
 import javax.jcr.Node;
 import javax.jcr.Session;
 import javax.jcr.security.AccessControlEntry;
@@ -42,7 +43,10 @@ public class ConcurrentWriteACLTest exte
 
     protected static final String ROOT_NODE_NAME = "test" + TEST_ID;
 
-    protected ConcurrentWriteACLTest() {
+    private int numItems;
+
+    public ConcurrentWriteACLTest(int numItems) {
+        this.numItems = numItems;
     }
 
     @Override
@@ -73,28 +77,31 @@ public class ConcurrentWriteACLTest exte
         Session session = null;
         try {
             session = loginWriter();
-            for (int i=0; i<10; i++) {
+            for (int i=0; i<numItems; i++) {
                 session.refresh(false);
                 int a = random.nextInt(NODE_COUNT);
                 int b = random.nextInt(NODE_COUNT);
                 String path = "/" + ROOT_NODE_NAME + "/node" + a + "/node" + b;
                 AccessControlManager acMgr = session.getAccessControlManager();
                 JackrabbitAccessControlList acl = AccessControlUtils.getAccessControlList(session, path);
-                Privilege[] privileges = new Privilege[] {
-                        acMgr.privilegeFromName(Privilege.JCR_READ),
-                        acMgr.privilegeFromName(Privilege.JCR_READ_ACCESS_CONTROL)
-                };
-                if (acl.addAccessControlEntry(EveryonePrincipal.getInstance(), privileges)) {
+                if (acl.isEmpty()) {
+                    Privilege[] privileges = new Privilege[] {
+                            acMgr.privilegeFromName(Privilege.JCR_READ),
+                            acMgr.privilegeFromName(Privilege.JCR_READ_ACCESS_CONTROL)
+                    };
+                    if (acl.addAccessControlEntry(EveryonePrincipal.getInstance(), privileges)) {
+                        acMgr.setPolicy(path, acl);
+                    }
+                } else {
+                    for (AccessControlEntry ace: acl.getAccessControlEntries()) {
+                        acl.removeAccessControlEntry(ace);
+                    }
                     acMgr.setPolicy(path, acl);
                 }
                 session.save();
-                for (AccessControlEntry ace: acl.getAccessControlEntries()) {
-                    acl.removeAccessControlEntry(ace);
-                }
-                acMgr.setPolicy(path, acl);
-                session.save();
             }
-        } catch (Exception e) {
+        } catch (InvalidItemStateException e) {
+            System.out.printf("error: %s%n", e);
             // ignore
         } finally {
             if (session != null) {



Mime
View raw message