Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/MembershipProvider.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/MembershipProvider.java (revision 1846651) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/MembershipProvider.java (revision ) @@ -223,6 +223,11 @@ return getMembers(groupTree, getContentID(groupTree), includeInherited, new HashSet()); } + @NotNull + Iterator getDeclaredMemberContentIDs(@NotNull Tree groupTree) { + return getDeclaredMemberReferenceIterator(groupTree); + } + /** * Returns an iterator over all member paths of the given group. * @@ -296,12 +301,7 @@ } String contentId = getContentID(authorizableTree); - MemberReferenceIterator refs = new MemberReferenceIterator(groupTree) { - @Override - protected boolean hasProcessedReference(@NotNull String value) { - return true; - } - }; + MemberReferenceIterator refs = getDeclaredMemberReferenceIterator(groupTree); return Iterators.contains(refs, contentId); } @@ -380,6 +380,15 @@ */ Set removeMembers(@NotNull Tree groupTree, @NotNull Map memberIds) { return writer.removeMembers(groupTree, memberIds); + } + + private MemberReferenceIterator getDeclaredMemberReferenceIterator(@NotNull Tree groupTree) { + return new MemberReferenceIterator(groupTree) { + @Override + protected boolean hasProcessedReference(@NotNull String value) { + return true; + } + }; } /** Index: oak-benchmarks/src/main/java/org/apache/jackrabbit/oak/benchmark/FindAuthorizableWithScopeTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-benchmarks/src/main/java/org/apache/jackrabbit/oak/benchmark/FindAuthorizableWithScopeTest.java (revision 1846651) +++ oak-benchmarks/src/main/java/org/apache/jackrabbit/oak/benchmark/FindAuthorizableWithScopeTest.java (revision ) @@ -19,14 +19,19 @@ import java.util.Iterator; import javax.jcr.Node; import javax.jcr.Session; +import javax.jcr.SimpleCredentials; +import javax.jcr.security.Privilege; import org.apache.jackrabbit.api.JackrabbitSession; +import org.apache.jackrabbit.api.security.JackrabbitAccessControlList; import org.apache.jackrabbit.api.security.user.Authorizable; import org.apache.jackrabbit.api.security.user.Group; import org.apache.jackrabbit.api.security.user.Query; import org.apache.jackrabbit.api.security.user.QueryBuilder; import org.apache.jackrabbit.api.security.user.User; import org.apache.jackrabbit.api.security.user.UserManager; +import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils; +import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal; import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl; import org.apache.jackrabbit.util.Text; @@ -35,37 +40,67 @@ private static final String GROUP_ID = "testGroup"; private final long numberOfUsers; + private final long numberOfMembership; + private final long maxCount; private final boolean setScope; + private final boolean declaredMembership; + private final boolean runAsAdmin; - private Session adminSession; - private UserManager userMgr; + private JackrabbitSession adminSession; + private UserManager testUserManager; - public FindAuthorizableWithScopeTest(long numberOfUsers, boolean setScope) { + public FindAuthorizableWithScopeTest(long numberOfUsers, long numberOfMembership, int maxCount, boolean setScope, boolean declaredMembership, boolean runAsAdmin) { this.numberOfUsers = numberOfUsers; + this.numberOfMembership = numberOfMembership; + this.maxCount = maxCount; this.setScope = setScope; + this.declaredMembership = declaredMembership; + this.runAsAdmin = runAsAdmin; } @Override protected void beforeSuite() throws Exception { - adminSession = loginWriter(); + adminSession = (JackrabbitSession) loginWriter(); - userMgr = ((JackrabbitSession) adminSession).getUserManager(); - Group gr = userMgr.createGroup(GROUP_ID, new PrincipalImpl(GROUP_ID), "test"); + UserManager userManager = adminSession.getUserManager(); + Group gr = userManager.createGroup(GROUP_ID, new PrincipalImpl(GROUP_ID), "test"); + User u = null; for (int i = 0; i < numberOfUsers; i++) { - User u = userMgr.createUser("testUser" + i, null, new PrincipalImpl("testUser" + i), "test"); + u = userManager.createUser("testUser" + i, "pw", new PrincipalImpl("testUser" + i), "test"); gr.addMember(u); } + + for (int i = 1; i < numberOfMembership; i++) { + String id = GROUP_ID + i; + Group g = userManager.createGroup(id, new PrincipalImpl(id), "test"); + g.addMember(u); + } + adminSession.save(); + + if (runAsAdmin) { + testUserManager = userManager; + } else { + JackrabbitAccessControlList acl = AccessControlUtils.getAccessControlList(adminSession, "/"); + if (acl != null) { + acl.addEntry(EveryonePrincipal.getInstance(), AccessControlUtils.privilegesFromNames(adminSession, Privilege.JCR_READ), true); + adminSession.getAccessControlManager().setPolicy("/", acl); + adminSession.save(); - } + } + Session reader = login(new SimpleCredentials(u.getID(), "pw".toCharArray())); + testUserManager = ((JackrabbitSession) reader).getUserManager(); + } + } @Override protected void afterSuite() throws Exception { - Authorizable gr = userMgr.getAuthorizable(GROUP_ID); + UserManager userManager = adminSession.getUserManager(); + Authorizable gr = userManager.getAuthorizable(GROUP_ID); if (gr != null) { Node n = adminSession.getNode(Text.getRelativeParent(gr.getPath(), 1)); n.remove(); } - Authorizable u1 = userMgr.getAuthorizable("testUser0"); + Authorizable u1 = userManager.getAuthorizable("testUser0"); if (u1 != null) { Node n = adminSession.getNode(Text.getRelativeParent(u1.getPath(), 1)); n.remove(); @@ -75,7 +110,7 @@ @Override protected void runTest() throws Exception { - Iterator result = userMgr.findAuthorizables(createQuery()); + Iterator result = testUserManager.findAuthorizables(createQuery()); while (result.hasNext()) { result.next(); @@ -83,19 +118,14 @@ } private Query createQuery() { - if (setScope) { - return new Query() { - public void build(QueryBuilder builder) { + return new Query() { + public void build(QueryBuilder builder) { - builder.nameMatches("testUser"); - builder.setScope(GROUP_ID, true); + builder.setLimit(0, maxCount); + builder.nameMatches("testUser%"); + if (setScope) { + builder.setScope(GROUP_ID, declaredMembership); } - }; - } else { - return new Query() { - public void build(QueryBuilder builder) { - builder.nameMatches("testUser"); - } - }; + } + }; - } } } \ No newline at end of file Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManager.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManager.java (revision 1846651) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManager.java (revision ) @@ -16,21 +16,17 @@ */ package org.apache.jackrabbit.oak.security.user.query; -import static org.apache.jackrabbit.oak.api.QueryEngine.NO_BINDINGS; - import java.text.ParseException; import java.util.Collections; import java.util.HashSet; import java.util.Iterator; import java.util.Set; - import javax.jcr.RepositoryException; import javax.jcr.Value; import com.google.common.base.Predicate; import com.google.common.base.Strings; import com.google.common.collect.Iterators; - import org.apache.jackrabbit.api.security.user.Authorizable; import org.apache.jackrabbit.api.security.user.Group; import org.apache.jackrabbit.api.security.user.Query; @@ -40,6 +36,7 @@ import org.apache.jackrabbit.oak.api.Root; import org.apache.jackrabbit.oak.commons.PathUtils; import org.apache.jackrabbit.oak.namepath.NamePathMapper; +import org.apache.jackrabbit.oak.security.user.DeclaredMembershipPredicate; import org.apache.jackrabbit.oak.security.user.UserManagerImpl; import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters; import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal; @@ -52,6 +49,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.apache.jackrabbit.oak.api.QueryEngine.NO_BINDINGS; + /** * Query manager for user specific searches. */ @@ -109,9 +108,14 @@ } else { // filtering by group name included in query -> enforce offset and limit on the result set. Iterator result = findAuthorizables(statement, Long.MAX_VALUE, 0, null); - Predicate groupFilter = new GroupPredicate(userManager, groupId, builder.isDeclaredMembersOnly()); - return ResultIterator.create(builder.getOffset(), builder.getMaxCount(), - Iterators.filter(result, groupFilter)); + Predicate filter; + if (builder.isDeclaredMembersOnly()) { + filter = new DeclaredMembershipPredicate(userManager, groupId); + } else { + filter = new GroupPredicate(userManager, groupId, false); + + } + return ResultIterator.create(builder.getOffset(), builder.getMaxCount(), Iterators.filter(result, filter)); } } Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/DeclaredMembershipPredicate.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/DeclaredMembershipPredicate.java (revision ) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/DeclaredMembershipPredicate.java (revision ) @@ -0,0 +1,88 @@ +/* + * 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.user; + +import java.util.HashSet; +import java.util.Iterator; +import java.util.Set; +import javax.jcr.RepositoryException; + +import com.google.common.base.Predicate; +import com.google.common.collect.Iterators; +import org.apache.jackrabbit.api.security.user.Authorizable; +import org.apache.jackrabbit.oak.api.Tree; +import org.apache.jackrabbit.oak.spi.security.user.AuthorizableType; +import org.jetbrains.annotations.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Predicate used to filter authorizables based on their declared group membership. + */ +public class DeclaredMembershipPredicate implements Predicate { + + static final Logger log = LoggerFactory.getLogger(DeclaredMembershipPredicate.class); + + private final MembershipProvider membershipProvider; + private final Iterator contentIdIterator; + private final Set declaredMemberContentIds = new HashSet(); + + public DeclaredMembershipPredicate(UserManagerImpl userManager, String groupId) throws RepositoryException { + this.membershipProvider = userManager.getMembershipProvider(); + Tree groupTree = membershipProvider.getByID(groupId, AuthorizableType.GROUP); + if (groupTree == null) { + contentIdIterator = Iterators.emptyIterator(); + } else { + contentIdIterator = membershipProvider.getDeclaredMemberContentIDs(membershipProvider.getByID(groupId, AuthorizableType.GROUP)); + } + } + + @Override + public boolean apply(@Nullable Authorizable authorizable) { + String id = saveGetContentId(authorizable); + if (id != null) { + if (declaredMemberContentIds.contains(id)) { + return true; + } else { + // not contained in ids that have already been processed => look + // for occurrence in the remaining iterator entries. + while (contentIdIterator.hasNext()) { + String memberContentId = contentIdIterator.next(); + if (memberContentId != null) { + declaredMemberContentIds.add(memberContentId); + if (memberContentId.equals(id)) { + return true; + } + } + } + } + } + return false; + } + + @Nullable + private String saveGetContentId(@Nullable Authorizable authorizable) { + if (authorizable != null) { + try { + return membershipProvider.getContentID(authorizable.getID()); + } catch (RepositoryException e) { + log.debug("Error while retrieving ID for authorizable {}", authorizable, e); + } + } + return null; + } +} Index: oak-benchmarks/src/main/java/org/apache/jackrabbit/oak/benchmark/BenchmarkRunner.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-benchmarks/src/main/java/org/apache/jackrabbit/oak/benchmark/BenchmarkRunner.java (revision 1846651) +++ oak-benchmarks/src/main/java/org/apache/jackrabbit/oak/benchmark/BenchmarkRunner.java (revision ) @@ -128,7 +128,11 @@ OptionSpec expiration = parser.accepts("expiration", "Expiration time (e.g. principal cache.") .withOptionalArg().ofType(Long.class).defaultsTo(AbstractLoginTest.NO_CACHE); OptionSpec numberOfGroups = parser.accepts("numberOfGroups", "Number of groups to create.") - .withOptionalArg().ofType(Integer.class).defaultsTo(LoginWithMembershipTest.NUMBER_OF_GROUPS_DEFAULT); + .withOptionalArg().ofType(Integer.class).defaultsTo(LoginWithMembershipTest.NUMBER_OF_GROUPS_DEFAULT); + OptionSpec queryMaxCount = parser.accepts("queryMaxCount", "Max number of query results.") + .withOptionalArg().ofType(Integer.class).defaultsTo(Integer.MAX_VALUE); + OptionSpec declaredMembership = parser.accepts("declaredMembership", "Only look for declared membership.") + .withOptionalArg().ofType(Boolean.class).defaultsTo(true); OptionSpec numberOfInitialAce = parser.accepts("numberOfInitialAce", "Number of ACE to create before running the test.") .withOptionalArg().ofType(Integer.class).defaultsTo(AceCreationTest.NUMBER_OF_INITIAL_ACE_DEFAULT); OptionSpec nestedGroups = parser.accepts("nestedGroups", "Use nested groups.") @@ -470,7 +474,7 @@ wikipedia.value(options), flatStructure.value(options), report.value(options), withStorage.value(options), withServer.value(options)), - new FindAuthorizableWithScopeTest(numberOfUsers.value(options), setScope.value(options)), + new FindAuthorizableWithScopeTest(numberOfUsers.value(options), numberOfGroups.value(options), queryMaxCount.value(options), setScope.value(options), declaredMembership.value(options), runAsAdmin.value(options)), new LucenePropertyFullTextTest( wikipedia.value(options), flatStructure.value(options),