You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by ju...@apache.org on 2014/01/07 22:51:08 UTC
svn commit: r1556370 - 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-jcr/
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/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/PermissionEntryProvider.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-jcr/pom.xml
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=1556370&r1=1556369&r2=1556370&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 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.AccessControlImporter;
import org.apache.jackrabbit.oak.security.authorization.accesscontrol.AccessControlManagerImpl;
import org.apache.jackrabbit.oak.security.authorization.accesscontrol.AccessControlValidatorProvider;
-import org.apache.jackrabbit.oak.security.authorization.permission.PermissionEntryCache;
import org.apache.jackrabbit.oak.security.authorization.permission.PermissionHook;
import org.apache.jackrabbit.oak.security.authorization.permission.PermissionProviderImpl;
import org.apache.jackrabbit.oak.security.authorization.permission.PermissionStoreValidatorProvider;
@@ -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/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=1556370&r1=1556369&r2=1556370&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 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/security/authorization/permission/PermissionEntryProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProvider.java?rev=1556370&r1=1556369&r2=1556370&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProvider.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/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/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=1556370&r1=1556369&r2=1556370&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 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/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=1556370&r1=1556369&r2=1556370&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 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/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=1556370&r1=1556369&r2=1556370&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 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=1556370&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#testCopyInvisibleAcContent <!-- OAK-920 -->
org.apache.jackrabbit.oak.jcr.security.authorization.SessionMoveTest#testMoveAddSubTreeWithRestriction <!-- OAK-1223 -->
+ org.apache.jackrabbit.oak.jcr.security.authorization.SessionMoveTest#testMoveAndRemoveSubTree3 <!-- OAK-710 -->
+ org.apache.jackrabbit.oak.jcr.security.authorization.SessionMoveTest#testMoveAndAddSubTree3 <!-- OAK-710 -->
+ org.apache.jackrabbit.oak.jcr.security.authorization.SessionMoveTest#testMoveAndAddProperty2 <!-- OAK-710 -->
+ org.apache.jackrabbit.oak.jcr.security.authorization.SessionMoveTest#testMoveAndRemoveProperty2 <!-- OAK-710 -->
<!-- User Management -->
org.apache.jackrabbit.oak.jcr.security.user.RefreshTest#testAuthorizableGetProperty <!-- OAK-1124 -->
Re: svn commit: r1556370 - 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-jcr/
Posted by Tobias Bocanegra <tr...@apache.org>.
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 <tr...@adobe.com> 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 <an...@adobe.com> 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, "jukka@apache.org" <ju...@apache.org> 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 -->
>>>
>>>
>>
Re: svn commit: r1556370 - 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-jcr/
Posted by Jukka Zitting <ju...@gmail.com>.
Hi,
On Wed, Jan 8, 2014 at 1:54 PM, Tobias Bocanegra <tr...@adobe.com> wrote:
> 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.
OK, cool. Just make sure the resulting code is still deterministic.
BR,
Jukka Zitting
Re: svn commit: r1556370 - 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-jcr/
Posted by Tobias Bocanegra <tr...@adobe.com>.
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 <an...@adobe.com> 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, "jukka@apache.org" <ju...@apache.org> 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 -->
>>
>>
>
Re: svn commit: r1556370 - 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-jcr/
Posted by Angela Schreiber <an...@adobe.com>.
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, "jukka@apache.org" <ju...@apache.org> 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 -->
>
>