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 an...@apache.org on 2020/09/23 16:35:29 UTC

svn commit: r1881961 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/security/authorization/permission/ test/java/org/apache/jackrabbit/oak/security/authorization/permission/

Author: angela
Date: Wed Sep 23 16:35:28 2020
New Revision: 1881961

URL: http://svn.apache.org/viewvc?rev=1881961&view=rev
Log:
OAK-9203 : PermissionProvider.refresh: create dedicated benchmarks and evaluate potential improvements

Added:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CacheStrategy.java   (with props)
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CacheStrategyImpl.java   (with props)
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractCacheTest.java   (with props)
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/CacheStrategyImplTest.java   (with props)
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheDefaultNotExactTest.java   (with props)
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheDefaultTest.java   (with props)
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCachePathMapTest.java   (with props)
Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheBuilder.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/PermissionProviderImpl.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImplTest.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/EmptyPermissionCacheTest.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheBuilderTest.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImplTest.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStoreImplTest.java

Added: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CacheStrategy.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CacheStrategy.java?rev=1881961&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CacheStrategy.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CacheStrategy.java Wed Sep 23 16:35:28 2020
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.security.authorization.permission;
+
+interface CacheStrategy {
+
+    /**
+     * @return Maximal number of access controlled paths that should be read to populate the cache upon initialization.
+     */
+    long maxSize();
+
+    /**
+     * @param numEntriesSize Value of {@link NumEntries#size} for a given path.
+     * @param cnt The latest (estimated) count of number of access controlled paths for the principals processed so far.
+     * @return {@code true} if permission entries for the current principal should be read to populate the cache; {@code false} otherwise.
+     */
+    boolean loadFully(long numEntriesSize, long cnt);
+
+    /**
+     * @param cnt The final (estimated) count of number of access controlled paths after having initialized the cache for all principals.
+     * @return {@code true} if all entries should (or already have) been read into the cache and a simple lookup can be used;
+     * {@code false} if the default cache should be used instead.
+     */
+    boolean usePathEntryMap(long cnt);
+
+}

Propchange: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CacheStrategy.java
------------------------------------------------------------------------------
    svn:eol-style = native

Added: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CacheStrategyImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CacheStrategyImpl.java?rev=1881961&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CacheStrategyImpl.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CacheStrategyImpl.java Wed Sep 23 16:35:28 2020
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.security.authorization.permission;
+
+import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
+
+final class CacheStrategyImpl implements CacheStrategy {
+
+    static final String EAGER_CACHE_SIZE_PARAM = "eagerCacheSize";
+    private static final long DEFAULT_SIZE = 250;
+
+    static final String EAGER_CACHE_MAXPATHS_PARAM = "eagerCacheMaxPaths";
+    private static final long DEFAULT_MAX_PATHS = 10;
+
+    private final long maxSize;
+    private final long maxPaths;
+    private final long threshold;
+
+    private boolean fullyLoaded = true;
+
+    CacheStrategyImpl(ConfigurationParameters options, boolean isRefresh) {
+        maxSize = options.getConfigValue(EAGER_CACHE_SIZE_PARAM, DEFAULT_SIZE);
+        maxPaths = options.getConfigValue(EAGER_CACHE_MAXPATHS_PARAM, DEFAULT_MAX_PATHS);
+        // define upper boundary for reading all permission entries:
+        // - avoid repeatedly reading the entries for sessions that force a refresh by either writing (Session.save)
+        //   or explicit refresh (Session.refresh). see also OAK-9203
+        // - read-only sessions though that read permission entries only once, benefit from eagerly
+        //   reading permission entries for principals with few access controlled paths (original behavior)
+        if (isRefresh) {
+            threshold = maxSize;
+        } else {
+            threshold = Long.MAX_VALUE;
+        }
+    }
+
+
+    @Override
+    public long maxSize() {
+        return maxSize;
+    }
+
+    @Override
+    public boolean loadFully(long numEntriesSize, long cnt) {
+        if (numEntriesSize <= maxPaths && cnt < threshold) {
+            return true;
+        } else {
+            fullyLoaded = false;
+            return false;
+        }
+    }
+
+    @Override
+    public boolean usePathEntryMap(long cnt) {
+        if (cnt > 0) {
+            return fullyLoaded || cnt < maxSize;
+        } else {
+            return false;
+        }
+    }
+}

Propchange: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CacheStrategyImpl.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheBuilder.java?rev=1881961&r1=1881960&r2=1881961&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheBuilder.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheBuilder.java Wed Sep 23 16:35:28 2020
@@ -16,6 +16,11 @@
  */
 package org.apache.jackrabbit.oak.security.authorization.permission;
 
+import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.commons.LongUtils;
+import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants;
+import org.jetbrains.annotations.NotNull;
+
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -24,17 +29,10 @@ import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
 
