I created https://issues.apache.org/jira/browse/OAK-1311 and start working on it asap (==now) regards, toby
On Wed, Jan 8, 2014 at 10:54 AM, Tobias Bocanegra <[email protected]> wrote: > Hi, > > also, I think you removed some optimizations we did for the permission > evaluation (which are not directly related to the global cache). I'll > revert your changes and "disabled" the global cache instead. > > regards, toby > > On Wed, Jan 8, 2014 at 10:44 AM, Angela Schreiber <[email protected]> wrote: >> hi jukka >> >> maybe i missed the corresponding discussion on the list as i busy with >> other >> stuff lately... but the changes below don't 'disable' the cache as you >> claim >> in the commit message but rather remove it altogether. >> >> did you discuss this with tobi prior to the modifications? i think he >> knows >> best would could go wrong here and how to fix it. >> i would have appreciated if you would really just have disabled it in >> the evaluation. >> >> in case this is not yet done: can you please create an issue to get the >> cache fixed instead of just removing it? >> >> thanks >> angela >> >> On 07/01/14 22:51, "[email protected]" <[email protected]> wrote: >> >>>Author: jukka >>>Date: Tue Jan 7 21:51:07 2014 >>>New Revision: 1556370 >>> >>>URL: http://svn.apache.org/r1556370 >>>Log: >>>OAK-1247: Non-deterministic access control test failures >>> >>>Disable the PermissionEntryCache to make the test suite deterministic >>> >>>Modified: >>> >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/AuthorizationConfigurationImpl.java >>> >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/permission/CompiledPermissionImpl.java >>> >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/permission/PermissionEntryProvider.java >>> >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/permission/PermissionEntryProviderImpl.java >>> >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/permission/PermissionHook.java >>> >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/permission/PermissionProviderImpl.java >>> jackrabbit/oak/trunk/oak-jcr/pom.xml >>> >>>Modified: >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/AuthorizationConfigurationImpl.java >>>URL: >>>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/o >>>rg/apache/jackrabbit/oak/security/authorization/AuthorizationConfiguration >>>Impl.java?rev=1556370&r1=1556369&r2=1556370&view=diff >>>========================================================================== >>>==== >>>--- >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/AuthorizationConfigurationImpl.java (original) >>>+++ >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/AuthorizationConfigurationImpl.java Tue Jan 7 >>>21:51:07 2014 >>>@@ -32,7 +32,6 @@ import org.apache.jackrabbit.oak.plugins >>> import >>>org.apache.jackrabbit.oak.security.authorization.accesscontrol.AccessContr >>>olImporter; >>> import >>>org.apache.jackrabbit.oak.security.authorization.accesscontrol.AccessContr >>>olManagerImpl; >>> import >>>org.apache.jackrabbit.oak.security.authorization.accesscontrol.AccessContr >>>olValidatorProvider; >>>-import >>>org.apache.jackrabbit.oak.security.authorization.permission.PermissionEntr >>>yCache; >>> import >>>org.apache.jackrabbit.oak.security.authorization.permission.PermissionHook >>>; >>> import >>>org.apache.jackrabbit.oak.security.authorization.permission.PermissionProv >>>iderImpl; >>> import >>>org.apache.jackrabbit.oak.security.authorization.permission.PermissionStor >>>eValidatorProvider; >>>@@ -61,8 +60,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(); >>> } >>>@@ -94,7 +91,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 >>>@@ -131,7 +128,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/secu >>>rity/authorization/permission/CompiledPermissionImpl.java >>>URL: >>>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/o >>>rg/apache/jackrabbit/oak/security/authorization/permission/CompiledPermiss >>>ionImpl.java?rev=1556370&r1=1556369&r2=1556370&view=diff >>>========================================================================== >>>==== >>>--- >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/permission/CompiledPermissionImpl.java (original) >>>+++ >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/permission/CompiledPermissionImpl.java Tue Jan 7 >>>21:51:07 2014 >>>@@ -86,8 +86,7 @@ final class CompiledPermissionImpl imple >>> >>> private PrivilegeBitsProvider bitsProvider; >>> >>>- 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 Set<String> readPaths) { >>>@@ -109,20 +108,19 @@ final class CompiledPermissionImpl imple >>> } >>> } >>> >>>- userStore = new PermissionEntryProviderImpl(store, cache, >>>userNames); >>>- groupStore = new PermissionEntryProviderImpl(store, cache, >>>groupNames); >>>+ userStore = new PermissionEntryProviderImpl(store, userNames); >>>+ groupStore = new PermissionEntryProviderImpl(store, groupNames); >>> } >>> >>> 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 { >>> Set<String> readPaths = >>>acConfig.getParameters().getConfigValue(PARAM_READ_PATHS, >>>DEFAULT_READ_PATHS); >>>- return new CompiledPermissionImpl(cache, principals, root, >>>workspaceName, acConfig.getRestrictionProvider(), readPaths); >>>+ return new CompiledPermissionImpl(principals, root, >>>workspaceName, acConfig.getRestrictionProvider(), readPaths); >>> } >>> } >>> >>>@@ -132,8 +130,6 @@ final class CompiledPermissionImpl imple >>> this.root = root; >>> this.bitsProvider = new PrivilegeBitsProvider(root); >>> store.flush(root); >>>- userStore.flush(); >>>- groupStore.flush(); >>> } >>> >>> @Override >>> >>>Modified: >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/permission/PermissionEntryProvider.java >>>URL: >>>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/o >>>rg/apache/jackrabbit/oak/security/authorization/permission/PermissionEntry >>>Provider.java?rev=1556370&r1=1556369&r2=1556370&view=diff >>>========================================================================== >>>==== >>>--- >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/permission/PermissionEntryProvider.java (original) >>>+++ >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/permission/PermissionEntryProvider.java Tue Jan 7 >>>21:51:07 2014 >>>@@ -38,5 +38,4 @@ interface PermissionEntryProvider { >>> @Nonnull >>> Collection<PermissionEntry> getEntries(@Nonnull String >>>accessControlledPath); >>> >>>- void flush(); >>>-} >>>\ No newline at end of file >>>+} >>> >>>Modified: >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/permission/PermissionEntryProviderImpl.java >>>URL: >>>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/o >>>rg/apache/jackrabbit/oak/security/authorization/permission/PermissionEntry >>>ProviderImpl.java?rev=1556370&r1=1556369&r2=1556370&view=diff >>>========================================================================== >>>==== >>>--- >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/permission/PermissionEntryProviderImpl.java (original) >>>+++ >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/permission/PermissionEntryProviderImpl.java Tue Jan 7 >>>21:51:07 2014 >>>@@ -18,10 +18,7 @@ package org.apache.jackrabbit.oak.securi >>> >>> 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; >>> >>>@@ -39,98 +36,37 @@ import com.google.common.collect.Iterato >>> */ >>> class PermissionEntryProviderImpl implements PermissionEntryProvider { >>> >>>- private static final long MAX_SIZE = 250; // TODO define size or >>>make configurable >>>- >>> private final Set<String> principalNames; >>> >>>- private final Set<String> existingNames = new HashSet<String>(); >>>- >>>- private Map<String, Collection<PermissionEntry>> pathEntryMap; >>>- >>> private final PermissionStore store; >>> >>>- private final PermissionEntryCache.Local cache; >>>- >>>- PermissionEntryProviderImpl(@Nonnull PermissionStore store, @Nonnull >>>PermissionEntryCache.Local cache, >>>+ PermissionEntryProviderImpl(@Nonnull PermissionStore store, >>> @Nonnull Set<String> principalNames) { >>> this.store = store; >>>- this.cache = cache; >>> this.principalNames = >>>Collections.unmodifiableSet(principalNames); >>>- init(); >>>- } >>>- >>>- private void init() { >>>- long cnt = 0; >>>- existingNames.clear(); >>>- for (String name: principalNames) { >>>- if (cnt > MAX_SIZE) { >>>- if (cache.hasEntries(store, name)) { >>>- existingNames.add(name); >>>- } >>>- } else { >>>- long n = cache.getNumEntries(store, name); >>>- cnt+= n; >>>- if (n > 0) { >>>- existingNames.add(name); >>>- } >>>- } >>>- } >>>- if (cnt < MAX_SIZE) { >>>- // cache all entries of all principals >>>- pathEntryMap = new HashMap<String, >>>Collection<PermissionEntry>>(); >>>- for (String name: principalNames) { >>>- cache.load(store, pathEntryMap, name); >>>- } >>>- } else { >>>- pathEntryMap = null; >>>- } >>>- } >>>- >>>- public void flush() { >>>- cache.flush(principalNames); >>>- init(); >>> } >>> >>> @Nonnull >>> public Iterator<PermissionEntry> getEntryIterator(@Nonnull >>>EntryPredicate predicate) { >>>- if (existingNames.isEmpty()) { >>>- return Iterators.emptyIterator(); >>>- } else { >>>- return new EntryIterator(predicate); >>>- } >>>+ return new EntryIterator(predicate); >>> } >>> >>> @Nonnull >>> public Collection<PermissionEntry> getEntries(@Nonnull Tree >>>accessControlledTree) { >>>- if (existingNames.isEmpty()) { >>>- return Collections.emptyList(); >>>- } else if (pathEntryMap != null) { >>>- Collection<PermissionEntry> entries = >>>pathEntryMap.get(accessControlledTree.getPath()); >>>- return (entries != null) ? entries : >>>Collections.<PermissionEntry>emptyList(); >>>+ if >>>(accessControlledTree.hasChild(AccessControlConstants.REP_POLICY)) { >>>+ return getEntries(accessControlledTree.getPath()); >>> } else { >>>- return >>>(accessControlledTree.hasChild(AccessControlConstants.REP_POLICY)) ? >>>- loadEntries(accessControlledTree.getPath()) : >>>- Collections.<PermissionEntry>emptyList(); >>>+ return Collections.<PermissionEntry>emptyList(); >>> } >>> } >>> >>> @Nonnull >>> public Collection<PermissionEntry> getEntries(@Nonnull String path) { >>>- if (existingNames.isEmpty()) { >>>- return Collections.emptyList(); >>>- } else if (pathEntryMap != null) { >>>- Collection<PermissionEntry> entries = pathEntryMap.get(path); >>>- return (entries != null) ? entries : >>>Collections.<PermissionEntry>emptyList(); >>>- } else { >>>- return loadEntries(path); >>>- } >>>- } >>>- >>>- @Nonnull >>>- private Collection<PermissionEntry> loadEntries(@Nonnull String >>>path) { >>> Collection<PermissionEntry> ret = new TreeSet<PermissionEntry>(); >>>- for (String name: existingNames) { >>>- cache.load(store, ret, name, path); >>>+ for (String name: principalNames) { >>>+ // todo: conditionally load entries if too many >>>+ PrincipalPermissionEntries ppe = store.load(name); >>>+ ret.addAll(ppe.getEntries(path)); >>> } >>> return ret; >>> } >>> >>>Modified: >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/permission/PermissionHook.java >>>URL: >>>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/o >>>rg/apache/jackrabbit/oak/security/authorization/permission/PermissionHook. >>>java?rev=1556370&r1=1556369&r2=1556370&view=diff >>>========================================================================== >>>==== >>>--- >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/permission/PermissionHook.java (original) >>>+++ >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/permission/PermissionHook.java Tue Jan 7 21:51:07 2014 >>>@@ -84,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; >>>@@ -96,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 >>>@@ -128,7 +126,6 @@ public class PermissionHook implements P >>> for (Map.Entry<String, Acl> entry : modified.entrySet()) { >>> entry.getValue().update(principalNames); >>> } >>>- cache.flush(principalNames); >>> } >>> >>> @Nonnull >>> >>>Modified: >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/permission/PermissionProviderImpl.java >>>URL: >>>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/o >>>rg/apache/jackrabbit/oak/security/authorization/permission/PermissionProvi >>>derImpl.java?rev=1556370&r1=1556369&r2=1556370&view=diff >>>========================================================================== >>>==== >>>--- >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/permission/PermissionProviderImpl.java (original) >>>+++ >>>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu >>>rity/authorization/permission/PermissionProviderImpl.java Tue Jan 7 >>>21:51:07 2014 >>>@@ -58,8 +58,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; >>>@@ -69,7 +68,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-jcr/pom.xml >>>URL: >>>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/pom.xml?rev=1556 >>>370&r1=1556369&r2=1556370&view=diff >>>========================================================================== >>>==== >>>--- jackrabbit/oak/trunk/oak-jcr/pom.xml (original) >>>+++ jackrabbit/oak/trunk/oak-jcr/pom.xml Tue Jan 7 21:51:07 2014 >>>@@ -117,6 +117,10 @@ >>> >>>org.apache.jackrabbit.oak.jcr.security.authorization.CopyTest#testCopyInvi >>>sibleAcContent <!-- OAK-920 --> >>> >>> >>>org.apache.jackrabbit.oak.jcr.security.authorization.SessionMoveTest#testM >>>oveAddSubTreeWithRestriction <!-- OAK-1223 --> >>>+ >>>org.apache.jackrabbit.oak.jcr.security.authorization.SessionMoveTest#testM >>>oveAndRemoveSubTree3 <!-- OAK-710 --> >>>+ >>>org.apache.jackrabbit.oak.jcr.security.authorization.SessionMoveTest#testM >>>oveAndAddSubTree3 <!-- OAK-710 --> >>>+ >>>org.apache.jackrabbit.oak.jcr.security.authorization.SessionMoveTest#testM >>>oveAndAddProperty2 <!-- OAK-710 --> >>>+ >>>org.apache.jackrabbit.oak.jcr.security.authorization.SessionMoveTest#testM >>>oveAndRemoveProperty2 <!-- OAK-710 --> >>> >>> <!-- User Management --> >>> >>>org.apache.jackrabbit.oak.jcr.security.user.RefreshTest#testAuthorizableGe >>>tProperty <!-- OAK-1124 --> >>> >>> >>
