jackrabbit-oak-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tobias Bocanegra <tri...@apache.org>
Subject 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/
Date Wed, 08 Jan 2014 19:01:27 GMT
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 <tripod@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 <anchela@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" <jukka@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 -->
>>>
>>>
>>

Mime
View raw message