-import org.apache.jackrabbit.oak.api.Tree;
-import org.apache.jackrabbit.oak.commons.LongUtils;
-import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants;
-import org.jetbrains.annotations.NotNull;
-
 import static com.google.common.base.Preconditions.checkState;
 
 final class PermissionCacheBuilder {
 
-    private static final long MAX_PATHS_SIZE = 10;
-
     private final PermissionStore store;
     private final PermissionEntryCache peCache;
 
@@ -48,11 +46,11 @@ final class PermissionCacheBuilder {
         this.peCache = new PermissionEntryCache();
     }
 
-    boolean init(@NotNull Set<String> principalNames, long maxSize) {
+    boolean init(@NotNull Set<String> principalNames, @NotNull CacheStrategy cacheStrategy) {
         existingNames = new HashSet<>();
         long cnt = 0;
         for (String name : principalNames) {
-            NumEntries ne = store.getNumEntries(name, maxSize);
+            NumEntries ne = store.getNumEntries(name, cacheStrategy.maxSize());
             long n = ne.size;
             /*
             if getNumEntries (n) returns a number bigger than 0, we
@@ -60,7 +58,7 @@ final class PermissionCacheBuilder {
             */
             if (n > 0) {
                 existingNames.add(name);
-                if (n <= MAX_PATHS_SIZE) {
+                if (cacheStrategy.loadFully(n, cnt)) {
                     peCache.getFullyLoadedEntries(store, name);
                 } else {
                     long expectedSize = (ne.isExact) ? n : Long.MAX_VALUE;
@@ -84,7 +82,7 @@ final class PermissionCacheBuilder {
             }
         }
 
-        usePathEntryMap = (cnt > 0 && cnt < maxSize);
+        usePathEntryMap = cacheStrategy.usePathEntryMap(cnt);
         initialized = true;
         return existingNames.isEmpty();
     }
@@ -155,7 +153,7 @@ final class PermissionCacheBuilder {
         public Collection<PermissionEntry> getEntries(@NotNull Tree accessControlledTree) {
             return (accessControlledTree.hasChild(AccessControlConstants.REP_POLICY)) ?
                     getEntries(accessControlledTree.getPath()) :
-                    Collections.<PermissionEntry>emptyList();
+                    Collections.emptyList();
         }
     }
 
@@ -176,14 +174,14 @@ final class PermissionCacheBuilder {
         @Override
         public Collection<PermissionEntry> getEntries(@NotNull String path) {
             Collection<PermissionEntry> entries = pathEntryMap.get(path);
-            return (entries != null) ? entries : Collections.<PermissionEntry>emptyList();
+            return (entries != null) ? entries : Collections.emptyList();
         }
 
         @NotNull
         @Override
         public Collection<PermissionEntry> getEntries(@NotNull Tree accessControlledTree) {
             Collection<PermissionEntry> entries = pathEntryMap.get(accessControlledTree.getPath());
-            return (entries != null) ? entries : Collections.<PermissionEntry>emptyList();
+            return (entries != null) ? entries : Collections.emptyList();
         }
     }
 
@@ -199,13 +197,13 @@ final class PermissionCacheBuilder {
         @NotNull
         @Override
         public Collection<PermissionEntry> getEntries(@NotNull String path) {
-            return Collections.<PermissionEntry>emptyList();
+            return Collections.emptyList();
         }
 
         @NotNull
         @Override
         public Collection<PermissionEntry> getEntries(@NotNull Tree accessControlledTree) {
-            return Collections.<PermissionEntry>emptyList();
+            return Collections.emptyList();
         }
     }
 

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=1881961&r1=1881960&r2=1881961&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 Wed Sep 23 16:35:28 2020
@@ -16,21 +16,18 @@
  */
 package org.apache.jackrabbit.oak.security.authorization.permission;
 
-import java.util.Collection;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.Set;
 import com.google.common.base.Strings;
 import org.apache.jackrabbit.commons.iterator.AbstractLazyIterator;
 import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
 import org.jetbrains.annotations.NotNull;
 
-class PermissionEntryProviderImpl implements PermissionEntryProvider {
-
-    private static final String EAGER_CACHE_SIZE_PARAM = "eagerCacheSize";
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.Set;
 
-    private static final long DEFAULT_SIZE = 250;
+class PermissionEntryProviderImpl implements PermissionEntryProvider {
 
     /**
      * The set of principal names for which this {@code PermissionEntryProvider}
@@ -40,7 +37,7 @@ class PermissionEntryProviderImpl implem
 
     private final PermissionStore store;
 
-    private final long maxSize;
+    private final ConfigurationParameters options;
 
     /**
      * Flag to indicate if the the store contains any permission entries for the
@@ -48,30 +45,37 @@ class PermissionEntryProviderImpl implem
      */
     private boolean noExistingNames;
 
+    private boolean initialized = false;
+    private boolean isRefreshed = false;
+
     private PermissionCache permissionCache;
 
     PermissionEntryProviderImpl(@NotNull PermissionStore store, @NotNull Set<String> principalNames, @NotNull ConfigurationParameters options) {
         this.store = store;
         this.principalNames = Collections.unmodifiableSet(principalNames);
-        this.maxSize = options.getConfigValue(EAGER_CACHE_SIZE_PARAM, DEFAULT_SIZE);
-        init();
+        this.options = options;
     }
 
     private void init() {
-        PermissionCacheBuilder builder = new PermissionCacheBuilder(store);
-        noExistingNames = builder.init(principalNames, maxSize);
-        permissionCache = builder.build();
+        if (!initialized) {
+            PermissionCacheBuilder builder = new PermissionCacheBuilder(store);
+            noExistingNames = builder.init(principalNames, new CacheStrategyImpl(options, isRefreshed));
+            permissionCache = builder.build();
+            initialized = true;
+        }
     }
 
     //--------------------------------------------< PermissionEntryProvider >---
     @Override
     public void flush() {
-        init();
+        initialized = false;
+        isRefreshed = true;
     }
 
     @Override
     @NotNull
     public Iterator<PermissionEntry> getEntryIterator(@NotNull EntryPredicate predicate) {
+        init();
         if (noExistingNames) {
             return Collections.emptyIterator();
         } else {
@@ -82,14 +86,11 @@ class PermissionEntryProviderImpl implem
     @Override
     @NotNull
     public Collection<PermissionEntry> getEntries(@NotNull Tree accessControlledTree) {
+        init();
         return permissionCache.getEntries(accessControlledTree);
     }
 
     //------------------------------------------------------------< private >---
-    @NotNull
-    private Collection<PermissionEntry> getEntries(@NotNull String path) {
-        return permissionCache.getEntries(path);
-    }
 
     private final class EntryIterator extends AbstractLazyIterator<PermissionEntry> {
 
@@ -125,5 +126,10 @@ class PermissionEntryProviderImpl implem
             }
             return next;
         }
+
+        @NotNull
+        private Collection<PermissionEntry> getEntries(@NotNull String path) {
+            return permissionCache.getEntries(path);
+        }
     }
 }

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=1881961&r1=1881960&r2=1881961&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 Wed Sep 23 16:35:28 2020
@@ -16,8 +16,6 @@
  */
 package org.apache.jackrabbit.oak.security.authorization.permission;
 
-import java.security.Principal;
-import java.util.Set;
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.api.Root;
 import org.apache.jackrabbit.oak.api.Tree;
@@ -41,6 +39,9 @@ import org.apache.jackrabbit.oak.spi.ver
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+import java.security.Principal;
+import java.util.Set;
+
 public class PermissionProviderImpl implements PermissionProvider, AccessControlConstants, PermissionConstants, AggregatedPermissionProvider {
 
     private final Root root;

Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractCacheTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractCacheTest.java?rev=1881961&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractCacheTest.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractCacheTest.java Wed Sep 23 16:35:28 2020
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.security.authorization.permission;
+
+import com.google.common.collect.ImmutableSet;
+import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
+import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionPattern;
+import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeBits;
+import org.jetbrains.annotations.NotNull;
+import org.junit.Before;
+
+import static org.mockito.Mockito.mock;
+
+public abstract class AbstractCacheTest {
+
+    static final String EMPTY_CLASS_NAME = "org.apache.jackrabbit.oak.security.authorization.permission.PermissionCacheBuilder$EmptyCache";
+    static final String ENTRYMAP_CLASS_NAME = "org.apache.jackrabbit.oak.security.authorization.permission.PermissionCacheBuilder$PathEntryMapCache";
+    static final String DEFAULT_CLASS_NAME = "org.apache.jackrabbit.oak.security.authorization.permission.PermissionCacheBuilder$DefaultPermissionCache";
+
+    PermissionStore store;
+    PermissionCacheBuilder permissionCacheBuilder;
+
+    @Before
+    public void before() {
+        store = mock(PermissionStore.class);
+        permissionCacheBuilder = new PermissionCacheBuilder(store);
+    }
+
+    @NotNull
+    static PrincipalPermissionEntries generatedPermissionEntries(@NotNull String path, boolean isAllow, int index, @NotNull String privilegeName) {
+        PrincipalPermissionEntries ppe = new PrincipalPermissionEntries(1);
+        ppe.putEntriesByPath(path, ImmutableSet.of(new PermissionEntry(path, isAllow, index, PrivilegeBits.BUILT_IN.get(privilegeName), RestrictionPattern.EMPTY)));
+        return ppe;
+    }
+
+
+
+    @NotNull
+    static CacheStrategy createStrategy(long maxSize, long maxPaths, boolean isRefresh) {
+        return new CacheStrategyImpl(ConfigurationParameters.of(
+                CacheStrategyImpl.EAGER_CACHE_SIZE_PARAM, maxSize,
+                CacheStrategyImpl.EAGER_CACHE_MAXPATHS_PARAM, maxPaths), isRefresh);
+    }
+}
\ No newline at end of file

Propchange: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractCacheTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/CacheStrategyImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/CacheStrategyImplTest.java?rev=1881961&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/CacheStrategyImplTest.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/CacheStrategyImplTest.java Wed Sep 23 16:35:28 2020
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.security.authorization.permission;
+
+import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
+import org.junit.Test;
+
+import static org.apache.jackrabbit.oak.security.authorization.permission.CacheStrategyImpl.EAGER_CACHE_MAXPATHS_PARAM;
+import static org.apache.jackrabbit.oak.security.authorization.permission.CacheStrategyImpl.EAGER_CACHE_SIZE_PARAM;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class CacheStrategyImplTest {
+
+    @Test
+    public void testMaxSize() {
+        long maxSize = 30L;
+        assertEquals(maxSize, new CacheStrategyImpl(ConfigurationParameters.of(EAGER_CACHE_SIZE_PARAM, maxSize), false).maxSize());
+        assertEquals(maxSize, new CacheStrategyImpl(ConfigurationParameters.of(EAGER_CACHE_SIZE_PARAM, maxSize), true).maxSize());
+    }
+
+    @Test
+    public void testDefaultMaxSize() {
+        long defaultMaxSize = 250L;
+        assertEquals(defaultMaxSize, new CacheStrategyImpl(ConfigurationParameters.EMPTY, false).maxSize());
+        assertEquals(defaultMaxSize, new CacheStrategyImpl(ConfigurationParameters.EMPTY, true).maxSize());
+    }
+
+    @Test
+    public void testThreshold() {
+        long maxPaths = 5;
+        CacheStrategy cs = new CacheStrategyImpl(ConfigurationParameters.of(EAGER_CACHE_MAXPATHS_PARAM, maxPaths), false);
+
+        long[] cnts = new long[] {Long.MIN_VALUE, 0L, 1L, 10L, 1000L, Long.MAX_VALUE-1};
+        for (long cnt : cnts) {
+            assertTrue(cs.loadFully(maxPaths-1, cnt));
+            assertTrue(cs.loadFully(maxPaths, cnt));
+            assertFalse(cs.loadFully(maxPaths+1, cnt));
+        }
+        assertFalse(cs.loadFully(maxPaths+1, Long.MAX_VALUE));
+
+    }
+
+    @Test
+    public void testThresholdIsRefresh() {
+        long maxSize = 10;
+        long maxPaths = 5;
+        CacheStrategy cs = new CacheStrategyImpl(ConfigurationParameters.of(
+                EAGER_CACHE_SIZE_PARAM, maxSize,
+                EAGER_CACHE_MAXPATHS_PARAM, maxPaths), true);
+
+        for (long cnt : new long[] {Long.MIN_VALUE, 0L, 1L, 10L-1}) {
+            assertTrue(cs.loadFully(maxPaths-1, cnt));
+            assertTrue(cs.loadFully(maxPaths, cnt));
+            assertFalse(cs.loadFully(maxPaths+1, cnt));
+        }
+
+        for (long cnt : new long[] {10L, 1000L, Long.MAX_VALUE}) {
+            assertFalse(cs.loadFully(maxPaths-1, cnt));
+            assertFalse(cs.loadFully(maxPaths, cnt));
+            assertFalse(cs.loadFully(maxPaths+1, cnt));
+        }
+    }
+
+    @Test
+    public void testUseEntryMapCacheZeroCnt() {
+        long maxSize = 100;
+        long maxPaths = 1;
+        CacheStrategy cs = new CacheStrategyImpl(ConfigurationParameters.of(
+                EAGER_CACHE_SIZE_PARAM, maxSize,
+                EAGER_CACHE_MAXPATHS_PARAM, maxPaths), true);
+
+        long cnt = 0;
+        assertFalse(cs.usePathEntryMap(cnt));
+        assertTrue(cs.loadFully(1, cnt));
+        assertFalse(cs.usePathEntryMap(cnt));
+    }
+
+    @Test
+    public void testUseEntryMapCacheIsRefresh() {
+        long maxSize = 100;
+        long maxPaths = 1;
+        CacheStrategy cs = new CacheStrategyImpl(ConfigurationParameters.of(
+                EAGER_CACHE_SIZE_PARAM, maxSize,
+                EAGER_CACHE_MAXPATHS_PARAM, maxPaths), true);
+
+        long numentrysize = maxPaths;
+        long cnt = 0;
+        assertTrue(cs.loadFully(numentrysize, cnt));
+        assertTrue(cs.usePathEntryMap(++cnt));
+
+        cnt=10;
+        assertTrue(cs.loadFully(numentrysize, cnt));
+        assertTrue(cs.usePathEntryMap(++cnt));
+        assertTrue(cs.usePathEntryMap(maxSize));
+
+        numentrysize += 5;
+        assertFalse(cs.loadFully(numentrysize, cnt));
+        assertTrue(cs.usePathEntryMap(numentrysize+cnt));
+        assertFalse(cs.usePathEntryMap(maxSize));
+    }
+
+    @Test
+    public void testUseEntryMapCache() {
+        long maxSize = 100;
+        long maxPaths = 1;
+        CacheStrategy cs = new CacheStrategyImpl(ConfigurationParameters.of(
+                EAGER_CACHE_SIZE_PARAM, maxSize,
+                EAGER_CACHE_MAXPATHS_PARAM, maxPaths), false);
+
+        assertTrue(cs.loadFully(maxPaths, maxSize));
+        assertTrue(cs.usePathEntryMap(maxSize));
+
+        assertFalse(cs.loadFully(maxPaths+1, maxSize));
+        assertTrue(cs.usePathEntryMap(maxSize-1));
+        assertFalse(cs.usePathEntryMap(maxSize));
+    }
+}
\ No newline at end of file

Propchange: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/CacheStrategyImplTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImplTest.java?rev=1881961&r1=1881960&r2=1881961&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImplTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImplTest.java Wed Sep 23 16:35:28 2020
@@ -72,6 +72,7 @@ import static org.junit.Assert.assertSam
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.clearInvocations;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
@@ -123,18 +124,18 @@ public class CompiledPermissionImplTest
     }
 
     @NotNull
-    private CompiledPermissionImpl create(@NotNull ConfigurationParameters options, @NotNull Set<Principal> principals, @Nullable PermissionStore store) {
+    private PermissionStore mockPermissionStore(@NotNull Root r, @NotNull String wspName) {
+        return spy(new PermissionStoreImpl(r, wspName, getConfig(AuthorizationConfiguration.class).getRestrictionProvider()));
+    }
+
+    private CompiledPermissionImpl create(@NotNull Root r, @NotNull String workspaceName, @NotNull Set<Principal> principals, @NotNull PermissionStore store, @NotNull ConfigurationParameters options) {
         AuthorizationConfiguration config = getConfig(AuthorizationConfiguration.class);
         assertTrue(config instanceof CompositeAuthorizationConfiguration);
 
         AuthorizationConfiguration defConfig = ((CompositeAuthorizationConfiguration) config).getDefaultConfig();
         assertTrue(defConfig instanceof AuthorizationConfigurationImpl);
 
-        Root r = getRootProvider().createReadOnlyRoot(testSession.getLatestRoot());
-        String workspaceName = testSession.getWorkspaceName();
-        PermissionStore pStore = (store == null) ? new PermissionStoreImpl(r, workspaceName, config.getRestrictionProvider()) : store;
-
-        CompiledPermissions cp = CompiledPermissionImpl.create(r, workspaceName, pStore, principals, options, config.getContext(), (AuthorizationConfigurationImpl) defConfig);
+        CompiledPermissions cp = CompiledPermissionImpl.create(r, workspaceName, store, principals, options, config.getContext(), (AuthorizationConfigurationImpl) defConfig);
         assertTrue(cp instanceof CompiledPermissionImpl);
 
         return (CompiledPermissionImpl) cp;
@@ -142,7 +143,11 @@ public class CompiledPermissionImplTest
 
     @NotNull
     private CompiledPermissionImpl createForTestSession(@NotNull ConfigurationParameters options) {
-        return create(options, testSession.getAuthInfo().getPrincipals(), null);
+        Root r = getRootProvider().createReadOnlyRoot(testSession.getLatestRoot());
+        String workspaceName = testSession.getWorkspaceName();
+        PermissionStore pStore = mockPermissionStore(r, workspaceName);
+
+        return create(r, workspaceName, testSession.getAuthInfo().getPrincipals(), pStore, options);
     }
 
     @NotNull
@@ -216,7 +221,7 @@ public class CompiledPermissionImplTest
             assertFalse(cp.hasPrivileges(t, PrivilegeConstants.REP_READ_NODES));
             assertFalse(cp.hasPrivileges(t, JCR_READ));
 
-            assertEquals(0, cp.getPrivileges(t).size());
+            assertTrue(cp.getPrivileges(t).isEmpty());
 
             TreePermission tp = createTreePermission(cp, readPath);
             assertFalse(tp.canRead());
@@ -230,21 +235,7 @@ public class CompiledPermissionImplTest
         // cp with DefaultReadPolicy
         CompiledPermissionImpl cp = createForTestSession(ConfigurationParameters.EMPTY);
         for (String readPath : PermissionConstants.DEFAULT_READ_PATHS) {
-            assertTrue(cp.isGranted(readPath, Permissions.READ_NODE));
-
-            Tree t = createReadonlyTree(readPath);
-            assertTrue(cp.isGranted(t, null, Permissions.READ_NODE));
-            assertTrue(cp.isGranted(t, t.getProperty(JCR_PRIMARYTYPE), Permissions.READ_PROPERTY));
-
-            assertTrue(cp.hasPrivileges(t, PrivilegeConstants.REP_READ_NODES));
-            assertTrue(cp.hasPrivileges(t, JCR_READ));
-
-            assertEquals(ImmutableSet.of(JCR_READ), cp.getPrivileges(t));
-
-            TreePermission tp = createTreePermission(cp, readPath);
-            assertTrue(tp.canRead());
-            assertTrue(tp.canRead(mock(PropertyState.class)));
-            assertFalse(tp.canReadAll());
+            assertReadPath(cp, readPath);
         }
     }
 
@@ -254,22 +245,26 @@ public class CompiledPermissionImplTest
         CompiledPermissionImpl cp = createForTestSession(ConfigurationParameters.of(PARAM_READ_PATHS, ImmutableSet.of(TEST_PATH, "/another", "/yet/another")));
 
         for (String readPath : new String[]{TEST_PATH, SUBTREE_PATH, TEST_PATH + "/nonExisting"}) {
-            assertTrue(cp.isGranted(readPath, Permissions.READ_NODE));
+            assertReadPath(cp, readPath);
+        }
+    }
 
-            Tree t = createReadonlyTree(readPath);
-            assertTrue(cp.isGranted(t, null, Permissions.READ_NODE));
-            assertTrue(cp.isGranted(t, t.getProperty(JCR_PRIMARYTYPE), Permissions.READ_PROPERTY));
+    private void assertReadPath(@NotNull CompiledPermissionImpl cp, @NotNull String readPath) {
+        assertTrue(cp.isGranted(readPath, Permissions.READ_NODE));
 
-            assertTrue(cp.hasPrivileges(t, PrivilegeConstants.REP_READ_NODES));
-            assertTrue(cp.hasPrivileges(t, JCR_READ));
+        Tree t = createReadonlyTree(readPath);
+        assertTrue(cp.isGranted(t, null, Permissions.READ_NODE));
+        assertTrue(cp.isGranted(t, t.getProperty(JCR_PRIMARYTYPE), Permissions.READ_PROPERTY));
 
-            assertEquals(ImmutableSet.of(JCR_READ), cp.getPrivileges(t));
+        assertTrue(cp.hasPrivileges(t, PrivilegeConstants.REP_READ_NODES));
+        assertTrue(cp.hasPrivileges(t, JCR_READ));
 
-            TreePermission tp = createTreePermission(cp, readPath);
-            assertTrue(tp.canRead());
-            assertTrue(tp.canRead(mock(PropertyState.class)));
-            assertFalse(tp.canReadAll());
-        }
+        assertEquals(ImmutableSet.of(JCR_READ), cp.getPrivileges(t));
+
+        TreePermission tp = createTreePermission(cp, readPath);
+        assertTrue(tp.canRead());
+        assertTrue(tp.canRead(mock(PropertyState.class)));
+        assertFalse(tp.canReadAll());
     }
 
     @Test
@@ -443,6 +438,44 @@ public class CompiledPermissionImplTest
     }
 
     @Test
+    public void testCacheBuildOnDemand() throws Exception {
+        grant(ACCESS_CONTROLLED_PATH, getTestUser().getPrincipal(), JCR_VERSION_MANAGEMENT);
+        grant(ACCESS_CONTROLLED_PATH, EveryonePrincipal.getInstance(), JCR_READ);
+        root.commit();
+
+        Root readOnlyRoot = getRootProvider().createReadOnlyRoot(testSession.getLatestRoot());
+        String wspName = testSession.getWorkspaceName();
+
+        PermissionStore store = mockPermissionStore(readOnlyRoot, wspName);
+        Set<Principal> principals = ImmutableSet.of(getTestUser().getPrincipal(), EveryonePrincipal.getInstance());
+        CompiledPermissionImpl cp = create(readOnlyRoot, wspName, principals, store, ConfigurationParameters.EMPTY);
+
+        // verify lazy initialization of the permission cache
+        verify(store, never()).getNumEntries(anyString(), anyLong());
+
+        cp.isGranted(ACCESS_CONTROLLED_PATH, Permissions.VERSION_MANAGEMENT);
+        verify(store, times(2)).getNumEntries(anyString(), anyLong());
+
+        cp.isGranted(ACCESS_CONTROLLED_PATH, Permissions.READ);
+        verify(store, times(2)).getNumEntries(anyString(), anyLong());
+
+        clearInvocations(store);
+
+        // subsequent reads must not trigger re-loading of the permission cache.
+        cp.isGranted(ACCESS_CONTROLLED_PATH, Permissions.WRITE);
+        verify(store, never()).getNumEntries(anyString(), anyLong());
+
+        // flush must not eagerly re-load permission cache
+        cp.refresh(readOnlyRoot, wspName);
+        verify(store, times(1)).flush(readOnlyRoot);
+        verify(store, never()).getNumEntries(anyString(), anyLong());
+
+        // verifying permissions that requires init of both user and group store
+        cp.isGranted(ACCESS_CONTROLLED_PATH, Permissions.MODIFY_PROPERTY);
+        verify(store, times(2)).getNumEntries(anyString(), anyLong());
+    }
+
+    @Test
     public void testMissingGroupStore() throws Exception {
         grant(ACCESS_CONTROLLED_PATH, getTestUser().getPrincipal(), JCR_VERSION_MANAGEMENT);
         root.commit();
@@ -451,13 +484,18 @@ public class CompiledPermissionImplTest
         String wspName = testSession.getWorkspaceName();
 
         // create cp for user principal only (no group principals that hold the permission setup)
-        PermissionStore store = spy(new PermissionStoreImpl(readOnlyRoot, wspName, getConfig(AuthorizationConfiguration.class).getRestrictionProvider()));
-        CompiledPermissionImpl cp = create(ConfigurationParameters.EMPTY, ImmutableSet.of(getTestUser().getPrincipal()), store);
+        PermissionStore store = mockPermissionStore(readOnlyRoot, wspName);
+        CompiledPermissionImpl cp = create(readOnlyRoot, wspName, ImmutableSet.of(getTestUser().getPrincipal()), store, ConfigurationParameters.EMPTY);
+
+        verify(store, never()).getNumEntries(anyString(), anyLong());
 
+        cp.isGranted(ACCESS_CONTROLLED_PATH, Permissions.READ_NODE);
         verify(store, times(1)).getNumEntries(anyString(), anyLong());
 
         cp.refresh(readOnlyRoot, wspName);
+        verify(store, times(1)).getNumEntries(anyString(), anyLong());
 
+        cp.isGranted(ACCESS_CONTROLLED_PATH, Permissions.READ_NODE);
         verify(store, times(2)).getNumEntries(anyString(), anyLong());
 
         assertFalse(cp.isGranted(ACCESS_CONTROLLED_PATH, SET_PROPERTY));
@@ -479,18 +517,23 @@ public class CompiledPermissionImplTest
     }
 
     @Test
-    public void testMissingUserStore() throws Exception {
+    public void testMissingUserStore() {
         Root readOnlyRoot = getRootProvider().createReadOnlyRoot(testSession.getLatestRoot());
         String wspName = testSession.getWorkspaceName();
 
         // create cp for group principal only (no user principal)
         PermissionStore store = spy(new PermissionStoreImpl(readOnlyRoot, wspName, getConfig(AuthorizationConfiguration.class).getRestrictionProvider()));
-        CompiledPermissionImpl cp = create(ConfigurationParameters.EMPTY, ImmutableSet.of(EveryonePrincipal.getInstance()), store);
+        CompiledPermissionImpl cp = create(readOnlyRoot, wspName, ImmutableSet.of(EveryonePrincipal.getInstance()), store, ConfigurationParameters.EMPTY);
+
+        verify(store, never()).getNumEntries(anyString(), anyLong());
 
+        cp.isGranted(ACCESS_CONTROLLED_PATH, Permissions.READ_NODE);
         verify(store, times(1)).getNumEntries(anyString(), anyLong());
 
         cp.refresh(readOnlyRoot, wspName);
+        verify(store, times(1)).getNumEntries(anyString(), anyLong());
 
+        cp.isGranted(ACCESS_CONTROLLED_PATH, Permissions.READ_NODE);
         verify(store, times(2)).getNumEntries(anyString(), anyLong());
 
         assertTrue(cp.isGranted(ACCESS_CONTROLLED_PATH, SET_PROPERTY));

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/EmptyPermissionCacheTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/EmptyPermissionCacheTest.java?rev=1881961&r1=1881960&r2=1881961&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/EmptyPermissionCacheTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/EmptyPermissionCacheTest.java Wed Sep 23 16:35:28 2020
@@ -18,6 +18,7 @@ package org.apache.jackrabbit.oak.securi
 
 import com.google.common.collect.ImmutableSet;
 import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.Mockito;
@@ -32,7 +33,7 @@ public class EmptyPermissionCacheTest {
     @Before
     public void before() {
         PermissionCacheBuilder builder = new PermissionCacheBuilder(Mockito.mock(PermissionStore.class));
-        builder.init(ImmutableSet.of(), Long.MAX_VALUE);
+        builder.init(ImmutableSet.of(), new CacheStrategyImpl(ConfigurationParameters.EMPTY, false));
         empty = builder.build();
     }
 

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheBuilderTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheBuilderTest.java?rev=1881961&r1=1881960&r2=1881961&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheBuilderTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheBuilderTest.java Wed Sep 23 16:35:28 2020
@@ -20,10 +20,6 @@ import com.google.common.collect.Immutab
 import com.google.common.collect.Sets;
 import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.commons.PathUtils;
-import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionPattern;
-import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeBits;
-import org.jetbrains.annotations.NotNull;
-import org.junit.Before;
 import org.junit.Test;
 
 import java.util.Set;
@@ -33,37 +29,15 @@ import static org.apache.jackrabbit.oak.
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
-import static org.mockito.Matchers.anyLong;
-import static org.mockito.Matchers.anyString;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
-public class PermissionCacheBuilderTest {
-
-    private static final int MAX_PATH_SIZE = 10;
-
-    private static final String EMPTY_CLASS_NAME = "org.apache.jackrabbit.oak.security.authorization.permission.PermissionCacheBuilder$EmptyCache";
-    private static final String SIMPLE_CLASS_NAME = "org.apache.jackrabbit.oak.security.authorization.permission.PermissionCacheBuilder$PathEntryMapCache";
-    private static final String DEFAULT_CLASS_NAME = "org.apache.jackrabbit.oak.security.authorization.permission.PermissionCacheBuilder$DefaultPermissionCache";
-
-    private PermissionStore store;
-    private PermissionCacheBuilder permissionCacheBuilder;
-
-    @Before
-    public void before() {
-        store = mock(PermissionStore.class);
-        permissionCacheBuilder = new PermissionCacheBuilder(store);
-    }
-
-    @NotNull
-    private static PrincipalPermissionEntries generatedPermissionEntries(@NotNull String path, boolean isAllow, int index, @NotNull String privilegeName) {
-        PrincipalPermissionEntries ppe = new PrincipalPermissionEntries(1);
-        ppe.putEntriesByPath(path, ImmutableSet.of(new PermissionEntry(path, isAllow, index, PrivilegeBits.BUILT_IN.get(privilegeName), RestrictionPattern.EMPTY)));
-        return ppe;
-    }
+public class PermissionCacheBuilderTest extends AbstractCacheTest {
 
     @Test(expected = IllegalStateException.class)
     public void testBuildBeforeInitialized() {
@@ -72,7 +46,7 @@ public class PermissionCacheBuilderTest
 
     @Test
     public void testBuildForEmptyPrincipals() {
-        assertTrue(permissionCacheBuilder.init(ImmutableSet.of(), Long.MAX_VALUE));
+        assertTrue(permissionCacheBuilder.init(ImmutableSet.of(), createStrategy(Long.MAX_VALUE, 2, false)));
         PermissionCache cache = permissionCacheBuilder.build();
         assertEquals(EMPTY_CLASS_NAME, cache.getClass().getName());
 
@@ -88,7 +62,7 @@ public class PermissionCacheBuilderTest
 
         Set<String> principalNames = Sets.newHashSet("noEntries", "noEntries2", "noEntries3");
 
-        assertTrue(permissionCacheBuilder.init(principalNames, Long.MAX_VALUE));
+        assertTrue(permissionCacheBuilder.init(principalNames, createStrategy(Long.MAX_VALUE, 10, false)));
 
         PermissionCache cache = permissionCacheBuilder.build();
         assertEquals(EMPTY_CLASS_NAME, cache.getClass().getName());
@@ -111,10 +85,10 @@ public class PermissionCacheBuilderTest
         when(store.getNumEntries(anyString(), anyLong())).thenReturn(NumEntries.valueOf(1, true));
 
         Set<String> principalNames = Sets.newHashSet("a", "b");
-        assertFalse(permissionCacheBuilder.init(principalNames, Long.MAX_VALUE));
+        assertFalse(permissionCacheBuilder.init(principalNames, createStrategy(Long.MAX_VALUE, 10, true)));
 
         PermissionCache cache = permissionCacheBuilder.build();
-        assertEquals(SIMPLE_CLASS_NAME, cache.getClass().getName());
+        assertEquals(ENTRYMAP_CLASS_NAME, cache.getClass().getName());
 
         verify(store, times(2)).getNumEntries(anyString(), anyLong());
         verify(store, times(2)).load(anyString());
@@ -128,10 +102,10 @@ public class PermissionCacheBuilderTest
         when(store.getNumEntries(anyString(), anyLong())).thenReturn(NumEntries.valueOf(1, true));
 
         Set<String> principalNames = Sets.newHashSet("a", "b");
-        assertFalse(permissionCacheBuilder.init(principalNames, Long.MAX_VALUE));
+        assertFalse(permissionCacheBuilder.init(principalNames, createStrategy(Long.MAX_VALUE, 10, false)));
 
         PermissionCache cache = permissionCacheBuilder.build();
-        assertEquals(SIMPLE_CLASS_NAME, cache.getClass().getName());
+        assertEquals(ENTRYMAP_CLASS_NAME, cache.getClass().getName());
 
         verify(store, times(2)).getNumEntries(anyString(), anyLong());
         verify(store, times(2)).load(anyString());
@@ -145,10 +119,10 @@ public class PermissionCacheBuilderTest
         when(store.getNumEntries(anyString(), anyLong())).thenReturn(NumEntries.valueOf(1, false));
 
         Set<String> principalNames = Sets.newHashSet("a", "b");
-        assertFalse(permissionCacheBuilder.init(principalNames, Long.MAX_VALUE));
+        assertFalse(permissionCacheBuilder.init(principalNames, createStrategy(Long.MAX_VALUE, 10, true)));
 
         PermissionCache cache = permissionCacheBuilder.build();
-        assertEquals(SIMPLE_CLASS_NAME, cache.getClass().getName());
+        assertEquals(ENTRYMAP_CLASS_NAME, cache.getClass().getName());
 
         verify(store, times(2)).getNumEntries(anyString(), anyLong());
         verify(store, times(2)).load(anyString());
@@ -157,11 +131,12 @@ public class PermissionCacheBuilderTest
 
     @Test
     public void testBuildPathEntryMapResultsInEmptyCache() {
+        long maxPaths = 10;
         when(store.load(anyString())).thenReturn(new PrincipalPermissionEntries());
-        when(store.getNumEntries(anyString(), anyLong())).thenReturn(NumEntries.valueOf(MAX_PATH_SIZE+1, false));
+        when(store.getNumEntries(anyString(), anyLong())).thenReturn(NumEntries.valueOf(maxPaths+1, false));
 
         Set<String> principalNames = Sets.newHashSet("a", "b");
-        assertFalse(permissionCacheBuilder.init(principalNames, Long.MAX_VALUE));
+        assertFalse(permissionCacheBuilder.init(principalNames, createStrategy(Long.MAX_VALUE, maxPaths, false)));
 
         PermissionCache cache = permissionCacheBuilder.build();
         assertEquals(EMPTY_CLASS_NAME, cache.getClass().getName());
@@ -172,7 +147,7 @@ public class PermissionCacheBuilderTest
     }
 
     @Test
-    public void testBuildMaxEntriesReached() throws Exception {
+    public void testBuildMaxEntriesReachedAllFullyLoaded() {
         PrincipalPermissionEntries ppeA = generatedPermissionEntries("/path1",false, 0, REP_READ_NODES);
         PrincipalPermissionEntries ppeB = generatedPermissionEntries("/path2",false, 0, REP_READ_NODES);
 
@@ -182,10 +157,10 @@ public class PermissionCacheBuilderTest
 
         Set<String> principalNames = Sets.newHashSet("a", "b");
         long maxSize = 1;
-        assertFalse(permissionCacheBuilder.init(principalNames, maxSize));
+        assertFalse(permissionCacheBuilder.init(principalNames, createStrategy(maxSize, 10, false)));
 
         PermissionCache cache = permissionCacheBuilder.build();
-        assertEquals(DEFAULT_CLASS_NAME, cache.getClass().getName());
+        assertEquals(ENTRYMAP_CLASS_NAME, cache.getClass().getName());
 
         verify(store, times(2)).getNumEntries(anyString(), anyLong());
         verify(store, times(2)).load(anyString());
@@ -193,10 +168,59 @@ public class PermissionCacheBuilderTest
     }
 
     @Test
+    public void testBuildMaxEntriesReachedPartiallyFullyLoaded() {
+        PrincipalPermissionEntries ppeA = generatedPermissionEntries("/path1",false, 0, REP_READ_NODES);
+
+        long maxSize = 1;
+        long maxPaths = 10;
+
+        when(store.load("a")).thenReturn(ppeA);
+        when(store.getNumEntries("a", maxSize)).thenReturn(NumEntries.valueOf(1, true));
+        when(store.getNumEntries("b", maxSize)).thenReturn(NumEntries.valueOf(maxPaths+1, true));
+
+        Set<String> principalNames = Sets.newLinkedHashSet(ImmutableSet.of("a", "b"));
+        assertFalse(permissionCacheBuilder.init(principalNames, createStrategy(maxSize, maxPaths, false)));
+
+        PermissionCache cache = permissionCacheBuilder.build();
+        assertEquals(DEFAULT_CLASS_NAME, cache.getClass().getName());
+
+        verify(store, times(2)).getNumEntries(anyString(), anyLong());
+        verify(store, times(1)).load(anyString());
+        verify(store, never()).load(anyString(), anyString());
+
+        cache.getEntries("/path");
+        verify(store, never()).load("a", "/path");
+        verify(store, times(1)).load("b", "/path");
+    }
+
+    @Test
+    public void testBuildLazilyLoaded() {
+        PrincipalPermissionEntries ppeA = generatedPermissionEntries("/path1",false, 0, REP_READ_NODES);
+
+        long maxSize = 1;
+        long maxPaths = 0;
+
+        when(store.load("a")).thenReturn(ppeA);
+        when(store.getNumEntries("a", maxSize)).thenReturn(NumEntries.valueOf(maxPaths+1, true));
+        when(store.getNumEntries("b", maxSize)).thenReturn(NumEntries.valueOf(maxPaths+1, false));
+
+        Set<String> principalNames = Sets.newLinkedHashSet(ImmutableSet.of("a", "b"));
+        assertFalse(permissionCacheBuilder.init(principalNames, createStrategy(maxSize, maxPaths, false)));
+
+        PermissionCache cache = permissionCacheBuilder.build();
+        assertEquals(DEFAULT_CLASS_NAME, cache.getClass().getName());
+
+        verify(store, times(2)).getNumEntries(anyString(), anyLong());
+        verify(store, never()).load(anyString());
+        verify(store, never()).load(anyString(), anyString());
+    }
+
+    @Test
     public void testInitNumEntriesExceedMaxPathExact() {
-        when(store.getNumEntries(anyString(), anyLong())).thenReturn(NumEntries.valueOf(MAX_PATH_SIZE+1, true));
+        long maxPaths = 2;
+        when(store.getNumEntries(anyString(), anyLong())).thenReturn(NumEntries.valueOf(maxPaths+1, true));
 
-        assertFalse(permissionCacheBuilder.init(ImmutableSet.of("a", "b", "c"), Long.MAX_VALUE));
+        assertFalse(permissionCacheBuilder.init(ImmutableSet.of("a", "b", "c"), createStrategy(Long.MAX_VALUE, maxPaths, false)));
 
         verify(store, times(3)).getNumEntries(anyString(), anyLong());
         verify(store, never()).load(anyString());
@@ -205,9 +229,10 @@ public class PermissionCacheBuilderTest
 
     @Test
     public void testInitNumEntriesExceedMaxPathNotExact() {
-        when(store.getNumEntries(anyString(), anyLong())).thenReturn(NumEntries.valueOf(MAX_PATH_SIZE+1, false));
+        long maxPaths = 5;
+        when(store.getNumEntries(anyString(), anyLong())).thenReturn(NumEntries.valueOf(maxPaths+1, false));
 
-        assertFalse(permissionCacheBuilder.init(ImmutableSet.of("a", "b", "c"), Long.MAX_VALUE));
+        assertFalse(permissionCacheBuilder.init(ImmutableSet.of("a", "b", "c"), createStrategy(Long.MAX_VALUE, maxPaths, false)));
 
         verify(store, times(3)).getNumEntries(anyString(), anyLong());
         verify(store, never()).load(anyString());
@@ -218,7 +243,7 @@ public class PermissionCacheBuilderTest
     public void testInitNumEntriesExceedsMaxLong() {
         when(store.getNumEntries(anyString(), anyLong())).thenReturn(NumEntries.valueOf(Long.MAX_VALUE, false));
 
-        assertFalse(permissionCacheBuilder.init(ImmutableSet.of("a", "b", "c"), Long.MAX_VALUE));
+        assertFalse(permissionCacheBuilder.init(ImmutableSet.of("a", "b", "c"), createStrategy(Long.MAX_VALUE, 25, false)));
 
         verify(store, times(3)).getNumEntries(anyString(), anyLong());
         verify(store, never()).load(anyString());

Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheDefaultNotExactTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheDefaultNotExactTest.java?rev=1881961&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheDefaultNotExactTest.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheDefaultNotExactTest.java Wed Sep 23 16:35:28 2020
@@ -0,0 +1,41 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.security.authorization.permission;
+
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+public class PermissionCacheDefaultNotExactTest extends PermissionCacheDefaultTest {
+
+    boolean isExact() {
+        return false;
+    }
+
+    int loadInvocationCnt() {
+        // exact number of access controlled paths is unknown => store needs to check all paths for all principals
+        return 6;
+    }
+
+    void verifyByPath() {
+        verify(store, times(1)).load("a", "/path1");
+        verify(store, times(1)).load("b", "/path1");
+        verify(store, times(1)).load("a", "/path2");
+        verify(store, times(1)).load("b", "/path2");
+        verify(store, times(1)).load("a", "/any/other/path");
+        verify(store, times(1)).load("b", "/any/other/path");
+    }
+}
\ No newline at end of file

Propchange: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheDefaultNotExactTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheDefaultTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheDefaultTest.java?rev=1881961&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheDefaultTest.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheDefaultTest.java Wed Sep 23 16:35:28 2020
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.security.authorization.permission;
+
+import com.google.common.collect.Sets;
+import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants;
+import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionPattern;
+import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeBits;
+import org.jetbrains.annotations.NotNull;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Set;
+
+import static org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants.JCR_MODIFY_ACCESS_CONTROL;
+import static org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants.REP_READ_NODES;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.clearInvocations;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+public class PermissionCacheDefaultTest extends AbstractCacheTest {
+
+    private PermissionCache cache;
+
+    @Before
+    public void before() {
+        super.before();
+
+        when(store.load("a", "/path1")).thenReturn(createPermissionEntryCollection("/path1", false, REP_READ_NODES));
+        when(store.load("b", "/path2")).thenReturn(createPermissionEntryCollection("/path2", true, JCR_MODIFY_ACCESS_CONTROL));
+        when(store.load("a", "/path2")).thenReturn(null);
+        when(store.load("b", "/path1")).thenReturn(null);
+        when(store.load("a", "/another/path")).thenReturn(null);
+        when(store.load("b", "/another/path")).thenReturn(null);
+        when(store.getNumEntries(anyString(), anyLong())).thenReturn(NumEntries.valueOf(1, isExact()));
+
+        Set<String> principalNames = Sets.newHashSet("a", "b");
+        assertFalse(permissionCacheBuilder.init(principalNames, createStrategy(1, 0, true)));
+
+        cache = permissionCacheBuilder.build();
+        assertEquals(DEFAULT_CLASS_NAME, cache.getClass().getName());
+
+        verify(store, never()).load(anyString());
+        clearInvocations(store);
+    }
+
+    boolean isExact() {
+        return true;
+    }
+
+    int loadInvocationCnt() {
+        // exact number of access controlled paths is known => store can determine when all entries have been already loaded.
+        return 3;
+    }
+
+    void verifyByPath() {
+        verify(store, times(1)).load("a", "/path1");
+        verify(store, times(1)).load("b", "/path1");
+
+        verify(store, times(1)).load("b", "/path2");
+        verify(store, never()).load("a", "/path2");
+
+        verify(store, never()).load("a", "/any/other/path");
+        verify(store, never()).load("b", "/any/other/path");
+    }
+
+    private static Collection<PermissionEntry> createPermissionEntryCollection(@NotNull String path, boolean isAllow, @NotNull String privilegeName) {
+        return Collections.singleton(new PermissionEntry(path, isAllow, 0, PrivilegeBits.BUILT_IN.get(privilegeName), RestrictionPattern.EMPTY));
+    }
+
+    @Test
+    public void testGetEntriesByPath() {
+        assertEquals(1, cache.getEntries("/path1").size());
+        assertEquals(1, cache.getEntries("/path2").size());
+        assertTrue(cache.getEntries("/any/other/path").isEmpty());
+
+        verify(store, never()).load(anyString());
+        verify(store, times(loadInvocationCnt())).load(anyString(), anyString());
+        verifyByPath();
+
+        // second access reads from cache
+        assertEquals(1, cache.getEntries("/path1").size());
+        assertEquals(1, cache.getEntries("/path2").size());
+        assertTrue(cache.getEntries("/any/other/path").isEmpty());
+
+        verify(store, times(loadInvocationCnt())).load(anyString(), anyString());
+        verifyByPath();
+    }
+
+    @Test
+    public void testGetEntriesByTree() {
+        Tree t = when(mock(Tree.class).getPath()).thenReturn("/not/access/controlled").getMock();
+        when(t.hasChild(AccessControlConstants.REP_POLICY)).thenReturn(false);
+
+        assertTrue(cache.getEntries(t).isEmpty());
+
+        verify(store, never()).load(anyString());
+        verify(store, never()).load(anyString(), anyString());
+    }
+
+    @Test
+    public void testGetEntriesByAccessControlledTree() {
+
+        Tree t = when(mock(Tree.class).getPath()).thenReturn("/path1", "/path2", "/any/other/path", "/path1", "/path2", "/any/other/path").getMock();
+        when(t.hasChild(AccessControlConstants.REP_POLICY)).thenReturn(true);
+
+        assertEquals(1, cache.getEntries(t).size());
+        assertEquals(1, cache.getEntries(t).size());
+        assertTrue(cache.getEntries(t).isEmpty());
+
+        verify(store, never()).load(anyString());
+        verify(store, times(loadInvocationCnt())).load(anyString(), anyString());
+        verifyByPath();
+
+        // second access reads from cache
+        assertEquals(1, cache.getEntries(t).size());
+        assertEquals(1, cache.getEntries(t).size());
+        assertTrue(cache.getEntries(t).isEmpty());
+
+        verify(store, times(loadInvocationCnt())).load(anyString(), anyString());
+        verifyByPath();
+    }
+}
\ No newline at end of file

Propchange: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCacheDefaultTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCachePathMapTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCachePathMapTest.java?rev=1881961&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCachePathMapTest.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCachePathMapTest.java Wed Sep 23 16:35:28 2020
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.security.authorization.permission;
+
+import com.google.common.collect.Sets;
+import org.apache.jackrabbit.oak.api.Tree;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.Set;
+
+import static org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants.JCR_MODIFY_ACCESS_CONTROL;
+import static org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants.REP_READ_NODES;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.clearInvocations;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+public class PermissionCachePathMapTest extends AbstractCacheTest {
+
+    private PermissionCache cache;
+
+    @Before
+    public void before() {
+        super.before();
+
+        when(store.load("a")).thenReturn(generatedPermissionEntries("/path1",false, 0, REP_READ_NODES));
+        when(store.load("b")).thenReturn(generatedPermissionEntries("/path2", true, 0, JCR_MODIFY_ACCESS_CONTROL));
+        when(store.getNumEntries(anyString(), anyLong())).thenReturn(NumEntries.valueOf(1, true));
+
+        Set<String> principalNames = Sets.newHashSet("a", "b");
+        assertFalse(permissionCacheBuilder.init(principalNames, createStrategy(250, 10, true)));
+
+        cache = permissionCacheBuilder.build();
+        assertEquals(ENTRYMAP_CLASS_NAME, cache.getClass().getName());
+
+        verify(store, times(2)).load(anyString());
+        clearInvocations(store);
+    }
+
+    @Test
+    public void testGetEntriesByPath() {
+        assertEquals(1, cache.getEntries("/path1").size());
+        assertEquals(1, cache.getEntries("/path2").size());
+        assertTrue(cache.getEntries("/any/other/path").isEmpty());
+
+        verify(store, never()).load(anyString());
+        verify(store, never()).load(anyString(), anyString());
+    }
+
+    @Test
+    public void testGetEntriesByTree() {
+        Tree t = when(mock(Tree.class).getPath()).thenReturn("/path1", "/path2", "/any/other/path").getMock();
+        assertEquals(1, cache.getEntries(t).size());
+        assertEquals(1, cache.getEntries(t).size());
+        assertTrue(cache.getEntries(t).isEmpty());
+
+        verify(store, never()).load(anyString());
+        verify(store, never()).load(anyString(), anyString());
+    }
+}
\ No newline at end of file

Propchange: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionCachePathMapTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImplTest.java?rev=1881961&r1=1881960&r2=1881961&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImplTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImplTest.java Wed Sep 23 16:35:28 2020
@@ -23,6 +23,7 @@ import java.util.Set;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import org.apache.jackrabbit.oak.api.Root;
+import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -31,6 +32,7 @@ import org.junit.Test;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
 
 public class PermissionEntryProviderImplTest {
 
@@ -57,7 +59,7 @@ public class PermissionEntryProviderImpl
 
         // test that PermissionEntryProviderImpl.noExistingNames nevertheless is
         // properly set
-        assertFalse(getNoExistingNames(provider));
+        assertFalse(getBooleanField(provider, "noExistingNames"));
         assertNotSame(Collections.emptyIterator(), provider.getEntryIterator(EntryPredicate.create()));
     }
 
@@ -77,7 +79,7 @@ public class PermissionEntryProviderImpl
         Long.MAX_VALUE
         */
         PermissionEntryProviderImpl provider = new PermissionEntryProviderImpl(store, principalNames, ConfigurationParameters.EMPTY);
-        assertFalse(getNoExistingNames(provider));
+        assertFalse(getBooleanField(provider, "noExistingNames"));
 
         assertNotSame(Collections.emptyIterator(), provider.getEntryIterator(EntryPredicate.create()));
     }
@@ -94,7 +96,7 @@ public class PermissionEntryProviderImpl
         same as before but principal-set contains a name for which not entries exist
         */
         PermissionEntryProviderImpl provider = new PermissionEntryProviderImpl(store, principalNames, ConfigurationParameters.EMPTY);
-        assertFalse(getNoExistingNames(provider));
+        assertFalse(getBooleanField(provider, "noExistingNames"));
     }
 
     @Test
@@ -103,23 +105,41 @@ public class PermissionEntryProviderImpl
         Set<String> principalNames = Sets.newHashSet("noEntries", "noEntries2", "noEntries3");
 
         PermissionEntryProviderImpl provider = new PermissionEntryProviderImpl(store, principalNames, ConfigurationParameters.EMPTY);
+        assertFalse(getBooleanField(provider, "noExistingNames"));
 
-        assertTrue(getNoExistingNames(provider));
+        // force init
+        provider.getEntryIterator(EntryPredicate.create());
+        assertTrue(getBooleanField(provider, "noExistingNames"));
+    }
+    
+    @Test
+    public void testInit() throws Exception {
+        MockPermissionStore store = new MockPermissionStore();
+        Set<String> principalNames = Sets.newHashSet("noEntries", "noEntries2", "noEntries3");
+
+        PermissionEntryProviderImpl provider = new PermissionEntryProviderImpl(store, principalNames, ConfigurationParameters.EMPTY);
+        assertFalse(getBooleanField(provider, "initialized"));
+
+        provider.getEntryIterator(EntryPredicate.create());
+        assertTrue(getBooleanField(provider, "initialized"));
+
+        provider.flush();
+        assertFalse(getBooleanField(provider, "initialized"));
+
+        provider.getEntries(mock(Tree.class));
+        assertTrue(getBooleanField(provider, "initialized"));
     }
 
     /**
-     * Use reflection to access the private "existingNames" field storing the
-     * names of those principals associated with the entry provider for which
-     * any permission entries exist.
+     * Use reflection to access a private boolean field
      *
      * @param provider The permission entry provider
-     * @return the existingNames set.
-     * @throws Exception
+     * @return the value of the boolean field
      */
-    private static boolean getNoExistingNames(@NotNull PermissionEntryProviderImpl provider) throws Exception {
-        Field noExistingNamesField = provider.getClass().getDeclaredField("noExistingNames");
-        noExistingNamesField.setAccessible(true);
-        return (boolean) noExistingNamesField.get(provider);
+    private static boolean getBooleanField(@NotNull PermissionEntryProviderImpl provider, @NotNull String name) throws Exception {
+        Field f = provider.getClass().getDeclaredField(name);
+        f.setAccessible(true);
+        return (boolean) f.get(provider);
     }
 
     // Inner Classes

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStoreImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStoreImplTest.java?rev=1881961&r1=1881960&r2=1881961&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStoreImplTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStoreImplTest.java Wed Sep 23 16:35:28 2020
@@ -27,7 +27,6 @@ import org.apache.jackrabbit.oak.spi.sec
 import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
 import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeBits;
 import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
-import org.apache.jackrabbit.oak.util.NodeUtil;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.junit.After;
@@ -61,9 +60,9 @@ public class PermissionStoreImplTest ext
         super.before();
         testPrincipal = getTestUser().getPrincipal();
 
-        NodeUtil rootNode = new NodeUtil(root.getTree("/"), namePathMapper);
-        NodeUtil testNode = rootNode.addChild("testPath", JcrConstants.NT_UNSTRUCTURED);
-        testNode.addChild("childNode", JcrConstants.NT_UNSTRUCTURED);
+        Tree rootNode = root.getTree("/");
+        Tree testNode = TreeUtil.addChild(rootNode, "testPath", JcrConstants.NT_UNSTRUCTURED);
+        TreeUtil.addChild(testNode, "childNode", JcrConstants.NT_UNSTRUCTURED);
 
         addAcl(testPath, EveryonePrincipal.getInstance());
         addAcl(childPath, EveryonePrincipal.getInstance());