Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStoreImpl.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStoreImpl.java (revision 1844566) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionStoreImpl.java (revision ) @@ -95,7 +95,7 @@ } } } - return entries == null || entries.isEmpty() ? null : entries; + return entries; } @NotNull Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PrincipalPermissionEntries.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PrincipalPermissionEntries.java (revision 1844566) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PrincipalPermissionEntries.java (revision ) @@ -17,8 +17,12 @@ package org.apache.jackrabbit.oak.security.authorization.permission; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; +import java.util.Set; + import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -38,6 +42,7 @@ * map of permission entries, accessed by path */ private Map> entries = new HashMap<>(); + private Set emptyPaths = new HashSet(); PrincipalPermissionEntries() { this(Long.MAX_VALUE); @@ -45,10 +50,11 @@ PrincipalPermissionEntries(long expectedSize) { this.expectedSize = expectedSize; + fullyLoaded = (expectedSize == 0); } long getSize() { - return entries.size(); + return entries.size() + emptyPaths.size(); } boolean isFullyLoaded() { @@ -66,7 +72,7 @@ @Nullable Collection getEntriesByPath(@NotNull String path) { - return entries.get(path); + return (emptyPaths.contains(path)) ? Collections.emptySet() : entries.get(path); } void putEntriesByPath(@NotNull String path, @NotNull Collection pathEntries) { @@ -74,6 +80,10 @@ if (entries.size() >= expectedSize) { setFullyLoaded(true); } + } + + void rememberNotAccessControlled(@NotNull String path) { + emptyPaths.add(path); } void putAllEntries(@NotNull Map> allEntries) { Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCache.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCache.java (revision 1844566) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCache.java (revision ) @@ -17,7 +17,6 @@ package org.apache.jackrabbit.oak.security.authorization.permission; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.Map; import org.jetbrains.annotations.NotNull; @@ -71,7 +70,7 @@ // nothing to add to the result collection 'ret'. // nevertheless, remember the absence of any permission entries // in the cache to avoid reading from store again. - ppe.putEntriesByPath(path, Collections.emptySet()); + ppe.rememberNotAccessControlled(path); } else { ppe.putEntriesByPath(path, pes); ret.addAll(pes); Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/MountPermissionProvider.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/MountPermissionProvider.java (revision 1844566) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/MountPermissionProvider.java (revision ) @@ -29,6 +29,7 @@ import org.apache.jackrabbit.oak.spi.security.Context; import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionProvider; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import static com.google.common.collect.Lists.newArrayList; @@ -74,13 +75,13 @@ this.stores = stores; } - @NotNull + @Nullable @Override public Collection load(@NotNull String principalName, @NotNull String path) { for (PermissionStoreImpl store : stores) { Collection col = store.load(principalName, path); - if (col != null && !col.isEmpty()) { + if (col != null) { return col; } } Index: oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PrincipalPermissionEntriesTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PrincipalPermissionEntriesTest.java (revision 1844566) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PrincipalPermissionEntriesTest.java (revision ) @@ -19,6 +19,7 @@ import java.lang.reflect.Field; import java.util.Collection; import java.util.Map; + import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionPattern; @@ -28,6 +29,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -55,7 +57,7 @@ @Test public void testIsFullyLoadedUponCreation() { - assertFalse(new PrincipalPermissionEntries(0).isFullyLoaded()); + assertTrue(new PrincipalPermissionEntries(0).isFullyLoaded()); assertFalse(new PrincipalPermissionEntries(1).isFullyLoaded()); assertFalse(new PrincipalPermissionEntries().isFullyLoaded()); } @@ -140,9 +142,33 @@ ppe.putEntriesByPath("/path", ImmutableSet.of(permissionEntry)); assertEquals(1, ppe.getEntries().size()); + assertEquals(1, ppe.getSize()); } @Test + public void testPutEmptyEntriesByPath() { + PrincipalPermissionEntries ppe = new PrincipalPermissionEntries(1); + ppe.putEntriesByPath("/path", ImmutableSet.of()); + + assertTrue(ppe.isFullyLoaded()); + assertEquals(1, ppe.getSize()); + assertEquals(1, ppe.getEntries().size()); + } + + @Test + public void testRememberNotAccessControlled() { + PrincipalPermissionEntries ppe = new PrincipalPermissionEntries(1); + ppe.rememberNotAccessControlled("/path"); + + assertFalse(ppe.isFullyLoaded()); + assertEquals(1, ppe.getSize()); + assertTrue(ppe.getEntries().isEmpty()); + Collection c = ppe.getEntriesByPath("/path"); + assertNotNull(c); + assertTrue(c.isEmpty()); + } + + @Test public void testGetEntriesByPath() { PrincipalPermissionEntries ppe = new PrincipalPermissionEntries(2); Collection collection = ImmutableSet.of(permissionEntry); @@ -154,6 +180,23 @@ assertEquals(collection, ppe.getEntriesByPath("/path")); assertEquals(collection, ppe.getEntriesByPath("/path2")); assertNull(ppe.getEntriesByPath("/nonExisting")); + } + + @Test + public void testGetInitialSize() { + assertEquals(0, new PrincipalPermissionEntries().getSize()); + assertEquals(0, new PrincipalPermissionEntries(1).getSize()); + } + + @Test + public void testGetSize() { + PrincipalPermissionEntries ppe = new PrincipalPermissionEntries(); + + ppe.putEntriesByPath("/path", ImmutableSet.of(permissionEntry)); + assertEquals(1, ppe.getSize()); + + ppe.rememberNotAccessControlled("/path2"); + assertEquals(2, ppe.getSize()); } private static final long inspectExpectedSize(@NotNull PrincipalPermissionEntries ppe) throws Exception { Index: oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCacheTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCacheTest.java (revision 1844566) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCacheTest.java (revision ) @@ -163,6 +163,28 @@ } @Test + public void testLoadNonExistingNotCompleted2() throws Exception { + cache.init("a", 1); + when(store.load("a", "/path")).thenReturn(null); + + Collection result = Sets.newHashSet(); + cache.load(store, result, "a", "/path"); + + assertTrue(result.isEmpty()); + + PrincipalPermissionEntries inspectedEntries = inspectEntries(cache, "a"); + assertFalse(inspectedEntries.isFullyLoaded()); + + // requesting the entries again must NOT hit the store + when(store.load("a", "/path")).thenThrow(IllegalStateException.class); + + result.clear(); + cache.load(store, result, "a", "/path"); + + assertTrue(result.isEmpty()); + } + + @Test public void testLoadNonExistingCompleted() throws Exception { cache.init("a", 0); when(store.load("a", "/path")).thenReturn(null);