Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCache.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCache.java (revision ) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCache.java (revision ) @@ -0,0 +1,29 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.jackrabbit.oak.security.authorization.permission; + +import java.util.Collection; +import javax.annotation.Nonnull; + +import org.apache.jackrabbit.oak.api.Tree; + +interface PermissionCache { + + Collection getEntries(@Nonnull String path); + + Collection getEntries(@Nonnull Tree accessControlledTree); +} \ No newline at end of file Index: oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/package-info.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/package-info.java (revision 1830726) +++ oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/package-info.java (revision ) @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -@Version("4.0.0") +@Version("4.1.0") package org.apache.jackrabbit.oak.spi.security.authorization.permission; import org.osgi.annotation.versioning.Version; Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheBuilder.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheBuilder.java (revision ) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheBuilder.java (revision ) @@ -0,0 +1,207 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +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 javax.annotation.Nonnull; + +import org.apache.jackrabbit.oak.api.Tree; +import org.apache.jackrabbit.oak.commons.LongUtils; +import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants; + +import static com.google.common.base.Preconditions.checkState; + +final class PermissionCacheBuilder { + + private static final long MAX_PATHS_SIZE = 10; + + private final PermissionStore store; + private final PermissionEntryCache peCache; + + private Set existingNames; + private boolean usePathEntryMap; + + private boolean initialized = false; + + PermissionCacheBuilder(@Nonnull PermissionStore store) { + this.store = store; + this.peCache = new PermissionEntryCache(); + } + + Set init(@Nonnull Set principalNames, long maxSize) { + existingNames = new HashSet<>(); + long cnt = 0; + for (String name : principalNames) { + NumEntries ne = store.getNumEntries(name, maxSize); + long n = ne.size; + /* + if getNumEntries (n) returns a number bigger than 0, we + remember this principal name int the 'existingNames' set + */ + if (n > 0) { + existingNames.add(name); + if (n <= MAX_PATHS_SIZE) { + peCache.getFullyLoadedEntries(store, name); + } else { + long expectedSize = (ne.isExact) ? n : Long.MAX_VALUE; + peCache.init(name, expectedSize); + } + } + /* + Estimate the total number of access controlled paths (cnt) defined + for the given set of principals in order to be able to determine if + the pathEntryMap should be loaded upfront. + Note however that cache.getNumEntries (n) may return Long.MAX_VALUE + if the underlying implementation does not know the exact value, and + the child node count is higher than maxSize (see OAK-2465). + */ + if (cnt < Long.MAX_VALUE) { + if (Long.MAX_VALUE == n) { + cnt = Long.MAX_VALUE; + } else { + cnt = LongUtils.safeAdd(cnt, n); + } + } + } + + usePathEntryMap = (cnt > 0 && cnt < maxSize); + initialized = true; + return existingNames; + } + + PermissionCache build() { + checkState(initialized); + if (existingNames.isEmpty()) { + return EmptyCache.INSTANCE; + } + if (usePathEntryMap) { + // the total number of access controlled paths is smaller that maxSize, + // so we can load all permission entries for all principals having + // any entries right away into the pathEntryMap + Map> pathEntryMap = new HashMap<>(); + for (String name : existingNames) { + PrincipalPermissionEntries ppe = peCache.getFullyLoadedEntries(store, name); + for (Map.Entry> e : ppe.getEntries().entrySet()) { + String path = e.getKey(); + Collection pathEntries = pathEntryMap.get(path); + if (pathEntries == null) { + pathEntries = new TreeSet(e.getValue()); + pathEntryMap.put(path, pathEntries); + } else { + pathEntries.addAll(e.getValue()); + } + } + } + if (pathEntryMap.isEmpty()) { + return EmptyCache.INSTANCE; + } else { + return new PathEntryMapCache(pathEntryMap); + } + } else { + return new DefaultPermissionCache(store, peCache, existingNames); + } + + } + + //------------------------------------< PermissionCache Implementations >--- + /** + * Default implementation of {@code PermissionCache} wrapping the + * {@code PermissionEntryCache}, which was previously hold as shared field + * inside the {@code PermissionEntryProviderImpl} + */ + private static final class DefaultPermissionCache implements PermissionCache { + private final PermissionStore store; + private final PermissionEntryCache cache; + private final Set existingNames; + + DefaultPermissionCache(@Nonnull PermissionStore store, @Nonnull PermissionEntryCache cache, Set existingNames) { + this.store = store; + this.cache = cache; + this.existingNames = existingNames; + } + + @Override + public Collection getEntries(@Nonnull String path) { + Collection ret = new TreeSet(); + for (String name : existingNames) { + cache.load(store, ret, name, path); + } + return ret; + } + + @Override + public Collection getEntries(@Nonnull Tree accessControlledTree) { + return (accessControlledTree.hasChild(AccessControlConstants.REP_POLICY)) ? + getEntries(accessControlledTree.getPath()) : + Collections.emptyList(); + } + } + + /** + * Fixed size implementation of {@code PermissionCache} that holds a map + * containing all existing entries that in this case have been read eagerly + * upfront. This implementation replaces the optional {@code pathEntryMap} + * previously present inside the the {@code PermissionEntryProviderImpl}. + */ + private static final class PathEntryMapCache implements PermissionCache { + private final Map> pathEntryMap; + + PathEntryMapCache(Map> pathEntryMap) { + this.pathEntryMap = pathEntryMap; + } + + @Override + public Collection getEntries(@Nonnull String path) { + Collection entries = pathEntryMap.get(path); + return (entries != null) ? entries : Collections.emptyList(); + } + + @Override + public Collection getEntries(@Nonnull Tree accessControlledTree) { + Collection entries = pathEntryMap.get(accessControlledTree.getPath()); + return (entries != null) ? entries : Collections.emptyList(); + } + } + + /** + * Empty implementation of {@code PermissionCache} for those cases where + * for a given (possibly empty) set of principals no permission entries are + * present. + */ + private static final class EmptyCache implements PermissionCache { + + private static final PermissionCache INSTANCE = new EmptyCache(); + + @Override + public Collection getEntries(@Nonnull String path) { + return Collections.emptyList(); + } + + @Override + public Collection getEntries(@Nonnull Tree accessControlledTree) { + return Collections.emptyList(); + } + } + +} \ No newline at end of file Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/NumEntries.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/NumEntries.java (revision ) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/NumEntries.java (revision ) @@ -0,0 +1,58 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.jackrabbit.oak.security.authorization.permission; + +import com.google.common.base.Objects; + +final class NumEntries { + + static final NumEntries ZERO = new NumEntries(0, true); + + final long size; + final boolean isExact; + + private NumEntries(long size, boolean isExact) { + this.size = size; + this.isExact = isExact; + } + + @Override + public int hashCode() { + return Objects.hashCode(size, isExact); + } + + @Override + public boolean equals(Object obj) { + if (obj == this) { + return true; + } + if (obj instanceof NumEntries) { + NumEntries other = (NumEntries) obj; + return size == other.size && isExact == other.isExact; + } else { + return false; + } + } + + static NumEntries valueOf(long size, boolean isExact) { + if (size == 0 && isExact) { + return ZERO; + } else { + return new NumEntries(size, isExact); + } + } +} \ No newline at end of file Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStoreImpl.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStoreImpl.java (revision 1830726) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStoreImpl.java (revision ) @@ -23,7 +23,6 @@ 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; @@ -52,7 +51,7 @@ private final RestrictionProvider restrictionProvider; - private final Map principalTreeMap = new HashMap(); + private final Map principalTreeMap = new HashMap<>(); private Tree permissionsTree; private PrivilegeBits allBits; @@ -77,19 +76,20 @@ //----------------------------------------------------< PermissionStore >--- @Override @CheckForNull - public Collection load(@Nullable Collection entries, @Nonnull String principalName, @Nonnull String path) { + public Collection load(@Nonnull String principalName, @Nonnull String path) { Tree principalRoot = getPrincipalRoot(principalName); + Collection entries = null; 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); + entries = loadPermissionEntries(path, child); } else { // check for child node for (Tree node : child.getChildren()) { if (PermissionUtil.checkACLPath(node, path)) { - entries = loadPermissionEntries(path, entries, node); + entries = loadPermissionEntries(path, node); } } } @@ -98,12 +98,22 @@ return entries == null || entries.isEmpty() ? null : entries; } + @Nonnull @Override - public long getNumEntries(@Nonnull String principalName, long max) { - // we ignore the hash-collisions here + public NumEntries getNumEntries(@Nonnull String principalName, long max) { Tree tree = getPrincipalRoot(principalName); - return tree == null ? 0 : tree.getChildrenCount(max); + if (tree == null) { + return NumEntries.ZERO; + } else { + // if rep:numPermissions is present it contains the exact number of + // access controlled nodes for the given principal name. + // if this property is missing (backward compat) we use the old + // mechanism and use child-cnt with a max value to get a rough idea + // about the magnitude (note: this approximation ignores the hash-collisions) + long l = TreeUtil.getLong(tree, REP_NUM_PERMISSIONS, -1); + return (l >= 0) ? NumEntries.valueOf(l, true) : NumEntries.valueOf(tree.getChildrenCount(max), false); - } + } + } @Override @Nonnull @@ -113,7 +123,7 @@ Tree principalRoot = getPrincipalRoot(principalName); if (principalRoot != null) { for (Tree entryTree : principalRoot.getChildren()) { - loadPermissionEntries(entryTree, ret.getEntries()); + loadPermissionEntries(entryTree, ret); } } ret.setFullyLoaded(true); @@ -140,17 +150,17 @@ } private void loadPermissionEntries(@Nonnull Tree tree, - @Nonnull Map> pathEntryMap) { + @Nonnull PrincipalPermissionEntries principalPermissionEntries) { String path = TreeUtil.getString(tree, PermissionConstants.REP_ACCESS_CONTROLLED_PATH); if (path != null) { - Collection entries = pathEntryMap.get(path); + Collection entries = principalPermissionEntries.getEntriesByPath(path); if (entries == null) { - entries = new TreeSet(); - pathEntryMap.put(path, entries); + entries = new TreeSet<>(); + principalPermissionEntries.putEntriesByPath(path, entries); } for (Tree child : tree.getChildren()) { if (child.getName().charAt(0) == 'c') { - loadPermissionEntries(child, pathEntryMap); + loadPermissionEntries(child, principalPermissionEntries); } else { entries.add(createPermissionEntry(path, child)); } @@ -162,13 +172,10 @@ @CheckForNull private Collection loadPermissionEntries(@Nonnull String path, - @Nullable Collection ret, @Nonnull Tree tree) { + Collection ret = new TreeSet<>(); for (Tree ace : tree.getChildren()) { if (ace.getName().charAt(0) != 'c') { - if (ret == null) { - ret = new TreeSet(); - } ret.add(createPermissionEntry(path, ace)); } } Index: oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/PermissionConstants.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/PermissionConstants.java (revision 1830726) +++ oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/PermissionConstants.java (revision ) @@ -39,6 +39,7 @@ String REP_ACCESS_CONTROLLED_PATH = "rep:accessControlledPath"; String REP_IS_ALLOW = "rep:isAllow"; String REP_PRIVILEGE_BITS = "rep:privileges"; + String REP_NUM_PERMISSIONS = "rep:numPermissions"; Set PERMISSION_NODETYPE_NAMES = ImmutableSet.of(NT_REP_PERMISSIONS, NT_REP_PERMISSION_STORE); Set PERMISSION_NODE_NAMES = ImmutableSet.of(REP_PERMISSION_STORE); Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java (revision 1830726) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java (revision ) @@ -17,6 +17,7 @@ package org.apache.jackrabbit.oak.security.authorization.permission; import java.security.Principal; +import java.security.acl.Group; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -49,7 +50,6 @@ import org.apache.jackrabbit.oak.spi.security.authorization.permission.RepositoryPermission; import org.apache.jackrabbit.oak.spi.security.authorization.permission.TreePermission; import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionProvider; -import org.apache.jackrabbit.oak.spi.security.principal.GroupPrincipals; 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; @@ -103,16 +103,15 @@ Set userNames = new HashSet(principals.size()); Set groupNames = new HashSet(principals.size()); for (Principal principal : principals) { - if (GroupPrincipals.isGroup(principal)) { + if (principal instanceof Group) { groupNames.add(principal.getName()); } else { userNames.add(principal.getName()); } } - PermissionEntryCache cache = new PermissionEntryCache(); - userStore = new PermissionEntryProviderImpl(store, cache, userNames, options); - groupStore = new PermissionEntryProviderImpl(store, cache, groupNames, options); + userStore = new PermissionEntryProviderImpl(store, userNames, options); + groupStore = new PermissionEntryProviderImpl(store, groupNames, options); typeProvider = new TreeTypeProvider(ctx); } Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/MountPermissionProvider.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/MountPermissionProvider.java (revision 1830726) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/MountPermissionProvider.java (revision ) @@ -25,7 +25,6 @@ import javax.annotation.CheckForNull; import javax.annotation.Nonnull; -import javax.annotation.Nullable; import org.apache.jackrabbit.oak.api.Root; import org.apache.jackrabbit.oak.commons.LongUtils; @@ -81,10 +80,10 @@ @CheckForNull @Override - public Collection load(@Nullable Collection entries, @Nonnull String principalName, + public Collection load(@Nonnull String principalName, - @Nonnull String path) { + @Nonnull String path) { for (PermissionStoreImpl store : stores) { - Collection col = store.load(null, principalName, path); + Collection col = store.load(principalName, path); if (col != null && !col.isEmpty()) { return col; } @@ -97,22 +96,28 @@ public PrincipalPermissionEntries load(@Nonnull String principalName) { PrincipalPermissionEntries ppe = new PrincipalPermissionEntries(); for (PermissionStoreImpl store : stores) { - ppe.getEntries().putAll(store.load(principalName).getEntries()); + ppe.putAllEntries(store.load(principalName).getEntries()); } ppe.setFullyLoaded(true); return ppe; } + @Nonnull @Override - public long getNumEntries(@Nonnull String principalName, long max) { + public NumEntries getNumEntries(@Nonnull String principalName, long max) { long num = 0; + boolean isExact = true; for (PermissionStoreImpl store : stores) { - num = LongUtils.safeAdd(num, store.getNumEntries(principalName, max)); - if (num >= max) { + NumEntries ne = store.getNumEntries(principalName, max); + num = LongUtils.safeAdd(num, ne.size); + if (!ne.isExact) { + isExact = false; + } + if (num >= max && !isExact) { break; } } - return num; + return NumEntries.valueOf(num, isExact); } @Override Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PrincipalPermissionEntries.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PrincipalPermissionEntries.java (revision 1830726) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PrincipalPermissionEntries.java (revision ) @@ -19,6 +19,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.Map; +import javax.annotation.CheckForNull; import javax.annotation.Nonnull; /** @@ -26,6 +27,8 @@ */ class PrincipalPermissionEntries { + private final long expectedSize; + /** * indicating if all entries were loaded. */ @@ -34,11 +37,16 @@ /** * map of permission entries, accessed by path */ - private Map> entries = new HashMap>(); + private Map> entries = new HashMap<>(); PrincipalPermissionEntries() { + this(Long.MAX_VALUE); } + PrincipalPermissionEntries(long expectedSize) { + this.expectedSize = expectedSize; + } + long getSize() { return entries.size(); } @@ -54,5 +62,22 @@ @Nonnull Map> getEntries() { return entries; + } + + @CheckForNull + Collection getEntriesByPath(@Nonnull String path) { + return entries.get(path); + } + + void putEntriesByPath(@Nonnull String path, @Nonnull Collection pathEntries) { + entries.put(path, pathEntries); + if (entries.size() >= expectedSize) { + setFullyLoaded(true); + } + } + + void putAllEntries(@Nonnull Map> allEntries) { + entries.putAll(allEntries); + setFullyLoaded(true); } } \ No newline at end of file Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStoreEditor.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStoreEditor.java (revision 1830726) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStoreEditor.java (revision ) @@ -92,7 +92,7 @@ new AcEntry(ace, accessControlledPath, index, isAllow, privilegeBits, restrictions); List list = entries.get(entry.principalName); if (list == null) { - list = new ArrayList(); + list = new ArrayList<>(); entries.put(entry.principalName, list); } list.add(entry); @@ -117,6 +117,7 @@ for (String principalName : entries.keySet()) { if (permissionRoot.hasChildNode(principalName)) { NodeBuilder principalRoot = permissionRoot.getChildNode(principalName); + boolean removed = false; // find the ACL node that for this path and principal NodeBuilder parent = principalRoot.getChildNode(nodeName); @@ -139,11 +140,13 @@ newParent.setChildNode(childName, child.getNodeState()); } } + if (newParent != null) { - // replace the 'parent', which needs to be removed + // replace the 'parent', which got removed principalRoot.setChildNode(nodeName, newParent.getNodeState()); + removed = true; } else { - parent.remove(); + removed = parent.remove(); } } else { // check if any of the child nodes match @@ -153,10 +156,13 @@ } NodeBuilder child = parent.getChildNode(childName); if (PermissionUtil.checkACLPath(child, accessControlledPath)) { - child.remove(); + removed = child.remove(); } } } + if (removed) { + updateNumEntries(principalRoot, -1); + } } else { log.error("Unable to remove permission entry {}: Principal root missing.", this); } @@ -210,8 +216,12 @@ parent.setProperty(REP_ACCESS_CONTROLLED_PATH, accessControlledPath); } updateEntries(parent, entry.getValue()); + + if (parent.isNew()) { + updateNumEntries(principalRoot, +1); - } - } + } + } + } private void updateEntries(NodeBuilder parent, List list) { // remove old entries @@ -225,6 +235,17 @@ } } + private static void updateNumEntries(@Nonnull NodeBuilder principalRoot, int cnt) { + PropertyState ps = principalRoot.getProperty(REP_NUM_PERMISSIONS); + long numEntries = ((ps == null) ? 0 : ps.getValue(Type.LONG)) + cnt; + if (ps == null && !principalRoot.isNew() || numEntries < 0) { + // existing principal root that doesn't have the rep:numEntries set + // or numEntries turned negative + return; + } + principalRoot.setProperty(REP_NUM_PERMISSIONS, numEntries, Type.LONG); + } + private final class JcrAllAcEntry extends AcEntry { private JcrAllAcEntry(@Nonnull NodeState node, @@ -251,7 +272,7 @@ private final int index; private int hashCode = -1; - private AcEntry(@Nonnull NodeState node, @Nonnull String accessControlledPath, int index, + AcEntry(@Nonnull NodeState node, @Nonnull String accessControlledPath, int index, boolean isAllow, @Nonnull PrivilegeBits privilegeBits, @Nonnull Set restrictions) { this.accessControlledPath = accessControlledPath; Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImpl.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImpl.java (revision 1830726) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImpl.java (revision ) @@ -18,20 +18,14 @@ import java.util.Collection; import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; import java.util.Iterator; -import java.util.Map; import java.util.Set; -import java.util.TreeSet; import javax.annotation.Nonnull; import com.google.common.base.Strings; import org.apache.jackrabbit.commons.iterator.AbstractLazyIterator; import org.apache.jackrabbit.oak.api.Tree; -import org.apache.jackrabbit.oak.commons.LongUtils; import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters; -import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants; class PermissionEntryProviderImpl implements PermissionEntryProvider { @@ -45,6 +39,10 @@ */ private final Set principalNames; + private final PermissionStore store; + + private final long maxSize; + /** * The set of principal names for which the store contains any permission * entries. This set is equals or just a subset of the {@code principalNames} @@ -52,70 +50,26 @@ * this set is empty and thus no permission entries exist for the specified * set of principal. */ - private final Set existingNames = new HashSet(); + private Set existingNames; - private final PermissionStore store; + private PermissionCache permissionCache; - private final PermissionEntryCache cache; - - private final long maxSize; - - private Map> pathEntryMap; - - PermissionEntryProviderImpl(@Nonnull PermissionStore store, @Nonnull PermissionEntryCache cache, - @Nonnull Set principalNames, @Nonnull ConfigurationParameters options) { + PermissionEntryProviderImpl(@Nonnull PermissionStore store, @Nonnull Set principalNames, @Nonnull ConfigurationParameters options) { this.store = store; - this.cache = cache; this.principalNames = Collections.unmodifiableSet(principalNames); this.maxSize = options.getConfigValue(EAGER_CACHE_SIZE_PARAM, DEFAULT_SIZE); init(); } private void init() { - long cnt = 0; - existingNames.clear(); - for (String name : principalNames) { - long n = store.getNumEntries(name, maxSize); - /* - if cache.getNumEntries (n) returns a number bigger than 0, we - remember this principal name int the 'existingNames' set - */ - if (n > 0) { - existingNames.add(name); + PermissionCacheBuilder builder = new PermissionCacheBuilder(store); + existingNames = builder.init(principalNames, maxSize); + permissionCache = builder.build(); - } + } - /* - Calculate the total number of permission entries (cnt) defined for the - given set of principals in order to be able to determine if the cache - should be loaded upfront. - Note however that cache.getNumEntries (n) may return Long.MAX_VALUE - if the underlying implementation does not know the exact value, and - the child node count is higher than maxSize (see OAK-2465). - */ - if (cnt < Long.MAX_VALUE) { - if (Long.MAX_VALUE == n) { - cnt = Long.MAX_VALUE; - } else { - cnt = LongUtils.safeAdd(cnt, n); - } - } - } - if (cnt > 0 && cnt < maxSize) { - // the total number of entries is smaller that maxSize, so we can - // cache all entries for all principals having any entries right away - pathEntryMap = new HashMap>(); - for (String name : existingNames) { - cache.load(store, pathEntryMap, name); - } - } else { - pathEntryMap = null; - } - } - //--------------------------------------------< PermissionEntryProvider >--- @Override public void flush() { - cache.flush(principalNames); init(); } @@ -132,38 +86,13 @@ @Override @Nonnull public Collection getEntries(@Nonnull Tree accessControlledTree) { - if (existingNames.isEmpty()) { - return Collections.emptyList(); - } else if (pathEntryMap != null) { - Collection entries = pathEntryMap.get(accessControlledTree.getPath()); - return (entries != null) ? entries : Collections.emptyList(); - } else { - return (accessControlledTree.hasChild(AccessControlConstants.REP_POLICY)) ? - loadEntries(accessControlledTree.getPath()) : - Collections.emptyList(); + return permissionCache.getEntries(accessControlledTree); - } + } - } //------------------------------------------------------------< private >--- @Nonnull private Collection getEntries(@Nonnull String path) { - if (existingNames.isEmpty()) { - return Collections.emptyList(); - } else if (pathEntryMap != null) { - Collection entries = pathEntryMap.get(path); - return (entries != null) ? entries : Collections.emptyList(); - } else { - return loadEntries(path); - } - } - - @Nonnull - private Collection loadEntries(@Nonnull String path) { - Collection ret = new TreeSet(); - for (String name : existingNames) { - cache.load(store, ret, name, path); - } - return ret; + return permissionCache.getEntries(path); } private final class EntryIterator extends AbstractLazyIterator { \ No newline at end of file Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStore.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStore.java (revision 1830726) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStore.java (revision ) @@ -20,7 +20,6 @@ import javax.annotation.CheckForNull; import javax.annotation.Nonnull; -import javax.annotation.Nullable; import org.apache.jackrabbit.oak.api.Root; @@ -37,18 +36,18 @@ * 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 load(@Nullable Collection entries, @Nonnull String principalName, @Nonnull String path); + Collection load(@Nonnull String principalName, @Nonnull String path); @Nonnull PrincipalPermissionEntries load(@Nonnull String principalName); - long getNumEntries(@Nonnull String principalName, long max); + @Nonnull + NumEntries getNumEntries(@Nonnull String principalName, long max); void flush(@Nonnull Root root); Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCache.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCache.java (revision 1830726) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCache.java (revision ) @@ -20,82 +20,66 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; -import java.util.Set; -import java.util.TreeSet; import javax.annotation.Nonnull; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + /** * {@code PermissionEntryCache} caches the permission entries of principals. * 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: - * - 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 - * fallback to the direct store.load() methods when providing the entries. if those principals with many entries - * are used often, they might get elected to live in the global cache; memory permitting. */ class PermissionEntryCache { - private final Map entries = new HashMap(); + private static final Logger log = LoggerFactory.getLogger(PermissionEntryCache.class); + private final Map entries = new HashMap<>(); + @Nonnull - PrincipalPermissionEntries getEntries(@Nonnull PermissionStore store, + PrincipalPermissionEntries getFullyLoadedEntries(@Nonnull PermissionStore store, - @Nonnull String principalName) { + @Nonnull String principalName) { PrincipalPermissionEntries ppe = entries.get(principalName); - if (ppe == null) { + if (ppe == null || !ppe.isFullyLoaded()) { ppe = store.load(principalName); entries.put(principalName, ppe); - } else { - if (!ppe.isFullyLoaded()) { - ppe = store.load(principalName); - entries.put(principalName, ppe); - } + } - } return ppe; } - void load(@Nonnull PermissionStore store, - @Nonnull Map> pathEntryMap, - @Nonnull String principalName) { - // todo: conditionally load entries if too many - PrincipalPermissionEntries ppe = getEntries(store, principalName); - for (Map.Entry> e: ppe.getEntries().entrySet()) { - Collection pathEntries = pathEntryMap.get(e.getKey()); - if (pathEntries == null) { - pathEntries = new TreeSet(e.getValue()); - pathEntryMap.put(e.getKey(), pathEntries); - } else { - pathEntries.addAll(e.getValue()); + void init(@Nonnull String principalName, long expectedSize) { + if (!entries.containsKey(principalName)) { + entries.put(principalName, new PrincipalPermissionEntries(expectedSize)); - } - } + } + } - } void load(@Nonnull PermissionStore store, @Nonnull Collection ret, @Nonnull String principalName, @Nonnull String path) { + if (entries.containsKey(principalName)) { - PrincipalPermissionEntries ppe = entries.get(principalName); + PrincipalPermissionEntries ppe = entries.get(principalName); - if (ppe == null) { - ppe = new PrincipalPermissionEntries(); - entries.put(principalName, ppe); + Collection pes = ppe.getEntriesByPath(path); + if (ppe.isFullyLoaded() || pes != null) { + // no need to read from store + if (pes != null) { + ret.addAll(pes); - } + } - Collection pes = ppe.getEntries().get(path); + } else { + // read entries for path from store + pes = store.load(principalName, path); - if (pes == null) { + if (pes == null) { - pes = store.load(null, principalName, path); - if (pes == null) { - pes = Collections.emptySet(); + // nothing to add to the result collection 'ret'. + // nevertheless, remember the absence of any permission entries + // in the cache to avoid reading from store again. + ppe.putEntriesByPath(path, Collections.emptySet()); - } else { + } else { + ppe.putEntriesByPath(path, pes); - ret.addAll(pes); - } + ret.addAll(pes); + } - ppe.getEntries().put(path, pes); + } } else { - ret.addAll(pes); + log.error("Failed to load entries for principal '%s' at path %s", principalName, path); } - } - - void flush(@Nonnull Set principalNames) { - entries.keySet().removeAll(principalNames); } } \ No newline at end of file