Index: src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java =================================================================== --- src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java (revision 1858199) +++ src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java (working copy) @@ -160,7 +160,7 @@ return new RepositoryPermission() { @Override public boolean isGranted(long repositoryPermissions) { - EntryPredicate predicate = new EntryPredicate(); + EntryPredicate predicate = EntryPredicate.create(); return hasPermissions(getEntryIterator(predicate), predicate, repositoryPermissions, null); } }; @@ -284,7 +284,7 @@ @Override public boolean isGranted(@NotNull String path, long permissions) { - EntryPredicate predicate = new EntryPredicate(path, Permissions.respectParentPermissions(permissions)); + EntryPredicate predicate = EntryPredicate.create(path, Permissions.respectParentPermissions(permissions)); return hasPermissions(getEntryIterator(predicate), predicate, permissions, path); } @@ -302,7 +302,7 @@ //------------------------------------------------------------< private >--- private boolean internalIsGranted(@NotNull Tree tree, @Nullable PropertyState property, long permissions) { - EntryPredicate predicate = new EntryPredicate(tree, property, Permissions.respectParentPermissions(permissions)); + EntryPredicate predicate = EntryPredicate.create(tree, property, Permissions.respectParentPermissions(permissions)); return hasPermissions(getEntryIterator(predicate), predicate, permissions, tree.getPath()); } @@ -400,8 +400,8 @@ @NotNull private PrivilegeBits getPrivilegeBits(@Nullable Tree tree) { EntryPredicate pred = (tree == null) - ? new EntryPredicate() - : new EntryPredicate(tree, null, false); + ? EntryPredicate.create() + : EntryPredicate.create(tree, null, false); Iterator entries = getEntryIterator(pred); PrivilegeBits allowBits = PrivilegeBits.getInstance(); @@ -566,7 +566,7 @@ @Override public boolean isGranted(long permissions) { - EntryPredicate predicate = new EntryPredicate(tree, null, Permissions.respectParentPermissions(permissions)); + EntryPredicate predicate = EntryPredicate.create(tree, null, Permissions.respectParentPermissions(permissions)); Iterator it = getIterator(predicate); return hasPermissions(it, predicate, permissions, tree.getPath()); } @@ -573,7 +573,7 @@ @Override public boolean isGranted(long permissions, @NotNull PropertyState property) { - EntryPredicate predicate = new EntryPredicate(tree, property, Permissions.respectParentPermissions(permissions)); + EntryPredicate predicate = EntryPredicate.create(tree, property, Permissions.respectParentPermissions(permissions)); Iterator it = getIterator(predicate); return hasPermissions(it, predicate, permissions, tree.getPath()); } @@ -580,7 +580,7 @@ //--------------------------------------------------------< private >--- private Iterator getIterator(@Nullable PropertyState property, long permissions) { - EntryPredicate predicate = new EntryPredicate(tree, property, Permissions.respectParentPermissions(permissions)); + EntryPredicate predicate = EntryPredicate.create(tree, property, Permissions.respectParentPermissions(permissions)); return getIterator(predicate); } Index: src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/EntryPredicate.java =================================================================== --- src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/EntryPredicate.java (revision 1858199) +++ src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/EntryPredicate.java (working copy) @@ -27,67 +27,65 @@ * Predicate used to evaluation if a given {@code PermissionEntry} matches * the specified tree, property or path. */ -final class EntryPredicate implements Predicate { +interface EntryPredicate extends Predicate { - private final Tree tree; - private final PropertyState property; - private final String path; + @Nullable + String getPath(); - private final String parentPath; - private final Tree parent; - private final boolean respectParent; - - EntryPredicate() { - this(null, null, null, false); + default boolean apply(@Nullable PermissionEntry entry) { + return entry != null && apply(entry, true); } - EntryPredicate(@NotNull Tree tree, @Nullable PropertyState property, boolean respectParent) { - this(tree, property, tree.getPath(), respectParent); - } + boolean apply(@NotNull PermissionEntry entry, boolean respectParent); - EntryPredicate(@NotNull String path, boolean respectParent) { - this(null, null, path, respectParent); - } + static EntryPredicate create() { + return new EntryPredicate() { + @Nullable + @Override + public String getPath() { + return null; + } - private EntryPredicate(@Nullable Tree tree, @Nullable PropertyState property, - @Nullable String path, boolean respectParent) { - this.tree = tree; - this.property = property; - this.path = path; - - if (respectParent) { - parentPath = (path == null || "/".equals(path)) ? null : PathUtils.getParentPath(path); - parent = (tree == null || tree.isRoot()) ? null : tree.getParent(); - } else { - parentPath = null; - parent = null; - } - this.respectParent = parent != null || parentPath != null; + @Override + public boolean apply(@NotNull PermissionEntry entry, boolean respectParent) { + return entry.matches(); + } + }; } - @Nullable - String getPath() { - return path; - } + static EntryPredicate create(@NotNull Tree tree, @Nullable PropertyState property, boolean respectParent) { + Tree parent = (!respectParent || tree.isRoot()) ? null : tree.getParent(); + boolean rp = respectParent && parent != null; + return new EntryPredicate() { + @NotNull + @Override + public String getPath() { + return tree.getPath(); + } - //----------------------------------------------------------< Predicate >--- - @Override - public boolean apply(@Nullable PermissionEntry entry) { - return apply(entry, true); + @Override + public boolean apply(@NotNull PermissionEntry entry, boolean respectParent) { + respectParent &= rp; + return entry.matches(tree, property) || (respectParent && entry.matches(parent, null)); + } + }; } - public boolean apply(@Nullable PermissionEntry entry, boolean respectParent) { - if (entry == null) { - return false; - } - respectParent &= this.respectParent; + static EntryPredicate create(@NotNull String path, boolean respectParent) { + String parentPath = (!respectParent || PathUtils.ROOT_PATH.equals(path)) ? null : PathUtils.getParentPath(path); + boolean rp = respectParent && parentPath != null; + return new EntryPredicate() { + @NotNull + @Override + public String getPath() { + return path; + } - if (tree != null) { - return entry.matches(tree, property) || (respectParent && parent != null && entry.matches(parent, null)); - } else if (path != null) { - return entry.matches(path) || (respectParent && parentPath != null && entry.matches(parentPath)); - } else { - return entry.matches(); - } + @Override + public boolean apply(@NotNull PermissionEntry entry, boolean respectParent) { + respectParent &= rp; + return entry.matches(path) || (respectParent && entry.matches(parentPath)); + } + }; } } Index: src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/EntryPredicateTest.java =================================================================== --- src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/EntryPredicateTest.java (revision 1858199) +++ src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/EntryPredicateTest.java (working copy) @@ -31,7 +31,6 @@ import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; -import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -55,14 +54,12 @@ @Test public void testPredicateRepositoryLevel() { - EntryPredicate pred = new EntryPredicate(); + EntryPredicate pred = EntryPredicate.create(); assertNull(pred.getPath()); when(pattern.matches()).thenReturn(true); assertFalse(pred.apply(null)); - assertFalse(pred.apply(null, true)); - assertFalse(pred.apply(null, false)); assertTrue(pred.apply(entry)); assertTrue(pred.apply(entry, true)); @@ -73,7 +70,7 @@ @Test public void testPredicatePathRespectParent() { - EntryPredicate pred = new EntryPredicate(path, true); + EntryPredicate pred = EntryPredicate.create(path, true); assertEquals(path, pred.getPath()); // pattern neither matches path nor parent path @@ -89,8 +86,6 @@ when(pattern.matches(parentPath)).thenReturn(true); assertFalse(pred.apply(null)); - assertFalse(pred.apply(null, true)); - assertFalse(pred.apply(null, false)); assertTrue(pred.apply(entry)); assertTrue(pred.apply(entry, true)); @@ -118,7 +113,7 @@ @Test public void testPredicatePathDontRespectParent() { - EntryPredicate pred = new EntryPredicate(path, false); + EntryPredicate pred = EntryPredicate.create(path, false); assertEquals(path, pred.getPath()); // pattern neither matches path nor parent path @@ -134,8 +129,6 @@ when(pattern.matches(parentPath)).thenReturn(true); assertFalse(pred.apply(null)); - assertFalse(pred.apply(null, true)); - assertFalse(pred.apply(null, false)); assertTrue(pred.apply(entry)); assertTrue(pred.apply(entry, true)); @@ -168,7 +161,7 @@ PropertyState ps = mock(PropertyState.class); when(ps.getName()).thenReturn("property"); - EntryPredicate pred = new EntryPredicate(tree, ps, true); + EntryPredicate pred = EntryPredicate.create(tree, ps, true); assertEquals(path, pred.getPath()); // pattern neither matches path nor parent path @@ -186,8 +179,6 @@ when(pattern.matches(parent, null)).thenReturn(true); assertFalse(pred.apply(null)); - assertFalse(pred.apply(null, true)); - assertFalse(pred.apply(null, false)); assertTrue(pred.apply(entry)); assertTrue(pred.apply(entry, true)); @@ -223,7 +214,7 @@ PropertyState ps = mock(PropertyState.class); when(ps.getName()).thenReturn("property"); - EntryPredicate pred = new EntryPredicate(tree, ps,false); + EntryPredicate pred = EntryPredicate.create(tree, ps,false); assertEquals(path, pred.getPath()); // pattern neither matches path nor parent path @@ -241,8 +232,6 @@ when(pattern.matches(parent, null)).thenReturn(true); assertFalse(pred.apply(null)); - assertFalse(pred.apply(null, true)); - assertFalse(pred.apply(null, false)); assertTrue(pred.apply(entry)); assertTrue(pred.apply(entry, true)); @@ -273,7 +262,7 @@ @Test public void testPredicateRootPath() { - EntryPredicate pred = new EntryPredicate(PathUtils.ROOT_PATH, true); + EntryPredicate pred = EntryPredicate.create(PathUtils.ROOT_PATH, true); assertEquals(PathUtils.ROOT_PATH, pred.getPath()); // pattern doesn't match path @@ -295,7 +284,7 @@ @Test public void testPredicateRootPathDontRespectParent() { - EntryPredicate pred = new EntryPredicate(PathUtils.ROOT_PATH, false); + EntryPredicate pred = EntryPredicate.create(PathUtils.ROOT_PATH, false); assertEquals(PathUtils.ROOT_PATH, pred.getPath()); // pattern doesn't match path @@ -320,7 +309,7 @@ Tree tree = mockTree(PathUtils.ROOT_PATH, null); when(tree.isRoot()).thenReturn(true); - EntryPredicate pred = new EntryPredicate(tree, null,true); + EntryPredicate pred = EntryPredicate.create(tree, null,true); assertEquals(PathUtils.ROOT_PATH, pred.getPath()); // pattern doesn't match path @@ -346,7 +335,7 @@ Tree tree = mockTree(PathUtils.ROOT_PATH, null); when(tree.isRoot()).thenReturn(true); - EntryPredicate pred = new EntryPredicate(tree, null,false); + EntryPredicate pred = EntryPredicate.create(tree, null,false); assertEquals(PathUtils.ROOT_PATH, pred.getPath()); // pattern doesn't match path Index: src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImplTest.java =================================================================== --- src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImplTest.java (revision 1858199) +++ src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImplTest.java (working copy) @@ -58,7 +58,7 @@ // test that PermissionEntryProviderImpl.noExistingNames nevertheless is // properly set assertFalse(getNoExistingNames(provider)); - assertNotSame(Collections.emptyIterator(), provider.getEntryIterator(new EntryPredicate())); + assertNotSame(Collections.emptyIterator(), provider.getEntryIterator(EntryPredicate.create())); } /** @@ -79,7 +79,7 @@ PermissionEntryProviderImpl provider = new PermissionEntryProviderImpl(store, principalNames, ConfigurationParameters.EMPTY); assertFalse(getNoExistingNames(provider)); - assertNotSame(Collections.emptyIterator(), provider.getEntryIterator(new EntryPredicate())); + assertNotSame(Collections.emptyIterator(), provider.getEntryIterator(EntryPredicate.create())); } /**